Skip to content

SSA: Tolerate multiple variables being read at the same CFG node - #22744

Draft
asgerf wants to merge 11 commits into
github:mainfrom
asgerf:ssa/multiple-reads-v2
Draft

asgerf wants to merge 11 commits into
github:mainfrom
asgerf:ssa/multiple-reads-v2

Conversation

@asgerf

@asgerf asgerf commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes an issue with the SSA data-flow integration library that occurs when multiple variables are read at the same CFG node.

This situation happens when it is instantiated from the VariableCapture library. It also happens in unified due to how we use synthetic reads to mark post-update positions.

The assumption has a subtle presence in this bit of code:

    private class ReadNodeImpl extends ExprNodeImpl {
      private BasicBlock bb_;
      private int i_;

      ReadNodeImpl() {
        variableRead(bb_, i_, v_, true) and
        this.getExpr().hasCfgNode(bb_, i_)
      }

Since this is only bound to the expression, the variableRead join can fan out to multiple variables when not uniquely determined by the CFG node. Even if Expr is constructed to be unique to the variable, the correlation is forgotten in this join and they get mixed up anyway. The fix was to make TExprNode unique to a specific variable.

The real culprit is the Expr class from the data-flow integration input. I propose we remove Expr entirely and replace ExprNode with ReadNode: a canonical representative for a (bb,i,v) triple from the variableRead input, and likewise for PostUpdateNode. But such a change is too large for this PR as it requires language-specific refactorings. But the fix is essentially a precursor to this solution: when Expr is a CFG node, ReadNode is effectively a (bb,i,v) triple as it rightfully should be.

Comment on lines +2150 to +2152
/**
* The given `read` reads multiple variables at once. `var` is bound to one of them.
*/
asgerf added 5 commits October 2, 2026 16:22
These consistency violations originate from the VariableCapture instantation
in C#, JS, Python, and Ruby.
The previously-added consistency errors are gone.
Unified also had consistency errors from its LocalSSA instantiation, due
to its use of synthetic read nodes to represent post-update positions.
Many variables can have a post-update at the same CFG node.
@asgerf
asgerf force-pushed the ssa/multiple-reads-v2 branch from bfd363b to 1bbd5d5 Compare October 2, 2026 14:23
asgerf added 3 commits October 2, 2026 16:47
Many tests passed for the wrong reasons, due to the SSA bug.

We need more library/operator modelling to actually find these flows.
Switched to TaintTracking and adds some very ad-hoc steps to recover
most of the results.

Some more tests pass and others fail; these are now consistent with what
we actually model.
@asgerf
asgerf force-pushed the ssa/multiple-reads-v2 branch 2 times, most recently from 1bbd5d5 to 2c637ab Compare October 2, 2026 14:55
@asgerf asgerf changed the title Ssa/multiple reads v2 SSA: Tolerate multiple variables being read at the same CFG node Oct 2, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants