fix(sdg): wire actual_in ports from formal_in, not from CFG ENTRY (#220) - #221
Merged
Merged
Conversation
The SDG edges were correct: param_in carried actual_in -> formal_in and the summary edges were present. The DDG feeding them was wired wrong on the way in. Reaching-def analysis names the synthetic CFG ENTRY node as the definition site of every parameter, capture and read global. build_formals already remapped that source onto the matching formal_in vertex. build_actuals runs after it, allocates the actual_in ports, and wired them straight from _defs_reaching_call_matching, which returns the raw source. For a parameter that source is ENTRY, and nothing remapped it, so the graph got ENTRY -> actual_in:N where it needed formal_in:N -> actual_in:N. The return direction was already correct, and that asymmetry is the tell. Because resolve_value returns the formal_in vertex, a forward walk started at a vertex with no path to the argument port, never crossed PARAM_IN, and stopped inside the caller. slice_forward stayed in one function, flows_to_call and flows_to_argument answered False, paths_between returned nothing, and taint() reported a clean exhausted with complete=True and an empty ledger, which is a refutation for a flow visible in three lines of source. Extract the ENTRY-to-formal_in lookup into formal_for(var) on the assembler and use it from both build_formals and build_actuals, for argument ports and global-read ports. Fall back to the original source when the variable is not one of the callable's formals, so no edge is lost. On the reproducer in #220 a two-boundary flow now resolves end to end and taint returns the witness. Regression test asserts an actual_in port carrying a parameter is fed from formal_in and never from ENTRY; it fails on 1.5.3. Full suite: 520 passed, 6 skipped. Closes #220
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 #220.
build_actualswired eachactual_inport from the raw reaching-def source. For aparameter that source is the synthetic CFG ENTRY node, so the graph got
ENTRY -> actual_in:Nwhere it neededformal_in:N -> actual_in:N.build_formalsalready had the correct remap;
build_actualsdid not use it.Since
resolve_valuereturns theformal_invertex, every forward walk started at avertex with no path to the argument port and stopped inside the caller.
taint()therefore returned a clean
exhaustedfor real flows.Before / after on the #220 reproducer
slice_forward("request").totalflows_to_callFalseTruepaths_between(request -> raw)[]taint(...).exhausted[('request','raw')][]This matches what
codeanalyzer-typescript1.6.0 already answers for the identical fixture.Regression test in
test/test_dataflow_sdg.pyfails on 1.5.3 and passes here.Full suite: 520 passed, 6 skipped.
Also bumps to 1.5.4 with a CHANGELOG entry.