Skip to content

Commit 32f1da7

Browse files
authored
Merge pull request github#5327 from MathiasVP/less-field-to-obj-flow
C++: Remove more field-to-object flow
2 parents a55246c + c86fc22 commit 32f1da7

File tree

17 files changed

+135
-27
lines changed

17 files changed

+135
-27
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -693,7 +693,11 @@ private predicate simpleInstructionLocalFlowStep(Operand opFrom, Instruction iTo
693693
exists(ChiInstruction chi | chi = iTo |
694694
opFrom.getAnyDef() instanceof WriteSideEffectInstruction and
695695
chi.getPartialOperand() = opFrom and
696-
not chi.isResultConflated()
696+
not chi.isResultConflated() and
697+
// In a call such as `set_value(&x->val);` we don't want the memory representing `x` to receive
698+
// dataflow by a simple step. Instead, this is handled by field flow. If we add a simple step here
699+
// we can get field-to-object flow.
700+
not chi.isPartialUpdate()
697701
)
698702
or
699703
// Flow through modeled functions

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2055,6 +2055,13 @@ class ChiInstruction extends Instruction {
20552055
final predicate getUpdatedInterval(int startBit, int endBit) {
20562056
Construction::getIntervalUpdatedByChi(this, startBit, endBit)
20572057
}
2058+
2059+
/**
2060+
* Holds if the `ChiPartialOperand` totally, but not exactly, overlaps with the `ChiTotalOperand`.
2061+
* This means that the `ChiPartialOperand` will not override the entire memory associated with the
2062+
* `ChiTotalOperand`.
2063+
*/
2064+
final predicate isPartialUpdate() { Construction::chiOnlyPartiallyUpdatesLocation(this) }
20582065
}
20592066

20602067
/**

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,11 @@ MemoryLocation getOperandMemoryLocation(MemoryOperand operand) {
629629
}
630630

631631
/** Gets the start bit offset of a `MemoryLocation`, if any. */
632-
int getStartBitOffset(VariableMemoryLocation location) { result = location.getStartBitOffset() }
632+
int getStartBitOffset(VariableMemoryLocation location) {
633+
result = location.getStartBitOffset() and Ints::hasValue(result)
634+
}
633635

634636
/** Gets the end bit offset of a `MemoryLocation`, if any. */
635-
int getEndBitOffset(VariableMemoryLocation location) { result = location.getEndBitOffset() }
637+
int getEndBitOffset(VariableMemoryLocation location) {
638+
result = location.getEndBitOffset() and Ints::hasValue(result)
639+
}

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,22 @@ private module Cached {
178178
)
179179
}
180180

181+
/**
182+
* Holds if the `ChiPartialOperand` only partially overlaps with the `ChiTotalOperand`.
183+
* This means that the `ChiPartialOperand` will not override the entire memory associated
184+
* with the `ChiTotalOperand`.
185+
*/
186+
cached
187+
predicate chiOnlyPartiallyUpdatesLocation(ChiInstruction chi) {
188+
exists(Alias::MemoryLocation location, OldInstruction oldInstruction |
189+
oldInstruction = getOldInstruction(chi.getPartial()) and
190+
location = Alias::getResultMemoryLocation(oldInstruction)
191+
|
192+
Alias::getStartBitOffset(location) != 0 or
193+
Alias::getEndBitOffset(location) != 8 * location.getType().getByteSize()
194+
)
195+
}
196+
181197
/**
182198
* Holds if `instr` is part of a cycle in the operand graph that doesn't go
183199
* through a phi instruction and therefore should be impossible.

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2055,6 +2055,13 @@ class ChiInstruction extends Instruction {
20552055
final predicate getUpdatedInterval(int startBit, int endBit) {
20562056
Construction::getIntervalUpdatedByChi(this, startBit, endBit)
20572057
}
2058+
2059+
/**
2060+
* Holds if the `ChiPartialOperand` totally, but not exactly, overlaps with the `ChiTotalOperand`.
2061+
* This means that the `ChiPartialOperand` will not override the entire memory associated with the
2062+
* `ChiTotalOperand`.
2063+
*/
2064+
final predicate isPartialUpdate() { Construction::chiOnlyPartiallyUpdatesLocation(this) }
20582065
}
20592066

