Skip to content

C++: Prepare for model generation adoption #19274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 16 additions & 8 deletions cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ private predicate isFunctionConstructedFrom(Function f, Function templateFunc) {
}

/** Gets the fully templated version of `f`. */
private Function getFullyTemplatedFunction(Function f) {
Function getFullyTemplatedFunction(Function f) {
not f.isFromUninstantiatedTemplate(_) and
(
exists(Class c, Class templateClass, int i |
Expand Down Expand Up @@ -559,27 +559,35 @@ private string getTypeName(Type t, boolean needsSpace) {

/**
* Gets a type name for the `n`'th parameter of `f` without any template
* arguments. The result may be a string representing a type for which the
* typedefs have been resolved.
* arguments.
*
* If `canonical = false` then the result may be a string representing a type
* for which the typedefs have been resolved. If `canonical = true` then the
* result will be a string representing a type without resolving `typedefs`.
*/
bindingset[f]
pragma[inline_late]
string getParameterTypeWithoutTemplateArguments(Function f, int n) {
string getParameterTypeWithoutTemplateArguments(Function f, int n, boolean canonical) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where/how will canonical be used? I only see this predicate used with a wildcard argument for that parameter in this PR.

Copy link
Contributor Author

@MathiasVP MathiasVP Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your observation is correct! Currently, we're fairly lenient in with the parameter names we accept when mapping a MaD representation to the targeted function. For example:

using MyIntType = int;
void f(MyIntType t);

will both match the parameter list (int) and (MyIntType) (because, annoyingly, some implementations of the C++ standard library differ on whether they use an internal typedef'd name, and what that internal typedef is even called).

However when we want to generate a MaD row (i.e., what model generation will be doing) we don't want to map a function to many parameter lists. So:

  • When we consume MaD rows we will always use _ for the canonical parameter (because we want to accept many possible names for the parameter type), but
  • When we produce MaD rows we need to pick 1 parameter type name. So when this PR is merged I'll open the PR that actually adds the model generation, and this library will use true for canonical.

exists(string s, string base, string specifiers, Type t |
t = f.getParameter(n).getType() and
// The name of the string can either be the possibly typedefed name
// or an alternative name where typedefs has been resolved.
// `getTypeName(t, _)` is almost equal to `t.resolveTypedefs().getName()`,
// except that `t.resolveTypedefs()` doesn't have a result when the
// resulting type doesn't appear in the database.
s = [t.getName(), getTypeName(t, _)] and
(
s = t.getName() and canonical = true
or
s = getTypeName(t, _) and canonical = false
) and
parseAngles(s, base, _, specifiers) and
result = base + specifiers
)
or
f.isVarargs() and
n = f.getNumberOfParameters() and
result = "..."
result = "..." and
canonical = true
}

/**
Expand All @@ -590,7 +598,7 @@ private string getTypeNameWithoutFunctionTemplates(Function f, int n, int remain
exists(Function templateFunction |
templateFunction = getFullyTemplatedFunction(f) and
remaining = templateFunction.getNumberOfTemplateArguments() and
result = getParameterTypeWithoutTemplateArguments(templateFunction, n)
result = getParameterTypeWithoutTemplateArguments(templateFunction, n, _)
)
or
exists(string mid, TypeTemplateParameter tp, Function templateFunction |
Expand Down Expand Up @@ -627,7 +635,7 @@ private string getTypeNameWithoutClassTemplates(Function f, int n, int remaining
}

/** Gets the string representation of the `i`'th parameter of `c`. */
private string getParameterTypeName(Function c, int i) {
string getParameterTypeName(Function c, int i) {
result = getTypeNameWithoutClassTemplates(c, i, 0)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ private class PrimaryArgumentNode extends ArgumentNode, OperandNode {
PrimaryArgumentNode() { exists(CallInstruction call | op = call.getAnArgumentOperand()) }

override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
op = call.getArgumentOperand(pos.(DirectPosition).getIndex())
op = call.getArgumentOperand(pos.(DirectPosition).getArgumentIndex())
}
}

Expand Down Expand Up @@ -410,8 +410,16 @@ class ParameterPosition = Position;
class ArgumentPosition = Position;

abstract class Position extends TPosition {
/** Gets a textual representation of this position. */
abstract string toString();

/**
* Gets the argument index of this position. The qualifier of a call has
* argument index `-1`.
*/
abstract int getArgumentIndex();

/** Gets the indirection index of this position. */
abstract int getIndirectionIndex();
}

Expand All @@ -428,7 +436,7 @@ class DirectPosition extends Position, TDirectPosition {
result = index.toString()
}

int getIndex() { result = index }
override int getArgumentIndex() { result = index }

final override int getIndirectionIndex() { result = 0 }
}
Expand All @@ -445,16 +453,29 @@ class IndirectionPosition extends Position, TIndirectionPosition {
else result = repeatStars(indirectionIndex) + argumentIndex.toString()
}

int getArgumentIndex() { result = argumentIndex }
override int getArgumentIndex() { result = argumentIndex }

final override int getIndirectionIndex() { result = indirectionIndex }
}

newtype TPosition =
TDirectPosition(int argumentIndex) { exists(any(CallInstruction c).getArgument(argumentIndex)) } or
TDirectPosition(int argumentIndex) {
exists(any(CallInstruction c).getArgument(argumentIndex))
or
// Handle the rare case where there is a function definition but no call to
// the function.
exists(any(Cpp::Function f).getParameter(argumentIndex))
} or
TIndirectionPosition(int argumentIndex, int indirectionIndex) {
Ssa::hasIndirectOperand(any(CallInstruction call).getArgumentOperand(argumentIndex),
indirectionIndex)
or
// Handle the rare case where there is a function definition but no call to
// the function.
exists(Cpp::Function f, Cpp::Parameter p |
p = f.getParameter(argumentIndex) and
indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(p.getUnspecifiedType()) - 1]
)
}

private newtype TReturnKind =
Expand Down Expand Up @@ -501,6 +522,15 @@ class ReturnKind extends TReturnKind {

/** Gets a textual representation of this return kind. */
abstract string toString();

/** Holds if this `ReturnKind` is generated from a `return` statement. */
abstract predicate isNormalReturn();

/**
* Holds if this `ReturnKind` is generated from a write to the parameter with
* index `argumentIndex`
*/
abstract predicate isIndirectReturn(int argumentIndex);
}

/**
Expand All @@ -514,6 +544,10 @@ class NormalReturnKind extends ReturnKind, TNormalReturnKind {
override int getIndirectionIndex() { result = indirectionIndex }

override string toString() { result = "indirect return" }

override predicate isNormalReturn() { any() }

override predicate isIndirectReturn(int argumentIndex) { none() }
}

/**
Expand All @@ -528,6 +562,10 @@ private class IndirectReturnKind extends ReturnKind, TIndirectReturnKind {
override int getIndirectionIndex() { result = indirectionIndex }

override string toString() { result = "indirect outparam[" + argumentIndex.toString() + "]" }

override predicate isNormalReturn() { none() }

override predicate isIndirectReturn(int argumentIndex_) { argumentIndex_ = argumentIndex }
}

/** A data flow node that occurs as the result of a `ReturnStmt`. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,7 @@ private class ExplicitParameterInstructionNode extends AbstractExplicitParameter
ExplicitParameterInstructionNode() { exists(instr.getParameter()) }

override predicate isSourceParameterOf(Function f, ParameterPosition pos) {
f.getParameter(pos.(DirectPosition).getIndex()) = instr.getParameter()
f.getParameter(pos.(DirectPosition).getArgumentIndex()) = instr.getParameter()
}

override string toStringImpl() { result = instr.getParameter().toString() }
Expand All @@ -1460,7 +1460,7 @@ class ThisParameterInstructionNode extends AbstractExplicitParameterNode,
ThisParameterInstructionNode() { instr.getIRVariable() instanceof IRThisVariable }

override predicate isSourceParameterOf(Function f, ParameterPosition pos) {
pos.(DirectPosition).getIndex() = -1 and
pos.(DirectPosition).getArgumentIndex() = -1 and
instr.getEnclosingFunction() = f
}

Expand Down Expand Up @@ -1494,7 +1494,7 @@ private class DirectBodyLessParameterNode extends AbstractExplicitParameterNode,

override predicate isSourceParameterOf(Function f, ParameterPosition pos) {
this.getFunction() = f and
f.getParameter(pos.(DirectPosition).getIndex()) = p
f.getParameter(pos.(DirectPosition).getArgumentIndex()) = p
}

override Parameter getParameter() { result = p }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,11 @@ private module SpeculativeTaintFlow {
not exists(DataFlowDispatch::viableCallable(call)) and
src.(DataFlowPrivate::ArgumentNode).argumentOf(call, argpos)
|
not argpos.(DirectPosition).getIndex() = -1 and
not argpos.(DirectPosition).getArgumentIndex() = -1 and
sink.(PostUpdateNode)
.getPreUpdateNode()
.(DataFlowPrivate::ArgumentNode)
.argumentOf(call, any(DirectPosition qualpos | qualpos.getIndex() = -1))
.argumentOf(call, any(DirectPosition qualpos | qualpos.getArgumentIndex() = -1))
or
sink.(DataFlowPrivate::OutNode).getCall() = call
)
Expand Down