From 0f2107572207762458abcc6a2981f3caf70db0c2 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 14 May 2025 17:59:01 +0100 Subject: [PATCH 1/5] C++: Add a test that demonstrate missing asExpr for aggregate literals. --- .../test/library-tests/dataflow/asExpr/test.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp index ae7127401440..5835abb297dd 100644 --- a/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp @@ -21,3 +21,20 @@ A& get_ref(); void test2() { take_ref(get_ref()); // $ asExpr="call to get_ref" asIndirectExpr="call to get_ref" } + +struct S { + int a; + int b; +}; + +void test_aggregate_literal() { + S s1 = {1, 2}; // $ asExpr=1 asExpr=2 + const S s2 = {3, 4}; // $ asExpr=3 asExpr=4 + S s3 = (S){5, 6}; // $ asExpr=5 asExpr=6 + const S s4 = (S){7, 8}; // $ asExpr=7 asExpr=8 + + S s5 = {.a = 1, .b = 2}; // $ asExpr=1 asExpr=2 + + int xs[] = {1, 2, 3}; // $ asExpr=1 asExpr=2 asExpr=3 + const int ys[] = {[0] = 4, [1] = 5, [0] = 6}; // $ asExpr=4 asExpr=5 asExpr=6 +} \ No newline at end of file From 783560cff65c909175f7f3458c3ea0af16333823 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 15 May 2025 16:45:10 +0100 Subject: [PATCH 2/5] C++: Add a subclass of PostUpdateNodes and ensure that 'node.asExpr() instanceof ClassAggregateLiteral' holds for this new node subclass. --- .../cpp/ir/dataflow/internal/ExprNodes.qll | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll index c2b89a67f692..146967fa6538 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll @@ -264,6 +264,41 @@ private module Cached { e = getConvertedResultExpression(node.asInstruction(), n) } + /** + * The IR doesn't have an instruction `i` for which this holds: + * ``` + * i.getUnconvertedResultExpression() instanceof ClassAggregateLiteral + * ``` + * and thus we don't automaticallt get a dataflow node for which: + * ``` + * node.asExpr() instanceof ClassAggregateLiteral + * ``` + * This is because the IR represents a `ClassAggregateLiteral` as a sequence + * of field writes. To work around this we map `asExpr` on the + * `PostUpdateNode` for the last field write to the class aggregate literal. + */ + private class ClassAggregateInitializerPostUpdateNode extends PostFieldUpdateNode { + ClassAggregateLiteral aggr; + + ClassAggregateInitializerPostUpdateNode() { + exists(Node node1, FieldContent fc, int position, StoreInstruction store | + store.getSourceValue().getUnconvertedResultExpression() = + aggr.getFieldExpr(fc.getField(), position) and + node1.asInstruction() = store and + // This is the last field write from the aggregate initialization. + not exists(aggr.getFieldExpr(_, position + 1)) and + storeStep(node1, fc, this) + ) + } + + ClassAggregateLiteral getClassAggregateLiteral() { result = aggr } + } + + private predicate exprNodeShouldBePostUpdateNode(Node node, Expr e, int n) { + node.(ClassAggregateInitializerPostUpdateNode).getClassAggregateLiteral() = e and + n = 0 + } + /** Holds if `node` should be an `IndirectInstruction` that maps `node.asIndirectExpr()` to `e`. */ private predicate indirectExprNodeShouldBeIndirectInstruction( IndirectInstruction node, Expr e, int n, int indirectionIndex @@ -294,7 +329,8 @@ private module Cached { exprNodeShouldBeInstruction(_, e, n) or exprNodeShouldBeOperand(_, e, n) or exprNodeShouldBeIndirectOutNode(_, e, n) or - exprNodeShouldBeIndirectOperand(_, e, n) + exprNodeShouldBeIndirectOperand(_, e, n) or + exprNodeShouldBePostUpdateNode(_, e, n) } private class InstructionExprNode extends ExprNodeBase, InstructionNode { @@ -442,6 +478,12 @@ private module Cached { final override Expr getConvertedExpr(int n) { exprNodeShouldBeIndirectOperand(this, result, n) } } + private class PostUpdateExprNode extends ExprNodeBase instanceof PostUpdateNode { + PostUpdateExprNode() { exprNodeShouldBePostUpdateNode(this, _, _) } + + final override Expr getConvertedExpr(int n) { exprNodeShouldBePostUpdateNode(this, result, n) } + } + /** * An expression, viewed as a node in a data flow graph. */ From c3c6bb6e607387430049a269e1d51434b8d99420 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 15 May 2025 16:45:23 +0100 Subject: [PATCH 3/5] C++: Accept test changes. --- cpp/ql/test/library-tests/dataflow/asExpr/test.cpp | 14 +++++++------- .../dataflow/dataflow-tests/localFlow-ir.expected | 2 +- .../dataflow/fields/ir-path-flow.expected | 12 ++++++------ 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp index 5835abb297dd..c81b86aa8ae3 100644 --- a/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp @@ -28,13 +28,13 @@ struct S { }; void test_aggregate_literal() { - S s1 = {1, 2}; // $ asExpr=1 asExpr=2 - const S s2 = {3, 4}; // $ asExpr=3 asExpr=4 - S s3 = (S){5, 6}; // $ asExpr=5 asExpr=6 - const S s4 = (S){7, 8}; // $ asExpr=7 asExpr=8 + S s1 = {1, 2}; // $ asExpr=1 asExpr=2 asExpr={...} + const S s2 = {3, 4}; // $ asExpr=3 asExpr=4 asExpr={...} + S s3 = (S){5, 6}; // $ asExpr=5 asExpr=6 asExpr={...} + const S s4 = (S){7, 8}; // $ asExpr=7 asExpr=8 asExpr={...} - S s5 = {.a = 1, .b = 2}; // $ asExpr=1 asExpr=2 + S s5 = {.a = 1, .b = 2}; // $ asExpr=1 asExpr=2 asExpr={...} - int xs[] = {1, 2, 3}; // $ asExpr=1 asExpr=2 asExpr=3 - const int ys[] = {[0] = 4, [1] = 5, [0] = 6}; // $ asExpr=4 asExpr=5 asExpr=6 + int xs[] = {1, 2, 3}; // $ asExpr=1 asExpr=2 asExpr=3 MISSING: asExpr={...} + const int ys[] = {[0] = 4, [1] = 5, [0] = 6}; // $ asExpr=4 asExpr=5 asExpr=6 MISSING: asExpr={...} } \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow-ir.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow-ir.expected index 513c23e3c6eb..f41def013155 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow-ir.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow-ir.expected @@ -17,7 +17,6 @@ | example.c:17:11:17:16 | *definition of coords | example.c:17:11:17:16 | *definition of coords | | example.c:17:11:17:16 | *definition of coords | example.c:17:11:17:16 | *definition of coords | | example.c:17:11:17:16 | *definition of coords | example.c:24:13:24:18 | *coords | -| example.c:17:11:17:16 | *definition of coords [post update] | example.c:17:11:17:16 | *definition of coords | | example.c:17:11:17:16 | *definition of coords [post update] | example.c:24:13:24:18 | *coords | | example.c:17:11:17:16 | definition of coords | example.c:17:11:17:16 | *definition of coords | | example.c:17:11:17:16 | definition of coords | example.c:17:11:17:16 | definition of coords | @@ -27,6 +26,7 @@ | example.c:17:11:17:16 | definition of coords | example.c:24:13:24:18 | coords | | example.c:17:11:17:16 | definition of coords [post update] | example.c:17:11:17:16 | definition of coords | | example.c:17:11:17:16 | definition of coords [post update] | example.c:24:13:24:18 | coords | +| example.c:17:11:17:16 | {...} | example.c:17:11:17:16 | *definition of coords | | example.c:17:19:17:22 | {...} | example.c:17:19:17:22 | {...} | | example.c:17:21:17:21 | 0 | example.c:17:21:17:21 | 0 | | example.c:19:6:19:6 | *b | example.c:15:37:15:37 | *b | diff --git a/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected b/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected index 43725bb4524e..6852a5dd3cd4 100644 --- a/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected +++ b/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected @@ -863,12 +863,12 @@ edges | struct_init.c:24:10:24:12 | absink output argument [a] | struct_init.c:28:5:28:7 | *& ... [a] | provenance | | | struct_init.c:26:16:26:20 | *definition of outer [nestedAB, a] | struct_init.c:31:8:31:12 | *outer [nestedAB, a] | provenance | | | struct_init.c:26:16:26:20 | *definition of outer [nestedAB, a] | struct_init.c:36:11:36:15 | *outer [nestedAB, a] | provenance | | -| 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 | | | 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 | | +| struct_init.c:26:16:26:20 | {...} [*pointerAB, a] | struct_init.c:33:8:33:12 | *outer [*pointerAB, a] | provenance | | | 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 | | | struct_init.c:27:7:27:16 | call to user_input | struct_init.c:26:23:29:3 | *{...} [post update] [a] | provenance | | | struct_init.c:27:7:27:16 | call to user_input | struct_init.c:27:7:27:16 | call to user_input | provenance | | -| struct_init.c:28:5:28:7 | *& ... [a] | struct_init.c:26:16:26:20 | *definition of outer [post update] [*pointerAB, a] | provenance | | +| struct_init.c:28:5:28:7 | *& ... [a] | struct_init.c:26:16:26:20 | {...} [*pointerAB, a] | provenance | | | struct_init.c:31:8:31:12 | *outer [nestedAB, a] | struct_init.c:31:14:31:21 | *nestedAB [a] | provenance | | | struct_init.c:31:14:31:21 | *nestedAB [a] | struct_init.c:31:23:31:23 | a | provenance | | | 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 | 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 | | | 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 | | | struct_init.c:40:20:40:29 | call to user_input | struct_init.c:40:20:40:29 | call to user_input | provenance | | -| 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 | | -| struct_init.c:43:5:43:7 | *& ... [a] | struct_init.c:41:16:41:20 | *definition of outer [post update] [*pointerAB, a] | provenance | | +| struct_init.c:41:16:41:20 | {...} [*pointerAB, a] | struct_init.c:46:10:46:14 | *outer [*pointerAB, a] | provenance | | +| struct_init.c:43:5:43:7 | *& ... [a] | struct_init.c:41:16:41:20 | {...} [*pointerAB, a] | provenance | | | struct_init.c:46:10:46:14 | *outer [*pointerAB, a] | struct_init.c:46:16:46:24 | *pointerAB [a] | provenance | | | struct_init.c:46:16:46:24 | *pointerAB [a] | struct_init.c:14:24:14:25 | *ab [a] | provenance | | nodes @@ -1773,8 +1773,8 @@ nodes | struct_init.c:24:10:24:12 | *& ... [a] | semmle.label | *& ... [a] | | struct_init.c:24:10:24:12 | absink output argument [a] | semmle.label | absink output argument [a] | | struct_init.c:26:16:26:20 | *definition of outer [nestedAB, a] | semmle.label | *definition of outer [nestedAB, a] | -| struct_init.c:26:16:26:20 | *definition of outer [post update] [*pointerAB, a] | semmle.label | *definition of outer [post update] [*pointerAB, a] | | struct_init.c:26:16:26:20 | *definition of outer [post update] [nestedAB, a] | semmle.label | *definition of outer [post update] [nestedAB, a] | +| struct_init.c:26:16:26:20 | {...} [*pointerAB, a] | semmle.label | {...} [*pointerAB, a] | | struct_init.c:26:23:29:3 | *{...} [post update] [a] | semmle.label | *{...} [post update] [a] | | struct_init.c:27:7:27:16 | call to user_input | semmle.label | call to user_input | | struct_init.c:27:7:27:16 | call to user_input | semmle.label | call to user_input | @@ -1791,7 +1791,7 @@ nodes | struct_init.c:40:13:40:14 | *definition of ab [post update] [a] | semmle.label | *definition of ab [post update] [a] | | struct_init.c:40:20:40:29 | call to user_input | semmle.label | call to user_input | | struct_init.c:40:20:40:29 | call to user_input | semmle.label | call to user_input | -| struct_init.c:41:16:41:20 | *definition of outer [post update] [*pointerAB, a] | semmle.label | *definition of outer [post update] [*pointerAB, a] | +| struct_init.c:41:16:41:20 | {...} [*pointerAB, a] | semmle.label | {...} [*pointerAB, a] | | struct_init.c:43:5:43:7 | *& ... [a] | semmle.label | *& ... [a] | | struct_init.c:46:10:46:14 | *outer [*pointerAB, a] | semmle.label | *outer [*pointerAB, a] | | struct_init.c:46:16:46:24 | *pointerAB [a] | semmle.label | *pointerAB [a] | From f731d0e630d9cfe9fc1773e41c88f6394d2e61ec Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 15 May 2025 17:39:51 +0100 Subject: [PATCH 4/5] C++: Add change note. --- .../lib/change-notes/2025-05-15-class-aggregate-literals.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/lib/change-notes/2025-05-15-class-aggregate-literals.md diff --git a/cpp/ql/lib/change-notes/2025-05-15-class-aggregate-literals.md b/cpp/ql/lib/change-notes/2025-05-15-class-aggregate-literals.md new file mode 100644 index 000000000000..ea821d7d48d6 --- /dev/null +++ b/cpp/ql/lib/change-notes/2025-05-15-class-aggregate-literals.md @@ -0,0 +1,4 @@ +--- +category: fix +--- +* Fixed a problem where `asExpr()` on `DataFlow::Node` would never return `ClassAggregateLiteral`s. \ No newline at end of file From e11ab0f125458f76567329733fbb010c75bf2419 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 16 May 2025 12:06:25 +0100 Subject: [PATCH 5/5] Update cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll index 146967fa6538..5514bd80eeec 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll @@ -269,7 +269,7 @@ private module Cached { * ``` * i.getUnconvertedResultExpression() instanceof ClassAggregateLiteral * ``` - * and thus we don't automaticallt get a dataflow node for which: + * and thus we don't automatically get a dataflow node for which: * ``` * node.asExpr() instanceof ClassAggregateLiteral * ```