Skip to content

C++: Fix infinite range analysis loop on invalid SSA #19477

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
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: fix
---
* Fixed an infinite loop in `semmle.code.cpp.rangeanalysis.new.RangeAnalysis` when computing ranges in very large and complex function bodies.
Original file line number Diff line number Diff line change
Expand Up @@ -1567,7 +1567,7 @@ private int countNumberOfBranchesUsingParameter(SwitchInstruction switch, Parame
|
exists(Ssa::UseImpl use | use.hasIndexInBlock(useblock, _, sv))
or
exists(Ssa::DefImpl def | def.hasIndexInBlock(useblock, _, sv))
exists(Ssa::DefImpl def | def.hasIndexInBlock(sv, useblock, _))
)
)
)
Expand Down Expand Up @@ -1814,7 +1814,7 @@ module IteratorFlow {
*/
private predicate isIteratorWrite(Instruction write, Operand address) {
exists(Ssa::DefImpl writeDef, IRBlock bb, int i |
writeDef.hasIndexInBlock(bb, i, _) and
writeDef.hasIndexInBlock(_, bb, i) and
bb.getInstruction(i) = write and
address = writeDef.getAddressOperand()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ abstract class DefImpl extends TDefImpl {
* Holds if this definition (or use) has index `index` in block `block`,
* and is a definition (or use) of the variable `sv`
*/
final predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) {
final predicate hasIndexInBlock(SourceVariable sv, IRBlock block, int index) {
this.hasIndexInBlock(block, index) and
sv = this.getSourceVariable()
}
Expand Down Expand Up @@ -891,12 +891,12 @@ private module SsaInput implements SsaImplCommon::InputSig<Location> {
predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {
DataFlowImplCommon::forceCachingInSameStage() and
(
exists(DefImpl def | def.hasIndexInBlock(bb, i, v) |
exists(DefImpl def | def.hasIndexInBlock(v, bb, i) |
if def.isCertain() then certain = true else certain = false
)
or
exists(GlobalDefImpl global |
global.hasIndexInBlock(bb, i, v) and
global.hasIndexInBlock(v, bb, i) and
certain = true
)
)
Expand Down Expand Up @@ -934,10 +934,11 @@ module SsaCached {
}

/** Gets the `DefImpl` corresponding to `def`. */
pragma[nomagic]
private DefImpl getDefImpl(SsaImpl::Definition def) {
exists(SourceVariable sv, IRBlock bb, int i |
def.definesAt(sv, bb, i) and
result.hasIndexInBlock(bb, i, sv)
result.hasIndexInBlock(sv, bb, i)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,12 @@ class IRFunction extends IRFunctionBase {
* Gets all blocks in this function.
*/
final IRBlock getABlock() { result.getEnclosingIRFunction() = this }

/**
* Holds if this function may have incomplete def-use information.
*
* Def-use information may be omitted for a function when it is too expensive
* to compute.
*/
final predicate hasIncompleteSsa() { Construction::hasIncompleteSsa(this) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ private newtype TMemoryLocation =
*
* Some of these memory locations will be filtered out for performance reasons before being passed to SSA construction.
*/
abstract private class MemoryLocation0 extends TMemoryLocation {
abstract class MemoryLocation0 extends TMemoryLocation {
final string toString() {
if this.isMayAccess()
then result = "?" + this.toStringInternal()
Expand Down Expand Up @@ -874,7 +874,7 @@ private int numberOfOverlappingUses(MemoryLocation0 def) {
* Holds if `def` is a busy definition. That is, it has a large number of
* overlapping uses.
*/
private predicate isBusyDef(MemoryLocation0 def) { numberOfOverlappingUses(def) > 1024 }
predicate isBusyDef(MemoryLocation0 def) { numberOfOverlappingUses(def) > 1024 }

/** Holds if `use` is a use that overlaps with a busy definition. */
private predicate useOverlapWithBusyDef(MemoryLocation0 use) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,20 @@ private module Cached {
or
instruction = getChi(result.(UninitializedGroupInstruction))
}

/**
* Holds if the def-use information for `f` may have been omitted because it
* was too expensive to compute. This happens if one of the memory allocations
* in `f` is a busy definition (i.e., it has many different overlapping uses).
*/
pragma[nomagic]
cached
predicate hasIncompleteSsa(IRFunction f) {
exists(Alias::MemoryLocation0 defLocation |
Alias::isBusyDef(pragma[only_bind_into](defLocation)) and
defLocation.getIRFunction() = f
)
}
}

private Instruction getNewInstruction(OldInstruction instr) { getOldInstruction(result) = instr }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,12 @@ class IRFunction extends IRFunctionBase {
* Gets all blocks in this function.
*/
final IRBlock getABlock() { result.getEnclosingIRFunction() = this }

/**
* Holds if this function may have incomplete def-use information.
*
* Def-use information may be omitted for a function when it is too expensive
* to compute.
*/
final predicate hasIncompleteSsa() { Construction::hasIncompleteSsa(this) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ Instruction getMemoryOperandDefinition(
none()
}

predicate hasIncompleteSsa(IRFunction f) { none() }

/**
* Holds if the operand totally overlaps with its definition and consumes the
* bit range `[startBitOffset, endBitOffset)`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,12 @@ class IRFunction extends IRFunctionBase {
* Gets all blocks in this function.
*/
final IRBlock getABlock() { result.getEnclosingIRFunction() = this }

/**
* Holds if this function may have incomplete def-use information.
*
* Def-use information may be omitted for a function when it is too expensive
* to compute.
*/
final predicate hasIncompleteSsa() { Construction::hasIncompleteSsa(this) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,20 @@ private module Cached {
or
instruction = getChi(result.(UninitializedGroupInstruction))
}

/**
* Holds if the def-use information for `f` may have been omitted because it
* was too expensive to compute. This happens if one of the memory allocations
* in `f` is a busy definition (i.e., it has many different overlapping uses).
*/
pragma[nomagic]
cached
predicate hasIncompleteSsa(IRFunction f) {
exists(Alias::MemoryLocation0 defLocation |
Alias::isBusyDef(pragma[only_bind_into](defLocation)) and
defLocation.getIRFunction() = f
)
}
}

private Instruction getNewInstruction(OldInstruction instr) { getOldInstruction(result) = instr }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ class MemoryLocation extends TMemoryLocation {
final predicate canReuseSsa() { canReuseSsaForVariable(var) }
}

class MemoryLocation0 = MemoryLocation;

predicate canReuseSsaForOldResult(Instruction instr) { none() }

abstract class VariableGroup extends Unit {
Expand Down Expand Up @@ -141,3 +143,9 @@ int getStartBitOffset(MemoryLocation location) { none() }

/** Gets the end bit offset of a `MemoryLocation`, if any. */
int getEndBitOffset(MemoryLocation location) { none() }

/**
* Holds if `def` is a busy definition. That is, it has a large number of
* overlapping uses.
*/
predicate isBusyDef(MemoryLocation def) { none() }
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,14 @@ module SemanticExprConfig {
}

/** Holds if no range analysis should be performed on the phi edges in `f`. */
private predicate excludeFunction(Cpp::Function f) { count(f.getEntryPoint()) > 1 }
private predicate excludeFunction(Cpp::Function f) {
count(f.getEntryPoint()) > 1
or
exists(IR::IRFunction irFunction |
irFunction.getFunction() = f and
irFunction.hasIncompleteSsa()
)
}

SemType getUnknownExprType(Expr expr) { result = getSemanticType(expr.getResultIRType()) }

Expand Down