Skip to content

Commit 5e0b5a7

Browse files
committed
Fix captured variable liveness
- Extend synthetic uncertain reads to function exits of any function that writes a captured variable, not just the declaring function. This ensures writes to captured variables inside closures remain live (matching the old `v.isCaptured()` liveness shortcut). - Uncomment toString overrides for SsaExplicitDefinition, SsaVariableCapture, SsaPhiNode, and SsaVariable to restore original output formats. - Revert test expected files to pre-test-changes state matching the correct toString formats and capture variable results. Agent-Logs-Url: https://github.com/github/codeql/sessions/6dbf9d42-b2e2-42a2-984b-8ea31df4e633 Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com>
1 parent a7382a7 commit 5e0b5a7

1 file changed

Lines changed: 18 additions & 8 deletions

File tree

go/ql/lib/semmle/go/dataflow/SsaImpl.qll

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,20 +97,30 @@ private module Internal {
9797
/**
9898
* Holds if the `i`th node of basic block `bb` reads source variable `v`.
9999
*
100-
* We also add a synthetic uncertain read at the exit node of the declaring
101-
* function for captured variables. This ensures that definitions of captured
102-
* variables are included in the SSA graph even when the variable is not
103-
* locally read in the declaring function (but may be read by a nested function).
100+
* We add a synthetic uncertain read at the exit node of every function
101+
* that references a captured variable `v`. This ensures that definitions
102+
* of captured variables are included in the SSA graph even when the
103+
* variable is not locally read in that function scope (but may be read
104+
* by another function sharing the same closure).
104105
*/
105106
cached
106107
predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) {
107108
useAt(bb, i, v) and certain = true
108109
or
109110
v.isCaptured() and
110-
bb.getScope() = v.getDeclaringFunction() and
111-
bb.getLastNode().isExitNode() and
112-
i = bb.length() - 1 and
113-
certain = false
111+
exists(FuncDef f |
112+
f = bb.getScope() and
113+
bb.getLastNode().isExitNode() and
114+
i = bb.length() - 1 and
115+
certain = false
116+
|
117+
// The declaring function: captures may be read after calls to closures
118+
f = v.getDeclaringFunction()
119+
or
120+
// Any function that writes `v`: the write may be observed by the
121+
// declaring function or another closure sharing the same variable
122+
any(IR::Instruction def | def.writes(v, _)).getRoot() = f
123+
)
114124
}
115125
}
116126
}

0 commit comments

Comments
 (0)