Anchor several regex/substring identifier matches against false-positive suffixes - #1274
Merged
Merged
Conversation
…ive suffixes Follow-on to #1262/#1266, which share the same root cause: an unanchored match mistaking a substring for a whole token. A scan for the same pattern elsewhere in the codebase turned up: - generator_test_results_backtrace.rb: matching an unresolved crashed test case's C symbol against a gdb backtrace frame had no \b before the escaped symbol, so a shorter unresolved symbol that's a suffix of a longer one actually named in the frame (e.g. "foo" vs "my_foo") could steal the crash attribution -- confirmed with a two-member parameterized group. - generator_test_runner.rb: remap_line_numbers! matched a test name against source lines completely unescaped and unanchored. A static helper function between two test cases whose name merely contains a later test's name as a substring (e.g. reset_test_ab_state() before test_ab) could false-match first, assigning the wrong line number and cascading misalignment to every subsequent test case in the file -- confirmed the same way. - c_extractor_declarations.rb: extract_type's rindex(name) is a suffix- substring risk in principle (not shown to misfire on realistic declarations, since everything that could trail a real declared name is already stripped before this runs) -- now \b-anchored defensively via a single greedy match instead of a raw rindex. - generator_test_results_backtrace.rb (extract_simple_crash_output): a filename interpolated into a regex without Regexp.escape -- hardened even though no concrete failing input was found. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
[skip ci] Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Port of #1273 (green on `next_version`) to `master`, for inclusion in 1.1.8.
Follow-on to #1262 and #1266. Both fixes shared the same root cause — an unanchored match mistaking a substring for a whole token — so after each was fixed, I scanned the rest of the codebase for the same pattern. This PR addresses what that scan found.
Confirmed bugs (empirically reproduced)
`generator_test_results_backtrace.rb` — `do_gdb`'s "prefer whichever unresolved member's own symbol is named in the backtrace" search had no `\b` before the escaped symbol:
```ruby
crash_result[:output].match?( /#{Regexp.escape(tc[:symbol])}\s*()\sat/ )
```
A shorter unresolved symbol that's a suffix of a longer one actually named in the frame (e.g. `foo` vs `my_foo`) could steal the crash attribution. Reproduced with a two-member parameterized group.
`generator_test_runner.rb` — `remap_line_numbers!` matched a test name against source lines completely unescaped and unanchored:
```ruby
if (line =~ /#{next_case[:test]}/)
```
A static helper function whose name merely contains a later test's name as a substring (e.g. `reset_test_ab_state()` before `test_ab`) could false-match first, cascading misalignment to every test case after it in the file. Reproduced the same way.
Defensive hardening
Tests
Same coverage as #1273: new regression tests for both confirmed bugs (verified failing pre-fix, passing after), plus a defensive test for the `extract_type` hardening. Unit tests only, no system tests. `bundle exec rake specs:units` clean (1994 examples, 0 failures, 1 pre-existing unrelated pending).
🤖 Generated with Claude Code