Skip to content

Commit fa79423

Browse files
authored
Merge pull request #19477 from MathiasVP/fix-infinite-range-analysis-on-incomplete-ssa
C++: Fix infinite range analysis loop on invalid SSA
2 parents c608a90 + f255fc2 commit fa79423

File tree

12 files changed

+83
-9
lines changed

12 files changed

+83
-9
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: fix
3+
---
4+
* Fixed an infinite loop in `semmle.code.cpp.rangeanalysis.new.RangeAnalysis` when computing ranges in very large and complex function bodies.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,7 +1567,7 @@ private int countNumberOfBranchesUsingParameter(SwitchInstruction switch, Parame
15671567
|
15681568
exists(Ssa::UseImpl use | use.hasIndexInBlock(useblock, _, sv))
15691569
or
1570-
exists(Ssa::DefImpl def | def.hasIndexInBlock(useblock, _, sv))
1570+
exists(Ssa::DefImpl def | def.hasIndexInBlock(sv, useblock, _))
15711571
)
15721572
)
15731573
)
@@ -1814,7 +1814,7 @@ module IteratorFlow {
18141814
*/
18151815
private predicate isIteratorWrite(Instruction write, Operand address) {
18161816
exists(Ssa::DefImpl writeDef, IRBlock bb, int i |
1817-
writeDef.hasIndexInBlock(bb, i, _) and
1817+
writeDef.hasIndexInBlock(_, bb, i) and
18181818
bb.getInstruction(i) = write and
18191819
address = writeDef.getAddressOperand()
18201820
)

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ abstract class DefImpl extends TDefImpl {
191191
* Holds if this definition (or use) has index `index` in block `block`,
192192
* and is a definition (or use) of the variable `sv`
193193
*/
194-
final predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) {
194+
final predicate hasIndexInBlock(SourceVariable sv, IRBlock block, int index) {
195195
this.hasIndexInBlock(block, index) and
196196
sv = this.getSourceVariable()
197197
}
@@ -891,12 +891,12 @@ private module SsaInput implements SsaImplCommon::InputSig<Location> {
891891
predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {
892892
DataFlowImplCommon::forceCachingInSameStage() and
893893
(
894-
exists(DefImpl def | def.hasIndexInBlock(bb, i, v) |
894+
exists(DefImpl def | def.hasIndexInBlock(v, bb, i) |
895895
if def.isCertain() then certain = true else certain = false
896896
)
897897
or
898898
exists(GlobalDefImpl global |
899-
global.hasIndexInBlock(bb, i, v) and
899+
global.hasIndexInBlock(v, bb, i) and
900900
certain = true
901901
)
902902
)
@@ -934,10 +934,11 @@ module SsaCached {
934934
}
935935

936936
/** Gets the `DefImpl` corresponding to `def`. */
937+
pragma[nomagic]
937938
private DefImpl getDefImpl(SsaImpl::Definition def) {
938939
exists(SourceVariable sv, IRBlock bb, int i |
939940
def.definesAt(sv, bb, i) and
940-
result.hasIndexInBlock(bb, i, sv)
941+
result.hasIndexInBlock(sv, bb, i)
941942
)
942943
}
943944

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/IRFunction.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,12 @@ class IRFunction extends IRFunctionBase {
5858
* Gets all blocks in this function.
5959
*/
6060
final IRBlock getABlock() { result.getEnclosingIRFunction() = this }
61+
62+
/**
63+
* Holds if this function may have incomplete def-use information.
64+
*
65+
* Def-use information may be omitted for a function when it is too expensive
66+
* to compute.
67+
*/
68+
final predicate hasIncompleteSsa() { Construction::hasIncompleteSsa(this) }
6169
}

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ private newtype TMemoryLocation =
235235
*
236236
* Some of these memory locations will be filtered out for performance reasons before being passed to SSA construction.
237237
*/
238-
abstract private class MemoryLocation0 extends TMemoryLocation {
238+
abstract class MemoryLocation0 extends TMemoryLocation {
239239
final string toString() {
240240
if this.isMayAccess()
241241
then result = "?" + this.toStringInternal()
@@ -874,7 +874,7 @@ private int numberOfOverlappingUses(MemoryLocation0 def) {
874874
* Holds if `def` is a busy definition. That is, it has a large number of
875875
* overlapping uses.
876876
*/
877-
private predicate isBusyDef(MemoryLocation0 def) { numberOfOverlappingUses(def) > 1024 }
877+
predicate isBusyDef(MemoryLocation0 def) { numberOfOverlappingUses(def) > 1024 }
878878

879879
/** Holds if `use` is a use that overlaps with a busy definition. */
880880
private predicate useOverlapWithBusyDef(MemoryLocation0 use) {

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,20 @@ private module Cached {
731731
or
732732
instruction = getChi(result.(UninitializedGroupInstruction))
733733
}
734+
735+
/**
736+
* Holds if the def-use information for `f` may have been omitted because it
737+
* was too expensive to compute. This happens if one of the memory allocations
738+
* in `f` is a busy definition (i.e., it has many different overlapping uses).
739+
*/
740+
pragma[nomagic]
741+
cached
742+
predicate hasIncompleteSsa(IRFunction f) {
743+
exists(Alias::MemoryLocation0 defLocation |
744+
Alias::isBusyDef(pragma[only_bind_into](defLocation)) and
745+
defLocation.getIRFunction() = f
746+
)
747+
}
734748
}
735749

736750
private Instruction getNewInstruction(OldInstruction instr) { getOldInstruction(result) = instr }

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRFunction.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,12 @@ class IRFunction extends IRFunctionBase {
5858
* Gets all blocks in this function.
5959
*/
6060
final IRBlock getABlock() { result.getEnclosingIRFunction() = this }
61+
62+
/**
63+
* Holds if this function may have incomplete def-use information.
64+
*
65+
* Def-use information may be omitted for a function when it is too expensive
66+
* to compute.
67+
*/
68+
final predicate hasIncompleteSsa() { Construction::hasIncompleteSsa(this) }
6169
}

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,8 @@ Instruction getMemoryOperandDefinition(
220220
none()
221221
}
222222

223+
predicate hasIncompleteSsa(IRFunction f) { none() }
224+
223225
/**
224226
* Holds if the operand totally overlaps with its definition and consumes the
225227
* bit range `[startBitOffset, endBitOffset)`.

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/IRFunction.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,12 @@ class IRFunction extends IRFunctionBase {
5858
* Gets all blocks in this function.
5959
*/
6060
final IRBlock getABlock() { result.getEnclosingIRFunction() = this }
61+
62+
/**
63+
* Holds if this function may have incomplete def-use information.
64+
*
65+
* Def-use information may be omitted for a function when it is too expensive
66+
* to compute.
67+
*/
68+
final predicate hasIncompleteSsa() { Construction::hasIncompleteSsa(this) }
6169
}

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,20 @@ private module Cached {
731731
or
732732
instruction = getChi(result.(UninitializedGroupInstruction))
733733
}
734+
735+
/**
736+
* Holds if the def-use information for `f` may have been omitted because it
737+
* was too expensive to compute. This happens if one of the memory allocations
738+
* in `f` is a busy definition (i.e., it has many different overlapping uses).
739+
*/
740+
pragma[nomagic]
741+
cached
742+
predicate hasIncompleteSsa(IRFunction f) {
743+
exists(Alias::MemoryLocation0 defLocation |
744+
Alias::isBusyDef(pragma[only_bind_into](defLocation)) and
745+
defLocation.getIRFunction() = f
746+
)
747+
}
734748
}
735749

736750
private Instruction getNewInstruction(OldInstruction instr) { getOldInstruction(result) = instr }

0 commit comments

Comments
 (0)