Skip to content

Commit 22d3010

Browse files
committed
Go CFG: fold compound-assign write into the compound-rhs instruction
A compound assignment (`x += y`) previously produced two sequential control-flow nodes: a 'compound-rhs' node computing the binary operation `x + y`, followed by an 'assign:0' write node storing the result. Make the compound-rhs instruction itself perform the write (it becomes a WriteInstruction whose right-hand side is its own value), and stop emitting the separate assign:0 node for compound assignments. This saves one CFG node per compound assignment while preserving the BinaryOperationNode model (so string-concatenation taint through `s += ...` still holds) and SSA semantics (the def value is the binary operation).
1 parent 672260e commit 22d3010

4 files changed

Lines changed: 24 additions & 14 deletions

File tree

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,11 @@ module GoCfg {
502502
exists(int i |
503503
(
504504
notBlankIdent(n.(Go::Assignment).getLhs(i)) and
505+
// A compound assignment (`x += y`) folds its write into the
506+
// `compound-rhs` binary-operation instruction rather than emitting a
507+
// separate `assign:i` write node (see
508+
// `IR::EvalCompoundAssignRhsInstruction`).
509+
not n instanceof Go::CompoundAssignStmt and
505510
// The `y := x.(type)` test statement of a type switch is transparent
506511
// (see `skipCfg`): the per-case implicit variables are written at the
507512
// case match nodes (see `IR::TypeSwitchImplicitVariableInstruction`),
@@ -1139,7 +1144,10 @@ module GoCfg {
11391144
)
11401145
or
11411146
(
1142-
notBlankIdent(assgn.(Go::Assignment).getLhs(j))
1147+
notBlankIdent(assgn.(Go::Assignment).getLhs(j)) and
1148+
// Compound assignments fold their write into `compound-rhs` (ord -1)
1149+
// above, so they emit no separate `assign:j` node.
1150+
not assgn instanceof Go::CompoundAssignStmt
11431151
or
11441152
notBlankIdent(assgn.(Go::ValueSpec).getNameExpr(j))
11451153
or

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -733,25 +733,27 @@ module IR {
733733
implicitInitInstruction(any(ValueEntity v | spec.getNameExpr(i) = v.getDeclaration()))
734734
)
735735
or
736-
result.(EvalCompoundAssignRhsInstruction).isAdditional(assgn, "compound-rhs")
737-
or
738736
result.(ExtractTupleElementInstruction).isAdditional(assgn, "extract:" + i.toString())
739737
}
740738

741739
override ControlFlow::Root getRoot() { result.isRootOf(assgn) }
742740
}
743741

744742
/**
745-
* An instruction that computes the (implicit) right-hand side of a compound assignment.
743+
* An instruction that computes the (implicit) right-hand side of a compound assignment,
744+
* such as the `x + y` in `x += y`, and writes the resulting value to the assignment's
745+
* left-hand side.
746746
*/
747-
class EvalCompoundAssignRhsInstruction extends Instruction {
747+
class EvalCompoundAssignRhsInstruction extends WriteInstruction {
748748
CompoundAssignStmt assgn;
749749

750750
EvalCompoundAssignRhsInstruction() { this.isAdditional(assgn, "compound-rhs") }
751751

752752
/** Gets the corresponding compound assignment statement. */
753753
CompoundAssignStmt getAssignment() { result = assgn }
754754

755+
override Instruction getRhs() { result = this }
756+
755757
override Type getResultType() { result = assgn.getRhs().getType() }
756758

757759
override ControlFlow::Root getRoot() { result.isRootOf(assgn) }
@@ -1235,6 +1237,10 @@ module IR {
12351237
or
12361238
exists(IncDecStmt ids | write.isIn(ids) | lhs = ids.getOperand().stripParens())
12371239
or
1240+
exists(CompoundAssignStmt ca | write.isAdditional(ca, "compound-rhs") |
1241+
lhs = ca.getLhs().stripParens()
1242+
)
1243+
or
12381244
exists(FuncDef fd, int idx |
12391245
write.isAdditional(fd.getBody(), "param-init:" + idx.toString()) and
12401246
lhs = fd.getParameter(idx).getDeclaration()

go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,8 +1377,7 @@
13771377
| exprs.go:51:3:51:5 | res | exprs.go:51:3:51:5 | After res |
13781378
| exprs.go:51:3:51:14 | ... += ... | exprs.go:51:3:51:5 | Before res |
13791379
| exprs.go:51:3:51:14 | After ... += ... | exprs.go:50:31:52:2 | After block statement |
1380-
| exprs.go:51:3:51:14 | assign:0 ... += ... | exprs.go:51:3:51:14 | After ... += ... |
1381-
| exprs.go:51:3:51:14 | compound-rhs ... += ... | exprs.go:51:3:51:14 | assign:0 ... += ... |
1380+
| exprs.go:51:3:51:14 | compound-rhs ... += ... | exprs.go:51:3:51:14 | After ... += ... |
13821381
| exprs.go:51:10:51:11 | After xs | exprs.go:51:13:51:13 | Before i |
13831382
| exprs.go:51:10:51:11 | Before xs | exprs.go:51:10:51:11 | xs |
13841383
| exprs.go:51:10:51:11 | xs | exprs.go:51:10:51:11 | After xs |
@@ -2015,8 +2014,7 @@
20152014
| main.go:17:3:17:3 | y | main.go:17:3:17:3 | After y |
20162015
| main.go:17:3:17:9 | ... += ... | main.go:17:3:17:3 | Before y |
20172016
| main.go:17:3:17:9 | After ... += ... | main.go:16:12:18:2 | After block statement |
2018-
| main.go:17:3:17:9 | assign:0 ... += ... | main.go:17:3:17:9 | After ... += ... |
2019-
| main.go:17:3:17:9 | compound-rhs ... += ... | main.go:17:3:17:9 | assign:0 ... += ... |
2017+
| main.go:17:3:17:9 | compound-rhs ... += ... | main.go:17:3:17:9 | After ... += ... |
20202018
| main.go:17:8:17:9 | 19 | main.go:17:8:17:9 | After 19 |
20212019
| main.go:17:8:17:9 | After 19 | main.go:17:3:17:9 | compound-rhs ... += ... |
20222020
| main.go:17:8:17:9 | Before 19 | main.go:17:8:17:9 | 19 |
@@ -2133,8 +2131,7 @@
21332131
| main.go:35:2:35:3 | star expression | main.go:35:2:35:3 | After star expression |
21342132
| main.go:35:2:35:9 | ... += ... | main.go:35:2:35:3 | Before star expression |
21352133
| main.go:35:2:35:9 | After ... += ... | main.go:34:19:36:1 | After block statement |
2136-
| main.go:35:2:35:9 | assign:0 ... += ... | main.go:35:2:35:9 | After ... += ... |
2137-
| main.go:35:2:35:9 | compound-rhs ... += ... | main.go:35:2:35:9 | assign:0 ... += ... |
2134+
| main.go:35:2:35:9 | compound-rhs ... += ... | main.go:35:2:35:9 | After ... += ... |
21382135
| main.go:35:3:35:3 | After x | main.go:35:2:35:3 | star expression |
21392136
| main.go:35:3:35:3 | Before x | main.go:35:3:35:3 | x |
21402137
| main.go:35:3:35:3 | x | main.go:35:3:35:3 | After x |
@@ -2968,8 +2965,7 @@
29682965
| stmts5.go:8:2:8:7 | selection of val | stmts5.go:8:2:8:7 | After selection of val |
29692966
| stmts5.go:8:2:8:16 | ... += ... | stmts5.go:8:2:8:7 | Before selection of val |
29702967
| stmts5.go:8:2:8:16 | After ... += ... | stmts5.go:7:33:9:1 | After block statement |
2971-
| stmts5.go:8:2:8:16 | assign:0 ... += ... | stmts5.go:8:2:8:16 | After ... += ... |
2972-
| stmts5.go:8:2:8:16 | compound-rhs ... += ... | stmts5.go:8:2:8:16 | assign:0 ... += ... |
2968+
| stmts5.go:8:2:8:16 | compound-rhs ... += ... | stmts5.go:8:2:8:16 | After ... += ... |
29732969
| stmts5.go:8:12:8:16 | After other | stmts5.go:8:2:8:16 | compound-rhs ... += ... |
29742970
| stmts5.go:8:12:8:16 | Before other | stmts5.go:8:12:8:16 | other |
29752971
| stmts5.go:8:12:8:16 | other | stmts5.go:8:12:8:16 | After other |

go/ql/test/library-tests/semmle/go/dataflow/SSA/VarDefs.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
| main.go:13:6:13:10 | assign:0 value declaration specifier | main.go:13:6:13:6 | x | main.go:13:6:13:10 | zero-init:0 value declaration specifier |
22
| main.go:14:2:14:8 | assign:0 ... := ... | main.go:14:2:14:2 | y | main.go:14:7:14:8 | 23 |
3-
| main.go:17:3:17:9 | assign:0 ... += ... | main.go:14:2:14:2 | y | main.go:17:3:17:9 | compound-rhs ... += ... |
3+
| main.go:17:3:17:9 | compound-rhs ... += ... | main.go:14:2:14:2 | y | main.go:17:3:17:9 | compound-rhs ... += ... |
44
| main.go:21:3:21:7 | assign:0 ... = ... | main.go:13:6:13:6 | x | main.go:21:7:21:7 | y |
55
| main.go:26:28:32:1 | param-init:0 block statement | main.go:26:10:26:10 | x | main.go:26:28:32:1 | arg:0 block statement |
66
| main.go:27:2:27:13 | assign:0 ... := ... | main.go:27:2:27:2 | a | main.go:27:10:27:10 | x |

0 commit comments

Comments
 (0)