Skip to content

Commit 03d6633

Browse files
authored
Merge pull request #19501 from MathiasVP/as-expr-class-aggregate-literal
C++: Make `node.asExpr() instanceof ClassAggregateLiteral` satisfiable
2 parents 579cf4a + e11ab0f commit 03d6633

File tree

5 files changed

+71
-8
lines changed

5 files changed

+71
-8
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: fix
3+
---
4+
* Fixed a problem where `asExpr()` on `DataFlow::Node` would never return `ClassAggregateLiteral`s.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,41 @@ private module Cached {
264264
e = getConvertedResultExpression(node.asInstruction(), n)
265265
}
266266

267+
/**
268+
* The IR doesn't have an instruction `i` for which this holds:
269+
* ```
270+
* i.getUnconvertedResultExpression() instanceof ClassAggregateLiteral
271+
* ```
272+
* and thus we don't automatically get a dataflow node for which:
273+
* ```
274+
* node.asExpr() instanceof ClassAggregateLiteral
275+
* ```
276+
* This is because the IR represents a `ClassAggregateLiteral` as a sequence
277+
* of field writes. To work around this we map `asExpr` on the
278+
* `PostUpdateNode` for the last field write to the class aggregate literal.
279+
*/
280+
private class ClassAggregateInitializerPostUpdateNode extends PostFieldUpdateNode {
281+
ClassAggregateLiteral aggr;
282+
283+
ClassAggregateInitializerPostUpdateNode() {
284+
exists(Node node1, FieldContent fc, int position, StoreInstruction store |
285+
store.getSourceValue().getUnconvertedResultExpression() =
286+
aggr.getFieldExpr(fc.getField(), position) and
287+
node1.asInstruction() = store and
288+
// This is the last field write from the aggregate initialization.
289+
not exists(aggr.getFieldExpr(_, position + 1)) and
290+
storeStep(node1, fc, this)
291+
)
292+
}
293+
294+
ClassAggregateLiteral getClassAggregateLiteral() { result = aggr }
295+
}
296+
297+
private predicate exprNodeShouldBePostUpdateNode(Node node, Expr e, int n) {
298+
node.(ClassAggregateInitializerPostUpdateNode).getClassAggregateLiteral() = e and
299+
n = 0
300+
}
301+
267302
/** Holds if `node` should be an `IndirectInstruction` that maps `node.asIndirectExpr()` to `e`. */
268303
private predicate indirectExprNodeShouldBeIndirectInstruction(
269304
IndirectInstruction node, Expr e, int n, int indirectionIndex
@@ -294,7 +329,8 @@ private module Cached {
294329
exprNodeShouldBeInstruction(_, e, n) or
295330
exprNodeShouldBeOperand(_, e, n) or
296331
exprNodeShouldBeIndirectOutNode(_, e, n) or
297-
exprNodeShouldBeIndirectOperand(_, e, n)
332+
exprNodeShouldBeIndirectOperand(_, e, n) or
333+
exprNodeShouldBePostUpdateNode(_, e, n)
298334
}
299335

300336
private class InstructionExprNode extends ExprNodeBase, InstructionNode {
@@ -442,6 +478,12 @@ private module Cached {
442478
final override Expr getConvertedExpr(int n) { exprNodeShouldBeIndirectOperand(this, result, n) }
443479
}
444480

481+
private class PostUpdateExprNode extends ExprNodeBase instanceof PostUpdateNode {
482+
PostUpdateExprNode() { exprNodeShouldBePostUpdateNode(this, _, _) }
483+
484+
final override Expr getConvertedExpr(int n) { exprNodeShouldBePostUpdateNode(this, result, n) }
485+
}
486+
445487
/**
446488
* An expression, viewed as a node in a data flow graph.
447489
*/

cpp/ql/test/library-tests/dataflow/asExpr/test.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,20 @@ A& get_ref();
2121
void test2() {
2222
take_ref(get_ref()); // $ asExpr="call to get_ref" asIndirectExpr="call to get_ref"
2323
}
24+
25+
struct S {
26+
int a;
27+
int b;
28+
};
29+
30+
void test_aggregate_literal() {
31+
S s1 = {1, 2}; // $ asExpr=1 asExpr=2 asExpr={...}
32+
const S s2 = {3, 4}; // $ asExpr=3 asExpr=4 asExpr={...}
33+
S s3 = (S){5, 6}; // $ asExpr=5 asExpr=6 asExpr={...}
34+
const S s4 = (S){7, 8}; // $ asExpr=7 asExpr=8 asExpr={...}
35+
36+
S s5 = {.a = 1, .b = 2}; // $ asExpr=1 asExpr=2 asExpr={...}
37+
38+
int xs[] = {1, 2, 3}; // $ asExpr=1 asExpr=2 asExpr=3 MISSING: asExpr={...}
39+
const int ys[] = {[0] = 4, [1] = 5, [0] = 6}; // $ asExpr=4 asExpr=5 asExpr=6 MISSING: asExpr={...}
40+
}

cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow-ir.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
| example.c:17:11:17:16 | *definition of coords | example.c:17:11:17:16 | *definition of coords |
1818
| example.c:17:11:17:16 | *definition of coords | example.c:17:11:17:16 | *definition of coords |
1919
| example.c:17:11:17:16 | *definition of coords | example.c:24:13:24:18 | *coords |
20-
| example.c:17:11:17:16 | *definition of coords [post update] | example.c:17:11:17:16 | *definition of coords |
2120
| example.c:17:11:17:16 | *definition of coords [post update] | example.c:24:13:24:18 | *coords |
2221
| example.c:17:11:17:16 | definition of coords | example.c:17:11:17:16 | *definition of coords |
2322
| example.c:17:11:17:16 | definition of coords | example.c:17:11:17:16 | definition of coords |
@@ -27,6 +26,7 @@
2726
| example.c:17:11:17:16 | definition of coords | example.c:24:13:24:18 | coords |
2827
| example.c:17:11:17:16 | definition of coords [post update] | example.c:17:11:17:16 | definition of coords |
2928
| example.c:17:11:17:16 | definition of coords [post update] | example.c:24:13:24:18 | coords |
29+
| example.c:17:11:17:16 | {...} | example.c:17:11:17:16 | *definition of coords |
3030
| example.c:17:19:17:22 | {...} | example.c:17:19:17:22 | {...} |
3131
| example.c:17:21:17:21 | 0 | example.c:17:21:17:21 | 0 |
3232
| example.c:19:6:19:6 | *b | example.c:15:37:15:37 | *b |

cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -863,12 +863,12 @@ edges
863863
| struct_init.c:24:10:24:12 | absink output argument [a] | struct_init.c:28:5:28:7 | *& ... [a] | provenance | |
864864
| struct_init.c:26:16:26:20 | *definition of outer [nestedAB, a] | struct_init.c:31:8:31:12 | *outer [nestedAB, a] | provenance | |
865865
| struct_init.c:26:16:26:20 | *definition of outer [nestedAB, a] | struct_init.c:36:11:36:15 | *outer [nestedAB, a] | provenance | |
866-
| struct_init.c:26:16:26:20 | *definition of outer [post update] [*pointerAB, a] | struct_init.c:33:8:33:12 | *outer [*pointerAB, a] | provenance | |
867866
| struct_init.c:26:16:26:20 | *definition of outer [post update] [nestedAB, a] | struct_init.c:26:16:26:20 | *definition of outer [nestedAB, a] | provenance | |
867+
| struct_init.c:26:16:26:20 | {...} [*pointerAB, a] | struct_init.c:33:8:33:12 | *outer [*pointerAB, a] | provenance | |
868868
| struct_init.c:26:23:29:3 | *{...} [post update] [a] | struct_init.c:26:16:26:20 | *definition of outer [post update] [nestedAB, a] | provenance | |
869869
| struct_init.c:27:7:27:16 | call to user_input | struct_init.c:26:23:29:3 | *{...} [post update] [a] | provenance | |
870870
| struct_init.c:27:7:27:16 | call to user_input | struct_init.c:27:7:27:16 | call to user_input | provenance | |
871-
| struct_init.c:28:5:28:7 | *& ... [a] | struct_init.c:26:16:26:20 | *definition of outer [post update] [*pointerAB, a] | provenance | |
871+
| struct_init.c:28:5:28:7 | *& ... [a] | struct_init.c:26:16:26:20 | {...} [*pointerAB, a] | provenance | |
872872
| struct_init.c:31:8:31:12 | *outer [nestedAB, a] | struct_init.c:31:14:31:21 | *nestedAB [a] | provenance | |
873873
| struct_init.c:31:14:31:21 | *nestedAB [a] | struct_init.c:31:23:31:23 | a | provenance | |
874874
| struct_init.c:33:8:33:12 | *outer [*pointerAB, a] | struct_init.c:33:14:33:22 | *pointerAB [a] | provenance | |
@@ -879,8 +879,8 @@ edges
879879
| struct_init.c:40:13:40:14 | *definition of ab [post update] [a] | struct_init.c:40:13:40:14 | *definition of ab [a] | provenance | |
880880
| struct_init.c:40:20:40:29 | call to user_input | struct_init.c:40:13:40:14 | *definition of ab [post update] [a] | provenance | |
881881
| struct_init.c:40:20:40:29 | call to user_input | struct_init.c:40:20:40:29 | call to user_input | provenance | |
882-
| struct_init.c:41:16:41:20 | *definition of outer [post update] [*pointerAB, a] | struct_init.c:46:10:46:14 | *outer [*pointerAB, a] | provenance | |
883-
| struct_init.c:43:5:43:7 | *& ... [a] | struct_init.c:41:16:41:20 | *definition of outer [post update] [*pointerAB, a] | provenance | |
882+
| struct_init.c:41:16:41:20 | {...} [*pointerAB, a] | struct_init.c:46:10:46:14 | *outer [*pointerAB, a] | provenance | |
883+
| struct_init.c:43:5:43:7 | *& ... [a] | struct_init.c:41:16:41:20 | {...} [*pointerAB, a] | provenance | |
884884
| struct_init.c:46:10:46:14 | *outer [*pointerAB, a] | struct_init.c:46:16:46:24 | *pointerAB [a] | provenance | |
885885
| struct_init.c:46:16:46:24 | *pointerAB [a] | struct_init.c:14:24:14:25 | *ab [a] | provenance | |
886886
nodes
@@ -1773,8 +1773,8 @@ nodes
17731773
| struct_init.c:24:10:24:12 | *& ... [a] | semmle.label | *& ... [a] |
17741774
| struct_init.c:24:10:24:12 | absink output argument [a] | semmle.label | absink output argument [a] |
17751775
| struct_init.c:26:16:26:20 | *definition of outer [nestedAB, a] | semmle.label | *definition of outer [nestedAB, a] |
1776-
| struct_init.c:26:16:26:20 | *definition of outer [post update] [*pointerAB, a] | semmle.label | *definition of outer [post update] [*pointerAB, a] |
17771776
| struct_init.c:26:16:26:20 | *definition of outer [post update] [nestedAB, a] | semmle.label | *definition of outer [post update] [nestedAB, a] |
1777+
| struct_init.c:26:16:26:20 | {...} [*pointerAB, a] | semmle.label | {...} [*pointerAB, a] |
17781778
| struct_init.c:26:23:29:3 | *{...} [post update] [a] | semmle.label | *{...} [post update] [a] |
17791779
| struct_init.c:27:7:27:16 | call to user_input | semmle.label | call to user_input |
17801780
| struct_init.c:27:7:27:16 | call to user_input | semmle.label | call to user_input |
@@ -1791,7 +1791,7 @@ nodes
17911791
| struct_init.c:40:13:40:14 | *definition of ab [post update] [a] | semmle.label | *definition of ab [post update] [a] |
17921792
| struct_init.c:40:20:40:29 | call to user_input | semmle.label | call to user_input |
17931793
| struct_init.c:40:20:40:29 | call to user_input | semmle.label | call to user_input |
1794-
| struct_init.c:41:16:41:20 | *definition of outer [post update] [*pointerAB, a] | semmle.label | *definition of outer [post update] [*pointerAB, a] |
1794+
| struct_init.c:41:16:41:20 | {...} [*pointerAB, a] | semmle.label | {...} [*pointerAB, a] |
17951795
| struct_init.c:43:5:43:7 | *& ... [a] | semmle.label | *& ... [a] |
17961796
| struct_init.c:46:10:46:14 | *outer [*pointerAB, a] | semmle.label | *outer [*pointerAB, a] |
17971797
| struct_init.c:46:16:46:24 | *pointerAB [a] | semmle.label | *pointerAB [a] |

0 commit comments

Comments
 (0)