Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1b41bae
C++: Add tests with missing flow.
MathiasVP Sep 7, 2026
0169e18
C++: Add a missing utility predicate on Call instructions.
MathiasVP Sep 8, 2026
3777020
C++: Sync identical files.
MathiasVP Sep 8, 2026
77bb230
C++: Small refactor.
MathiasVP Sep 8, 2026
04c2ac4
C++: We will need the template resolution for something other than the
MathiasVP Sep 7, 2026
48de4cb
C++: Add MaD support for models that specify argument forwarding.
MathiasVP Sep 7, 2026
4f2fc96
C++: Accept test changes.
MathiasVP Sep 7, 2026
db2a462
Apply batched suggestions from code review
MathiasVP Sep 9, 2026
908e32b
C++: Respond to Copilot comments.
MathiasVP Sep 9, 2026
10981ae
C++: Autoformat after Copilot suggestions.
MathiasVP Sep 9, 2026
1511488
C++: Fix spelling.
MathiasVP Sep 9, 2026
cca9f3f
C++: Add false positive from lack of overload handling.
MathiasVP Sep 9, 2026
246c486
C++: Fix FP by ensuring that the targeted constructor has at least as…
MathiasVP Sep 9, 2026
dcda06a
C++: Add a testcase that uses a hardcoded constructed type.
MathiasVP Sep 16, 2026
f381119
C++: Expand QLDoc.
MathiasVP Sep 16, 2026
8149caa
C++: Add a testcase that uses type parameters in the function name fo…
MathiasVP Sep 16, 2026
d9331e9
C++: Add a testcase with forwarding without a constructor.
MathiasVP Sep 10, 2026
f52d18c
C++: Handle forwarding without a constructor.
MathiasVP Sep 16, 2026
f00b553
C++: Accept test changes.
MathiasVP Sep 16, 2026
dee5142
C++: Add a high-level description of the forwarding model.
MathiasVP Sep 16, 2026
18affcd
Merge branch 'main' into flow-through-forwards-using-callbacks-3
MathiasVP Sep 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cpp/ql/lib/ext/empty.model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,7 @@ extensions:
pack: codeql/cpp-all
extensible: summaryModel
data: []
- addsTo:
pack: codeql/cpp-all
extensible: forwardsModel
data: []
192 changes: 187 additions & 5 deletions cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* `namespace; type; subtypes; name; signature; ext; output; kind; provenance`
* - BarrierGuards:
* `namespace; type; subtypes; name; signature; ext; input; acceptingValue; kind; provenance`
* - Forwards:
* `namespace; type; subtypes; name; signature; ext; start; constructor; output; provenance`
Comment thread
geoffw0 marked this conversation as resolved.
*
* The interpretation of a row is similar to API-graphs with a left-to-right
* reading.
Expand Down Expand Up @@ -108,13 +110,23 @@
* - "manual": The model has been written by hand.
* This information is used in a heuristic for dataflow analysis to determine, if a
* model or source code should be used for determining flow.
*
* The "Forwards" relation allows modeling of function that perform C++11-style "perfect
* forwarding" where a function receives a number of arguments and forwards those arguments
* to a constructor of another type. For example, the row:
* `"std"; "vector<T>"; "True"; "emplace"; ""; ""; "1"; T; Argument[-1].Element; manual`
* says that `std::vector<T>::emplace(arg0, arg1, ..., argn)` forwards arguments
* `arg1, ..., argn` to a constructor for `T`, and the result of `T(arg1, ..., argn)`
* flows to `Argument[-1].Element` (see information about the semantics of the `output`
* column further above).
*/

import cpp
private import new.DataFlow
private import semmle.code.cpp.controlflow.IRGuards
private import semmle.code.cpp.ir.dataflow.internal.DataFlowNodes as Nodes
private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate as Private
private import semmle.code.cpp.ir.dataflow.internal.SsaImpl as SsaImpl
private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
private import internal.FlowSummaryImpl
private import internal.FlowSummaryImpl::Public
Expand Down Expand Up @@ -160,6 +172,20 @@ predicate summaryModel(
)
}

/**
* Holds if a forward model exists for the given parameters.
*/
predicate forwardsModel(
string namespace, string type, boolean subtypes, string name, string signature, string ext,
string start, string constructor, string output, string provenance, string model
) {
exists(QlBuiltins::ExtensionId madId |
Extensions::forwardsModel(namespace, type, subtypes, name, signature, ext, start, constructor,
output, provenance, madId) and
model = "MaD:" + madId.toString()
)
}

