Skip to content

Commit adb7410

Browse files
MathiasVPMathiasVP
authored
Merge pull request #19511 from MathiasVP/as-expr-array-aggregate-literal
C++: Make `node.asExpr() instanceof ArrayAggregateLiteral` satisfiable
2 parents 9351702 + ff11aaf commit adb7410

File tree

5 files changed

+49
-13
lines changed

5 files changed

+49
-13
lines changed

‎cpp/ql/lib/change-notes/2025-05-16-array-aggregate-literals.md‎

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 `ArrayAggregateLiteral`s.

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,28 @@ private module Cached {
4545
)
4646
}
4747

48+
private Expr getRankedElementExpr(ArrayAggregateLiteral aggr, int rnk) {
49+
result =
50+
rank[rnk + 1](Expr e, int elementIndex, int position |
51+
e = aggr.getElementExpr(elementIndex, position)
52+
|
53+
e order by elementIndex, position
54+
)
55+
}
56+
57+
private class LastArrayAggregateStore extends StoreInstruction {
58+
ArrayAggregateLiteral aggr;
59+
60+
LastArrayAggregateStore() {
61+
exists(int rnk |
62+
this.getSourceValue().getUnconvertedResultExpression() = getRankedElementExpr(aggr, rnk) and
63+
not exists(getRankedElementExpr(aggr, rnk + 1))
64+
)
65+
}
66+
67+
ArrayAggregateLiteral getArrayAggregateLiteral() { result = aggr }
68+
}
69+
4870
private Expr getConvertedResultExpressionImpl0(Instruction instr) {
4971
// IR construction inserts an additional cast to a `size_t` on the extent
5072
// of a `new[]` expression. The resulting `ConvertInstruction` doesn't have
@@ -95,6 +117,16 @@ private module Cached {
95117
tco.producesExprResult() and
96118
result = asDefinitionImpl0(instr)
97119
)
120+
or
121+
// IR construction breaks an array aggregate literal `{1, 2, 3}` into a
122+
// sequence of `StoreInstruction`s. So there's no instruction `i` for which
123+
// `i.getUnconvertedResultExpression() instanceof ArrayAggregateLiteral`.
124+
// So we map the instruction node corresponding to the last `Store`
125+
// instruction of the sequence to the result of the array aggregate
126+
// literal. This makes sense since this store will immediately flow into
127+
// the indirect node representing the array. So this node does represent
128+
// the array after it has been fully initialized.
129+
result = instr.(LastArrayAggregateStore).getArrayAggregateLiteral()
98130
}
99131

100132
private Expr getConvertedResultExpressionImpl(Instruction instr) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,6 @@ void test_aggregate_literal() {
3535

3636
S s5 = {.a = 1, .b = 2}; // $ asExpr=1 asExpr=2 asExpr={...}
3737

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={...}
38+
int xs[] = {1, 2, 3}; // $ asExpr=1 asExpr=2 asExpr=3 asExpr={...}
39+
const int ys[] = {[0] = 4, [1] = 5, [0] = 6}; // $ asExpr=4 asExpr=5 asExpr=6 asExpr={...}
4040
}

