Skip to content

Comments

Fix iterator crash on single-element trees and compilation errors#25

Merged
rafaelkallis merged 5 commits intomainfrom
copilot/fix-coredump-in-loop
Nov 23, 2025
Merged

Fix iterator crash on single-element trees and compilation errors#25
rafaelkallis merged 5 commits intomainfrom
copilot/fix-coredump-in-loop

Conversation

Copy link
Contributor

Copilot AI commented Nov 23, 2025

Iterating past the last element in a single-element ART triggers a segmentation fault due to multiple bugs in the iterator implementation.

art::art<int> table;
table.set("key1", 1);
for(auto it = table.begin(); it != table.end(); ++it) {
    // Crash occurs on increment past last element
}

Root Causes Fixed

1. Missing #include <cstdint> in node.hpp (Pre-existing)

  • uint16_t prefix_len_ was undefined, causing 138 compilation errors that prevented any build
  • Fix: Added #include <cstdint> to node.hpp

2. Buffer underflow in step::operator++() (Pre-existing)

  • When incrementing the sentinel node (depth_ = 0, key_ = nullptr), code attempted to access key_[-1], causing segmentation fault
  • Fix: Added check if (depth_ > 0) before accessing key_[depth_ - 1]

3. Empty stack access in seek_leaf() (Original issue)

  • After popping traversal stack, code called get_step() without checking if stack was empty
  • get_step() asserts non-empty stack, causing crash
  • Fix: Added empty stack check after pop, return early if empty

Changes

  • node.hpp: Add missing #include <cstdint> to fix compilation
  • tree_it.hpp: Add empty stack check in seek_leaf() and depth check in step::operator++() to prevent buffer underflow
  • tree_it.cpp: Add test cases for single-element iteration edge case
  • .gitignore: Exclude CodeQL build artifacts

Test Results

All 25 tests now pass:

[doctest] test cases:     25 |     25 passed |      0 failed
[doctest] assertions: 41154954 | 41154954 passed |      0 failed
[doctest] Status: SUCCESS!
Original prompt

This section details on the original issue you should resolve

<issue_title>coredump with loop</issue_title>
<issue_description>Hi,I found that when the ART has only one key-value and loop it,it will core dump:
art::art table;
table.set("key1",1);
for(auto itor = table.begin(); itor != table.end(); itor++)
{
std::cout << itor.key() << std::endl;
std::cout << *itor << std::endl;
}</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits November 23, 2025 14:18
Added check for empty stack in seek_leaf() before calling get_step() to prevent assertion failure when incrementing past the last element in a tree.

Co-authored-by: rafaelkallis <9661903+rafaelkallis@users.noreply.github.com>
Co-authored-by: rafaelkallis <9661903+rafaelkallis@users.noreply.github.com>
Co-authored-by: rafaelkallis <9661903+rafaelkallis@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix coredump issue when looping over single key-value Fix iterator crash on single-element trees Nov 23, 2025
Copilot AI requested a review from rafaelkallis November 23, 2025 14:23
@rafaelkallis
Copy link
Owner

@copilot make sure the tests are passing

- Add missing #include <cstdint> to node.hpp (fixes uint16_t compilation errors)
- Fix buffer underflow in step::operator++() when depth_ is 0 (sentinel node case)

These pre-existing bugs prevented the repository from compiling and caused segfaults when iterating over single-element trees.

Co-authored-by: rafaelkallis <9661903+rafaelkallis@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 23, 2025

@copilot make sure the tests are passing

Tests are now passing! Fixed two pre-existing bugs that were preventing compilation and causing the crash:

  1. Missing #include <cstdint> in node.hpp (138 compilation errors)
  2. Buffer underflow in step::operator++() when accessing key_[-1] with sentinel node

All 25 tests pass:

[doctest] test cases:     25 |     25 passed |      0 failed
[doctest] assertions: 41154954 | 41154954 passed |      0 failed
[doctest] Status: SUCCESS!

Commit: 9183cfe

Copilot AI changed the title Fix iterator crash on single-element trees Fix iterator crash on single-element trees and compilation errors Nov 23, 2025
@rafaelkallis rafaelkallis marked this pull request as ready for review November 23, 2025 17:39
Copilot AI review requested due to automatic review settings November 23, 2025 17:39
Copy link
Owner

@rafaelkallis rafaelkallis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical segmentation fault that occurs when iterating past the last element in a single-element ART, along with a pre-existing compilation issue. The root cause was identified as two bugs in the iterator implementation: a buffer underflow when incrementing the sentinel step, and an empty stack access after popping the last element.

Key changes:

  • Fixed compilation errors by adding missing #include <cstdint> for uint16_t
  • Added bounds checking to prevent buffer underflow when incrementing iterator steps with zero depth
  • Added empty stack validation in seek_leaf() to prevent invalid memory access

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
include/art/node.hpp Adds missing #include <cstdint> to resolve compilation errors for uint16_t prefix_len_
include/art/tree_it.hpp Adds depth_ > 0 check before array access in step::operator++() and empty stack check in seek_leaf() to prevent crashes
test/tree_it.cpp Adds comprehensive test cases for single-element iteration covering both manual iteration and for-loop patterns
.gitignore Excludes CodeQL build artifacts from version control

@rafaelkallis rafaelkallis merged commit 99db793 into main Nov 23, 2025
9 checks passed
@rafaelkallis rafaelkallis deleted the copilot/fix-coredump-in-loop branch November 23, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

coredump with loop

2 participants