20602067
/**

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ predicate getIntervalUpdatedByChi(ChiInstruction chi, int startBit, int endBit)
194194
*/
195195
predicate getUsedInterval(Operand operand, int startBit, int endBit) { none() }
196196

197+
predicate chiOnlyPartiallyUpdatesLocation(ChiInstruction chi) { none() }
198+
197199
/** Gets a non-phi instruction that defines an operand of `instr`. */
198200
private Instruction getNonPhiOperandDef(Instruction instr) {
199201
result = getRegisterOperandDefinition(instr, _)

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2055,6 +2055,13 @@ class ChiInstruction extends Instruction {
20552055
final predicate getUpdatedInterval(int startBit, int endBit) {
20562056
Construction::getIntervalUpdatedByChi(this, startBit, endBit)
20572057
}
2058+
2059+
/**
2060+
* Holds if the `ChiPartialOperand` totally, but not exactly, overlaps with the `ChiTotalOperand`.
2061+
* This means that the `ChiPartialOperand` will not override the entire memory associated with the
2062+
* `ChiTotalOperand`.
2063+
*/
2064+
final predicate isPartialUpdate() { Construction::chiOnlyPartiallyUpdatesLocation(this) }
20582065
}
20592066

20602067
/**

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,22 @@ private module Cached {
178178
)
179179
}
180180

181+
/**
182+
* Holds if the `ChiPartialOperand` only partially overlaps with the `ChiTotalOperand`.
183+
* This means that the `ChiPartialOperand` will not override the entire memory associated
184+
* with the `ChiTotalOperand`.
185+
*/
186+
cached
187+
predicate chiOnlyPartiallyUpdatesLocation(ChiInstruction chi) {
188+
exists(Alias::MemoryLocation location, OldInstruction oldInstruction |
189+
oldInstruction = getOldInstruction(chi.getPartial()) and
190+
location = Alias::getResultMemoryLocation(oldInstruction)
191+
|
192+
Alias::getStartBitOffset(location) != 0 or
193+
Alias::getEndBitOffset(location) != 8 * location.getType().getByteSize()
194+
)
195+
}
196+
181197
/**
182198
* Holds if `instr` is part of a cycle in the operand graph that doesn't go
183199
* through a phi instruction and therefore should be impossible.

cpp/ql/test/library-tests/dataflow/fields/aliasing.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ void pointer_deref(int* xs) {
124124

125125
void pointer_deref_sub(int* xs) {
126126
taint_a_ptr(xs - 2);
127-
sink(*(xs - 2)); // $ ir MISSING: ast
127+
sink(*(xs - 2)); // $ MISSING: ast,ir
128128
}
129129

130130
void pointer_many_addrof_and_deref(int* xs) {
@@ -156,13 +156,13 @@ struct S_with_array {
156156
void pointer_member_deref() {
157157
S_with_array s;
158158
taint_a_ptr(s.data);
159-
sink(*s.data); // $ ir,ast
159+
sink(*s.data); // $ ast MISSING: ir
160160
}
161161

162162
void array_member_deref() {
163163
S_with_array s;
164164
taint_a_ptr(s.data);
165-
sink(s.data[0]); // $ ir,ast
165+
sink(s.data[0]); // $ ast MISSING: ir
166166
}
167167

168168
struct S2 {

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

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -50,27 +50,18 @@ edges
5050
| aliasing.cpp:98:10:98:19 | call to user_input | aliasing.cpp:98:3:98:21 | Chi [m1] |
5151
| aliasing.cpp:100:14:100:14 | Store [m1] | aliasing.cpp:102:8:102:10 | * ... |
5252
| aliasing.cpp:106:3:106:20 | Chi [array content] | aliasing.cpp:121:15:121:16 | taint_a_ptr output argument [array content] |
53-
| aliasing.cpp:106:3:106:20 | Chi [array content] | aliasing.cpp:126:15:126:20 | taint_a_ptr output argument [array content] |
5453
| aliasing.cpp:106:3:106:20 | Chi [array content] | aliasing.cpp:131:15:131:16 | taint_a_ptr output argument [array content] |
5554
| aliasing.cpp:106:3:106:20 | Chi [array content] | aliasing.cpp:136:15:136:17 | taint_a_ptr output argument [array content] |
56-
| aliasing.cpp:106:3:106:20 | Chi [array content] | aliasing.cpp:158:15:158:20 | taint_a_ptr output argument [array content] |
57-
| aliasing.cpp:106:3:106:20 | Chi [array content] | aliasing.cpp:164:15:164:20 | taint_a_ptr output argument [array content] |
5855
| aliasing.cpp:106:3:106:20 | Chi [array content] | aliasing.cpp:175:15:175:22 | taint_a_ptr output argument [array content] |
5956
| aliasing.cpp:106:3:106:20 | Chi [array content] | aliasing.cpp:187:15:187:22 | taint_a_ptr output argument [array content] |
6057
| aliasing.cpp:106:3:106:20 | Chi [array content] | aliasing.cpp:200:15:200:24 | taint_a_ptr output argument [array content] |
6158
| aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:106:3:106:20 | Chi [array content] |
6259
| aliasing.cpp:121:15:121:16 | Chi [array content] | aliasing.cpp:122:8:122:12 | access to array |
6360
| aliasing.cpp:121:15:121:16 | taint_a_ptr output argument [array content] | aliasing.cpp:121:15:121:16 | Chi [array content] |
64-
| aliasing.cpp:126:15:126:20 | Chi [array content] | aliasing.cpp:127:8:127:16 | * ... |
65-
| aliasing.cpp:126:15:126:20 | taint_a_ptr output argument [array content] | aliasing.cpp:126:15:126:20 | Chi [array content] |
6661
| aliasing.cpp:131:15:131:16 | Chi [array content] | aliasing.cpp:132:8:132:14 | * ... |
6762
| aliasing.cpp:131:15:131:16 | taint_a_ptr output argument [array content] | aliasing.cpp:131:15:131:16 | Chi [array content] |
6863
| aliasing.cpp:136:15:136:17 | Chi [array content] | aliasing.cpp:137:8:137:11 | * ... |
6964
| aliasing.cpp:136:15:136:17 | taint_a_ptr output argument [array content] | aliasing.cpp:136:15:136:17 | Chi [array content] |
70-
| aliasing.cpp:158:15:158:20 | Chi [array content] | aliasing.cpp:159:8:159:14 | * ... |
71-
| aliasing.cpp:158:15:158:20 | taint_a_ptr output argument [array content] | aliasing.cpp:158:15:158:20 | Chi [array content] |
72-
| aliasing.cpp:164:15:164:20 | Chi [array content] | aliasing.cpp:165:8:165:16 | access to array |
73-
| aliasing.cpp:164:15:164:20 | taint_a_ptr output argument [array content] | aliasing.cpp:164:15:164:20 | Chi [array content] |
7465
| aliasing.cpp:175:15:175:22 | Chi | aliasing.cpp:175:15:175:22 | Chi [m1] |
7566
| aliasing.cpp:175:15:175:22 | Chi [m1] | aliasing.cpp:176:13:176:14 | m1 |
7667
| aliasing.cpp:175:15:175:22 | taint_a_ptr output argument [array content] | aliasing.cpp:175:15:175:22 | Chi |
@@ -270,21 +261,12 @@ nodes
270261
| aliasing.cpp:121:15:121:16 | Chi [array content] | semmle.label | Chi [array content] |
271262
| aliasing.cpp:121:15:121:16 | taint_a_ptr output argument [array content] | semmle.label | taint_a_ptr output argument [array content] |
272263
| aliasing.cpp:122:8:122:12 | access to array | semmle.label | access to array |
273-
| aliasing.cpp:126:15:126:20 | Chi [array content] | semmle.label | Chi [array content] |
274-
| aliasing.cpp:126:15:126:20 | taint_a_ptr output argument [array content] | semmle.label | taint_a_ptr output argument [array content] |
275-
| aliasing.cpp:127:8:127:16 | * ... | semmle.label | * ... |
276264
| aliasing.cpp:131:15:131:16 | Chi [array content] | semmle.label | Chi [array content] |
277265
| aliasing.cpp:131:15:131:16 | taint_a_ptr output argument [array content] | semmle.label | taint_a_ptr output argument [array content] |
278266
| aliasing.cpp:132:8:132:14 | * ... | semmle.label | * ... |
279267
| aliasing.cpp:136:15:136:17 | Chi [array content] | semmle.label | Chi [array content] |
280268
| aliasing.cpp:136:15:136:17 | taint_a_ptr output argument [array content] | semmle.label | taint_a_ptr output argument [array content] |
281269
| aliasing.cpp:137:8:137:11 | * ... | semmle.label | * ... |
282-
| aliasing.cpp:158:15:158:20 | Chi [array content] | semmle.label | Chi [array content] |
283-
| aliasing.cpp:158:15:158:20 | taint_a_ptr output argument [array content] | semmle.label | taint_a_ptr output argument [array content] |
284-
| aliasing.cpp:159:8:159:14 | * ... | semmle.label | * ... |
285-
| aliasing.cpp:164:15:164:20 | Chi [array content] | semmle.label | Chi [array content] |
286-
| aliasing.cpp:164:15:164:20 | taint_a_ptr output argument [array content] | semmle.label | taint_a_ptr output argument [array content] |
287-
| aliasing.cpp:165:8:165:16 | access to array | semmle.label | access to array |
288270
| aliasing.cpp:175:15:175:22 | Chi | semmle.label | Chi |
289271
| aliasing.cpp:175:15:175:22 | Chi [m1] | semmle.label | Chi [m1] |
290272
| aliasing.cpp:175:15:175:22 | taint_a_ptr output argument [array content] | semmle.label | taint_a_ptr output argument [array content] |
@@ -441,11 +423,8 @@ nodes
441423
| 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 |
442424
| aliasing.cpp:102:8:102:10 | * ... | aliasing.cpp:98:10:98:19 | call to user_input | aliasing.cpp:102:8:102:10 | * ... | * ... flows from $@ | aliasing.cpp:98:10:98:19 | call to user_input | call to user_input |
443425
| aliasing.cpp:122:8:122:12 | access to array | aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:122:8:122:12 | access to array | access to array flows from $@ | aliasing.cpp:106:9:106:18 | call to user_input | call to user_input |
444-
| aliasing.cpp:127:8:127:16 | * ... | aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:127:8:127:16 | * ... | * ... flows from $@ | aliasing.cpp:106:9:106:18 | call to user_input | call to user_input |
445426
| aliasing.cpp:132:8:132:14 | * ... | aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:132:8:132:14 | * ... | * ... flows from $@ | aliasing.cpp:106:9:106:18 | call to user_input | call to user_input |
446427
| aliasing.cpp:137:8:137:11 | * ... | aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:137:8:137:11 | * ... | * ... flows from $@ | aliasing.cpp:106:9:106:18 | call to user_input | call to user_input |
447-
| aliasing.cpp:159:8:159:14 | * ... | aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:159:8:159:14 | * ... | * ... flows from $@ | aliasing.cpp:106:9:106:18 | call to user_input | call to user_input |
448-
| aliasing.cpp:165:8:165:16 | access to array | aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:165:8:165:16 | access to array | access to array flows from $@ | aliasing.cpp:106:9:106:18 | call to user_input | call to user_input |
449428
| aliasing.cpp:176:13:176:14 | m1 | aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:176:13:176:14 | m1 | m1 flows from $@ | aliasing.cpp:106:9:106:18 | call to user_input | call to user_input |
450429
| aliasing.cpp:189:15:189:16 | m1 | aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:189:15:189:16 | m1 | m1 flows from $@ | aliasing.cpp:106:9:106:18 | call to user_input | call to user_input |
451430
| aliasing.cpp:201:15:201:16 | m1 | aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:201:15:201:16 | m1 | m1 flows from $@ | aliasing.cpp:106:9:106:18 | call to user_input | call to user_input |

0 commit comments

Comments
 (0)