Skip to content

Commit 6f9ec04

Browse files
committed
C++: Use StackVariable in code that uses RangeSSA
1 parent 29f66ff commit 6f9ec04

File tree

3 files changed

+25
-31
lines changed

3 files changed

+25
-31
lines changed

cpp/ql/src/Likely Bugs/Format/SnprintfOverflow.ql

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ predicate flowsToExprImpl(Expr source, Expr sink, boolean pathMightOverflow) {
3737
pathMightOverflow = false and
3838
source.(FunctionCall).getTarget().(Snprintf).returnsFullFormatLength()
3939
or
40-
exists(RangeSsaDefinition def, LocalScopeVariable v |
40+
exists(RangeSsaDefinition def, StackVariable v |
4141
flowsToDef(source, def, v, pathMightOverflow) and
4242
sink = def.getAUse(v)
4343
)
@@ -63,9 +63,7 @@ predicate flowsToExprImpl(Expr source, Expr sink, boolean pathMightOverflow) {
6363
* `pathMightOverflow` is true if there is an arithmetic operation
6464
* on the path that might overflow.
6565
*/
66-
predicate flowsToDef(
67-
Expr source, RangeSsaDefinition def, LocalScopeVariable v, boolean pathMightOverflow
68-
) {
66+
predicate flowsToDef(Expr source, RangeSsaDefinition def, StackVariable v, boolean pathMightOverflow) {
6967
// Might the current definition overflow?
7068
exists(boolean otherMightOverflow | flowsToDefImpl(source, def, v, otherMightOverflow) |
7169
if defMightOverflow(def, v)
@@ -86,7 +84,7 @@ predicate flowsToDef(
8684
* the path. But it is a good way to reduce the number of false positives.
8785
*/
8886
predicate flowsToDefImpl(
89-
Expr source, RangeSsaDefinition def, LocalScopeVariable v, boolean pathMightOverflow
87+
Expr source, RangeSsaDefinition def, StackVariable v, boolean pathMightOverflow
9088
) {
9189
// Assignment or initialization: `e = v;`
9290
exists(Expr e |

cpp/ql/src/semmle/code/cpp/rangeanalysis/NanAnalysis.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ predicate nanExcludingComparison(ComparisonOperation guard, boolean polarity) {
2424
* by virtue of the guard in `def`.
2525
*/
2626
private predicate excludesNan(RangeSsaDefinition def, VariableAccess v) {
27-
exists(VariableAccess inCond, ComparisonOperation guard, boolean branch, LocalScopeVariable lsv |
27+
exists(VariableAccess inCond, ComparisonOperation guard, boolean branch, StackVariable lsv |
2828
def.isGuardPhi(inCond, guard, branch) and
2929
inCond.getTarget() = lsv and
3030
v = def.getAUse(lsv) and

cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ private predicate analyzableExpr(Expr e) {
120120
exists(exprMinVal(e.(Conversion).getExpr())) or
121121
// Also allow variable accesses, provided that they have SSA
122122
// information.
123-
exists(RangeSsaDefinition def, LocalScopeVariable v | e = def.getAUse(v))
123+
exists(RangeSsaDefinition def, StackVariable v | e = def.getAUse(v))
124124
)
125125
}
126126

@@ -136,7 +136,7 @@ private predicate analyzableExpr(Expr e) {
136136
* here.
137137
*/
138138
private predicate defDependsOnDef(
139-
RangeSsaDefinition def, LocalScopeVariable v, RangeSsaDefinition srcDef, LocalScopeVariable srcVar
139+
RangeSsaDefinition def, StackVariable v, RangeSsaDefinition srcDef, StackVariable srcVar
140140
) {
141141
// Definitions with a defining value.
142142
exists(Expr expr | assignmentDef(def, v, expr) | exprDependsOnDef(expr, srcDef, srcVar))
@@ -171,7 +171,7 @@ private predicate defDependsOnDef(
171171
* Helper predicate for `defDependsOnDef`. This predicate matches
172172
* the structure of `getLowerBoundsImpl` and `getUpperBoundsImpl`.
173173
*/
174-
private predicate exprDependsOnDef(Expr e, RangeSsaDefinition srcDef, LocalScopeVariable srcVar) {
174+
private predicate exprDependsOnDef(Expr e, RangeSsaDefinition srcDef, StackVariable srcVar) {
175175
exists(UnaryMinusExpr negateExpr | e = negateExpr |
176176
exprDependsOnDef(negateExpr.getOperand(), srcDef, srcVar)
177177
)
@@ -226,7 +226,7 @@ private predicate exprDependsOnDef(Expr e, RangeSsaDefinition srcDef, LocalScope
226226
* the structure of `getPhiLowerBounds` and `getPhiUpperBounds`.
227227
*/
228228
private predicate phiDependsOnDef(
229-
RangeSsaDefinition phi, LocalScopeVariable v, RangeSsaDefinition srcDef, LocalScopeVariable srcVar
229+
RangeSsaDefinition phi, StackVariable v, RangeSsaDefinition srcDef, StackVariable srcVar
230230
) {
231231
exists(VariableAccess access, ComparisonOperation guard |
232232
access = v.getAnAccess() and
@@ -241,19 +241,17 @@ private predicate phiDependsOnDef(
241241

242242
/** The transitive closure of `defDependsOnDef`. */
243243
private predicate defDependsOnDefTransitively(
244-
RangeSsaDefinition def, LocalScopeVariable v, RangeSsaDefinition srcDef, LocalScopeVariable srcVar
244+
RangeSsaDefinition def, StackVariable v, RangeSsaDefinition srcDef, StackVariable srcVar
245245
) {
246246
defDependsOnDef(def, v, srcDef, srcVar)
247247
or
248-
exists(RangeSsaDefinition midDef, LocalScopeVariable midVar |
249-
defDependsOnDef(def, v, midDef, midVar)
250-
|
248+
exists(RangeSsaDefinition midDef, StackVariable midVar | defDependsOnDef(def, v, midDef, midVar) |
251249
defDependsOnDefTransitively(midDef, midVar, srcDef, srcVar)
252250
)
253251
}
254252

255253
/** The set of definitions that depend recursively on themselves. */
256-
private predicate isRecursiveDef(RangeSsaDefinition def, LocalScopeVariable v) {
254+
private predicate isRecursiveDef(RangeSsaDefinition def, StackVariable v) {
257255
defDependsOnDefTransitively(def, v, def, v)
258256
}
259257

@@ -271,7 +269,7 @@ private predicate isRecursiveDef(RangeSsaDefinition def, LocalScopeVariable v) {
271269
*
272270
* This predicate finds all the definitions in the first set.
273271
*/
274-
private predicate assignmentDef(RangeSsaDefinition def, LocalScopeVariable v, Expr expr) {
272+
private predicate assignmentDef(RangeSsaDefinition def, StackVariable v, Expr expr) {
275273
v.getUnspecifiedType() instanceof ArithmeticType and
276274
(
277275
def = v.getInitializer().getExpr() and def = expr
@@ -285,7 +283,7 @@ private predicate assignmentDef(RangeSsaDefinition def, LocalScopeVariable v, Ex
285283
}
286284

287285
/** See comment above sourceDef. */
288-
private predicate analyzableDef(RangeSsaDefinition def, LocalScopeVariable v) {
286+
private predicate analyzableDef(RangeSsaDefinition def, StackVariable v) {
289287
assignmentDef(def, v, _) or defDependsOnDef(def, v, _, _)
290288
}
291289

@@ -613,7 +611,7 @@ private float getLowerBoundsImpl(Expr expr) {
613611
)
614612
or
615613
// Use SSA to get the lower bounds for a variable use.
616-
exists(RangeSsaDefinition def, LocalScopeVariable v | expr = def.getAUse(v) |
614+
exists(RangeSsaDefinition def, StackVariable v | expr = def.getAUse(v) |
617615
result = getDefLowerBounds(def, v)
618616
)
619617
}
@@ -766,7 +764,7 @@ private float getUpperBoundsImpl(Expr expr) {
766764
)
767765
or
768766
// Use SSA to get the upper bounds for a variable use.
769-
exists(RangeSsaDefinition def, LocalScopeVariable v | expr = def.getAUse(v) |
767+
exists(RangeSsaDefinition def, StackVariable v | expr = def.getAUse(v) |
770768
result = getDefUpperBounds(def, v)
771769
)
772770
}
@@ -860,7 +858,7 @@ private float boolConversionUpperBound(Expr expr) {
860858
* In this example, the lower bound of x is 0, but we can
861859
* use the guard to deduce that the lower bound is 2 inside the block.
862860
*/
863-
private float getPhiLowerBounds(LocalScopeVariable v, RangeSsaDefinition phi) {
861+
private float getPhiLowerBounds(StackVariable v, RangeSsaDefinition phi) {
864862
exists(
865863
VariableAccess access, ComparisonOperation guard, boolean branch, float defLB, float guardLB
866864
|
@@ -877,7 +875,7 @@ private float getPhiLowerBounds(LocalScopeVariable v, RangeSsaDefinition phi) {
877875
}
878876

879877
/** See comment for `getPhiLowerBounds`, above. */
880-
private float getPhiUpperBounds(LocalScopeVariable v, RangeSsaDefinition phi) {
878+
private float getPhiUpperBounds(StackVariable v, RangeSsaDefinition phi) {
881879
exists(
882880
VariableAccess access, ComparisonOperation guard, boolean branch, float defUB, float guardUB
883881
|
@@ -894,7 +892,7 @@ private float getPhiUpperBounds(LocalScopeVariable v, RangeSsaDefinition phi) {
894892
}
895893

896894
/** Only to be called by `getDefLowerBounds`. */
897-
private float getDefLowerBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v) {
895+
private float getDefLowerBoundsImpl(RangeSsaDefinition def, StackVariable v) {
898896
// Definitions with a defining value.
899897
exists(Expr expr | assignmentDef(def, v, expr) | result = getFullyConvertedLowerBounds(expr))
900898
or
@@ -936,7 +934,7 @@ private float getDefLowerBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v
936934
}
937935

938936
/** Only to be called by `getDefUpperBounds`. */
939-
private float getDefUpperBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v) {
937+
private float getDefUpperBoundsImpl(RangeSsaDefinition def, StackVariable v) {
940938
// Definitions with a defining value.
941939
exists(Expr expr | assignmentDef(def, v, expr) | result = getFullyConvertedUpperBounds(expr))
942940
or
@@ -982,7 +980,7 @@ private float getDefUpperBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v
982980
* done by `getDefLowerBoundsImpl`, but this is where widening is applied
983981
* to prevent the analysis from exploding due to a recursive definition.
984982
*/
985-
private float getDefLowerBounds(RangeSsaDefinition def, LocalScopeVariable v) {
983+
private float getDefLowerBounds(RangeSsaDefinition def, StackVariable v) {
986984
exists(float newLB, float truncatedLB |
987985
newLB = getDefLowerBoundsImpl(def, v) and
988986
if varMinVal(v) <= newLB and newLB <= varMaxVal(v)
@@ -1011,7 +1009,7 @@ private float getDefLowerBounds(RangeSsaDefinition def, LocalScopeVariable v) {
10111009
}
10121010

10131011
/** See comment for `getDefLowerBounds`, above. */
1014-
private float getDefUpperBounds(RangeSsaDefinition def, LocalScopeVariable v) {
1012+
private float getDefUpperBounds(RangeSsaDefinition def, StackVariable v) {
10151013
exists(float newUB, float truncatedUB |
10161014
newUB = getDefUpperBoundsImpl(def, v) and
10171015
if varMinVal(v) <= newUB and newUB <= varMaxVal(v)
@@ -1044,9 +1042,7 @@ private float getDefUpperBounds(RangeSsaDefinition def, LocalScopeVariable v) {
10441042
* unanalyzable definitions (such as function parameters) and make their
10451043
* bounds unknown.
10461044
*/
1047-
private predicate unanalyzableDefBounds(
1048-
RangeSsaDefinition def, LocalScopeVariable v, float lb, float ub
1049-
) {
1045+
private predicate unanalyzableDefBounds(RangeSsaDefinition def, StackVariable v, float lb, float ub) {
10501046
v = def.getAVariable() and
10511047
not analyzableDef(def, v) and
10521048
lb = varMinVal(v) and
@@ -1268,13 +1264,13 @@ private module SimpleRangeAnalysisCached {
12681264

12691265
/** Holds if the definition might overflow negatively. */
12701266
cached
1271-
predicate defMightOverflowNegatively(RangeSsaDefinition def, LocalScopeVariable v) {
1267+
predicate defMightOverflowNegatively(RangeSsaDefinition def, StackVariable v) {
12721268
getDefLowerBoundsImpl(def, v) < varMinVal(v)
12731269
}
12741270

12751271
/** Holds if the definition might overflow positively. */
12761272
cached
1277-
predicate defMightOverflowPositively(RangeSsaDefinition def, LocalScopeVariable v) {
1273+
predicate defMightOverflowPositively(RangeSsaDefinition def, StackVariable v) {
12781274
getDefUpperBoundsImpl(def, v) > varMaxVal(v)
12791275
}
12801276

@@ -1283,7 +1279,7 @@ private module SimpleRangeAnalysisCached {
12831279
* negatively).
12841280
*/
12851281
cached
1286-
predicate defMightOverflow(RangeSsaDefinition def, LocalScopeVariable v) {
1282+
predicate defMightOverflow(RangeSsaDefinition def, StackVariable v) {
12871283
defMightOverflowNegatively(def, v) or
12881284
defMightOverflowPositively(def, v)
12891285
}

0 commit comments

Comments
 (0)