Skip to content

Commit 6f96d17

Browse files
geoffw0geoffw0
authored
Merge pull request github#2077 from jbj/cfg-enable-pr
C++: enable the QL-based CFG code
2 parents 7127624 + 01a3a03 commit 6f96d17

File tree

9 files changed

+72
-153
lines changed

9 files changed

+72
-153
lines changed

‎change-notes/1.23/analysis-cpp.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,7 @@ The following changes in version 1.23 affect C/C++ analysis in all applications.
4444
clarity (e.g. `isOutReturnPointer()` to `isReturnValueDeref()`). The existing member predicates
4545
have been deprecated, and will be removed in a future release. Code that uses the old member
4646
predicates should be updated to use the corresponding new member predicate.
47+
* The control-flow graph is now computed in QL, not in the extractor. This can
48+
lead to regressions (or improvements) in how queries are optimized because
49+
optimization in QL relies on static size estimates, and the control-flow edge
50+
relations will now have different size estimates than before.

‎cpp/ql/src/semmle/code/cpp/controlflow/ControlFlowGraph.qll

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import cpp
22
import BasicBlocks
33
private import semmle.code.cpp.controlflow.internal.ConstantExprs
4+
private import semmle.code.cpp.controlflow.internal.CFG
45

56
/**
67
* A control-flow node is either a statement or an expression; in addition,
@@ -86,11 +87,11 @@ import ControlFlowGraphPublic
8687
class ControlFlowNodeBase extends ElementBase, @cfgnode { }
8788

8889
predicate truecond_base(ControlFlowNodeBase n1, ControlFlowNodeBase n2) {
89-
truecond(unresolveElement(n1), unresolveElement(n2))
90+
qlCFGTrueSuccessor(n1, n2)
9091
}
9192

9293
predicate falsecond_base(ControlFlowNodeBase n1, ControlFlowNodeBase n2) {
93-
falsecond(unresolveElement(n1), unresolveElement(n2))
94+
qlCFGFalseSuccessor(n1, n2)
9495
}
9596

9697
/**
@@ -120,7 +121,7 @@ abstract class AdditionalControlFlowEdge extends ControlFlowNodeBase {
120121
* `AdditionalControlFlowEdge`. Use this relation instead of `successors`.
121122
*/
122123
predicate successors_extended(ControlFlowNodeBase source, ControlFlowNodeBase target) {
123-
successors(unresolveElement(source), unresolveElement(target))
124+
qlCFGSuccessor(source, target)
124125
or
125126
source.(AdditionalControlFlowEdge).getAnEdgeTarget() = target
126127
}

