fix(fortran): func_start could phantom-match across a subroutine's own END boundary - #1555
Merged
Merged
Conversation
…n END boundary #1531: the RETURN TYPE section's lazy "Legacy Sizing/Attributes" class ([A-Za-z0-9_ \t\n&*,()=:]*?, unbounded) includes letters and newlines with no length cap. On a type-declaration line immediately preceding an unrelated later SUBROUTINE/FUNCTION (e.g. `CHARACTER(len=*), INTENT(IN) :: message` a few lines before that same subroutine's own `END SUBROUTINE wrf_message`), backtracking let the class swallow the entire intervening body -- including the trailing END statement -- and match keyword+identifier there instead. This produced a phantom duplicate function_data row whose start_line/args belonged to a type declaration, not any real definition -- confirmed reproducing #1531's exact wrf_message (start_line drift 5581 vs real 5580, args=1 vs real=2) and const_module_initialize symptoms, plus a third previously-unreported case (wrf_error_fatal) in the same corpus file. Bounded the class to {0,40} chars -- comfortably more than any real single/continued attribute list needs, well short of the ~95+ chars needed to reach a phantom match. A per-character `(?!\bEND\b)` exclusion was tried first and is more semantically precise, but empirically disables `re`'s fast path for a plain bounded character class: 0.01s vs. ~13s on a payload of thousands of repeated type-declaration lines with one distant END -- a real ReDoS regression on a plausible shape, not just a synthetic one. Caught by this PR's own ReDoS probe before it shipped; a plain numeric bound has none of that risk and was verified with the same probe. language-crucible/data/fortran/wrf/*.F: 3 duplicate names eliminated (wrf_message, wrf_error_fatal, const_module_initialize), each now resolving to its single real occurrence with correct start_line/args. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
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
1 participant
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.
Fixes #1531.
Summary
The RETURN TYPE section's lazy "Legacy Sizing/Attributes" class (
[A-Za-z0-9_ \t\n&*,()=:]*?) had no length cap and includes letters/newlines. On a type-declaration line immediately preceding an unrelated later SUBROUTINE/FUNCTION (e.g.CHARACTER(len=*), INTENT(IN) :: messagea few lines before that same subroutine's ownEND SUBROUTINE wrf_message), backtracking let the class swallow the entire intervening body -- including the trailing END statement -- and match keyword+identifier there instead. This produced a phantom duplicatefunction_datarow whosestart_line/argsbelonged to a type declaration, not any real definition.Confirmed reproducing the issue's exact symptoms against
language-crucible/data/fortran/wrf/module_initialize_real.F:wrf_message(phantom start_line 5581 vs. real 5580, args=1 vs. real 2) andconst_module_initialize, plus a third previously-unreported instance of the same bug shape in the same file (wrf_error_fatal).Fix
Bounded the attribute class to
{0,40}chars.A worthwhile detour on the way here: a per-character
(?!\bEND\b)exclusion was tried first -- more semantically precise (targets the actual mechanism instead of an arbitrary length) -- but empirically disables Pythonre's internal fast path for what would otherwise be a plain bounded character class. Direct timing on a payload of ~3000 repeated type-declaration lines followed by one distantEND(a realistic adversarial shape, not synthetic): 0.01s with a plain numeric bound vs. ~13s with the lookahead -- a real ReDoS regression that would have shipped if not caught by this PR's own probe before pushing. Went with the numeric bound instead, verified against the same payload.Test plan
pytest tests/extraction/languages/test_fortran.py-- 35 passedtree_sitter_accuracy_audit.py --all --ci-- clean across all 31 languagescrucible_check.py --mode both-- PASS/PASS after blessing expected drift (3 phantom duplicates removed from the corpus)audit_check.py --ci-- all clear🤖 Generated with Claude Code