Skip to content

Commit 99cc417

Browse files
committed
C++: Fix FPs by making 'isArgumentOfCallable' more robust.
1 parent f65fe34 commit 99cc417

File tree

3 files changed

+36
-30
lines changed

3 files changed

+36
-30
lines changed

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -684,8 +684,41 @@ predicate ssaFlow(Node nodeFrom, Node nodeTo) {
684684
)
685685
}
686686

687-
private predicate isArgumentOfCallable(DataFlowCall call, ArgumentNode arg) {
688-
arg.argumentOf(call, _)
687+
private predicate isArgumentOfCallableInstruction(DataFlowCall call, Instruction instr) {
688+
isArgumentOfCallableOperand(call, unique( | | getAUse(instr)))
689+
}
690+
691+
private predicate isArgumentOfCallableOperand(DataFlowCall call, Operand operand) {
692+
operand.(ArgumentOperand).getCall() = call
693+
or
694+
exists(FieldAddressInstruction fai |
695+
fai.getObjectAddressOperand() = operand and
696+
isArgumentOfCallableInstruction(call, fai)
697+
)
698+
or
699+
exists(Instruction deref |
700+
isArgumentOfCallableInstruction(call, deref) and
701+
isDereference(deref, operand, _)
702+
)
703+
or
704+
exists(Instruction instr |
705+
isArgumentOfCallableInstruction(call, instr) and
706+
conversionFlow(operand, instr, _, _)
707+
)
708+
}
709+
710+
private predicate isArgumentOfCallable(DataFlowCall call, Node n) {
711+
isArgumentOfCallableOperand(call, n.asOperand())
712+
or
713+
exists(Operand op |
714+
n.(IndirectOperand).hasOperandAndIndirectionIndex(op, _) and
715+
isArgumentOfCallableOperand(call, op)
716+
)
717+
or
718+
exists(Instruction instr |
719+
n.(IndirectInstruction).hasInstructionAndIndirectionIndex(instr, _) and
720+
isArgumentOfCallableInstruction(call, instr)
721+
)
689722
}
690723

691724
/** Holds if there is def-use or use-use flow from `pun` to `nodeTo`. */

cpp/ql/test/library-tests/dataflow/dataflow-tests/self_argument_flow.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace {
77

88
Foo* test_self_argument_flow() {
99
Foo *info;
10-
acquire(info->string); // $ SPURIOUS: self-arg-flow
10+
acquire(info->string); // clean
1111

1212
return info;
1313
}

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

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -267,12 +267,8 @@ edges
267267
| aliasing.cpp:105:23:105:24 | pa | aliasing.cpp:175:15:175:22 | taint_a_ptr output argument |
268268
| aliasing.cpp:105:23:105:24 | pa | aliasing.cpp:187:15:187:22 | taint_a_ptr output argument |
269269
| aliasing.cpp:105:23:105:24 | pa | aliasing.cpp:200:15:200:24 | taint_a_ptr output argument |
270-
| aliasing.cpp:105:23:105:24 | pa indirection | aliasing.cpp:105:23:105:24 | pa |
271270
| aliasing.cpp:106:9:106:18 | call to user_input | aliasing.cpp:105:23:105:24 | pa |
272271
| aliasing.cpp:121:15:121:16 | taint_a_ptr output argument | aliasing.cpp:122:8:122:12 | access to array |
273-
| aliasing.cpp:126:15:126:20 | ... - ... indirection | aliasing.cpp:105:23:105:24 | pa indirection |
274-
| aliasing.cpp:126:15:126:20 | ... - ... indirection | aliasing.cpp:126:15:126:20 | taint_a_ptr output argument |
275-
| aliasing.cpp:126:15:126:20 | taint_a_ptr output argument | aliasing.cpp:126:15:126:20 | ... - ... indirection |
276272
| aliasing.cpp:126:15:126:20 | taint_a_ptr output argument | aliasing.cpp:127:8:127:16 | * ... |
277273
| aliasing.cpp:131:15:131:16 | taint_a_ptr output argument | aliasing.cpp:132:8:132:14 | * ... |
278274
| aliasing.cpp:136:15:136:17 | taint_a_ptr output argument | aliasing.cpp:137:8:137:11 | * ... |
@@ -297,13 +293,8 @@ edges
297293
| aliasing.cpp:187:21:187:22 | s indirection [post update] [m1] | aliasing.cpp:187:19:187:19 | s2 indirection [post update] [s, m1] |
298294
| aliasing.cpp:189:8:189:11 | s2_2 indirection [s, m1] | aliasing.cpp:189:13:189:13 | s indirection [m1] |
299295
| aliasing.cpp:189:13:189:13 | s indirection [m1] | aliasing.cpp:189:15:189:16 | m1 |
300-
| aliasing.cpp:200:15:200:24 | & ... indirection | aliasing.cpp:105:23:105:24 | pa indirection |
301-
| aliasing.cpp:200:15:200:24 | & ... indirection | aliasing.cpp:200:15:200:24 | taint_a_ptr output argument |
302296
| aliasing.cpp:200:15:200:24 | taint_a_ptr output argument | aliasing.cpp:200:23:200:24 | s indirection [post update] [m1] |
303-
| aliasing.cpp:200:16:200:18 | ps2 indirection [s, m1] | aliasing.cpp:200:21:200:21 | s indirection [m1] |
304-
| aliasing.cpp:200:21:200:21 | ps2 indirection [post update] [s, m1] | aliasing.cpp:200:16:200:18 | ps2 indirection [s, m1] |
305297
| aliasing.cpp:200:21:200:21 | ps2 indirection [post update] [s, m1] | aliasing.cpp:201:8:201:10 | ps2 indirection [s, m1] |
306-
| aliasing.cpp:200:21:200:21 | s indirection [m1] | aliasing.cpp:200:15:200:24 | & ... indirection |
307298
| aliasing.cpp:200:23:200:24 | s indirection [post update] [m1] | aliasing.cpp:200:21:200:21 | ps2 indirection [post update] [s, m1] |
308299
| aliasing.cpp:201:8:201:10 | ps2 indirection [s, m1] | aliasing.cpp:201:13:201:13 | s indirection [m1] |
309300
| aliasing.cpp:201:13:201:13 | s indirection [m1] | aliasing.cpp:201:15:201:16 | m1 |
@@ -415,7 +406,6 @@ edges
415406
| by_reference.cpp:88:13:88:22 | call to user_input | by_reference.cpp:88:3:88:24 | ... = ... |
416407
| by_reference.cpp:91:25:91:26 | pa | by_reference.cpp:104:15:104:22 | taint_a_ptr output argument |
417408
| by_reference.cpp:91:25:91:26 | pa | by_reference.cpp:108:15:108:24 | taint_a_ptr output argument |
418-
| by_reference.cpp:91:25:91:26 | pa indirection | by_reference.cpp:91:25:91:26 | pa |
419409
| by_reference.cpp:92:9:92:18 | call to user_input | by_reference.cpp:91:25:91:26 | pa |
420410
| by_reference.cpp:95:25:95:26 | pa | by_reference.cpp:124:15:124:21 | taint_a_ref output argument |
421411
| by_reference.cpp:95:25:95:26 | pa | by_reference.cpp:128:15:128:23 | taint_a_ref output argument |
@@ -430,11 +420,7 @@ edges
430420
| by_reference.cpp:106:30:106:41 | pouter indirection [post update] [inner_nested, a] | by_reference.cpp:114:8:114:13 | pouter indirection [inner_nested, a] |
431421
| by_reference.cpp:107:29:107:37 | pouter indirection [post update] [inner_ptr indirection, a] | by_reference.cpp:115:8:115:13 | pouter indirection [inner_ptr indirection, a] |
432422
| by_reference.cpp:107:29:107:37 | taint_inner_a_ptr output argument [a] | by_reference.cpp:107:29:107:37 | pouter indirection [post update] [inner_ptr indirection, a] |
433-
| by_reference.cpp:108:15:108:24 | & ... indirection | by_reference.cpp:91:25:91:26 | pa indirection |
434-
| by_reference.cpp:108:15:108:24 | & ... indirection | by_reference.cpp:108:15:108:24 | taint_a_ptr output argument |
435423
| by_reference.cpp:108:15:108:24 | taint_a_ptr output argument | by_reference.cpp:108:24:108:24 | pouter indirection [post update] [a] |
436-
| by_reference.cpp:108:16:108:21 | pouter indirection [a] | by_reference.cpp:108:15:108:24 | & ... indirection |
437-
| by_reference.cpp:108:24:108:24 | pouter indirection [post update] [a] | by_reference.cpp:108:16:108:21 | pouter indirection [a] |
438424
| by_reference.cpp:108:24:108:24 | pouter indirection [post update] [a] | by_reference.cpp:116:8:116:13 | pouter indirection [a] |
439425
| by_reference.cpp:110:8:110:12 | outer indirection [inner_nested, a] | by_reference.cpp:110:14:110:25 | inner_nested indirection [a] |
440426
| by_reference.cpp:110:14:110:25 | inner_nested indirection [a] | by_reference.cpp:110:27:110:27 | a |
@@ -1104,12 +1090,9 @@ nodes
11041090
| aliasing.cpp:101:14:101:19 | s_copy indirection [m1] | semmle.label | s_copy indirection [m1] |
11051091
| aliasing.cpp:102:8:102:10 | * ... | semmle.label | * ... |
11061092
| aliasing.cpp:105:23:105:24 | pa | semmle.label | pa |
1107-
| aliasing.cpp:105:23:105:24 | pa | semmle.label | pa |
1108-
| aliasing.cpp:105:23:105:24 | pa indirection | semmle.label | pa indirection |
11091093
| aliasing.cpp:106:9:106:18 | call to user_input | semmle.label | call to user_input |
11101094
| aliasing.cpp:121:15:121:16 | taint_a_ptr output argument | semmle.label | taint_a_ptr output argument |
11111095
| aliasing.cpp:122:8:122:12 | access to array | semmle.label | access to array |
1112-
| aliasing.cpp:126:15:126:20 | ... - ... indirection | semmle.label | ... - ... indirection |
11131096
| aliasing.cpp:126:15:126:20 | taint_a_ptr output argument | semmle.label | taint_a_ptr output argument |
11141097
| aliasing.cpp:127:8:127:16 | * ... | semmle.label | * ... |
11151098
| aliasing.cpp:131:15:131:16 | taint_a_ptr output argument | semmle.label | taint_a_ptr output argument |
@@ -1141,11 +1124,8 @@ nodes
11411124
| aliasing.cpp:189:8:189:11 | s2_2 indirection [s, m1] | semmle.label | s2_2 indirection [s, m1] |
11421125
| aliasing.cpp:189:13:189:13 | s indirection [m1] | semmle.label | s indirection [m1] |
11431126
| aliasing.cpp:189:15:189:16 | m1 | semmle.label | m1 |
1144-
| aliasing.cpp:200:15:200:24 | & ... indirection | semmle.label | & ... indirection |
11451127
| aliasing.cpp:200:15:200:24 | taint_a_ptr output argument | semmle.label | taint_a_ptr output argument |
1146-
| aliasing.cpp:200:16:200:18 | ps2 indirection [s, m1] | semmle.label | ps2 indirection [s, m1] |
11471128
| aliasing.cpp:200:21:200:21 | ps2 indirection [post update] [s, m1] | semmle.label | ps2 indirection [post update] [s, m1] |
1148-
| aliasing.cpp:200:21:200:21 | s indirection [m1] | semmle.label | s indirection [m1] |
11491129
| aliasing.cpp:200:23:200:24 | s indirection [post update] [m1] | semmle.label | s indirection [post update] [m1] |
11501130
| aliasing.cpp:201:8:201:10 | ps2 indirection [s, m1] | semmle.label | ps2 indirection [s, m1] |
11511131
| aliasing.cpp:201:13:201:13 | s indirection [m1] | semmle.label | s indirection [m1] |
@@ -1250,8 +1230,6 @@ nodes
12501230
| by_reference.cpp:88:9:88:9 | inner indirection [post update] [a] | semmle.label | inner indirection [post update] [a] |
12511231
| by_reference.cpp:88:13:88:22 | call to user_input | semmle.label | call to user_input |
12521232
| by_reference.cpp:91:25:91:26 | pa | semmle.label | pa |
1253-
| by_reference.cpp:91:25:91:26 | pa | semmle.label | pa |
1254-
| by_reference.cpp:91:25:91:26 | pa indirection | semmle.label | pa indirection |
12551233
| by_reference.cpp:92:9:92:18 | call to user_input | semmle.label | call to user_input |
12561234
| by_reference.cpp:95:25:95:26 | pa | semmle.label | pa |
12571235
| by_reference.cpp:96:8:96:17 | call to user_input | semmle.label | call to user_input |
@@ -1265,9 +1243,7 @@ nodes
12651243
| by_reference.cpp:106:30:106:41 | pouter indirection [post update] [inner_nested, a] | semmle.label | pouter indirection [post update] [inner_nested, a] |
12661244
| by_reference.cpp:107:29:107:37 | pouter indirection [post update] [inner_ptr indirection, a] | semmle.label | pouter indirection [post update] [inner_ptr indirection, a] |
12671245
| by_reference.cpp:107:29:107:37 | taint_inner_a_ptr output argument [a] | semmle.label | taint_inner_a_ptr output argument [a] |
1268-
| by_reference.cpp:108:15:108:24 | & ... indirection | semmle.label | & ... indirection |
12691246
| by_reference.cpp:108:15:108:24 | taint_a_ptr output argument | semmle.label | taint_a_ptr output argument |
1270-
| by_reference.cpp:108:16:108:21 | pouter indirection [a] | semmle.label | pouter indirection [a] |
12711247
| by_reference.cpp:108:24:108:24 | pouter indirection [post update] [a] | semmle.label | pouter indirection [post update] [a] |
12721248
| by_reference.cpp:110:8:110:12 | outer indirection [inner_nested, a] | semmle.label | outer indirection [inner_nested, a] |
12731249
| by_reference.cpp:110:14:110:25 | inner_nested indirection [a] | semmle.label | inner_nested indirection [a] |
@@ -1692,8 +1668,6 @@ subpaths
16921668
| D.cpp:22:14:22:20 | call to getBox1 indirection [elem] | D.cpp:10:11:10:17 | this indirection [elem] | D.cpp:10:11:10:17 | getElem indirection | D.cpp:22:10:22:33 | call to getElem |
16931669
| D.cpp:37:21:37:21 | e | D.cpp:11:24:11:24 | e | D.cpp:11:29:11:32 | this indirection [post update] [elem] | D.cpp:37:8:37:10 | setElem output argument [elem] |
16941670
| D.cpp:51:27:51:27 | e | D.cpp:11:24:11:24 | e | D.cpp:11:29:11:32 | this indirection [post update] [elem] | D.cpp:51:8:51:14 | setElem output argument [elem] |
1695-
| aliasing.cpp:126:15:126:20 | ... - ... indirection | aliasing.cpp:105:23:105:24 | pa indirection | aliasing.cpp:105:23:105:24 | pa | aliasing.cpp:126:15:126:20 | taint_a_ptr output argument |
1696-
| aliasing.cpp:200:15:200:24 | & ... indirection | aliasing.cpp:105:23:105:24 | pa indirection | aliasing.cpp:105:23:105:24 | pa | aliasing.cpp:200:15:200:24 | taint_a_ptr output argument |
16971671
| by_reference.cpp:20:23:20:27 | value | by_reference.cpp:15:26:15:30 | value | by_reference.cpp:16:11:16:11 | this indirection [post update] [a] | by_reference.cpp:20:5:20:8 | setDirectly output argument [a] |
16981672
| by_reference.cpp:24:25:24:29 | value | by_reference.cpp:11:48:11:52 | value | by_reference.cpp:12:8:12:8 | s indirection [post update] [a] | by_reference.cpp:24:19:24:22 | nonMemberSetA output argument [a] |
16991673
| by_reference.cpp:40:12:40:15 | this indirection [a] | by_reference.cpp:35:9:35:19 | this indirection [a] | by_reference.cpp:35:9:35:19 | getDirectly indirection | by_reference.cpp:40:18:40:28 | call to getDirectly |
@@ -1706,7 +1680,6 @@ subpaths
17061680
| by_reference.cpp:63:8:63:8 | s indirection [a] | by_reference.cpp:43:9:43:27 | this indirection [a] | by_reference.cpp:43:9:43:27 | getThroughNonMember indirection | by_reference.cpp:63:10:63:28 | call to getThroughNonMember |
17071681
| by_reference.cpp:68:21:68:30 | call to user_input | by_reference.cpp:11:48:11:52 | value | by_reference.cpp:12:8:12:8 | s indirection [post update] [a] | by_reference.cpp:68:17:68:18 | nonMemberSetA output argument [a] |
17081682
| by_reference.cpp:69:22:69:23 | & ... indirection [a] | by_reference.cpp:31:46:31:46 | s indirection [a] | by_reference.cpp:31:16:31:28 | nonMemberGetA indirection | by_reference.cpp:69:8:69:20 | call to nonMemberGetA |
1709-
| by_reference.cpp:108:15:108:24 | & ... indirection | by_reference.cpp:91:25:91:26 | pa indirection | by_reference.cpp:91:25:91:26 | pa | by_reference.cpp:108:15:108:24 | taint_a_ptr output argument |
17101683
| complex.cpp:42:16:42:16 | f indirection [a_] | complex.cpp:9:7:9:7 | this indirection [a_] | complex.cpp:9:7:9:7 | a indirection | complex.cpp:42:18:42:18 | call to a |
17111684
| complex.cpp:43:16:43:16 | f indirection [b_] | complex.cpp:10:7:10:7 | this indirection [b_] | complex.cpp:10:7:10:7 | b indirection | complex.cpp:43:18:43:18 | call to b |
17121685
| complex.cpp:53:19:53:28 | call to user_input | complex.cpp:11:17:11:17 | a | complex.cpp:11:22:11:23 | this indirection [post update] [a_] | complex.cpp:53:12:53:12 | setA output argument [a_] |

0 commit comments

Comments
 (0)