Skip to content

Commit 317734f

Browse files
MathiasVPMathiasVP
committed
C++: Attach PostUpdateNodes to Chi nodes following aschackmull's suggestion
1 parent ce5d8d5 commit 317734f

3 files changed

Lines changed: 31 additions & 116 deletions

File tree

‎cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,10 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
148148
*/
149149
predicate readStep(Node node1, Content f, Node node2) {
150150
exists(FieldAddressInstruction fa, LoadInstruction load |
151-
load.getSourceAddress() = fa and
152-
node1.asInstruction() = fa.getObjectAddress() and
153151
fa.getField() = f.(FieldContent).getField() and
154-
load = node2.asInstruction()
152+
node1.asInstruction() = load and
153+
load.getSourceAddress() = fa and
154+
node2.asInstruction().getAnOperand().getAnyDef() = load
155155
)
156156
}
157157

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

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -245,30 +245,19 @@ abstract class PostUpdateNode extends InstructionNode {
245245
* setY(&x); // a partial definition of the object `x`.
246246
* ```
247247
*/
248-
abstract class PartialDefinitionNode extends PostUpdateNode, TInstructionNode {
249-
/**
250-
* Gets the instruction that partially defines the object. This includes
251-
* both the instruction that partially defines the object, and the chi
252-
* instruction that links up the partial definition to the object.
253-
*/
254-
final Instruction getInstructionOrChi() {
255-
exists(ChiInstruction chi |
256-
not chi.isResultConflated() and
257-
chi.getPartial() = getInstruction() and
258-
result = chi
259-
)
260-
or
261-
result = getInstruction()
262-
}
263-
}
248+
abstract class PartialDefinitionNode extends PostUpdateNode, TInstructionNode {}
264249

265250
private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
266-
override StoreInstruction instr;
267-
FieldAddressInstruction field;
251+
override ChiInstruction instr;
268252

269-
ExplicitFieldStoreQualifierNode() { field = instr.getDestinationAddress() }
253+
ExplicitFieldStoreQualifierNode() {
254+
not instr.isResultConflated() and
255+
exists(StoreInstruction store, FieldInstruction field |
256+
instr.getPartial() = store and field = store.getDestinationAddress()
257+
)
258+
}
270259

271-
override Node getPreUpdateNode() { result.asInstruction() = field.getObjectAddress() }
260+
override Node getPreUpdateNode() { result.asInstruction() = instr.getTotal() }
272261
}
273262

274263
/**
@@ -282,29 +271,29 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
282271
* `getVariableAccess()` equal to `x`.
283272
*/
284273
class DefinitionByReferenceNode extends PartialDefinitionNode {
285-
override WriteSideEffectInstruction instr;
274+
override ChiInstruction instr;
275+
WriteSideEffectInstruction write;
286276
CallInstruction call;
287277

288-
DefinitionByReferenceNode() { call = instr.getPrimaryInstruction() }
278+
DefinitionByReferenceNode() {
279+
instr.getPartial() = write and
280+
call = write.getPrimaryInstruction() }
289281

290282
override Node getPreUpdateNode() {
291-
result.asInstruction() = call.getPositionalArgument(instr.getIndex())
292-
or
293-
result.asInstruction() = call.getThisArgument() and
294-
instr.getIndex() = -1
283+
result.asInstruction() = instr.getTotal()
295284
}
296285

297286
/** Gets the argument corresponding to this node. */
298287
Expr getArgument() {
299-
result = call.getPositionalArgument(instr.getIndex()).getUnconvertedResultExpression()
288+
result = call.getPositionalArgument(write.getIndex()).getUnconvertedResultExpression()
300289
or
301290
result = call.getThisArgument().getUnconvertedResultExpression() and
302-
instr.getIndex() = -1
291+
write.getIndex() = -1
303292
}
304293

305294
/** Gets the parameter through which this value is assigned. */
306295
Parameter getParameter() {
307-
exists(CallInstruction ci | result = ci.getStaticCallTarget().getParameter(instr.getIndex()))
296+
exists(CallInstruction ci | result = ci.getStaticCallTarget().getParameter(write.getIndex()))
308297
}
309298
}
310299

@@ -393,10 +382,10 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) { simpleLocalFlowStep(nodeFr
393382
predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
394383
simpleInstructionLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asInstruction())
395384
or
396-
exists(LoadInstruction load |
397-
load.getSourceValueOperand().getAnyDef() =
398-
nodeFrom.(PartialDefinitionNode).getInstructionOrChi() and
399-
nodeTo.asInstruction() = load.getSourceAddress().(FieldAddressInstruction).getObjectAddress()
385+
exists(LoadInstruction load, ChiInstruction chi |
386+
nodeTo.asInstruction() = load and
387+
nodeFrom.asInstruction() = chi and
388+
load.getSourceValueOperand().getAnyDef() = chi
400389
)
401390
}
402391

@@ -423,6 +412,12 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
423412
//
424413
// Flow through the partial operand belongs in the taint-tracking libraries
425414
// for now.
415+
416+
// TODO: To capture flow from a partial definition of an object (i.e., a field write) to the object
417+
// we add dataflow through partial chi operands, but only if the chi node is not the chi node for all
418+
// aliased memory.
419+
iTo.getAnOperand().(ChiPartialOperand).getDef() = iFrom and not iFrom.isResultConflated()
420+
or
426421
iTo.getAnOperand().(ChiTotalOperand).getDef() = iFrom
427422
or
428423
// Flow through modeled functions

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

Lines changed: 0 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,107 +1,27 @@
11
edges
2-
| aliasing.cpp:9:3:9:22 | Store : void | aliasing.cpp:9:3:9:22 | Store [m1] : void |
3-
| aliasing.cpp:9:3:9:22 | Store [m1] : void | aliasing.cpp:25:17:25:19 | BufferMayWriteSideEffect [m1] : void |
4-
| aliasing.cpp:9:11:9:20 | call to user_input : void | aliasing.cpp:9:3:9:22 | Store : void |
5-
| aliasing.cpp:13:3:13:21 | Store : void | aliasing.cpp:13:3:13:21 | Store [m1] : void |
6-
| aliasing.cpp:13:3:13:21 | Store [m1] : void | aliasing.cpp:26:19:26:20 | BufferMayWriteSideEffect [m1] : void |
7-
| aliasing.cpp:13:10:13:19 | call to user_input : void | aliasing.cpp:13:3:13:21 | Store : void |
8-
| aliasing.cpp:25:17:25:19 | BufferMayWriteSideEffect [m1] : void | aliasing.cpp:29:8:29:9 | s1 [m1] : void |
9-
| aliasing.cpp:26:19:26:20 | BufferMayWriteSideEffect [m1] : void | aliasing.cpp:30:8:30:9 | s2 [m1] : void |
10-
| aliasing.cpp:29:8:29:9 | s1 [m1] : void | aliasing.cpp:29:11:29:12 | m1 |
11-
| aliasing.cpp:30:8:30:9 | s2 [m1] : void | aliasing.cpp:30:11:30:12 | m1 |
12-
| aliasing.cpp:37:3:37:24 | Store : void | aliasing.cpp:37:3:37:24 | Store [m1] : void |
13-
| aliasing.cpp:37:3:37:24 | Store : void | aliasing.cpp:38:11:38:12 | m1 |
14-
| aliasing.cpp:37:3:37:24 | Store [m1] : void | aliasing.cpp:38:8:38:9 | s1 [m1] : void |
15-
| aliasing.cpp:37:13:37:22 | call to user_input : void | aliasing.cpp:37:3:37:24 | Store : void |
162
| aliasing.cpp:37:13:37:22 | call to user_input : void | aliasing.cpp:38:11:38:12 | m1 |
17-
| aliasing.cpp:38:8:38:9 | s1 [m1] : void | aliasing.cpp:38:11:38:12 | m1 |
18-
| aliasing.cpp:42:3:42:22 | Store : void | aliasing.cpp:42:3:42:22 | Store [m1] : void |
19-
| aliasing.cpp:42:3:42:22 | Store : void | aliasing.cpp:43:13:43:14 | m1 |
20-
| aliasing.cpp:42:3:42:22 | Store [m1] : void | aliasing.cpp:43:8:43:11 | (reference dereference) [m1] : void |
21-
| aliasing.cpp:42:11:42:20 | call to user_input : void | aliasing.cpp:42:3:42:22 | Store : void |
223
| aliasing.cpp:42:11:42:20 | call to user_input : void | aliasing.cpp:43:13:43:14 | m1 |
23-
| aliasing.cpp:43:8:43:11 | (reference dereference) [m1] : void | aliasing.cpp:43:13:43:14 | m1 |
24-
| aliasing.cpp:79:3:79:22 | Store : void | aliasing.cpp:79:3:79:22 | Store [m1] : void |
25-
| aliasing.cpp:79:3:79:22 | Store : void | aliasing.cpp:80:12:80:13 | m1 |
26-
| aliasing.cpp:79:3:79:22 | Store [m1] : void | aliasing.cpp:80:10:80:10 | s [m1] : void |
27-
| aliasing.cpp:79:11:79:20 | call to user_input : void | aliasing.cpp:79:3:79:22 | Store : void |
284
| aliasing.cpp:79:11:79:20 | call to user_input : void | aliasing.cpp:80:12:80:13 | m1 |
29-
| aliasing.cpp:80:10:80:10 | s [m1] : void | aliasing.cpp:80:12:80:13 | m1 |
30-
| aliasing.cpp:86:3:86:21 | Store : void | aliasing.cpp:86:3:86:21 | Store [m1] : void |
31-
| aliasing.cpp:86:3:86:21 | Store : void | aliasing.cpp:87:12:87:13 | m1 |
32-
| aliasing.cpp:86:3:86:21 | Store [m1] : void | aliasing.cpp:87:10:87:10 | s [m1] : void |
33-
| aliasing.cpp:86:10:86:19 | call to user_input : void | aliasing.cpp:86:3:86:21 | Store : void |
345
| aliasing.cpp:86:10:86:19 | call to user_input : void | aliasing.cpp:87:12:87:13 | m1 |
35-
| aliasing.cpp:87:10:87:10 | s [m1] : void | aliasing.cpp:87:12:87:13 | m1 |
36-
| aliasing.cpp:92:3:92:23 | Store : void | aliasing.cpp:92:3:92:23 | Store [m1] : void |
37-
| aliasing.cpp:92:3:92:23 | Store : void | aliasing.cpp:93:12:93:13 | m1 |
38-
| aliasing.cpp:92:3:92:23 | Store [m1] : void | aliasing.cpp:93:10:93:10 | s [m1] : void |
39-
| aliasing.cpp:92:12:92:21 | call to user_input : void | aliasing.cpp:92:3:92:23 | Store : void |
406
| aliasing.cpp:92:12:92:21 | call to user_input : void | aliasing.cpp:93:12:93:13 | m1 |
41-
| aliasing.cpp:93:10:93:10 | s [m1] : void | aliasing.cpp:93:12:93:13 | m1 |
42-
| struct_init.c:20:20:20:29 | Store : void | struct_init.c:20:20:20:29 | Store [a] : void |
43-
| struct_init.c:20:20:20:29 | Store : void | struct_init.c:22:11:22:11 | a |
44-
| struct_init.c:20:20:20:29 | Store [a] : void | struct_init.c:22:8:22:9 | ab [a] : void |
45-
| struct_init.c:20:20:20:29 | call to user_input : void | struct_init.c:20:20:20:29 | Store : void |
467
| struct_init.c:20:20:20:29 | call to user_input : void | struct_init.c:22:11:22:11 | a |
47-
| struct_init.c:22:8:22:9 | ab [a] : void | struct_init.c:22:11:22:11 | a |
48-
| struct_init.c:27:7:27:16 | Store : void | struct_init.c:27:7:27:16 | Store [a] : void |
49-
| struct_init.c:27:7:27:16 | Store : void | struct_init.c:31:23:31:23 | a |
50-
| struct_init.c:27:7:27:16 | Store [a] : void | struct_init.c:31:14:31:21 | nestedAB [a] : void |
51-
| struct_init.c:27:7:27:16 | call to user_input : void | struct_init.c:27:7:27:16 | Store : void |
528
| struct_init.c:27:7:27:16 | call to user_input : void | struct_init.c:31:23:31:23 | a |
53-
| struct_init.c:31:14:31:21 | nestedAB [a] : void | struct_init.c:31:23:31:23 | a |
549
nodes
55-
| aliasing.cpp:9:3:9:22 | Store : void | semmle.label | Store : void |
56-
| aliasing.cpp:9:3:9:22 | Store [m1] : void | semmle.label | Store [m1] : void |
57-
| aliasing.cpp:9:11:9:20 | call to user_input : void | semmle.label | call to user_input : void |
58-
| aliasing.cpp:13:3:13:21 | Store : void | semmle.label | Store : void |
59-
| aliasing.cpp:13:3:13:21 | Store [m1] : void | semmle.label | Store [m1] : void |
60-
| aliasing.cpp:13:10:13:19 | call to user_input : void | semmle.label | call to user_input : void |
61-
| aliasing.cpp:25:17:25:19 | BufferMayWriteSideEffect [m1] : void | semmle.label | BufferMayWriteSideEffect [m1] : void |
62-
| aliasing.cpp:26:19:26:20 | BufferMayWriteSideEffect [m1] : void | semmle.label | BufferMayWriteSideEffect [m1] : void |
63-
| aliasing.cpp:29:8:29:9 | s1 [m1] : void | semmle.label | s1 [m1] : void |
64-
| aliasing.cpp:29:11:29:12 | m1 | semmle.label | m1 |
65-
| aliasing.cpp:30:8:30:9 | s2 [m1] : void | semmle.label | s2 [m1] : void |
66-
| aliasing.cpp:30:11:30:12 | m1 | semmle.label | m1 |
67-
| aliasing.cpp:37:3:37:24 | Store : void | semmle.label | Store : void |
68-
| aliasing.cpp:37:3:37:24 | Store [m1] : void | semmle.label | Store [m1] : void |
6910
| aliasing.cpp:37:13:37:22 | call to user_input : void | semmle.label | call to user_input : void |
70-
| aliasing.cpp:38:8:38:9 | s1 [m1] : void | semmle.label | s1 [m1] : void |
7111
| aliasing.cpp:38:11:38:12 | m1 | semmle.label | m1 |
72-
| aliasing.cpp:42:3:42:22 | Store : void | semmle.label | Store : void |
73-
| aliasing.cpp:42:3:42:22 | Store [m1] : void | semmle.label | Store [m1] : void |
7412
| aliasing.cpp:42:11:42:20 | call to user_input : void | semmle.label | call to user_input : void |
75-
| aliasing.cpp:43:8:43:11 | (reference dereference) [m1] : void | semmle.label | (reference dereference) [m1] : void |
7613
| aliasing.cpp:43:13:43:14 | m1 | semmle.label | m1 |
77-
| aliasing.cpp:79:3:79:22 | Store : void | semmle.label | Store : void |
78-
| aliasing.cpp:79:3:79:22 | Store [m1] : void | semmle.label | Store [m1] : void |
7914
| aliasing.cpp:79:11:79:20 | call to user_input : void | semmle.label | call to user_input : void |
80-
| aliasing.cpp:80:10:80:10 | s [m1] : void | semmle.label | s [m1] : void |
8115
| aliasing.cpp:80:12:80:13 | m1 | semmle.label | m1 |
82-
| aliasing.cpp:86:3:86:21 | Store : void | semmle.label | Store : void |
83-
| aliasing.cpp:86:3:86:21 | Store [m1] : void | semmle.label | Store [m1] : void |
8416
| aliasing.cpp:86:10:86:19 | call to user_input : void | semmle.label | call to user_input : void |
85-
| aliasing.cpp:87:10:87:10 | s [m1] : void | semmle.label | s [m1] : void |
8617
| aliasing.cpp:87:12:87:13 | m1 | semmle.label | m1 |
87-
| aliasing.cpp:92:3:92:23 | Store : void | semmle.label | Store : void |
88-
| aliasing.cpp:92:3:92:23 | Store [m1] : void | semmle.label | Store [m1] : void |
8918
| aliasing.cpp:92:12:92:21 | call to user_input : void | semmle.label | call to user_input : void |
90-
| aliasing.cpp:93:10:93:10 | s [m1] : void | semmle.label | s [m1] : void |
9119
| aliasing.cpp:93:12:93:13 | m1 | semmle.label | m1 |
92-
| struct_init.c:20:20:20:29 | Store : void | semmle.label | Store : void |
93-
| struct_init.c:20:20:20:29 | Store [a] : void | semmle.label | Store [a] : void |
9420
| struct_init.c:20:20:20:29 | call to user_input : void | semmle.label | call to user_input : void |
95-
| struct_init.c:22:8:22:9 | ab [a] : void | semmle.label | ab [a] : void |
9621
| struct_init.c:22:11:22:11 | a | semmle.label | a |
97-
| struct_init.c:27:7:27:16 | Store : void | semmle.label | Store : void |
98-
| struct_init.c:27:7:27:16 | Store [a] : void | semmle.label | Store [a] : void |
9922
| struct_init.c:27:7:27:16 | call to user_input : void | semmle.label | call to user_input : void |
100-
| struct_init.c:31:14:31:21 | nestedAB [a] : void | semmle.label | nestedAB [a] : void |
10123
| struct_init.c:31:23:31:23 | a | semmle.label | a |
10224
#select
103-
| aliasing.cpp:29:11:29:12 | m1 | aliasing.cpp:9:11:9:20 | call to user_input : void | aliasing.cpp:29:11:29:12 | m1 | m1 flows from $@ | aliasing.cpp:9:11:9:20 | call to user_input : void | call to user_input : void |
104-
| aliasing.cpp:30:11:30:12 | m1 | aliasing.cpp:13:10:13:19 | call to user_input : void | aliasing.cpp:30:11:30:12 | m1 | m1 flows from $@ | aliasing.cpp:13:10:13:19 | call to user_input : void | call to user_input : void |
10525
| aliasing.cpp:38:11:38:12 | m1 | aliasing.cpp:37:13:37:22 | call to user_input : void | aliasing.cpp:38:11:38:12 | m1 | m1 flows from $@ | aliasing.cpp:37:13:37:22 | call to user_input : void | call to user_input : void |
10626
| aliasing.cpp:43:13:43:14 | m1 | aliasing.cpp:42:11:42:20 | call to user_input : void | aliasing.cpp:43:13:43:14 | m1 | m1 flows from $@ | aliasing.cpp:42:11:42:20 | call to user_input : void | call to user_input : void |
10727
| aliasing.cpp:80:12:80:13 | m1 | aliasing.cpp:79:11:79:20 | call to user_input : void | aliasing.cpp:80:12:80:13 | m1 | m1 flows from $@ | aliasing.cpp:79:11:79:20 | call to user_input : void | call to user_input : void |

0 commit comments

Comments
 (0)