Skip to content

Commit 150d0a7

Browse files
committed
C++: Track function pointers using local dataflow and accept test changes.
1 parent 3649784 commit 150d0a7

3 files changed

Lines changed: 40 additions & 20 deletions

File tree

cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,16 @@ private module Input2 implements Impl::Private::InputSig2 {
161161
)
162162
}
163163

164+
private MemberFunction getFunctionFromType(Expr e) {
165+
result.getClassAndName("operator()").getADerivedClass*() = e.getUnspecifiedType()
166+
}
167+
168+
private Function getFunctionFromExpr(Expr e) {
169+
result = e.(FunctionAccess).getTarget()
170+
or
171+
result = e.(ConversionCall).getQualifier().(LambdaExpression).getLambdaFunction()
172+
}
173+
164174
class SourceSinkReportingElement extends Element {
165175
SourceSinkReportingElement() { this instanceof Expr or this instanceof Parameter }
166176

@@ -169,29 +179,29 @@ private module Input2 implements Impl::Private::InputSig2 {
169179
[this.(Expr).getEnclosingFunction(), this.(Parameter).getFunction()]
170180
}
171181

172-
/**
173-
* Gets the member function corresponding to an overloaded `operator()` when this element is
174-
* invoked.
175-
*/
176-
private MemberFunction getOperatorCallFunction() {
177-
// An `operator()` on a struct
178-
result.getClassAndName("operator()").getADerivedClass*() = this.(Expr).getUnspecifiedType()
179-
or
180-
// A lambda that has undergone "lambda to function-pointer conversion"
181-
result = this.(ConversionCall).getQualifier().(LambdaExpression).getLambdaFunction()
182-
}
183-
184182
/** Gets the function invoked when this element is used as a callback. */
185-
private Function getCallbackFunction() {
186-
// Taking the address of a function
187-
result = this.(FunctionAccess).getTarget()
183+
private Function getCallable() {
184+
// The expression is a struct which implements `operator()`.
185+
result = getFunctionFromType(this)
188186
or
189-
// Passing an object with an overloaded `operator()`
190-
result = this.getOperatorCallFunction()
187+
// The expression is a function pointer
188+
result = getFunctionFromExpr(this)
189+
or
190+
// The expression is an SSA read of an assignment of a callable
191+
exists(Ssa::Definition def |
192+
def.getAUse().getDef().getUnconvertedResultExpression() = this and
193+
result =
194+
getFunctionFromExpr(def.getAnUltimateDefinition()
195+
.(Ssa::DirectExplicitDefinition)
196+
.getAssignedInstruction()
197+
.(StoreInstruction)
198+
.getSourceValue()
199+
.getUnconvertedResultExpression())
200+
)
191201
}
192202

193203
SourceSinkReportingElement getASuccessor(Impl::Private::SummaryComponent sc) {
194-
exists(Function f | f = this.getCallbackFunction() |
204+
exists(Function f | f = this.getCallable() |
195205
exists(ParameterPosition pos | sc = Impl::Private::SummaryComponent::parameter(pos) |
196206
result = pos.getParameter(f)
197207
)

cpp/ql/test/library-tests/dataflow/external-models/flow.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,11 @@ edges
245245
| test.cpp:250:32:250:32 | source_from_callback_template output argument [value] | test.cpp:243:10:243:10 | *s [value] | provenance | |
246246
| test.cpp:251:27:251:27 | source_from_callback_ptr output argument | test.cpp:251:27:251:27 | source_from_callback_ptr output argument [value] | provenance | Src:MaD:37 |
247247
| test.cpp:251:27:251:27 | source_from_callback_ptr output argument [value] | test.cpp:243:10:243:10 | *s [value] | provenance | |
248+
| test.cpp:258:12:258:12 | *s [value] | test.cpp:258:15:258:19 | value | provenance | |
249+
| test.cpp:258:15:258:19 | value | test.cpp:258:15:258:19 | value | provenance | Sink:MaD:3 |
250+
| test.cpp:262:27:262:31 | source_from_callback_ptr output argument | test.cpp:262:27:262:31 | source_from_callback_ptr output argument [value] | provenance | Src:MaD:37 |
251+
| test.cpp:262:27:262:31 | source_from_callback_ptr output argument [value] | test.cpp:243:10:243:10 | *s [value] | provenance | |
252+
| test.cpp:262:27:262:31 | source_from_callback_ptr output argument [value] | test.cpp:258:12:258:12 | *s [value] | provenance | |
248253
| test.cpp:264:32:266:2 | source_from_callback_template output argument | test.cpp:264:32:266:2 | source_from_callback_template output argument [value] | provenance | Src:MaD:40 |
249254
| test.cpp:264:32:266:2 | source_from_callback_template output argument [value] | test.cpp:265:11:265:11 | *s [value] | provenance | |
250255
| test.cpp:265:11:265:11 | *s [value] | test.cpp:265:14:265:18 | value | provenance | |
@@ -629,6 +634,11 @@ nodes
629634
| test.cpp:250:32:250:32 | source_from_callback_template output argument [value] | semmle.label | source_from_callback_template output argument [value] |
630635
| test.cpp:251:27:251:27 | source_from_callback_ptr output argument | semmle.label | source_from_callback_ptr output argument |
631636
| test.cpp:251:27:251:27 | source_from_callback_ptr output argument [value] | semmle.label | source_from_callback_ptr output argument [value] |
637+
| test.cpp:258:12:258:12 | *s [value] | semmle.label | *s [value] |
638+
| test.cpp:258:15:258:19 | value | semmle.label | value |
639+
| test.cpp:258:15:258:19 | value | semmle.label | value |
640+
| test.cpp:262:27:262:31 | source_from_callback_ptr output argument | semmle.label | source_from_callback_ptr output argument |
641+
| test.cpp:262:27:262:31 | source_from_callback_ptr output argument [value] | semmle.label | source_from_callback_ptr output argument [value] |
632642
| test.cpp:264:32:266:2 | source_from_callback_template output argument | semmle.label | source_from_callback_template output argument |
633643
| test.cpp:264:32:266:2 | source_from_callback_template output argument [value] | semmle.label | source_from_callback_template output argument [value] |
634644
| test.cpp:265:11:265:11 | *s [value] | semmle.label | *s [value] |

cpp/ql/test/library-tests/dataflow/external-models/test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ using Callback = void(*)(const SourceWrapper*);
240240
void source_from_callback_ptr(Callback);
241241

242242
void f(const SourceWrapper* s) {
243-
ymlSink(s->value); // $ ir=250:32 ir=251:27 MISSING: ir=262:27
243+
ymlSink(s->value); // $ ir=250:32 ir=251:27 ir=262:27
244244
}
245245

246246
void test_source_access_path(bool b) {
@@ -255,7 +255,7 @@ void test_source_access_path(bool b) {
255255
f_var = f;
256256
} else {
257257
f_var = [](const SourceWrapper* s) {
258-
ymlSink(s->value); // $ MISSING: ir
258+
ymlSink(s->value); // $ ir
259259
};
260260
}
261261

0 commit comments

Comments
 (0)