diff --git a/cpp/ql/lib/change-notes/2025-05-13-range-analysis-infinite-loop.md b/cpp/ql/lib/change-notes/2025-05-13-range-analysis-infinite-loop.md new file mode 100644 index 000000000000..7452e024d53f --- /dev/null +++ b/cpp/ql/lib/change-notes/2025-05-13-range-analysis-infinite-loop.md @@ -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. \ No newline at end of file diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 39975d8883c4..e517a75edf97 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -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, _)) ) ) ) @@ -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() ) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 51829f13df51..bea6b68d5119 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -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() } @@ -891,12 +891,12 @@ private module SsaInput implements SsaImplCommon::InputSig { 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 ) ) @@ -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) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/IRFunction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/IRFunction.qll index 354ba41e3d1b..7fa08f57a006 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/IRFunction.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/IRFunction.qll @@ -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) } } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll index acb17006fefb..522cd393081e 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll @@ -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() @@ -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) { diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll index e0a6594e7400..1a1eb6a1876f 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll @@ -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 } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRFunction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRFunction.qll index 354ba41e3d1b..7fa08f57a006 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRFunction.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRFunction.qll @@ -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) } } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll index 5e2461ba8b7c..594e37b668d6 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll @@ -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)`. diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/IRFunction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/IRFunction.qll index 354ba41e3d1b..7fa08f57a006 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/IRFunction.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/IRFunction.qll @@ -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) } } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll index e0a6594e7400..1a1eb6a1876f 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll @@ -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 } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll index 8bee2bf86a77..39283d07b54c 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll @@ -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 { @@ -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() } diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExprSpecific.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExprSpecific.qll index 1b36ae2efc5e..1b83ae959d2c 100644 --- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExprSpecific.qll +++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExprSpecific.qll @@ -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()) }