/** Provides a query predicate to check the data for validation errors. */
module ModelValidation {
private string getInvalidModelInput() {
Expand All @@ -186,6 +212,8 @@ module ModelValidation {
sourceModel(_, _, _, _, _, _, output, _, _, _) and pred = "source"
or
summaryModel(_, _, _, _, _, _, _, output, _, _, _) and pred = "summary"
or
forwardsModel(_, _, _, _, _, _, _, _, output, _, _) and pred = "forwards"
|
invalidSpecComponent(output, part) and
not part = "" and
Expand Down Expand Up @@ -259,7 +287,8 @@ private predicate elementSpec(
sinkModel(namespace, type, subtypes, name, signature, ext, _, _, _, _) or
barrierModel(namespace, type, subtypes, name, signature, ext, _, _, _, _) or
barrierGuardModel(namespace, type, subtypes, name, signature, ext, _, _, _, _, _) or
summaryModel(namespace, type, subtypes, name, signature, ext, _, _, _, _, _)
summaryModel(namespace, type, subtypes, name, signature, ext, _, _, _, _, _) or
forwardsModel(namespace, type, subtypes, name, signature, ext, _, _, _, _, _)
}

/**
Expand Down Expand Up @@ -596,6 +625,14 @@ private string getAtIndex(string s, int i) {
not (s = "" and i = 0)
}

/** Gets the number of comma-separated arguments in `s`. */
bindingset[s]
private int getNumberOfArguments(string s) {
s = "" and result = 0
or
s != "" and result = count(s.indexOf(",")) + 1
}

/**
* Normalizes `partiallyNormalizedSignature` by replacing the `remaining`
* number of template arguments in `partiallyNormalizedSignature` with their
Expand All @@ -605,7 +642,7 @@ private string getSignatureWithoutClassTemplateNames(
string partiallyNormalizedSignature, string typeArgs, string nameArgs, int remaining
) {
elementSpecWithArguments0(_, _, _, partiallyNormalizedSignature, typeArgs, nameArgs) and
remaining = count(partiallyNormalizedSignature.indexOf(",")) + 1 and
remaining = getNumberOfArguments(typeArgs) and
result = partiallyNormalizedSignature
or
exists(string mid |
Expand All @@ -619,7 +656,7 @@ private string getSignatureWithoutClassTemplateNames(
)
or
// Make sure `remaining` is properly bound
remaining = [0 .. count(partiallyNormalizedSignature.indexOf(",")) + 1] and
remaining = [0 .. getNumberOfArguments(typeArgs)] and
not exists(getAtIndex(typeArgs, remaining)) and
result = mid
)
Expand All @@ -636,7 +673,7 @@ pragma[nomagic]
private string getSignatureWithoutFunctionTemplateNames(
string partiallyNormalizedSignature, string typeArgs, string nameArgs, int remaining
) {
remaining = count(partiallyNormalizedSignature.indexOf(",")) + 1 and
remaining = getNumberOfArguments(nameArgs) and
result =
getSignatureWithoutClassTemplateNames(partiallyNormalizedSignature, typeArgs, nameArgs, 0)
or
Expand All @@ -651,7 +688,7 @@ private string getSignatureWithoutFunctionTemplateNames(
)
or
// Make sure `remaining` is properly bound
remaining = [0 .. count(partiallyNormalizedSignature.indexOf(",")) + 1] and
remaining = [0 .. getNumberOfArguments(nameArgs)] and
not exists(getAtIndex(nameArgs, remaining)) and
result = mid
)
Expand Down Expand Up @@ -1046,6 +1083,148 @@ private module Cached {

import Cached

/** Gets the constructor type selected by `constructorType` in a forwarding model. */
private Type getForwardedConstructorType(
Function forwarder, string namespace, string type, boolean subtypes, string name,
string signature, string ext, string constructorType
) {
exists(int index |
forwardsModel(namespace, type, subtypes, name, signature, ext, _, constructorType, _, _, _) and
forwarder = interpretElement(namespace, type, subtypes, name, signature, ext)
|
exists(string typeArguments |
parseAngles(type, _, typeArguments, "") and
constructorType = getAtIndex(typeArguments, index) and
result = forwarder.getDeclaringType().getTemplateArgument(index)
)
or
exists(string nameArguments |
parseAngles(name, _, nameArguments, "") and
constructorType = getAtIndex(nameArguments, index) and
result = forwarder.getTemplateArgument(index)
)
)
}

/** Interprets a forwarding model, retaining its constructed type, output, and provenance. */
private predicate interpretForwardsModelType(
Function forwarder, Type constructedType, int start, string output, string provenance,
string model
) {
exists(
string namespace, string type, boolean subtypes, string name, string signature, string ext,
string startString, string constructorType
|
forwardsModel(namespace, type, subtypes, name, signature, ext, startString, constructorType,
output, provenance, model) and
forwarder = interpretElement(namespace, type, subtypes, name, signature, ext) and
start = startString.toInt()
|
// Either the row specifies forwarding to a type given by the type or
// function template, in which case we need to resolve that from the type
// or function name.
constructedType =
getForwardedConstructorType(forwarder, namespace, type, subtypes, name, signature, ext,
constructorType).getUnspecifiedType()
or
// Or the row specifies forwarding to a specific type.
not exists(
getForwardedConstructorType(forwarder, namespace, type, subtypes, name, signature, ext,
constructorType)
) and
classHasQualifiedName(constructedType, namespace, constructorType)
)
}

/**
* Holds if `forwarder` may forward its arguments starting at `start` to `constructor`. The
* actual constructor being forwarded to depends on the types of arguments from `start`
* at calls to `forwarder`.
*/
private predicate interpretForwardsModel(
Function forwarder, Constructor constructor, int start, string output, string provenance,
string model
) {
interpretForwardsModelType(forwarder, constructor.getDeclaringType(), start, output, provenance,
model)
}

/** Holds if `forwarder` forwards its arguments starting at `start` to `constructor`. */
predicate forwards(Function forwarder, Constructor constructor, int start) {
interpretForwardsModel(forwarder, constructor, start, _, _, _)
}

private int referenceIndirection(Type unspecified) {
if unspecified instanceof ReferenceType then result = 1 else result = 0
}

/** Gets `unspecified`, but with its outermost reference removed, if any. */
private Type stripReference(Type unspecified) {
result = unspecified.(ReferenceType).getBaseType().getUnspecifiedType()
or
not unspecified instanceof ReferenceType and
result = unspecified
}

/**
* In order to support flow summaries for functions that perform "perfect
* forwarding" we interpret a call such as:
* ```cpp
* struct Foo { Foo(int) };
* std::vector<Foo> v;
* v.emplace_back(42);
* ```
* as:
* ```cpp
* v.emplace_back(42, &Foo);
* ```
* and add two summaries:
* (1) One flow from `42` to the first argument of a call to `Foo`
* (2) One flow from the return value of `Foo` to the `this` argument of the call
* to `emplace_back` (with a sequence of output `Content`s).
*
* These two summaries are automatically generated when a forwarding model
* for `emplace_back` exists.
*/
private predicate interpretForwardingSummary(
Comment thread
MathiasVP marked this conversation as resolved.
Dismissed
Function forwarder, string input, string output, string provenance, string model
) {
exists(Constructor constructor, int start, string constructorOutput |
interpretForwardsModel(forwarder, constructor, start, constructorOutput, provenance, model)
|
// Generate the (1) summary
exists(int index, Parameter arg, Parameter p, int indirection |
arg = forwarder.getParameter(start + index) and
p = constructor.getParameter(index) and
indirection = [0 .. SsaImpl::getMaxIndirectionsForPRType(p.getUnspecifiedType())] and
input =
"Argument[" + repeatStars(indirection + referenceIndirection(arg.getUnspecifiedType())) +
(start + index) + "]" and
output =
"Argument[forward].Parameter[" +
repeatStars(indirection + referenceIndirection(p.getUnspecifiedType())) + index + "]"
)
or
// Generate the (2) summary
input = "Argument[forward].Parameter[-1]" and
output = constructorOutput
)
or
// Scalar types have no constructor to synthesize. In this case, directly
// preserve the value of the single forwarded argument at the modeled output.
exists(Type constructedType, int start, Parameter p, int indirection |
interpretForwardsModelType(forwarder, constructedType, start, output, provenance, model) and
not constructedType instanceof Class and
forwarder.getNumberOfParameters() = start + 1 and
p = forwarder.getParameter(start) and
stripReference(p.getUnspecifiedType()) = constructedType and
indirection = [0 .. SsaImpl::getMaxIndirectionsForPRType(constructedType)] and
input =
"Argument[" + repeatStars(indirection + referenceIndirection(p.getUnspecifiedType())) + start +
"]"
)
}

/**
* Holds if `node` is specified as a source with the given kind in a MaD flow
* model.
Expand Down Expand Up @@ -1074,6 +1253,9 @@ private predicate interpretSummary(
model) and
f = interpretElement(namespace, type, subtypes, name, signature, ext)
)
or
interpretForwardingSummary(f, input, output, provenance, model) and
kind = "value"
}

// adapter class for converting Mad summaries to `SummarizedCallable`s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ extensible predicate neutralModel(
string namespace, string type, string name, string signature, string kind, string provenance
);

/**
* Holds if a constructor forwarding model exists for the given parameters.
*/
extensible predicate forwardsModel(
string namespace, string type, boolean subtypes, string name, string signature, string ext,
string start, string constructor, string output, string provenance, QlBuiltins::ExtensionId madId
);

module Extensions implements SharedMaD::ExtensionsSig {
import ExternalFlowExtensions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CppDataFlow> {
pos = -1 and result = TIndirectionPosition(pos, indirection + 1)
)
)
or
argString = "forward" and
result = TForwardPosition()
}

bindingset[token]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ private module Cached {
TNonUnionContent(CanonicalField f, int indirectionIndex) {
// the indirection index for field content starts at 1 (because `TNonUnionContent` is thought of as
// the address of the field, `FieldAddress` in the IR).
indirectionIndex = [1 .. max(SsaImpl::getMaxIndirectionsForType(f.getAnUnspecifiedType()))] and
indirectionIndex = [1 .. max(SsaImpl::getMaxIndirectionsForGLType(f.getAnUnspecifiedType()))] and
// Reads and writes of union fields are tracked using `UnionContent`.
not f.getDeclaringType() instanceof Union
} or
Expand All @@ -156,7 +156,7 @@ private module Cached {
// field can be read by any read of the union's fields. Again, the indirection index
// is 1-based (because 0 is considered the address).
indirectionIndex =
[1 .. max(SsaImpl::getMaxIndirectionsForType(getAFieldWithSize(u, bytes)
[1 .. max(SsaImpl::getMaxIndirectionsForGLType(getAFieldWithSize(u, bytes)
.getAnUnspecifiedType())
)]
)
Expand Down Expand Up @@ -184,13 +184,16 @@ private module Cached {
TNode0(Node0Impl node) { DataFlowImplCommon::forceCachingInSameStage() } or
TGlobalLikeVariableNode(GlobalLikeVariable var, int indirectionIndex) {
indirectionIndex =
[getMinIndirectionsForType(var.getUnspecifiedType()) .. SsaImpl::getMaxIndirectionsForType(var.getUnspecifiedType())]
[getMinIndirectionsForType(var.getUnspecifiedType()) .. SsaImpl::getMaxIndirectionsForGLType(var.getUnspecifiedType())]
} or
TPostUpdateNodeImpl(Operand operand, int indirectionIndex) {
isPostUpdateNodeImpl(operand, indirectionIndex)
} or
TSsaSynthNode(SsaImpl::SynthNode n) or
TSsaIteratorNode(IteratorFlow::IteratorFlowNode n) or
TForwarderConstructorArgumentNode(CallInstruction call) {
isForwarderConstructorArgumentNodeImpl(call)
} or
TRawIndirectOperand0(Node0Impl node, int indirectionIndex) {
SsaImpl::hasRawIndirectOperand(node.asOperand(), indirectionIndex)
} or
Expand All @@ -209,10 +212,7 @@ private module Cached {
TBodyLessParameterNodeImpl(Parameter p, int indirectionIndex) {
// Rule out parameters of catch blocks.
not exists(p.getCatchBlock()) and
// We subtract one because `getMaxIndirectionsForType` returns the maximum
// indirection for a glvalue of a given type, and this doesn't apply to
// parameters.
indirectionIndex = [0 .. SsaImpl::getMaxIndirectionsForType(p.getUnspecifiedType()) - 1] and
indirectionIndex = [0 .. SsaImpl::getMaxIndirectionsForPRType(p.getUnspecifiedType())] and
not any(InitializeParameterInstruction init).getParameter() = p
} or
TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn)
Expand Down
Loading
Loading