hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-03 04:39:29 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #6568 from andersfugmann/andersfugmann/improve_upper_bound

Browse Source 
C++: Improve predicate upperBound in SimpleRangeAnalysis
This commit is contained in:
Anders Peter Fugmann
2021-09-10 09:49:48 +02:00
committed by GitHub
parent 3e17fdcaa3 270dbd2bf7
commit 1bbadb57a2
11 changed files with 127 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cpp/change-notes/2021-08-31-range-analysis-upper-bound.md Normal file
View File

@@ -0,0 +1,4 @@
lgtm,codescanning
* The `SimpleRangeAnalysis` library includes information from the
immediate guard for determining the upper bound of a stack
variable for improved accuracy.

15
cpp/ql/lib/semmle/code/cpp/rangeanalysis/RangeSSA.qll
View File

@@ -95,10 +95,19 @@ class RangeSsaDefinition extends ControlFlowNodeBase {
/**
* If this definition is a phi node corresponding to a guard,
* then return the variable and the guard.
* then return the variable access and the guard.
*/
predicate isGuardPhi(VariableAccess v, Expr guard, boolean branch) {
guard_defn(v, guard, this, branch)
predicate isGuardPhi(VariableAccess va, Expr guard, boolean branch) {
guard_defn(va, guard, this, branch)
}
/**
* If this definition is a phi node corresponding to a guard,
* then return the variable guarded, the variable access and the guard.
*/
predicate isGuardPhi(StackVariable v, VariableAccess va, Expr guard, boolean branch) {
guard_defn(va, guard, this, branch) and
va.getTarget() = v
}
/** Gets the primary location of this definition. */

29
cpp/ql/lib/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll
View File

@@ -1530,6 +1530,29 @@ private predicate isUnsupportedGuardPhi(Variable v, RangeSsaDefinition phi, Vari
)
}
/**
* Gets the upper bound of the expression, if the expression is guarded.
* An upper bound can only be found, if a guard phi node can be found, and the
* expression has only one immediate predecessor.
*/
private float getGuardedUpperBound(VariableAccess guardedAccess) {
exists(
RangeSsaDefinition def, StackVariable v, VariableAccess guardVa, Expr guard, boolean branch
|
def.isGuardPhi(v, guardVa, guard, branch) and
// If the basic block for the variable access being examined has
// more than one predecessor, the guard phi node could originate
// from one of the predecessors. This is because the guard phi
// node is attached to the block at the end of the edge and not on
// the actual edge. It is therefore not possible to determine which
// edge the guard phi node belongs to. The predicate below ensures
// that there is one predecessor, albeit somewhat conservative.
exists(unique(BasicBlock b | b = def.(BasicBlock).getAPredecessor())) and
guardedAccess = def.getAUse(v) and
result = max(float ub | upperBoundFromGuard(guard, guardVa, ub, branch))
)
}
cached
private module SimpleRangeAnalysisCached {
/**
@@ -1565,9 +1588,9 @@ private module SimpleRangeAnalysisCached {
*/
cached
float upperBound(Expr expr) {
// Combine the upper bounds returned by getTruncatedUpperBounds into a
// single maximum value.
result = max(float ub | ub = getTruncatedUpperBounds(expr) | ub)
// Combine the upper bounds returned by getTruncatedUpperBounds and
// getGuardedUpperBound into a single maximum value
result = min([max(getTruncatedUpperBounds(expr)), getGuardedUpperBound(expr)])
}
/**

33
cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected
View File

@@ -594,6 +594,11 @@
| test.c:659:9:659:9 | u | 0 |
| test.c:664:12:664:12 | s | -2147483648 |
| test.c:665:7:665:8 | s2 | -4 |
| test.c:670:7:670:7 | x | -2147483648 |
| test.c:671:9:671:9 | y | -2147483648 |
| test.c:675:7:675:7 | y | -2147483648 |
| test.c:684:7:684:7 | x | -2147483648 |
| test.c:689:7:689:7 | x | -2147483648 |
| test.cpp:10:7:10:7 | b | -2147483648 |
| test.cpp:11:5:11:5 | x | -2147483648 |
| test.cpp:13:10:13:10 | x | -2147483648 |
@@ -647,16 +652,18 @@
| test.cpp:97:10:97:10 | i | -2147483648 |
| test.cpp:97:22:97:22 | i | -2147483648 |
| test.cpp:98:5:98:5 | i | -2147483648 |
| test.cpp:105:7:105:7 | n | -32768 |
| test.cpp:108:7:108:7 | n | 0 |
| test.cpp:109:5:109:5 | n | 1 |
| test.cpp:111:5:111:5 | n | 0 |
| test.cpp:114:8:114:8 | n | 0 |
| test.cpp:115:5:115:5 | n | 0 |
| test.cpp:117:5:117:5 | n | 1 |
| test.cpp:120:3:120:3 | n | 0 |
| test.cpp:120:8:120:8 | n | 1 |
| test.cpp:120:12:120:12 | n | 0 |
| test.cpp:121:4:121:4 | n | 0 |
| test.cpp:121:8:121:8 | n | 0 |
| test.cpp:121:12:121:12 | n | 1 |
| test.cpp:98:9:98:9 | i | -2147483648 |
| test.cpp:99:5:99:5 | i | -2147483648 |
| test.cpp:106:7:106:7 | n | -32768 |
| test.cpp:109:7:109:7 | n | 0 |
| test.cpp:110:5:110:5 | n | 1 |
| test.cpp:112:5:112:5 | n | 0 |
| test.cpp:115:8:115:8 | n | 0 |
| test.cpp:116:5:116:5 | n | 0 |
| test.cpp:118:5:118:5 | n | 1 |
| test.cpp:121:3:121:3 | n | 0 |
| test.cpp:121:8:121:8 | n | 1 |
| test.cpp:121:12:121:12 | n | 0 |
| test.cpp:122:4:122:4 | n | 0 |
| test.cpp:122:8:122:8 | n | 0 |
| test.cpp:122:12:122:12 | n | 1 |

4
cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/ternaryLower.expected
View File

@@ -15,5 +15,5 @@
| test.c:394:20:394:36 | ... ? ... : ... | 0.0 | 0.0 | 100.0 |
| test.c:606:5:606:14 | ... ? ... : ... | 0.0 | 1.0 | 0.0 |
| test.c:607:5:607:14 | ... ? ... : ... | 0.0 | 0.0 | 1.0 |
| test.cpp:120:3:120:12 | ... ? ... : ... | 0.0 | 1.0 | 0.0 |
| test.cpp:121:3:121:12 | ... ? ... : ... | 0.0 | 0.0 | 1.0 |
| test.cpp:121:3:121:12 | ... ? ... : ... | 0.0 | 1.0 | 0.0 |
| test.cpp:122:3:122:12 | ... ? ... : ... | 0.0 | 0.0 | 1.0 |

4
cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/ternaryUpper.expected
View File

@@ -15,5 +15,5 @@
| test.c:394:20:394:36 | ... ? ... : ... | 100.0 | 99.0 | 100.0 |
| test.c:606:5:606:14 | ... ? ... : ... | 32767.0 | 32767.0 | 0.0 |
| test.c:607:5:607:14 | ... ? ... : ... | 32767.0 | 0.0 | 32767.0 |
| test.cpp:120:3:120:12 | ... ? ... : ... | 32767.0 | 32767.0 | 0.0 |
| test.cpp:121:3:121:12 | ... ? ... : ... | 32767.0 | 0.0 | 32767.0 |
| test.cpp:121:3:121:12 | ... ? ... : ... | 32767.0 | 32767.0 | 0.0 |
| test.cpp:122:3:122:12 | ... ? ... : ... | 32767.0 | 0.0 | 32767.0 |

25
cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.c
View File

@@ -664,3 +664,28 @@ void test_mod(int s) {
int s2 = s % 5;
out(s2); // -4 .. 4
}
void exit(int);
void guard_with_exit(int x, int y) {
if (x) {
if (y != 0) {
exit(0);
}
}
out(y); // ..
// This test ensures that we correctly identify
// that the upper bound for y is max_int when calling `out(y)`.
// The RangeSsa will place guardPhy on `out(y)`, and consequently there is no
// frontier phi node at out(y).
}
void test(int x) {
if (x >= 10) {
return;
}
// The basic below has two predecessors.
label:
out(x);
goto label;
}

1
cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.cpp
View File

@@ -95,6 +95,7 @@ int ref_to_number(int &i, const int &ci, int &aliased) {
return alias;
for (; i <= 12345; i++) { // test that widening works for references
i = i;
i;
}

41
cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected
View File

@@ -584,9 +584,9 @@
| test.c:639:9:639:10 | ss | 2 |
| test.c:645:8:645:8 | s | 2147483647 |
| test.c:645:15:645:15 | s | 127 |
| test.c:645:23:645:23 | s | 15 |
| test.c:646:18:646:18 | s | 15 |
| test.c:646:22:646:22 | s | 15 |
| test.c:645:23:645:23 | s | 9 |
| test.c:646:18:646:18 | s | 9 |
| test.c:646:22:646:22 | s | 9 |
| test.c:647:9:647:14 | result | 127 |
| test.c:653:7:653:7 | i | 0 |
| test.c:654:9:654:9 | i | 2147483647 |
@@ -594,6 +594,11 @@
| test.c:659:9:659:9 | u | 4294967295 |
| test.c:664:12:664:12 | s | 2147483647 |
| test.c:665:7:665:8 | s2 | 4 |
| test.c:670:7:670:7 | x | 2147483647 |
| test.c:671:9:671:9 | y | 2147483647 |
| test.c:675:7:675:7 | y | 2147483647 |
| test.c:684:7:684:7 | x | 2147483647 |
| test.c:689:7:689:7 | x | 15 |
| test.cpp:10:7:10:7 | b | 2147483647 |
| test.cpp:11:5:11:5 | x | 2147483647 |
| test.cpp:13:10:13:10 | x | 2147483647 |
@@ -646,17 +651,19 @@
| test.cpp:95:12:95:16 | alias | 2147483647 |
| test.cpp:97:10:97:10 | i | 65535 |
| test.cpp:97:22:97:22 | i | 32767 |
| test.cpp:98:5:98:5 | i | 32767 |
| test.cpp:105:7:105:7 | n | 32767 |
| test.cpp:108:7:108:7 | n | 32767 |
| test.cpp:109:5:109:5 | n | 32767 |
| test.cpp:111:5:111:5 | n | 0 |
| test.cpp:114:8:114:8 | n | 32767 |
| test.cpp:115:5:115:5 | n | 0 |
| test.cpp:117:5:117:5 | n | 32767 |
| test.cpp:120:3:120:3 | n | 32767 |
| test.cpp:120:8:120:8 | n | 32767 |
| test.cpp:120:12:120:12 | n | 0 |
| test.cpp:121:4:121:4 | n | 32767 |
| test.cpp:121:8:121:8 | n | 0 |
| test.cpp:121:12:121:12 | n | 32767 |
| test.cpp:98:5:98:5 | i | 2147483647 |
| test.cpp:98:9:98:9 | i | 12345 |
| test.cpp:99:5:99:5 | i | 32767 |
| test.cpp:106:7:106:7 | n | 32767 |
| test.cpp:109:7:109:7 | n | 32767 |
| test.cpp:110:5:110:5 | n | 32767 |
| test.cpp:112:5:112:5 | n | 0 |
| test.cpp:115:8:115:8 | n | 32767 |
| test.cpp:116:5:116:5 | n | 0 |
| test.cpp:118:5:118:5 | n | 32767 |
| test.cpp:121:3:121:3 | n | 32767 |
| test.cpp:121:8:121:8 | n | 32767 |
| test.cpp:121:12:121:12 | n | 0 |
| test.cpp:122:4:122:4 | n | 32767 |
| test.cpp:122:8:122:8 | n | 0 |
| test.cpp:122:12:122:12 | n | 32767 |

10
cpp/ql/test/query-tests/Critical/OverflowStatic/test.cpp
View File

@@ -46,3 +46,13 @@ void f2(char *src)
ptr = &(buffer[1]);
memcpy(ptr, src, 100); // BAD [NOT DETECTED]
}
void f3() {
int i;
char buffer[5];
for (i=0; i<10; i++) {
if (i < 5) {
buffer[i] = 0; // GOOD
}
}
}

2
cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.c
View File

@@ -115,7 +115,7 @@ int twoReasons(int a, int b) {
if (a <= 0 && b > 5) {
return a < b;
}
if (a <= 100 && b > 105) {
if (a <= 100 && b > 105) { // BUG [Not detected - this clause is always false]
return a > b;
}
return 0;