Skip to content
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.

Sign up for GitHub

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

C++: Clean up SSA and stop relying on memory edges for iterator flow #16345

Merged
merged 14 commits into from
May 2, 2024
Merged

C++: Clean up SSA and stop relying on memory edges for iterator flow #16345

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
044ee9b
C++: Delete old iterator flow using memory edges.
MathiasVP Apr 26, 2024
9874d40
C++: Make 'getAddressOperand' available on 'DefImpl'.
MathiasVP Apr 26, 2024
50775d0
C++: Get rid of 'UseImpl' in TSsaDefOrUse so that it now only contain…
MathiasVP Apr 27, 2024
5f0efc1
C++: Accept test changes.
MathiasVP Apr 26, 2024
ea1b8a3
C++: Implement 'getAnUltimateDefinition' on SSA definitions.
MathiasVP Apr 26, 2024
683fe26
C++: Add iterator flow based on dataflow SSA.
MathiasVP Apr 26, 2024
6e3ddde
C++: Use iterator flow in dataflow.
MathiasVP Apr 26, 2024
141af7c
C++: Subclasses for both 'begin' and 'end'.
MathiasVP Apr 26, 2024
d3d2e21
C++: Accept test changes.
MathiasVP Apr 26, 2024
527409d
C++: Fix iterator return types and accept test changes.
MathiasVP Apr 26, 2024
401717d
C++: Add a missing taint step and accept test changes.
MathiasVP Apr 26, 2024
4a47e11
C++: Fix QLDoc.
MathiasVP May 2, 2024
322fa36
C++: Fix QLDoc.
MathiasVP May 2, 2024
b912918
C++: Fix QLDoc.
MathiasVP May 2, 2024
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
34 changes: 0 additions & 34 deletions cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,6 @@ private newtype TDefOrUseImpl =
// a function body.
isGlobalDefImpl(v, f, _, indirectionIndex)
} or
TIteratorDef(
Operand iteratorDerefAddress, BaseSourceVariableInstruction container, int indirectionIndex
) {
isIteratorDef(container, iteratorDerefAddress, _, _, indirectionIndex)
} or
TIteratorUse(
Operand iteratorAddress, BaseSourceVariableInstruction container, int indirectionIndex
) {
isIteratorUse(container, iteratorAddress, _, indirectionIndex)
} or
TFinalParameterUse(Parameter p, int indirectionIndex) {
underlyingTypeIsModifiableAt(p.getUnderlyingType(), indirectionIndex) and
// Only create an SSA read for the final use of a parameter if there's
Expand Down Expand Up @@ -336,20 +326,6 @@ private class DirectDef extends OperandBasedDef, TDefImpl {
override predicate isCertain() { isDef(true, _, address, base, _, ind) }
}

private class IteratorDef extends OperandBasedDef, TIteratorDef {
BaseSourceVariableInstruction container;

IteratorDef() { this = TIteratorDef(address, container, ind) }

override BaseSourceVariableInstruction getBase() { result = container }

override int getIndirection() { isIteratorDef(container, address, _, result, ind) }

override Node0Impl getValue() { isIteratorDef(container, address, result, _, _) }

override predicate isCertain() { none() }
}

abstract class UseImpl extends DefOrUseImpl {
int ind;

Expand Down Expand Up @@ -418,16 +394,6 @@ private class DirectUse extends OperandBasedUse, TUseImpl {
override Node getNode() { nodeHasOperand(result, operand, ind) }
}

private class IteratorUse extends OperandBasedUse, TIteratorUse {
IteratorUse() { this = TIteratorUse(operand, base, ind) }

override int getIndirection() { isIteratorUse(base, operand, result, ind) }

override predicate isCertain() { none() }

override Node getNode() { nodeHasOperand(result, operand, ind) }
}

pragma[nomagic]
private predicate finalParameterNodeHasParameterAndIndex(
FinalParameterNode n, Parameter p, int indirectionIndex
Expand Down
242 changes: 0 additions & 242 deletions cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,14 +246,6 @@ private module IteratorIndirections {
baseType = super.getValueType()
}

override predicate isAdditionalDereference(Instruction deref, Operand address) {
exists(CallInstruction call |
operandForFullyConvertedCall(getAUse(deref), call) and
this = call.getStaticCallTarget().getClassAndName("operator*") and
address = call.getThisArgumentOperand()
)
}

jketema marked this conversation as resolved.
Show resolved Hide resolved
override predicate isAdditionalWrite(Node0Impl value, Operand address, boolean certain) {
exists(CallInstruction call | call.getArgumentOperand(0) = value.asOperand() |
this = call.getStaticCallTarget().getClassAndName("operator=") and
Expand All @@ -262,16 +254,6 @@ private module IteratorIndirections {
)
}

override predicate isAdditionalTaintStep(Node node1, Node node2) {
exists(CallInstruction call |
// Taint through `operator+=` and `operator-=` on iterators.
call.getStaticCallTarget() instanceof Iterator::IteratorAssignArithmeticOperator and
node2.(IndirectArgumentOutNode).getPreUpdateNode() = node1 and
node1.(IndirectOperand).hasOperandAndIndirectionIndex(call.getArgumentOperand(0), _) and
node1.getType().getUnspecifiedType() = this
)
}

override predicate isAdditionalConversionFlow(Operand opFrom, Instruction instrTo) {
// This is a bit annoying: Consider the following snippet:
// ```
Expand Down Expand Up @@ -589,230 +571,6 @@ private class BaseCallInstruction extends BaseSourceVariableInstruction, CallIns

cached
private module Cached {
private import semmle.code.cpp.models.interfaces.Iterator as Interfaces
private import semmle.code.cpp.models.implementations.Iterator as Iterator
private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs as IO

/**
* Holds if `next` is a instruction with a memory result that potentially
* updates the memory produced by `prev`.
*/
private predicate memorySucc(Instruction prev, Instruction next) {
prev = next.(ChiInstruction).getTotal()
or
// Phi inputs can be inexact.
prev = next.(PhiInstruction).getAnInputOperand().getAnyDef()
or
prev = next.(CopyInstruction).getSourceValue()
or
exists(ReadSideEffectInstruction read |
next = read.getPrimaryInstruction() and
isAdditionalConversionFlow(_, next) and
prev = read.getSideEffectOperand().getAnyDef()
)
}

/**
* Holds if `iteratorDerefAddress` is an address of an iterator dereference (i.e., `*it`)
* that is used for a write operation that writes the value `value`. The `memory` instruction
* represents the memory that the IR's SSA analysis determined was read by the call to `operator*`.
*
* The `numberOfLoads` integer represents the number of dereferences this write corresponds to
* on the underlying container that produced the iterator.
*/
private predicate isChiAfterIteratorDef(
Instruction memory, Operand iteratorDerefAddress, Node0Impl value, int numberOfLoads
) {
exists(
BaseSourceVariableInstruction iteratorBase, ReadSideEffectInstruction read,
Operand iteratorAddress
|
numberOfLoads >= 0 and
isDef(_, value, iteratorDerefAddress, iteratorBase, numberOfLoads + 2, 0) and
isUse(_, iteratorAddress, iteratorBase, numberOfLoads + 1, 0) and
iteratorBase.getResultType() instanceof Interfaces::Iterator and
isDereference(iteratorAddress.getDef(), read.getArgumentDef().getAUse(), _) and
memory = read.getSideEffectOperand().getAnyDef()
)
}

private predicate isSource(Instruction instr, Operand iteratorAddress, int numberOfLoads) {
getAUse(instr) = iteratorAddress and
exists(BaseSourceVariableInstruction iteratorBase |
iteratorBase.getResultType() instanceof Interfaces::Iterator and
not iteratorBase.getResultType() instanceof Cpp::PointerType and
isUse(_, iteratorAddress, iteratorBase, numberOfLoads - 1, 0)
)
}

private predicate isSink(Instruction instr, CallInstruction call) {
getAUse(instr).(ArgumentOperand).getCall() = call and
// Only include operations that may modify the object that the iterator points to.
// The following is a non-exhaustive list of things that may modify the value of the
// iterator, but never the value of what the iterator points to.
// The more things we can exclude here, the faster the small dataflow-like analysis
// done by `convertsIntoArgument` will converge.
not exists(Function f | f = call.getStaticCallTarget() |
f instanceof Iterator::IteratorCrementOperator or
f instanceof Iterator::IteratorBinaryArithmeticOperator or
f instanceof Iterator::IteratorAssignArithmeticOperator or
f instanceof Iterator::IteratorCrementMemberOperator or
f instanceof Iterator::IteratorBinaryArithmeticMemberOperator or
f instanceof Iterator::IteratorAssignArithmeticMemberOperator or
f instanceof Iterator::IteratorAssignmentMemberOperator
)
}

private predicate convertsIntoArgumentFwd(Instruction instr) {
isSource(instr, _, _)
or
exists(Instruction prev | convertsIntoArgumentFwd(prev) |
conversionFlow(unique( | | getAUse(prev)), instr, false, _)
)
}

private predicate convertsIntoArgumentRev(Instruction instr) {
convertsIntoArgumentFwd(instr) and
(
isSink(instr, _)
or
exists(Instruction next | convertsIntoArgumentRev(next) |
conversionFlow(unique( | | getAUse(instr)), next, false, _)
)
)
}

private predicate convertsIntoArgument(
Operand iteratorAddress, CallInstruction call, int numberOfLoads
) {
exists(Instruction iteratorAddressDef |
isSource(iteratorAddressDef, iteratorAddress, numberOfLoads) and
isSink(iteratorAddressDef, call) and
convertsIntoArgumentRev(pragma[only_bind_into](iteratorAddressDef))
)
}

private predicate isChiAfterIteratorArgument(
Instruction memory, Operand iteratorAddress, int numberOfLoads
) {
// Ideally, `iteratorAddress` would be an `ArgumentOperand`, but there might be
// various conversions applied to it before it becomes an argument.
// So we do a small amount of flow to find the call that the iterator is passed to.
exists(CallInstruction call | convertsIntoArgument(iteratorAddress, call, numberOfLoads) |
exists(ReadSideEffectInstruction read |
read.getPrimaryInstruction() = call and
read.getSideEffectOperand().getAnyDef() = memory
)
or
exists(LoadInstruction load |
iteratorAddress.getDef() = load and
memory = load.getSourceValueOperand().getAnyDef()
)
)
}

/**
* Holds if `iterator` is a `StoreInstruction` that stores the result of some function
* returning an iterator into an address computed started at `containerBase`.
*
* For example, given a declaration like `std::vector<int>::iterator it = v.begin()`,
* the `iterator` will be the `StoreInstruction` generated by the write to `it`, and
* `containerBase` will be the address of `v`.
*/
private predicate isChiAfterBegin(
BaseSourceVariableInstruction containerBase, StoreInstruction iterator
) {
exists(
CallInstruction getIterator, Iterator::GetIteratorFunction getIteratorFunction,
IO::FunctionInput input, int i
|
getIterator = iterator.getSourceValue() and
getIteratorFunction = getIterator.getStaticCallTarget() and
getIteratorFunction.getsIterator(input, _) and
isDef(_, any(Node0Impl n | n.asInstruction() = iterator), _, _, 1, 0) and
input.isParameterDerefOrQualifierObject(i) and
isUse(_, getIterator.getArgumentOperand(i), containerBase, 0, 0)
)
}

/**
* Holds if `iteratorAddress` is an address of an iterator that is used for
* a read operation. The `memory` instruction represents the memory that
* the IR's SSA analysis determined was read by the call to `operator*`.
*
* Finally, the `numberOfLoads` integer represents the number of dereferences
* this read corresponds to on the underlying container that produced the iterator.
*/
private predicate isChiBeforeIteratorUse(
Operand iteratorAddress, Instruction memory, int numberOfLoads
) {
exists(
BaseSourceVariableInstruction iteratorBase, LoadInstruction load,
ReadSideEffectInstruction read, Operand iteratorDerefAddress
|
numberOfLoads >= 0 and
isUse(_, iteratorAddress, iteratorBase, numberOfLoads + 1, 0) and
isUse(_, iteratorDerefAddress, iteratorBase, numberOfLoads + 2, 0) and
iteratorBase.getResultType() instanceof Interfaces::Iterator and
load.getSourceAddressOperand() = iteratorDerefAddress and
read.getPrimaryInstruction() = load.getSourceAddress() and
memory = read.getSideEffectOperand().getAnyDef()
)
}

/**
* Holds if `iteratorDerefAddress` is an address of an iterator dereference (i.e., `*it`)
* that is used for a write operation that writes the value `value` to a container that
* created the iterator. `container` represents the base of the address of the container
* that was used to create the iterator.
*/
cached
predicate isIteratorDef(
BaseSourceVariableInstruction container, Operand iteratorDerefAddress, Node0Impl value,
int numberOfLoads, int indirectionIndex
) {
exists(Instruction memory, Instruction begin, int upper, int ind |
isChiAfterIteratorDef(memory, iteratorDerefAddress, value, numberOfLoads) and
memorySucc*(begin, memory) and
isChiAfterBegin(container, begin) and
upper = countIndirectionsForCppType(getResultLanguageType(container)) and
ind = numberOfLoads + [1 .. upper] and
indirectionIndex = ind - (numberOfLoads + 1)
)
}

/**
* Holds if `iteratorAddress` is an address of an iterator that is used for a
* read operation to read a value from a container that created the iterator.
* `container` represents the base of the address of the container that was used
* to create the iterator.
*/
cached
predicate isIteratorUse(
BaseSourceVariableInstruction container, Operand iteratorAddress, int numberOfLoads,
int indirectionIndex
) {
// Direct use
exists(Instruction begin, Instruction memory, int upper, int ind |
isChiBeforeIteratorUse(iteratorAddress, memory, numberOfLoads) and
memorySucc*(begin, memory) and
isChiAfterBegin(container, begin) and
upper = countIndirectionsForCppType(getResultLanguageType(container)) and
ind = numberOfLoads + [1 .. upper] and
indirectionIndex = ind - (numberOfLoads + 1)
)
or
// Use through function output
exists(Instruction memory, Instruction begin, int upper, int ind |
isChiAfterIteratorArgument(memory, iteratorAddress, numberOfLoads) and
memorySucc*(begin, memory) and
isChiAfterBegin(container, begin) and
upper = countIndirectionsForCppType(getResultLanguageType(container)) and
ind = numberOfLoads + [1 .. upper] and
indirectionIndex = ind - (numberOfLoads - 1)
)
}

/** Holds if `op` is the only use of its defining instruction, and that op is used in a conversation */
private predicate isConversion(Operand op) {
exists(Instruction def, Operand use |
Expand Down