‎cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ edges
22
| consts.cpp:24:7:24:9 | **gv1 | consts.cpp:25:2:25:4 | *a | provenance | |
33
| consts.cpp:24:7:24:9 | **gv1 | consts.cpp:30:9:30:14 | *access to array | provenance | |
44
| consts.cpp:24:7:24:9 | **gv1 | consts.cpp:123:2:123:12 | *... = ... | provenance | |
5-
| consts.cpp:25:2:25:4 | *a | consts.cpp:26:2:26:4 | *b | provenance | |
6-
| consts.cpp:26:2:26:4 | *b | consts.cpp:24:7:24:9 | **gv1 | provenance | |
5+
| consts.cpp:25:2:25:4 | *a | consts.cpp:26:2:26:4 | *{...} | provenance | |
6+
| consts.cpp:26:2:26:4 | *{...} | consts.cpp:24:7:24:9 | **gv1 | provenance | |
77
| consts.cpp:29:7:29:25 | **nonConstFuncToArray | consts.cpp:126:9:126:30 | *call to nonConstFuncToArray | provenance | |
88
| consts.cpp:30:9:30:14 | *access to array | consts.cpp:29:7:29:25 | **nonConstFuncToArray | provenance | |
99
| consts.cpp:85:7:85:8 | gets output argument | consts.cpp:86:9:86:10 | *v1 | provenance | |
@@ -14,7 +14,7 @@ edges
1414
| consts.cpp:85:7:85:8 | gets output argument | consts.cpp:129:19:129:20 | *v1 | provenance | |
1515
| consts.cpp:85:7:85:8 | gets output argument | consts.cpp:135:9:135:11 | *v10 | provenance | TaintFunction |
1616
| consts.cpp:90:2:90:14 | *... = ... | consts.cpp:91:9:91:10 | *v2 | provenance | |
17-
| consts.cpp:90:2:90:14 | *... = ... | consts.cpp:115:21:115:22 | *v2 | provenance | |
17+
| consts.cpp:90:2:90:14 | *... = ... | consts.cpp:115:21:115:22 | *{...} | provenance | |
1818
| consts.cpp:90:7:90:10 | *call to gets | consts.cpp:90:2:90:14 | *... = ... | provenance | |
1919
| consts.cpp:90:12:90:13 | gets output argument | consts.cpp:94:13:94:14 | *v1 | provenance | |
2020
| consts.cpp:90:12:90:13 | gets output argument | consts.cpp:99:2:99:8 | *... = ... | provenance | |
@@ -28,9 +28,9 @@ edges
2828
| consts.cpp:106:13:106:19 | *call to varFunc | consts.cpp:107:9:107:10 | *v5 | provenance | |
2929
| consts.cpp:111:2:111:15 | *... = ... | consts.cpp:112:9:112:10 | *v6 | provenance | |
3030
| consts.cpp:111:7:111:13 | *call to varFunc | consts.cpp:111:2:111:15 | *... = ... | provenance | |
31-
| consts.cpp:115:17:115:18 | *v1 | consts.cpp:115:21:115:22 | *v2 | provenance | |
32-
| consts.cpp:115:21:115:22 | *v2 | consts.cpp:116:9:116:13 | *access to array | provenance | |
33-
| consts.cpp:115:21:115:22 | *v2 | consts.cpp:120:2:120:11 | *... = ... | provenance | |
31+
| consts.cpp:115:17:115:18 | *v1 | consts.cpp:115:21:115:22 | *{...} | provenance | |
32+
| consts.cpp:115:21:115:22 | *{...} | consts.cpp:116:9:116:13 | *access to array | provenance | |
33+
| consts.cpp:115:21:115:22 | *{...} | consts.cpp:120:2:120:11 | *... = ... | provenance | |
3434
| consts.cpp:120:2:120:11 | *... = ... | consts.cpp:121:9:121:10 | *v8 | provenance | |
3535
| consts.cpp:123:2:123:12 | *... = ... | consts.cpp:24:7:24:9 | **gv1 | provenance | |
3636
| consts.cpp:129:19:129:20 | *v1 | consts.cpp:130:9:130:10 | *v9 | provenance | |
@@ -39,7 +39,7 @@ edges
3939
nodes
4040
| consts.cpp:24:7:24:9 | **gv1 | semmle.label | **gv1 |
4141
| consts.cpp:25:2:25:4 | *a | semmle.label | *a |
42-
| consts.cpp:26:2:26:4 | *b | semmle.label | *b |
42+
| consts.cpp:26:2:26:4 | *{...} | semmle.label | *{...} |
4343
| consts.cpp:29:7:29:25 | **nonConstFuncToArray | semmle.label | **nonConstFuncToArray |
4444
| consts.cpp:30:9:30:14 | *access to array | semmle.label | *access to array |
4545
| consts.cpp:85:7:85:8 | gets output argument | semmle.label | gets output argument |
@@ -60,7 +60,7 @@ nodes
6060
| consts.cpp:111:7:111:13 | *call to varFunc | semmle.label | *call to varFunc |
6161
| consts.cpp:112:9:112:10 | *v6 | semmle.label | *v6 |
6262
| consts.cpp:115:17:115:18 | *v1 | semmle.label | *v1 |
63-
| consts.cpp:115:21:115:22 | *v2 | semmle.label | *v2 |
63+
| consts.cpp:115:21:115:22 | *{...} | semmle.label | *{...} |
6464
| consts.cpp:116:9:116:13 | *access to array | semmle.label | *access to array |
6565
| consts.cpp:120:2:120:11 | *... = ... | semmle.label | *... = ... |
6666
| consts.cpp:121:9:121:10 | *v8 | semmle.label | *v8 |

‎cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ edges
66
| test.cpp:28:10:28:29 | *http://example.com | test.cpp:11:26:11:28 | *url | provenance | |
77
| test.cpp:35:23:35:42 | *http://example.com | test.cpp:35:23:35:42 | *http://example.com | provenance | |
88
| test.cpp:35:23:35:42 | *http://example.com | test.cpp:39:11:39:15 | *url_l | provenance | |
9-
| test.cpp:36:26:36:45 | *http://example.com | test.cpp:36:26:36:45 | *http://example.com | provenance | |
10-
| test.cpp:36:26:36:45 | *http://example.com | test.cpp:40:11:40:17 | *access to array | provenance | |
9+
| test.cpp:36:26:36:45 | *http://example.com | test.cpp:36:26:36:45 | *{...} | provenance | |
10+
| test.cpp:36:26:36:45 | *{...} | test.cpp:40:11:40:17 | *access to array | provenance | |
1111
| test.cpp:38:11:38:15 | *url_g | test.cpp:11:26:11:28 | *url | provenance | |
1212
| test.cpp:39:11:39:15 | *url_l | test.cpp:11:26:11:28 | *url | provenance | |
1313
| test.cpp:40:11:40:17 | *access to array | test.cpp:11:26:11:28 | *url | provenance | |
@@ -29,7 +29,7 @@ nodes
2929
| test.cpp:35:23:35:42 | *http://example.com | semmle.label | *http://example.com |
3030
| test.cpp:35:23:35:42 | *http://example.com | semmle.label | *http://example.com |
3131
| test.cpp:36:26:36:45 | *http://example.com | semmle.label | *http://example.com |
32-
| test.cpp:36:26:36:45 | *http://example.com | semmle.label | *http://example.com |
32+
| test.cpp:36:26:36:45 | *{...} | semmle.label | *{...} |
3333
| test.cpp:38:11:38:15 | *url_g | semmle.label | *url_g |
3434
| test.cpp:39:11:39:15 | *url_l | semmle.label | *url_l |
3535
| test.cpp:40:11:40:17 | *access to array | semmle.label | *access to array |

0 commit comments

Comments
 (0)