Skip to content

Commit a6e619c

Browse files
committed
C++: Add field flow through single-field structs and accept tests
1 parent 6fca23b commit a6e619c

File tree

4 files changed

+51
-6
lines changed

4 files changed

+51
-6
lines changed

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,16 @@ private class ArrayContent extends Content, TArrayContent {
180180
override Type getType() { none() }
181181
}
182182

183-
/**
184-
* Holds if data can flow from `node1` to `node2` via an assignment to `f`.
185-
* Thus, `node2` references an object with a field `f` that contains the
186-
* value of `node1`.
187-
*/
188-
predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
183+
private predicate storeStepNoChi(Node node1, Content f, PostUpdateNode node2) {
184+
exists(FieldAddressInstruction fa, StoreInstruction store |
185+
store = node2.asInstruction() and
186+
store.getDestinationAddress() = fa and
187+
store.getSourceValue() = node1.asInstruction() and
188+
f.(FieldContent).getField() = fa.getField()
189+
)
190+
}
191+
192+
private predicate storeStepChi(Node node1, Content f, PostUpdateNode node2) {
189193
exists(FieldAddressInstruction fa, StoreInstruction store |
190194
node1.asInstruction() = store and
191195
store.getDestinationAddress() = fa and
@@ -194,6 +198,16 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
194198
)
195199
}
196200

201+
/**
202+
* Holds if data can flow from `node1` to `node2` via an assignment to `f`.
203+
* Thus, `node2` references an object with a field `f` that contains the
204+
* value of `node1`.
205+
*/
206+
predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
207+
storeStepNoChi(node1, f, node2) or
208+
storeStepChi(node1, f, node2)
209+
}
210+
197211
/**
198212
* Holds if data can flow from `node1` to `node2` via a read of `f`.
199213
* Thus, `node1` references an object with a field `f` whose value ends up in

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,19 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
270270
override Node getPreUpdateNode() { result.asInstruction() = instr.getTotal() }
271271
}
272272

273+
private class ExplicitSingleFieldStoreQualifierNode extends PartialDefinitionNode {
274+
override StoreInstruction instr;
275+
276+
ExplicitSingleFieldStoreQualifierNode() {
277+
exists(FieldAddressInstruction field |
278+
field = instr.getDestinationAddress() and
279+
not exists(ChiInstruction chi | chi.getPartial() = instr)
280+
)
281+
}
282+
283+
override Node getPreUpdateNode() { none() }
284+
}
285+
273286
/**
274287
* A node that represents the value of a variable after a function call that
275288
* may have changed the variable because it's passed by reference.
@@ -404,6 +417,8 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
404417
simpleInstructionLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asInstruction())
405418
}
406419

420+
private predicate hasSize(Type t, int size) { t.getSize() = size }
421+
407422
cached
408423
private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction iTo) {
409424
iTo.(CopyInstruction).getSourceValue() = iFrom
@@ -452,6 +467,13 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
452467
iTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = chi
453468
)
454469
or
470+
iTo.(CopyInstruction).getSourceValueOperand().getAnyDef() = iFrom and
471+
exists(Class c, int size |
472+
c = iTo.getResultType() and
473+
hasSize(c, size) and
474+
hasSize(iFrom.getResultType(), size)
475+
)
476+
or
455477
// Flow through modeled functions
456478
modelFlow(iFrom, iTo)
457479
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ unreachableNodeCCtx
1919
localCallNodes
2020
postIsNotPre
2121
postHasUniquePre
22+
| simple.cpp:65:5:65:22 | Store | PostUpdateNode should have one pre-update node but has 0. |
2223
uniquePostUpdate
2324
postIsInSameCallable
2425
reverseRead

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ edges
2525
| aliasing.cpp:79:11:79:20 | call to user_input | aliasing.cpp:80:12:80:13 | m1 |
2626
| aliasing.cpp:86:10:86:19 | call to user_input | aliasing.cpp:87:12:87:13 | m1 |
2727
| aliasing.cpp:92:12:92:21 | call to user_input | aliasing.cpp:93:12:93:13 | m1 |
28+
| simple.cpp:65:5:65:22 | Store [i] | simple.cpp:66:12:66:12 | Store [i] |
29+
| simple.cpp:65:11:65:20 | call to user_input | simple.cpp:65:5:65:22 | Store [i] |
30+
| simple.cpp:66:12:66:12 | Store [i] | simple.cpp:67:13:67:13 | i |
2831
| struct_init.c:20:20:20:29 | call to user_input | struct_init.c:22:11:22:11 | a |
2932
| struct_init.c:27:7:27:16 | call to user_input | struct_init.c:31:23:31:23 | a |
3033
nodes
@@ -63,6 +66,10 @@ nodes
6366
| aliasing.cpp:87:12:87:13 | m1 | semmle.label | m1 |
6467
| aliasing.cpp:92:12:92:21 | call to user_input | semmle.label | call to user_input |
6568
| aliasing.cpp:93:12:93:13 | m1 | semmle.label | m1 |
69+
| simple.cpp:65:5:65:22 | Store [i] | semmle.label | Store [i] |
70+
| simple.cpp:65:11:65:20 | call to user_input | semmle.label | call to user_input |
71+
| simple.cpp:66:12:66:12 | Store [i] | semmle.label | Store [i] |
72+
| simple.cpp:67:13:67:13 | i | semmle.label | i |
6673
| struct_init.c:20:20:20:29 | call to user_input | semmle.label | call to user_input |
6774
| struct_init.c:22:11:22:11 | a | semmle.label | a |
6875
| struct_init.c:27:7:27:16 | call to user_input | semmle.label | call to user_input |
@@ -78,5 +85,6 @@ nodes
7885
| aliasing.cpp:80:12:80:13 | m1 | aliasing.cpp:79:11:79:20 | call to user_input | aliasing.cpp:80:12:80:13 | m1 | m1 flows from $@ | aliasing.cpp:79:11:79:20 | call to user_input | call to user_input |
7986
| aliasing.cpp:87:12:87:13 | m1 | aliasing.cpp:86:10:86:19 | call to user_input | aliasing.cpp:87:12:87:13 | m1 | m1 flows from $@ | aliasing.cpp:86:10:86:19 | call to user_input | call to user_input |
8087
| aliasing.cpp:93:12:93:13 | m1 | aliasing.cpp:92:12:92:21 | call to user_input | aliasing.cpp:93:12:93:13 | m1 | m1 flows from $@ | aliasing.cpp:92:12:92:21 | call to user_input | call to user_input |
88+
| simple.cpp:67:13:67:13 | i | simple.cpp:65:11:65:20 | call to user_input | simple.cpp:67:13:67:13 | i | i flows from $@ | simple.cpp:65:11:65:20 | call to user_input | call to user_input |
8189
| struct_init.c:22:11:22:11 | a | struct_init.c:20:20:20:29 | call to user_input | struct_init.c:22:11:22:11 | a | a flows from $@ | struct_init.c:20:20:20:29 | call to user_input | call to user_input |
8290
| struct_init.c:31:23:31:23 | a | struct_init.c:27:7:27:16 | call to user_input | struct_init.c:31:23:31:23 | a | a flows from $@ | struct_init.c:27:7:27:16 | call to user_input | call to user_input |

0 commit comments

Comments
 (0)