‎cpp/ql/src/semmle/code/cpp/controlflow/internal/CFG.qll

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,21 @@ private predicate subEdge(Pos p1, Node n1, Node n2, Pos p2) {
851851
p2.nodeAfter(n2, s)
852852
)
853853
or
854+
// ConstexprIfStmt -> condition ; { then, else } -> // same as IfStmt
855+
exists(ConstexprIfStmt s |
856+
p1.nodeAt(n1, s) and
857+
p2.nodeBefore(n2, s.getCondition())
858+
or
859+
p1.nodeAfter(n1, s.getThen()) and
860+
p2.nodeBeforeDestructors(n2, s)
861+
or
862+
p1.nodeAfter(n1, s.getElse()) and
863+
p2.nodeBeforeDestructors(n2, s)
864+
or
865+
p1.nodeAfterDestructors(n1, s) and
866+
p2.nodeAfter(n2, s)
867+
)
868+
or
854869
// WhileStmt -> condition ; body -> condition ; after dtors -> after
855870
exists(WhileStmt s |
856871
p1.nodeAt(n1, s) and
@@ -1138,9 +1153,8 @@ private class ExceptionSource extends Node {
11381153
}
11391154

11401155
/**
1141-
* Holds if `test` is the test of a control-flow construct that will always
1142-
* have true/false sub-edges out of it, where the `truth`-sub-edge goes to
1143-
* `(n2, p2)`.
1156+
* Holds if `test` is the test of a control-flow construct where the `truth`
1157+
* sub-edge goes to `(n2, p2)`.
11441158
*/
11451159
private predicate conditionJumpsTop(Expr test, boolean truth, Node n2, Pos p2) {
11461160
exists(IfStmt s | test = s.getCondition() |
@@ -1155,6 +1169,24 @@ private predicate conditionJumpsTop(Expr test, boolean truth, Node n2, Pos p2) {
11551169
p2.nodeBeforeDestructors(n2, s)
11561170
)
11571171
or
1172+
exists(ConstexprIfStmt s, string cond |
1173+
test = s.getCondition() and
1174+
cond = test.getFullyConverted().getValue()
1175+
|
1176+
truth = true and
1177+
cond != "0" and
1178+
p2.nodeBefore(n2, s.getThen())
1179+
or
1180+
truth = false and
1181+
cond = "0" and
1182+
p2.nodeBefore(n2, s.getElse())
1183+
or
1184+
not exists(s.getElse()) and
1185+
truth = false and
1186+
cond = "0" and
1187+
p2.nodeBeforeDestructors(n2, s)
1188+
)
1189+
or
11581190
exists(Loop l |
11591191
(
11601192
l instanceof WhileStmt

‎cpp/ql/src/semmle/code/cpp/exprs/ArithmeticOperation.qll

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ class UnaryPlusExpr extends UnaryArithmeticOperation, @unaryplusexpr {
3232
*/
3333
class ConjugationExpr extends UnaryArithmeticOperation, @conjugation {
3434
override string getOperator() { result = "~" }
35+
36+
override string getCanonicalQLClass() { result = "ConjugationExpr" }
3537
}
3638

3739
/**
@@ -142,13 +144,17 @@ class PostfixDecrExpr extends DecrementOperation, PostfixCrementOperation, @post
142144
*/
143145
class RealPartExpr extends UnaryArithmeticOperation, @realpartexpr {
144146
override string getOperator() { result = "__real" }
147+
148+
override string getCanonicalQLClass() { result = "RealPartExpr" }
145149
}
146150

147151
/**
148152
* A C/C++ GNU imaginary part expression.
149153
*/
150154
class ImaginaryPartExpr extends UnaryArithmeticOperation, @imagpartexpr {
151155
override string getOperator() { result = "__imag" }
156+
157+
override string getCanonicalQLClass() { result = "ImaginaryPartExpr" }
152158
}
153159

154160
/**
@@ -217,6 +223,8 @@ class RemExpr extends BinaryArithmeticOperation, @remexpr {
217223
class ImaginaryMulExpr extends BinaryArithmeticOperation, @jmulexpr {
218224
override string getOperator() { result = "*" }
219225

226+
override string getCanonicalQLClass() { result = "ImaginaryMulExpr" }
227+
220228
override int getPrecedence() { result = 13 }
221229
}
222230

@@ -226,6 +234,8 @@ class ImaginaryMulExpr extends BinaryArithmeticOperation, @jmulexpr {
226234
class ImaginaryDivExpr extends BinaryArithmeticOperation, @jdivexpr {
227235
override string getOperator() { result = "/" }
228236

237+
override string getCanonicalQLClass() { result = "ImaginaryDivExpr" }
238+
229239
override int getPrecedence() { result = 13 }
230240
}
231241

@@ -235,6 +245,8 @@ class ImaginaryDivExpr extends BinaryArithmeticOperation, @jdivexpr {
235245
class RealImaginaryAddExpr extends BinaryArithmeticOperation, @fjaddexpr {
236246
override string getOperator() { result = "+" }
237247

248+
override string getCanonicalQLClass() { result = "RealImaginaryAddExpr" }
249+
238250
override int getPrecedence() { result = 12 }
239251
}
240252

@@ -244,6 +256,8 @@ class RealImaginaryAddExpr extends BinaryArithmeticOperation, @fjaddexpr {
244256
class ImaginaryRealAddExpr extends BinaryArithmeticOperation, @jfaddexpr {
245257
override string getOperator() { result = "+" }
246258

259+
override string getCanonicalQLClass() { result = "ImaginaryRealAddExpr" }
260+
247261
override int getPrecedence() { result = 12 }
248262
}
249263

@@ -253,6 +267,8 @@ class ImaginaryRealAddExpr extends BinaryArithmeticOperation, @jfaddexpr {
253267
class RealImaginarySubExpr extends BinaryArithmeticOperation, @fjsubexpr {
254268
override string getOperator() { result = "-" }
255269

270+
override string getCanonicalQLClass() { result = "RealImaginarySubExpr" }
271+
256272
override int getPrecedence() { result = 12 }
257273
}
258274

@@ -262,6 +278,8 @@ class RealImaginarySubExpr extends BinaryArithmeticOperation, @fjsubexpr {
262278
class ImaginaryRealSubExpr extends BinaryArithmeticOperation, @jfsubexpr {
263279
override string getOperator() { result = "-" }
264280

281+
override string getCanonicalQLClass() { result = "ImaginaryRealSubExpr" }
282+
265283
override int getPrecedence() { result = 12 }
266284
}
267285

@@ -270,13 +288,17 @@ class ImaginaryRealSubExpr extends BinaryArithmeticOperation, @jfsubexpr {
270288
*/
271289
class MinExpr extends BinaryArithmeticOperation, @minexpr {
272290
override string getOperator() { result = "<?" }
291+
292+
override string getCanonicalQLClass() { result = "MinExpr" }
273293
}
274294

275295
/**
276296
* A C/C++ GNU max expression.
277297
*/
278298
class MaxExpr extends BinaryArithmeticOperation, @maxexpr {
279299
override string getOperator() { result = ">?" }
300+
301+
override string getCanonicalQLClass() { result = "MaxExpr" }
280302
}
281303

282304
/**

‎cpp/ql/src/semmle/code/cpp/stmts/Stmt.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,8 @@ class IfStmt extends ConditionalStmt, @stmt_if {
294294
* ```
295295
*/
296296
class ConstexprIfStmt extends ConditionalStmt, @stmt_constexpr_if {
297+
override string getCanonicalQLClass() { result = "ConstexprIfStmt" }
298+
297299
/**
298300
* Gets the condition expression of this 'constexpr if' statement.
299301
*

‎cpp/ql/test/library-tests/complex_numbers/expr.expected

Lines changed: 0 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -1,177 +1,35 @@
1-
| conjugation.c:3:5:3:5 | x | AnalysedExpr |
2-
| conjugation.c:3:5:3:5 | x | CompileTimeVariableExpr |
3-
| conjugation.c:3:5:3:5 | x | DefOrUse |
41
| conjugation.c:3:5:3:5 | x | VariableAccess |
5-
| conjugation.c:3:5:3:10 | ... = ... | AnalysedExpr |
62
| conjugation.c:3:5:3:10 | ... = ... | AssignExpr |
7-
| conjugation.c:3:5:3:10 | ... = ... | CompileTimeVariableExpr |
8-
| conjugation.c:3:5:3:10 | ... = ... | Def |
9-
| conjugation.c:3:5:3:10 | ... = ... | ExprInVoidContext |
10-
| conjugation.c:3:5:3:10 | ... = ... | NameQualifiableElement |
11-
| conjugation.c:3:5:3:10 | ... = ... | RangeSsaDefinition |
12-
| conjugation.c:3:5:3:10 | ... = ... | SsaDefinition |
13-
| conjugation.c:3:9:3:10 | ~ ... | AnalysedExpr |
14-
| conjugation.c:3:9:3:10 | ~ ... | CompileTimeVariableExpr |
153
| conjugation.c:3:9:3:10 | ~ ... | ConjugationExpr |
16-
| conjugation.c:3:9:3:10 | ~ ... | DefOrUse |
17-
| conjugation.c:3:9:3:10 | ~ ... | NameQualifiableElement |
18-
| conjugation.c:3:10:3:10 | x | AnalysedExpr |
19-
| conjugation.c:3:10:3:10 | x | CompileTimeVariableExpr |
20-
| conjugation.c:3:10:3:10 | x | Use |
214
| conjugation.c:3:10:3:10 | x | VariableAccess |
22-
| test.c:5:5:5:5 | z | AnalysedExpr |
23-
| test.c:5:5:5:5 | z | CompileTimeVariableExpr |
24-
| test.c:5:5:5:5 | z | DefOrUse |
255
| test.c:5:5:5:5 | z | VariableAccess |
26-
| test.c:5:5:5:13 | ... = ... | AnalysedExpr |
276
| test.c:5:5:5:13 | ... = ... | AssignExpr |
28-
| test.c:5:5:5:13 | ... = ... | CompileTimeVariableExpr |
29-
| test.c:5:5:5:13 | ... = ... | Def |
30-
| test.c:5:5:5:13 | ... = ... | ExprInVoidContext |
31-
| test.c:5:5:5:13 | ... = ... | NameQualifiableElement |
32-
| test.c:5:5:5:13 | ... = ... | RangeSsaDefinition |
33-
| test.c:5:5:5:13 | ... = ... | SsaDefinition |
34-
| test.c:5:9:5:9 | x | AnalysedExpr |
35-
| test.c:5:9:5:9 | x | CompileTimeVariableExpr |
36-
| test.c:5:9:5:9 | x | Use |
377
| test.c:5:9:5:9 | x | VariableAccess |
38-
| test.c:5:9:5:13 | ... * ... | AnalysedExpr |
39-
| test.c:5:9:5:13 | ... * ... | CompileTimeVariableExpr |
40-
| test.c:5:9:5:13 | ... * ... | DefOrUse |
418
| test.c:5:9:5:13 | ... * ... | ImaginaryMulExpr |
42-
| test.c:5:9:5:13 | ... * ... | NameQualifiableElement |
43-
| test.c:5:13:5:13 | y | AnalysedExpr |
44-
| test.c:5:13:5:13 | y | CompileTimeVariableExpr |
45-
| test.c:5:13:5:13 | y | Use |
469
| test.c:5:13:5:13 | y | VariableAccess |
47-
| test.c:6:5:6:5 | z | AnalysedExpr |
48-
| test.c:6:5:6:5 | z | CompileTimeVariableExpr |
49-
| test.c:6:5:6:5 | z | DefOrUse |
5010
| test.c:6:5:6:5 | z | VariableAccess |
51-
| test.c:6:5:6:13 | ... = ... | AnalysedExpr |
5211
| test.c:6:5:6:13 | ... = ... | AssignExpr |
53-
| test.c:6:5:6:13 | ... = ... | CompileTimeVariableExpr |
54-
| test.c:6:5:6:13 | ... = ... | Def |
55-
| test.c:6:5:6:13 | ... = ... | ExprInVoidContext |
56-
| test.c:6:5:6:13 | ... = ... | NameQualifiableElement |
57-
| test.c:6:5:6:13 | ... = ... | RangeSsaDefinition |
58-
| test.c:6:5:6:13 | ... = ... | SsaDefinition |
59-
| test.c:6:9:6:9 | z | AnalysedExpr |
60-
| test.c:6:9:6:9 | z | CompileTimeVariableExpr |
61-
| test.c:6:9:6:9 | z | Use |
6212
| test.c:6:9:6:9 | z | VariableAccess |
63-
| test.c:6:9:6:13 | (double)... | AnalysedExpr |
6413
| test.c:6:9:6:13 | (double)... | CStyleCast |
65-
| test.c:6:9:6:13 | (double)... | CompileTimeVariableExpr |
66-
| test.c:6:9:6:13 | (double)... | DefOrUse |
67-
| test.c:6:9:6:13 | (double)... | FloatingPointConversion |
68-
| test.c:6:9:6:13 | (double)... | NameQualifiableElement |
69-
| test.c:6:9:6:13 | ... / ... | AnalysedExpr |
70-
| test.c:6:9:6:13 | ... / ... | CompileTimeVariableExpr |
71-
| test.c:6:9:6:13 | ... / ... | DefOrUse |
7214
| test.c:6:9:6:13 | ... / ... | ImaginaryDivExpr |
73-
| test.c:6:9:6:13 | ... / ... | NameQualifiableElement |
74-
| test.c:6:13:6:13 | y | AnalysedExpr |
75-
| test.c:6:13:6:13 | y | CompileTimeVariableExpr |
76-
| test.c:6:13:6:13 | y | Use |
7715
| test.c:6:13:6:13 | y | VariableAccess |
78-
| test.c:7:5:7:5 | w | AnalysedExpr |
79-
| test.c:7:5:7:5 | w | CompileTimeVariableExpr |
80-
| test.c:7:5:7:5 | w | DefOrUse |
8116
| test.c:7:5:7:5 | w | VariableAccess |
82-
| test.c:7:5:7:13 | ... = ... | AnalysedExpr |
8317
| test.c:7:5:7:13 | ... = ... | AssignExpr |
84-
| test.c:7:5:7:13 | ... = ... | CompileTimeVariableExpr |
85-
| test.c:7:5:7:13 | ... = ... | Def |
86-
| test.c:7:5:7:13 | ... = ... | ExprInVoidContext |
87-
| test.c:7:5:7:13 | ... = ... | NameQualifiableElement |
88-
| test.c:7:5:7:13 | ... = ... | RangeSsaDefinition |
89-
| test.c:7:5:7:13 | ... = ... | SsaDefinition |
90-
| test.c:7:9:7:9 | z | AnalysedExpr |
91-
| test.c:7:9:7:9 | z | CompileTimeVariableExpr |
92-
| test.c:7:9:7:9 | z | Use |
9318
| test.c:7:9:7:9 | z | VariableAccess |
94-
| test.c:7:9:7:13 | ... + ... | AnalysedExpr |
95-
| test.c:7:9:7:13 | ... + ... | CompileTimeVariableExpr |
96-
| test.c:7:9:7:13 | ... + ... | DefOrUse |
97-
| test.c:7:9:7:13 | ... + ... | NameQualifiableElement |
9819
| test.c:7:9:7:13 | ... + ... | RealImaginaryAddExpr |
99-
| test.c:7:13:7:13 | x | AnalysedExpr |
100-
| test.c:7:13:7:13 | x | CompileTimeVariableExpr |
101-
| test.c:7:13:7:13 | x | Use |
10220
| test.c:7:13:7:13 | x | VariableAccess |
103-
| test.c:8:5:8:5 | w | AnalysedExpr |
104-
| test.c:8:5:8:5 | w | CompileTimeVariableExpr |
105-
| test.c:8:5:8:5 | w | DefOrUse |
10621
| test.c:8:5:8:5 | w | VariableAccess |
107-
| test.c:8:5:8:13 | ... = ... | AnalysedExpr |
10822
| test.c:8:5:8:13 | ... = ... | AssignExpr |
109-
| test.c:8:5:8:13 | ... = ... | CompileTimeVariableExpr |
110-
| test.c:8:5:8:13 | ... = ... | Def |
111-
| test.c:8:5:8:13 | ... = ... | ExprInVoidContext |
112-
| test.c:8:5:8:13 | ... = ... | NameQualifiableElement |
113-
| test.c:8:5:8:13 | ... = ... | RangeSsaDefinition |
114-
| test.c:8:5:8:13 | ... = ... | SsaDefinition |
115-
| test.c:8:9:8:9 | x | AnalysedExpr |
116-
| test.c:8:9:8:9 | x | CompileTimeVariableExpr |
117-
| test.c:8:9:8:9 | x | Use |
11823
| test.c:8:9:8:9 | x | VariableAccess |
119-
| test.c:8:9:8:13 | ... + ... | AnalysedExpr |
120-
| test.c:8:9:8:13 | ... + ... | CompileTimeVariableExpr |
121-
| test.c:8:9:8:13 | ... + ... | DefOrUse |
12224
| test.c:8:9:8:13 | ... + ... | ImaginaryRealAddExpr |
123-
| test.c:8:9:8:13 | ... + ... | NameQualifiableElement |
124-
| test.c:8:13:8:13 | z | AnalysedExpr |
125-
| test.c:8:13:8:13 | z | CompileTimeVariableExpr |
126-
| test.c:8:13:8:13 | z | Use |
12725
| test.c:8:13:8:13 | z | VariableAccess |
128-
| test.c:9:5:9:5 | w | AnalysedExpr |
129-
| test.c:9:5:9:5 | w | CompileTimeVariableExpr |
130-
| test.c:9:5:9:5 | w | DefOrUse |
13126
| test.c:9:5:9:5 | w | VariableAccess |
132-
| test.c:9:5:9:13 | ... = ... | AnalysedExpr |
13327
| test.c:9:5:9:13 | ... = ... | AssignExpr |
134-
| test.c:9:5:9:13 | ... = ... | CompileTimeVariableExpr |
135-
| test.c:9:5:9:13 | ... = ... | Def |
136-
| test.c:9:5:9:13 | ... = ... | ExprInVoidContext |
137-
| test.c:9:5:9:13 | ... = ... | NameQualifiableElement |
138-
| test.c:9:5:9:13 | ... = ... | RangeSsaDefinition |
139-
| test.c:9:5:9:13 | ... = ... | SsaDefinition |
140-
| test.c:9:9:9:9 | z | AnalysedExpr |
141-
| test.c:9:9:9:9 | z | CompileTimeVariableExpr |
142-
| test.c:9:9:9:9 | z | Use |
14328
| test.c:9:9:9:9 | z | VariableAccess |
144-
| test.c:9:9:9:13 | ... - ... | AnalysedExpr |
145-
| test.c:9:9:9:13 | ... - ... | CompileTimeVariableExpr |
146-
| test.c:9:9:9:13 | ... - ... | DefOrUse |
147-
| test.c:9:9:9:13 | ... - ... | NameQualifiableElement |
14829
| test.c:9:9:9:13 | ... - ... | RealImaginarySubExpr |
149-
| test.c:9:13:9:13 | x | AnalysedExpr |
150-
| test.c:9:13:9:13 | x | CompileTimeVariableExpr |
151-
| test.c:9:13:9:13 | x | Use |
15230
| test.c:9:13:9:13 | x | VariableAccess |
153-
| test.c:10:5:10:5 | w | AnalysedExpr |
154-
| test.c:10:5:10:5 | w | CompileTimeVariableExpr |
155-
| test.c:10:5:10:5 | w | DefOrUse |
15631
| test.c:10:5:10:5 | w | VariableAccess |
157-
| test.c:10:5:10:13 | ... = ... | AnalysedExpr |
15832
| test.c:10:5:10:13 | ... = ... | AssignExpr |
159-
| test.c:10:5:10:13 | ... = ... | CompileTimeVariableExpr |
160-
| test.c:10:5:10:13 | ... = ... | Def |
161-
| test.c:10:5:10:13 | ... = ... | ExprInVoidContext |
162-
| test.c:10:5:10:13 | ... = ... | NameQualifiableElement |
163-
| test.c:10:5:10:13 | ... = ... | RangeSsaDefinition |
164-
| test.c:10:5:10:13 | ... = ... | SsaDefinition |
165-
| test.c:10:9:10:9 | x | AnalysedExpr |
166-
| test.c:10:9:10:9 | x | CompileTimeVariableExpr |
167-
| test.c:10:9:10:9 | x | Use |
16833
| test.c:10:9:10:9 | x | VariableAccess |
169-
| test.c:10:9:10:13 | ... - ... | AnalysedExpr |
170-
| test.c:10:9:10:13 | ... - ... | CompileTimeVariableExpr |
171-
| test.c:10:9:10:13 | ... - ... | DefOrUse |
17234
| test.c:10:9:10:13 | ... - ... | ImaginaryRealSubExpr |
173-
| test.c:10:9:10:13 | ... - ... | NameQualifiableElement |
174-
| test.c:10:13:10:13 | z | AnalysedExpr |
175-
| test.c:10:13:10:13 | z | CompileTimeVariableExpr |
176-
| test.c:10:13:10:13 | z | Use |
17735
| test.c:10:13:10:13 | z | VariableAccess |

‎cpp/ql/test/library-tests/complex_numbers/expr.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
import cpp
22

33
from Expr e
4-
select e, e.getAQlClass()
4+
select e, e.getCanonicalQLClass()

0 commit comments

Comments
 (0)