Skip to content

Commit a7382a7

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 9895d10 commit a7382a7

5 files changed

Lines changed: 148 additions & 495 deletions

File tree

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: 28 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -63,33 +63,25 @@ private predicate unresolvedIdentifier(Ident id, string name) {
6363
/**
6464
* An SSA variable.
6565
*/
66-
class SsaVariable extends TSsaDefinition {
67-
/** Gets the source variable corresponding to this SSA variable. */
68-
SsaSourceVariable getSourceVariable() { result = this.(SsaDefinition).getSourceVariable() }
69-
66+
class SsaVariable extends Definition {
7067
/** Gets the (unique) definition of this SSA variable. */
7168
SsaDefinition getDefinition() { result = this }
7269

7370
/** Gets the type of this SSA variable. */
7471
Type getType() { result = this.getSourceVariable().getType() }
7572

7673
/** Gets a use in basic block `bb` that refers to this SSA variable. */
77-
IR::Instruction getAUseIn(ReachableBasicBlock bb) {
74+
IR::Instruction getAUseIn(BasicBlock bb) {
7875
exists(int i, SsaSourceVariable v | v = this.getSourceVariable() |
7976
result = bb.getNode(i) and
80-
this = getDefinition(bb, i, v)
77+
ssaDefReachesRead(v, this, bb, i) and
78+
useAt(bb, i, v)
8179
)
8280
}
8381

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

87-
/** Gets a textual representation of this element. */
88-
string toString() { result = this.getDefinition().toString() }
89-
90-
/** Gets the location of this SSA variable. */
91-
Location getLocation() { result = this.getDefinition().getLocation() }
92-
9385
/**
9486
* DEPRECATED: Use `getLocation()` instead.
9587
*
@@ -109,50 +101,20 @@ class SsaVariable extends TSsaDefinition {
109101
/**
110102
* An SSA definition.
111103
*/
112-
class SsaDefinition extends TSsaDefinition {
104+
class SsaDefinition extends Definition {
113105
/** Gets the SSA variable defined by this definition. */
114106
SsaVariable getVariable() { result = this }
115107

116-
/** Gets the source variable defined by this definition. */
117-
abstract SsaSourceVariable getSourceVariable();
118-
119-
/**
120-
* Gets the basic block to which this definition belongs.
121-
*/
122-
abstract ReachableBasicBlock getBasicBlock();
123-
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();
108+
/** Gets the innermost function or file to which this SSA definition belongs. */
109+
ControlFlow::Root getRoot() { result = this.getBasicBlock().getScope() }
140110

141111
/**
142112
* INTERNAL: Do not use.
143113
*
144-
* Gets a pretty-printed representation of a reference to this SSA definition.
114+
* Gets a short string identifying the kind of this SSA definition,
115+
* used in reference formatting (e.g., `"def"`, `"capture"`, `"phi"`).
145116
*/
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();
117+
string getKind() { none() }
156118

157119
/**
158120
* DEPRECATED: Use `getLocation()` instead.
@@ -180,32 +142,23 @@ class SsaDefinition extends TSsaDefinition {
180142
/**
181143
* An SSA definition that corresponds to an explicit assignment or other variable definition.
182144
*/
183-
class SsaExplicitDefinition extends SsaDefinition, TExplicitDef {
145+
class SsaExplicitDefinition extends SsaDefinition, WriteDefinition {
146+
SsaExplicitDefinition() {
147+
exists(BasicBlock bb, int i, SsaSourceVariable v |
148+
this.definesAt(v, bb, i) and
149+
defAt(bb, i, v)
150+
)
151+
}
152+
184153
/** Gets the instruction where the definition happens. */
185154
IR::Instruction getInstruction() {
186-
exists(BasicBlock bb, int i | this = TExplicitDef(bb, i, _) | result = bb.getNode(i))
155+
exists(BasicBlock bb, int i | this.definesAt(_, bb, i) | result = bb.getNode(i))
187156
}
188157

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

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-
}
205-
206-
override string prettyPrintDef() { result = "SSA def(" + this.getSourceVariable() + ")" }
207-
208-
override Location getLocation() { result = this.getInstruction().getLocation() }
161+
override string getKind() { result = "def" }
209162
}
210163

211164
/** Provides a helper predicate for working with explicit SSA definitions. */
@@ -219,22 +172,7 @@ module SsaExplicitDefinition {
219172
/**
220173
* An SSA definition that does not correspond to an explicit variable definition.
221174
*/
222-
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-
236-
override Location getLocation() { result = this.getBasicBlock().getLocation() }
237-
}
175+
abstract class SsaImplicitDefinition extends SsaDefinition { }
238176

239177
/**
240178
* An SSA definition representing the capturing of an SSA-convertible variable
@@ -243,24 +181,8 @@ abstract class SsaImplicitDefinition extends SsaDefinition {
243181
* Capturing definitions appear at the beginning of such functions, as well as
244182
* at any function call that may affect the value of the variable.
245183
*/
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-
184+
class SsaVariableCapture extends SsaImplicitDefinition, UncertainWriteDefinition {
255185
override string getKind() { result = "capture" }
256-
257-
override string prettyPrintDef() { result = "SSA def(" + this.getSourceVariable() + ")" }
258-
259-
override Location getLocation() {
260-
exists(ReachableBasicBlock bb, int i | this.definesAt(bb, i, _) |
261-
result = bb.getNode(i).getLocation()
262-
)
263-
}
264186
}
265187

266188
/**
@@ -277,32 +199,21 @@ abstract class SsaPseudoDefinition extends SsaImplicitDefinition {
277199
* Gets a textual representation of the inputs of this pseudo-definition
278200
* in lexicographical order.
279201
*/
280-
string ppInputs() { result = concat(this.getAnInput().getDefinition().prettyPrintRef(), ", ") }
202+
string ppInputs() {
203+
result =
204+
concat(SsaVariable inp | inp = this.getAnInput() | inp.toString() order by inp.toString())
205+
}
281206
}
282207

283208
/**
284209
* An SSA phi node, that is, a pseudo-definition for a variable at a point
285210
* in the flow graph where otherwise two or more definitions for the variable
286211
* would be visible.
287212
*/
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) }
213+
class SsaPhiNode extends SsaPseudoDefinition, PhiNode {
214+
override SsaVariable getAnInput() { phiHasInputFromBlock(this, result, _) }
300215

301216
override string getKind() { result = "phi" }
302-
303-
override string prettyPrintDef() { result = "SSA phi(" + this.getSourceVariable() + ")" }
304-
305-
override Location getLocation() { result = this.getBasicBlock().getLocation() }
306217
}
307218

308219
/**

0 commit comments

Comments
 (0)