Skip to content

Commit d40322f

Browse files
MathiasVPMathiasVP
committed
C++: (Bugfix 3) Don't conflate summarized callables and source callables in 'nodeGetEnclosingCallable'.
1 parent 06bc8ad commit d40322f

File tree

5 files changed

+79
-185
lines changed

5 files changed

+79
-185
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowDispatch.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ private module VirtualDispatch {
152152
ReturnNode node, ReturnKind kind, DataFlowCallable callable
153153
) {
154154
node.getKind() = kind and
155-
node.getEnclosingCallable() = callable.getUnderlyingCallable()
155+
node.getFunction() = callable.getUnderlyingCallable()
156156
}
157157

158158
/** Call through a function pointer. */

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,7 @@ private module IndirectInstructions {
333333
import IndirectInstructions
334334

335335
/** Gets the callable in which this node occurs. */
336-
DataFlowCallable nodeGetEnclosingCallable(Node n) {
337-
result.getUnderlyingCallable() = n.getEnclosingCallable()
338-
}
336+
DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
339337

340338
/** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */
341339
predicate isParameterNode(ParameterNode p, DataFlowCallable c, ParameterPosition pos) {

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

Lines changed: 49 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ class Node extends TIRDataFlowNode {
147147
/**
148148
* INTERNAL: Do not use.
149149
*/
150-
Declaration getEnclosingCallable() { none() } // overridden in subclasses
150+
DataFlowCallable getEnclosingCallable() { none() } // overridden in subclasses
151151

152152
/** Gets the function to which this node belongs, if any. */
153153
Declaration getFunction() { none() } // overridden in subclasses
@@ -509,7 +509,9 @@ private class Node0 extends Node, TNode0 {
509509

510510
Node0() { this = TNode0(node) }
511511

512-
override Declaration getEnclosingCallable() { result = node.getEnclosingCallable() }
512+
override DataFlowCallable getEnclosingCallable() {
513+
result.asSourceCallable() = node.getEnclosingCallable()
514+
}
513515

514516
override Declaration getFunction() { result = node.getFunction() }
515517

@@ -574,7 +576,9 @@ class PostUpdateNodeImpl extends PartialDefinitionNode, TPostUpdateNodeImpl {
574576

575577
override Declaration getFunction() { result = operand.getUse().getEnclosingFunction() }
576578

577-
override Declaration getEnclosingCallable() { result = this.getFunction() }
579+
override DataFlowCallable getEnclosingCallable() {
580+
result = this.getPreUpdateNode().getEnclosingCallable()
581+
}
578582

579583
/** Gets the operand associated with this node. */
580584
Operand getOperand() { result = operand }
@@ -627,7 +631,9 @@ class SsaPhiNode extends Node, TSsaPhiNode {
627631
/** Gets the phi node associated with this node. */
628632
Ssa::PhiNode getPhiNode() { result = phi }
629633

630-
override Declaration getEnclosingCallable() { result = this.getFunction() }
634+
override DataFlowCallable getEnclosingCallable() {
635+
result.asSourceCallable() = this.getFunction()
636+
}
631637

632638
override Declaration getFunction() { result = phi.getBasicBlock().getEnclosingFunction() }
633639

@@ -710,7 +716,9 @@ class SsaPhiInputNode extends Node, TSsaPhiInputNode {
710716
/** Gets the basic block in which this input originates. */
711717
IRBlock getBlock() { result = block }
712718

713-
override Declaration getEnclosingCallable() { result = this.getFunction() }
719+
override DataFlowCallable getEnclosingCallable() {
720+
result.asSourceCallable() = this.getFunction()
721+
}
714722

715723
override Declaration getFunction() { result = phi.getBasicBlock().getEnclosingFunction() }
716724

@@ -739,7 +747,9 @@ class SsaIteratorNode extends Node, TSsaIteratorNode {
739747
/** Gets the phi node associated with this node. */
740748
IteratorFlow::IteratorFlowNode getIteratorFlowNode() { result = node }
741749

742-
override Declaration getEnclosingCallable() { result = this.getFunction() }
750+
override DataFlowCallable getEnclosingCallable() {
751+
result.asSourceCallable() = this.getFunction()
752+
}
743753

744754
override Declaration getFunction() { result = node.getFunction() }
745755

@@ -774,7 +784,9 @@ class SideEffectOperandNode extends Node instanceof IndirectOperand {
774784

775785
int getArgumentIndex() { result = argumentIndex }
776786

777-
override Declaration getEnclosingCallable() { result = this.getFunction() }
787+
override DataFlowCallable getEnclosingCallable() {
788+
result.asSourceCallable() = this.getFunction()
789+
}
778790

779791
override Declaration getFunction() { result = call.getEnclosingFunction() }
780792

@@ -795,7 +807,9 @@ class FinalGlobalValue extends Node, TFinalGlobalValue {
795807
/** Gets the underlying SSA use. */
796808
Ssa::GlobalUse getGlobalUse() { result = globalUse }
797809

798-
override Declaration getEnclosingCallable() { result = this.getFunction() }
810+
override DataFlowCallable getEnclosingCallable() {
811+
result.asSourceCallable() = this.getFunction()
812+
}
799813

800814
override Declaration getFunction() { result = globalUse.getIRFunction().getFunction() }
801815

@@ -825,7 +839,9 @@ class InitialGlobalValue extends Node, TInitialGlobalValue {
825839
/** Gets the underlying SSA definition. */
826840
Ssa::GlobalDef getGlobalDef() { result = globalDef }
827841

828-
override Declaration getEnclosingCallable() { result = this.getFunction() }
842+
override DataFlowCallable getEnclosingCallable() {
843+
result.asSourceCallable() = this.getFunction()
844+
}
829845

830846
override Declaration getFunction() { result = globalDef.getIRFunction().getFunction() }
831847

@@ -856,7 +872,9 @@ class BodyLessParameterNodeImpl extends Node, TBodyLessParameterNodeImpl {
856872

857873
BodyLessParameterNodeImpl() { this = TBodyLessParameterNodeImpl(p, indirectionIndex) }
858874

859-
override Declaration getEnclosingCallable() { result = this.getFunction() }
875+
override DataFlowCallable getEnclosingCallable() {
876+
result.asSourceCallable() = this.getFunction()
877+
}
860878

861879
override Declaration getFunction() { result = p.getFunction() }
862880

@@ -902,7 +920,9 @@ class FlowSummaryNode extends Node, TFlowSummaryNode {
902920
* Gets the enclosing callable. For a `FlowSummaryNode` this is always the
903921
* summarized function this node is part of.
904922
*/
905-
override Declaration getEnclosingCallable() { result = this.getSummarizedCallable() }
923+
override DataFlowCallable getEnclosingCallable() {
924+
result.asSummarizedCallable() = this.getSummarizedCallable()
925+
}
906926

907927
override Location getLocationImpl() { result = this.getSummarizedCallable().getLocation() }
908928

@@ -923,7 +943,7 @@ class IndirectReturnNode extends Node {
923943
.hasOperandAndIndirectionIndex(any(ReturnValueInstruction ret).getReturnAddressOperand(), _)
924944
}
925945

926-
override Declaration getEnclosingCallable() { result = this.getFunction() }
946+
override SourceCallable getEnclosingCallable() { result.asSourceCallable() = this.getFunction() }
927947

928948
/**
929949
* Holds if this node represents the value that is returned to the caller
@@ -1117,11 +1137,11 @@ private module RawIndirectNodes {
11171137
/** Gets the underlying indirection index. */
11181138
int getIndirectionIndex() { result = indirectionIndex }
11191139

1120-
override Declaration getFunction() {
1121-
result = this.getOperand().getDef().getEnclosingFunction()
1122-
}
1140+
override Declaration getFunction() { result = node.getFunction() }
11231141

1124-
override Declaration getEnclosingCallable() { result = this.getFunction() }
1142+
override DataFlowCallable getEnclosingCallable() {
1143+
result.asSourceCallable() = node.getEnclosingCallable()
1144+
}
11251145

11261146
override predicate isGLValue() { this.getOperand().isGLValue() }
11271147

@@ -1163,9 +1183,11 @@ private module RawIndirectNodes {
11631183
/** Gets the underlying indirection index. */
11641184
int getIndirectionIndex() { result = indirectionIndex }
11651185

1166-
override Declaration getFunction() { result = this.getInstruction().getEnclosingFunction() }
1186+
override Declaration getFunction() { result = node.getFunction() }
11671187

1168-
override Declaration getEnclosingCallable() { result = this.getFunction() }
1188+
override DataFlowCallable getEnclosingCallable() {
1189+
result.asSourceCallable() = node.getEnclosingCallable()
1190+
}
11691191

11701192
override predicate isGLValue() { this.getInstruction().isGLValue() }
11711193

@@ -1265,7 +1287,9 @@ class FinalParameterNode extends Node, TFinalParameterNode {
12651287

12661288
override Declaration getFunction() { result = p.getFunction() }
12671289

1268-
override Declaration getEnclosingCallable() { result = this.getFunction() }
1290+
override DataFlowCallable getEnclosingCallable() {
1291+
result.asSourceCallable() = this.getFunction()
1292+
}
12691293

12701294
override DataFlowType getType() { result = getTypeImpl(p.getUnderlyingType(), indirectionIndex) }
12711295

@@ -1385,12 +1409,14 @@ private class IndirectInstructionParameterNode extends AbstractIndirectParameter
13851409
/** Gets the parameter whose indirection is initialized. */
13861410
override Parameter getParameter() { result = init.getParameter() }
13871411

1388-
override Declaration getEnclosingCallable() { result = this.getFunction() }
1412+
override DataFlowCallable getEnclosingCallable() {
1413+
result.asSourceCallable() = this.getFunction()
1414+
}
13891415

13901416
override Declaration getFunction() { result = init.getEnclosingFunction() }
13911417

13921418
override predicate isSourceParameterOf(Function f, ParameterPosition pos) {
1393-
this.getEnclosingCallable() = f and
1419+
this.getFunction() = f and
13941420
exists(int argumentIndex, int indirectionIndex |
13951421
indirectPositionHasArgumentIndexAndIndex(pos, argumentIndex, indirectionIndex) and
13961422
indirectParameterNodeHasArgumentIndexAndIndex(this, argumentIndex, indirectionIndex)
@@ -1625,13 +1651,13 @@ class VariableNode extends Node, TGlobalLikeVariableNode {
16251651

16261652
override Declaration getFunction() { none() }
16271653

1628-
override Declaration getEnclosingCallable() {
1654+
override DataFlowCallable getEnclosingCallable() {
16291655
// When flow crosses from one _enclosing callable_ to another, the
16301656
// interprocedural data-flow library discards call contexts and inserts a
16311657
// node in the big-step relation used for human-readable path explanations.
16321658
// Therefore we want a distinct enclosing callable for each `VariableNode`,
16331659
// and that can be the `Variable` itself.
1634-
result = v
1660+
result.asSourceCallable() = v
16351661
}
16361662

16371663
override DataFlowType getType() {

‎cpp/ql/test/library-tests/dataflow/models-as-data/SummaryCall.expected‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,32 +102,49 @@ sourceCallables
102102
| tests.cpp:139:6:139:10 | value |
103103
| tests.cpp:140:6:140:11 | value2 |
104104
| tests.cpp:141:7:141:9 | ptr |
105+
| tests.cpp:144:5:144:19 | madArg0ToReturn |
105106
| tests.cpp:144:25:144:25 | x |
107+
| tests.cpp:145:6:145:28 | madArg0ToReturnIndirect |
106108
| tests.cpp:145:34:145:34 | x |
107109
| tests.cpp:146:5:146:15 | notASummary |
108110
| tests.cpp:146:21:146:21 | x |
111+
| tests.cpp:147:5:147:28 | madArg0ToReturnValueFlow |
109112
| tests.cpp:147:34:147:34 | x |
113+
| tests.cpp:148:5:148:27 | madArg0IndirectToReturn |
110114
| tests.cpp:148:34:148:34 | x |
115+
| tests.cpp:149:5:149:33 | madArg0DoubleIndirectToReturn |
111116
| tests.cpp:149:41:149:41 | x |
117+
| tests.cpp:150:5:150:30 | madArg0NotIndirectToReturn |
112118
| tests.cpp:150:37:150:37 | x |
119+
| tests.cpp:151:6:151:26 | madArg0ToArg1Indirect |
113120
| tests.cpp:151:32:151:32 | x |
114121
| tests.cpp:151:40:151:40 | y |
122+
| tests.cpp:152:6:152:34 | madArg0IndirectToArg1Indirect |
115123
| tests.cpp:152:47:152:47 | x |
116124
| tests.cpp:152:55:152:55 | y |
125+
| tests.cpp:153:5:153:18 | madArgsComplex |
117126
| tests.cpp:153:25:153:25 | a |
118127
| tests.cpp:153:33:153:33 | b |
119128
| tests.cpp:153:40:153:40 | c |
120129
| tests.cpp:153:47:153:47 | d |
130+
| tests.cpp:154:5:154:14 | madArgsAny |
121131
| tests.cpp:154:20:154:20 | a |
122132
| tests.cpp:154:28:154:28 | b |
133+
| tests.cpp:155:5:155:28 | madAndImplementedComplex |
123134
| tests.cpp:155:34:155:34 | a |
124135
| tests.cpp:155:41:155:41 | b |
125136
| tests.cpp:155:48:155:48 | c |
137+
| tests.cpp:160:5:160:24 | madArg0FieldToReturn |
126138
| tests.cpp:160:38:160:39 | mc |
139+
| tests.cpp:161:5:161:32 | madArg0IndirectFieldToReturn |
127140
| tests.cpp:161:47:161:48 | mc |
141+
| tests.cpp:162:5:162:32 | madArg0FieldIndirectToReturn |
128142
| tests.cpp:162:46:162:47 | mc |
143+
| tests.cpp:163:13:163:32 | madArg0ToReturnField |
129144
| tests.cpp:163:38:163:38 | x |
145+
| tests.cpp:164:14:164:41 | madArg0ToReturnIndirectField |
130146
| tests.cpp:164:47:164:47 | x |
147+
| tests.cpp:165:13:165:40 | madArg0ToReturnFieldIndirect |
131148
| tests.cpp:165:46:165:46 | x |
132149
| tests.cpp:167:13:167:30 | madFieldToFieldVar |
133150
| tests.cpp:168:13:168:38 | madFieldToIndirectFieldVar |
@@ -160,9 +177,13 @@ sourceCallables
160177
| tests.cpp:280:7:280:23 | qualifierArg0Sink |
161178
| tests.cpp:280:29:280:29 | x |
162179
| tests.cpp:281:7:281:24 | qualifierFieldSink |
180+
| tests.cpp:284:7:284:19 | madArg0ToSelf |
163181
| tests.cpp:284:25:284:25 | x |
182+
| tests.cpp:285:6:285:20 | madSelfToReturn |
164183
| tests.cpp:286:6:286:16 | notASummary |
184+
| tests.cpp:287:7:287:20 | madArg0ToField |
165185
| tests.cpp:287:26:287:26 | x |
186+
| tests.cpp:288:6:288:21 | madFieldToReturn |
166187
| tests.cpp:290:6:290:8 | val |
167188
| tests.cpp:293:7:293:7 | MyDerivedClass |
168189
| tests.cpp:293:7:293:7 | operator= |
@@ -183,6 +204,7 @@ sourceCallables
183204
| tests.cpp:308:52:308:52 | x |
184205
| tests.cpp:309:7:309:31 | namespaceMemberMadSinkVar |
185206
| tests.cpp:310:14:310:44 | namespaceStaticMemberMadSinkVar |
207+
| tests.cpp:313:7:313:30 | namespaceMadSelfToReturn |
186208
| tests.cpp:317:22:317:28 | source3 |
187209
| tests.cpp:319:6:319:23 | test_class_members |
188210
| tests.cpp:320:10:320:11 | mc |
@@ -208,10 +230,14 @@ sourceCallables
208230
| tests.cpp:429:8:429:14 | intPair |
209231
| tests.cpp:430:6:430:10 | first |
210232
| tests.cpp:431:6:431:11 | second |
233+
| tests.cpp:434:5:434:29 | madCallArg0ReturnToReturn |
211234
| tests.cpp:434:37:434:43 | fun_ptr |
235+
| tests.cpp:435:9:435:38 | madCallArg0ReturnToReturnFirst |
212236
| tests.cpp:435:46:435:52 | fun_ptr |
237+
| tests.cpp:436:6:436:25 | madCallArg0WithValue |
213238
| tests.cpp:436:34:436:40 | fun_ptr |
214239
| tests.cpp:436:53:436:57 | value |
240+
| tests.cpp:437:5:437:36 | madCallReturnValueIgnoreFunction |
215241
| tests.cpp:437:45:437:51 | fun_ptr |
216242
| tests.cpp:437:64:437:68 | value |
217243
| tests.cpp:439:5:439:14 | getTainted |
@@ -225,13 +251,15 @@ sourceCallables
225251
| tests.cpp:457:8:457:35 | StructWithTypedefInParameter<int> |
226252
| tests.cpp:458:12:458:15 | Type |
227253
| tests.cpp:459:5:459:31 | parameter_ref_to_return_ref |
254+
| tests.cpp:459:5:459:31 | parameter_ref_to_return_ref |
228255
| tests.cpp:459:45:459:45 | x |
229256
| tests.cpp:459:45:459:45 | x |
230257
| tests.cpp:462:6:462:37 | test_parameter_ref_to_return_ref |
231258
| tests.cpp:463:6:463:6 | x |
232259
| tests.cpp:464:36:464:36 | s |
233260
| tests.cpp:465:6:465:6 | y |
234261
| tests.cpp:469:7:469:9 | INT |
262+
| tests.cpp:471:5:471:17 | receive_array |
235263
| tests.cpp:471:23:471:23 | a |
236264
| tests.cpp:473:6:473:23 | test_receive_array |
237265
| tests.cpp:474:6:474:6 | x |

0 commit comments

Comments
 (0)