Skip to content

Commit e467cf6

Browse files
Copilotowen-mc
andcommitted
Make Go use the shared SSA library (codeql.ssa.Ssa)
Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/codeql/sessions/b400ebd5-4095-401e-8811-fb550600b3c4
1 parent e3bcd3b commit e467cf6

File tree

5 files changed

+156
-478
lines changed

5 files changed

+156
-478
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The Go SSA library now uses the shared SSA library (`codeql.ssa.Ssa`), consistent with other CodeQL languages such as C#, Java, Ruby, Rust, and Swift. This may result in minor changes to SSA construction in some edge cases.

go/ql/lib/qlpack.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ dependencies:
1010
codeql/controlflow: ${workspace}
1111
codeql/dataflow: ${workspace}
1212
codeql/mad: ${workspace}
13+
codeql/ssa: ${workspace}
1314
codeql/threat-models: ${workspace}
1415
codeql/tutorial: ${workspace}
1516
codeql/util: ${workspace}

go/ql/lib/semmle/go/controlflow/BasicBlocks.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,17 @@ class BasicBlock = BbImpl::BasicBlock;
4848

4949
class EntryBasicBlock = BbImpl::EntryBasicBlock;
5050

51+
/** Provides a `CfgSig` view of Go's control-flow graph for use with the shared SSA library. */
52+
module Cfg implements BB::CfgSig<Location> {
53+
class ControlFlowNode = BbImpl::ControlFlowNode;
54+
55+
class BasicBlock = BbImpl::BasicBlock;
56+
57+
class EntryBasicBlock = BbImpl::EntryBasicBlock;
58+
59+
predicate dominatingEdge = BbImpl::dominatingEdge/2;
60+
}
61+
5162
cached
5263
private predicate reachableBB(BasicBlock bb) {
5364
bb instanceof EntryBasicBlock

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

Lines changed: 49 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ private predicate unresolvedIdentifier(Ident id, string name) {
6363
/**
6464
* An SSA variable.
6565
*/
66-
class SsaVariable extends TSsaDefinition {
66+
class SsaVariable extends Definition {
6767
/** Gets the source variable corresponding to this SSA variable. */
68-
SsaSourceVariable getSourceVariable() { result = this.(SsaDefinition).getSourceVariable() }
68+
SsaSourceVariable getSourceVariable() { this.definesAt(result, _, _) }
6969

7070
/** Gets the (unique) definition of this SSA variable. */
7171
SsaDefinition getDefinition() { result = this }
@@ -74,21 +74,31 @@ class SsaVariable extends TSsaDefinition {
7474
Type getType() { result = this.getSourceVariable().getType() }
7575

7676
/** Gets a use in basic block `bb` that refers to this SSA variable. */
77-
IR::Instruction getAUseIn(ReachableBasicBlock bb) {
77+
IR::Instruction getAUseIn(BasicBlock bb) {
7878
exists(int i, SsaSourceVariable v | v = this.getSourceVariable() |
7979
result = bb.getNode(i) and
80-
this = getDefinition(bb, i, v)
80+
ssaDefReachesRead(v, this, bb, i) and
81+
useAt(bb, i, v)
8182
)
8283
}
8384

8485
/** Gets a use that refers to this SSA variable. */
8586
IR::Instruction getAUse() { result = this.getAUseIn(_) }
8687

87-
/** Gets a textual representation of this element. */
88-
string toString() { result = this.getDefinition().prettyPrintRef() }
88+
/**
89+
* Gets a textual representation of this element.
90+
*
91+
* The format is `kind@LINE:COL`, where `kind` is one of `def`, `capture`, or `phi`.
92+
*/
93+
override string toString() {
94+
exists(Location loc | loc = this.(SsaDefinition).getLocation() |
95+
result =
96+
this.(SsaDefinition).getKind() + "@" + loc.getStartLine() + ":" + loc.getStartColumn()
97+
)
98+
}
8999

90100
/** Gets the location of this SSA variable. */
91-
Location getLocation() { result = this.getDefinition().getLocation() }
101+
Location getLocation() { result = this.(SsaDefinition).getLocation() }
92102

93103
/**
94104
* DEPRECATED: Use `getLocation()` instead.
@@ -109,50 +119,28 @@ class SsaVariable extends TSsaDefinition {
109119
/**
110120
* An SSA definition.
111121
*/
112-
class SsaDefinition extends TSsaDefinition {
122+
class SsaDefinition extends Definition {
113123
/** Gets the SSA variable defined by this definition. */
114124
SsaVariable getVariable() { result = this }
115125

116126
/** Gets the source variable defined by this definition. */
117-
abstract SsaSourceVariable getSourceVariable();
127+
SsaSourceVariable getSourceVariable() { this.definesAt(result, _, _) }
118128

119129
/**
120130
* Gets the basic block to which this definition belongs.
121131
*/
122-
abstract ReachableBasicBlock getBasicBlock();
132+
BasicBlock getBasicBlock() { this.definesAt(_, result, _) }
123133

124-
/**
125-
* INTERNAL: Use `getBasicBlock()` and `getSourceVariable()` instead.
126-
*
127-
* Holds if this is a definition of source variable `v` at index `idx` in basic block `bb`.
128-
*
129-
* Phi nodes are considered to be at index `-1`, all other definitions at the index of
130-
* the control flow node they correspond to.
131-
*/
132-
abstract predicate definesAt(ReachableBasicBlock bb, int idx, SsaSourceVariable v);
133-
134-
/**
135-
* INTERNAL: Use `toString()` instead.
136-
*
137-
* Gets a pretty-printed representation of this SSA definition.
138-
*/
139-
abstract string prettyPrintDef();
134+
/** Gets the innermost function or file to which this SSA definition belongs. */
135+
ControlFlow::Root getRoot() { result = this.getBasicBlock().getScope() }
140136

141137
/**
142138
* INTERNAL: Do not use.
143139
*
144-
* Gets a pretty-printed representation of a reference to this SSA definition.
140+
* Gets a short string identifying the kind of this SSA definition,
141+
* used in reference formatting (e.g., `"def"`, `"capture"`, `"phi"`).
145142
*/
146-
abstract string prettyPrintRef();
147-
148-
/** Gets the innermost function or file to which this SSA definition belongs. */
149-
ControlFlow::Root getRoot() { result = this.getBasicBlock().getScope() }
150-
151-
/** Gets a textual representation of this element. */
152-
string toString() { result = this.prettyPrintDef() }
153-
154-
/** Gets the source location for this element. */
155-
abstract Location getLocation();
143+
string getKind() { none() }
156144

157145
/**
158146
* DEPRECATED: Use `getLocation()` instead.
@@ -180,32 +168,25 @@ class SsaDefinition extends TSsaDefinition {
180168
/**
181169
* An SSA definition that corresponds to an explicit assignment or other variable definition.
182170
*/
183-
class SsaExplicitDefinition extends SsaDefinition, TExplicitDef {
171+
class SsaExplicitDefinition extends SsaDefinition, WriteDefinition {
172+
SsaExplicitDefinition() {
173+
exists(BasicBlock bb, int i, SsaSourceVariable v |
174+
this.definesAt(v, bb, i) and
175+
defAt(bb, i, v)
176+
)
177+
}
178+
184179
/** Gets the instruction where the definition happens. */
185180
IR::Instruction getInstruction() {
186-
exists(BasicBlock bb, int i | this = TExplicitDef(bb, i, _) | result = bb.getNode(i))
181+
exists(BasicBlock bb, int i | this.definesAt(_, bb, i) | result = bb.getNode(i))
187182
}
188183

189184
/** Gets the right-hand side of the definition. */
190185
IR::Instruction getRhs() { this.getInstruction().writes(_, result) }
191186

192-
override predicate definesAt(ReachableBasicBlock bb, int i, SsaSourceVariable v) {
193-
this = TExplicitDef(bb, i, v)
194-
}
195-
196-
override ReachableBasicBlock getBasicBlock() { this.definesAt(result, _, _) }
197-
198-
override SsaSourceVariable getSourceVariable() { this = TExplicitDef(_, _, result) }
199-
200-
override string prettyPrintRef() {
201-
exists(Location loc | loc = this.getLocation() |
202-
result = "def@" + loc.getStartLine() + ":" + loc.getStartColumn()
203-
)
204-
}
187+
override string getKind() { result = "def" }
205188

206-
override string prettyPrintDef() { result = "definition of " + this.getSourceVariable() }
207-
208-
override Location getLocation() { result = this.getInstruction().getLocation() }
189+
override string toString() { result = "definition of " + this.getSourceVariable() }
209190
}
210191

211192
/** Provides a helper predicate for working with explicit SSA definitions. */
@@ -220,19 +201,6 @@ module SsaExplicitDefinition {
220201
* An SSA definition that does not correspond to an explicit variable definition.
221202
*/
222203
abstract class SsaImplicitDefinition extends SsaDefinition {
223-
/**
224-
* INTERNAL: Do not use.
225-
*
226-
* Gets the definition kind to include in `prettyPrintRef`.
227-
*/
228-
abstract string getKind();
229-
230-
override string prettyPrintRef() {
231-
exists(Location loc | loc = this.getLocation() |
232-
result = this.getKind() + "@" + loc.getStartLine() + ":" + loc.getStartColumn()
233-
)
234-
}
235-
236204
override Location getLocation() { result = this.getBasicBlock().getLocation() }
237205
}
238206

@@ -243,24 +211,16 @@ abstract class SsaImplicitDefinition extends SsaDefinition {
243211
* Capturing definitions appear at the beginning of such functions, as well as
244212
* at any function call that may affect the value of the variable.
245213
*/
246-
class SsaVariableCapture extends SsaImplicitDefinition, TCapture {
247-
override predicate definesAt(ReachableBasicBlock bb, int i, SsaSourceVariable v) {
248-
this = TCapture(bb, i, v)
249-
}
250-
251-
override ReachableBasicBlock getBasicBlock() { this.definesAt(result, _, _) }
252-
253-
override SsaSourceVariable getSourceVariable() { this.definesAt(_, _, result) }
254-
255-
override string getKind() { result = "capture" }
256-
257-
override string prettyPrintDef() { result = "capture variable " + this.getSourceVariable() }
258-
214+
class SsaVariableCapture extends SsaImplicitDefinition, UncertainWriteDefinition {
259215
override Location getLocation() {
260-
exists(ReachableBasicBlock bb, int i | this.definesAt(bb, i, _) |
216+
exists(BasicBlock bb, int i | this.definesAt(_, bb, i) |
261217
result = bb.getNode(i).getLocation()
262218
)
263219
}
220+
221+
override string getKind() { result = "capture" }
222+
223+
override string toString() { result = "capture variable " + this.getSourceVariable() }
264224
}
265225

266226
/**
@@ -277,34 +237,25 @@ abstract class SsaPseudoDefinition extends SsaImplicitDefinition {
277237
* Gets a textual representation of the inputs of this pseudo-definition
278238
* in lexicographical order.
279239
*/
280-
string ppInputs() { result = concat(this.getAnInput().getDefinition().prettyPrintRef(), ", ") }
240+
string ppInputs() {
241+
result =
242+
concat(SsaVariable inp | inp = this.getAnInput() | inp.toString() order by inp.toString())
243+
}
281244
}
282245

283246
/**
284247
* An SSA phi node, that is, a pseudo-definition for a variable at a point
285248
* in the flow graph where otherwise two or more definitions for the variable
286249
* would be visible.
287250
*/
288-
class SsaPhiNode extends SsaPseudoDefinition, TPhi {
289-
override SsaVariable getAnInput() {
290-
result = getDefReachingEndOf(this.getBasicBlock().getAPredecessor(_), this.getSourceVariable())
291-
}
292-
293-
override predicate definesAt(ReachableBasicBlock bb, int i, SsaSourceVariable v) {
294-
bb = this.getBasicBlock() and v = this.getSourceVariable() and i = -1
295-
}
296-
297-
override ReachableBasicBlock getBasicBlock() { this = TPhi(result, _) }
298-
299-
override SsaSourceVariable getSourceVariable() { this = TPhi(_, result) }
251+
class SsaPhiNode extends SsaPseudoDefinition, PhiNode {
252+
override SsaVariable getAnInput() { phiHasInputFromBlock(this, result, _) }
300253

301254
override string getKind() { result = "phi" }
302255

303-
override string prettyPrintDef() {
256+
override string toString() {
304257
result = this.getSourceVariable() + " = phi(" + this.ppInputs() + ")"
305258
}
306-
307-
override Location getLocation() { result = this.getBasicBlock().getLocation() }
308259
}
309260

310261
/**

0 commit comments

Comments
 (0)