Skip to content

Commit 2512403

Browse files
committed
C++: Fix asPartialDefinition for IR dataflow nodes and accept testcases
1 parent c5c3ffa commit 2512403

File tree

3 files changed

+38
-30
lines changed

3 files changed

+38
-30
lines changed

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,7 @@ class Node extends TIRDataFlowNode {
7171
* `x.set(taint())` is a partial definition of `x`, and `transfer(&x, taint())` is
7272
* a partial definition of `&x`).
7373
*/
74-
Expr asPartialDefinition() {
75-
result = this.(PartialDefinitionNode).getInstruction().getUnconvertedResultExpression()
76-
}
74+
Expr asPartialDefinition() { result = this.(PartialDefinitionNode).getDefinedExpr() }
7775

7876
/**
7977
* DEPRECATED: See UninitializedNode.
@@ -251,14 +249,17 @@ abstract class PostUpdateNode extends InstructionNode {
251249
* setY(&x); // a partial definition of the object `x`.
252250
* ```
253251
*/
254-
abstract private class PartialDefinitionNode extends PostUpdateNode, TInstructionNode { }
252+
abstract class PartialDefinitionNode extends PostUpdateNode, TInstructionNode {
253+
abstract Expr getDefinedExpr();
254+
}
255255

256-
private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
256+
class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
257257
override ChiInstruction instr;
258+
FieldAddressInstruction field;
258259

259260
ExplicitFieldStoreQualifierNode() {
260261
not instr.isResultConflated() and
261-
exists(StoreInstruction store, FieldInstruction field |
262+
exists(StoreInstruction store |
262263
instr.getPartial() = store and field = store.getDestinationAddress()
263264
)
264265
}
@@ -268,6 +269,10 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
268269
// DataFlowImplConsistency::Consistency. However, it's not clear what (if any) implications
269270
// this consistency failure has.
270271
override Node getPreUpdateNode() { result.asInstruction() = instr.getTotal() }
272+
273+
override Expr getDefinedExpr() {
274+
result = field.getObjectAddress().getUnconvertedResultExpression()
275+
}
271276
}
272277

273278
/**
@@ -278,15 +283,18 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
278283
*/
279284
private class ExplicitSingleFieldStoreQualifierNode extends PartialDefinitionNode {
280285
override StoreInstruction instr;
286+
FieldAddressInstruction field;
281287

282288
ExplicitSingleFieldStoreQualifierNode() {
283-
exists(FieldAddressInstruction field |
284-
field = instr.getDestinationAddress() and
285-
not exists(ChiInstruction chi | chi.getPartial() = instr)
286-
)
289+
field = instr.getDestinationAddress() and
290+
not exists(ChiInstruction chi | chi.getPartial() = instr)
287291
}
288292

289293
override Node getPreUpdateNode() { none() }
294+
295+
override Expr getDefinedExpr() {
296+
result = field.getObjectAddress().getUnconvertedResultExpression()
297+
}
290298
}
291299

292300
/**

cpp/ql/test/library-tests/dataflow/fields/partial-definition-diff.expected

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
| a | AST only |
5252
| a | AST only |
5353
| a | AST only |
54-
| a | AST only |
5554
| a_ | AST only |
5655
| a_ | AST only |
5756
| a_ | AST only |
@@ -87,7 +86,6 @@
8786
| b | AST only |
8887
| b | AST only |
8988
| b | AST only |
90-
| b | AST only |
9189
| b1 | AST only |
9290
| b1 | AST only |
9391
| b1 | AST only |
@@ -151,7 +149,6 @@
151149
| c1 | AST only |
152150
| c1 | AST only |
153151
| c1 | AST only |
154-
| c1 | AST only |
155152
| call to get | AST only |
156153
| call to get | AST only |
157154
| call to getBox1 | AST only |
@@ -174,7 +171,6 @@
174171
| call to user_input | AST only |
175172
| call to user_input | AST only |
176173
| cc | AST only |
177-
| copy1 | AST only |
178174
| ct | AST only |
179175
| d | AST only |
180176
| d | AST only |
@@ -239,10 +235,6 @@
239235
| inner | AST only |
240236
| inner | AST only |
241237
| inner | AST only |
242-
| inner | AST only |
243-
| inner | AST only |
244-
| inner | AST only |
245-
| inner | AST only |
246238
| inner_nested | AST only |
247239
| inner_nested | AST only |
248240
| inner_nested | AST only |
@@ -331,21 +323,9 @@
331323
| pouter | AST only |
332324
| raw | AST only |
333325
| raw | AST only |
334-
| ref1 | AST only |
335-
| s | AST only |
336-
| s | AST only |
337326
| s | AST only |
338327
| s | AST only |
339328
| s | AST only |
340-
| s | AST only |
341-
| s | AST only |
342-
| s | AST only |
343-
| s | AST only |
344-
| s | AST only |
345-
| s | AST only |
346-
| s2 | AST only |
347-
| s2 | AST only |
348-
| s2 | AST only |
349329
| s2 | AST only |
350330
| s3 | AST only |
351331
| this | AST only |
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
| A.cpp:100:5:100:6 | c1 |
2+
| A.cpp:142:7:142:7 | b |
3+
| aliasing.cpp:9:3:9:3 | s |
4+
| aliasing.cpp:13:3:13:3 | s |
5+
| aliasing.cpp:17:3:17:3 | s |
6+
| aliasing.cpp:37:3:37:6 | ref1 |
7+
| aliasing.cpp:42:3:42:4 | s2 |
8+
| aliasing.cpp:49:3:49:7 | copy1 |
9+
| aliasing.cpp:54:3:54:4 | s2 |
10+
| aliasing.cpp:60:3:60:4 | s2 |
11+
| aliasing.cpp:72:3:72:3 | s |
12+
| aliasing.cpp:79:3:79:3 | s |
13+
| aliasing.cpp:86:3:86:3 | s |
14+
| aliasing.cpp:92:5:92:5 | s |
15+
| by_reference.cpp:12:5:12:5 | s |
16+
| by_reference.cpp:84:3:84:7 | inner |
17+
| by_reference.cpp:88:3:88:7 | inner |
18+
| qualifiers.cpp:12:49:12:53 | inner |
19+
| qualifiers.cpp:13:51:13:55 | inner |
20+
| simple.cpp:65:5:65:5 | a |

0 commit comments

Comments
 (0)