Grow the delete-handling deque in tre_tnfa_run_approx() dynamically - #145
Open
kevinushey wants to merge 2 commits into
Open
Grow the delete-handling deque in tre_tnfa_run_approx() dynamically#145kevinushey wants to merge 2 commits into
kevinushey wants to merge 2 commits into
Conversation
tre_tnfa_run_approx() kept its delete-handling deque in a fixed 512-pointer array on the C stack, guarded only by assert(). A TNFA with more than 512 states reachable at one position overflows the array: with NDEBUG this corrupts the stack silently, and even with asserts enabled the initial fill loop walks past the array without ever tripping the assertion (it does not wrap), so the overflow is never caught. An alternation of ~200 distinct literals is enough to crash tre_regaexec() (observed on Windows x64, mingw-w64 gcc). Start with a small buffer on the stack and move it to the heap, growing as needed, when it fills during the initial enqueue of reachable states. The circular enqueue in the propagation loop keeps its existing assert() backstop; its capacity references now track the dynamic size. Addresses the '/* XXX - dynamic ringbuffer size */' note. This fix has been carried in GNU R's vendored copy of TRE since 2011 (R commits 155c77fa, 4e5d77d3, d7630204).
test_exec() runs every test through the approximate matcher via REG_APPROX_MATCHER, so this exercises the dynamic deque growth in tre_tnfa_run_approx() added in this branch; against the previous fixed-size deque this test crashes.
kevinushey
force-pushed
the
fix-approx-ringbuffer
branch
from
August 9, 2026 19:57
7f81e56 to
d216eb2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These pull requests were generated with Claude Code, but were reviewed by me (@kevinushey) before posting. I'll respond personally to any feedback; please let me know if this is okay.
This makes the delete-handling deque in
tre_tnfa_run_approx()grow dynamically, addressing the existing/* XXX - dynamic ringbuffer size */note.Problem. The deque lives in a fixed 512-pointer array on the C stack, guarded only by
assert(). A TNFA with more than 512 states reachable at one position overflows it. Worse, the initial fill loop does not wrap, sodeque_endwalks past the array without ever equalingdeque_start— the assertion can never fire there, even in debug builds, and the overflow writes through the enclosing stack frame.An alternation of ~200 distinct short literals is enough:
(observed on Windows x64, mingw-w64 gcc 14; the crash threshold depends on platform and optimization level)
Approach. Start with a 256-entry array on the stack; when the initial enqueue of reachable states fills it, move to the heap with
xmalloc/xrealloc, growing by 512 entries at a time. The circular enqueue in the later propagation loop keeps its existingassert()backstop, with its capacity references now tracking the dynamic size. Heap buffer is freed on all exits, including the existingREG_ESPACEerror-path convention forbuf.Testing. Alternations of 60–800 distinct literals through
tre_regaexec()atmax_cost1–2, all matching correctly where the unpatched code crashes at ≥ ~200; existing small-pattern behavior unchanged.Provenance. This fix has been carried in GNU R's vendored copy of TRE since 2011, where it backs R's
agrep()/adist().