Skip to content

Commit fbe42fb

Browse files
committed
C++: Support != constant in range analysis
1 parent d061b09 commit fbe42fb

File tree

3 files changed

+76
-27
lines changed

3 files changed

+76
-27
lines changed

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

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,6 +1160,17 @@ private float getPhiLowerBounds(StackVariable v, RangeSsaDefinition phi) {
11601160
if guardLB > defLB then result = guardLB else result = defLB
11611161
)
11621162
or
1163+
exists(VariableAccess access, float neConstant, float lower |
1164+
isNEPhi(v, phi, access, neConstant) and
1165+
lower = getFullyConvertedLowerBounds(access) and
1166+
if lower = neConstant then result = lower + 1 else result = lower
1167+
)
1168+
or
1169+
exists(VariableAccess access |
1170+
isUnsupportedGuardPhi(v, phi, access) and
1171+
result = getFullyConvertedLowerBounds(access)
1172+
)
1173+
or
11631174
result = getDefLowerBounds(phi.getAPhiInput(v), v)
11641175
}
11651176

@@ -1177,6 +1188,17 @@ private float getPhiUpperBounds(StackVariable v, RangeSsaDefinition phi) {
11771188
if guardUB < defUB then result = guardUB else result = defUB
11781189
)
11791190
or
1191+
exists(VariableAccess access, float neConstant, float upper |
1192+
isNEPhi(v, phi, access, neConstant) and
1193+
upper = getFullyConvertedUpperBounds(access) and
1194+
if upper = neConstant then result = upper - 1 else result = upper
1195+
)
1196+
or
1197+
exists(VariableAccess access |
1198+
isUnsupportedGuardPhi(v, phi, access) and
1199+
result = getFullyConvertedUpperBounds(access)
1200+
)
1201+
or
11801202
result = getDefUpperBounds(phi.getAPhiInput(v), v)
11811203
}
11821204

@@ -1501,22 +1523,13 @@ private predicate linearBoundFromGuard(
15011523
// 1. x <= upperbound(RHS)
15021524
// 2. x >= lowerbound(RHS)
15031525
//
1504-
// For x != RHS, we create trivial bounds:
1505-
//
1506-
// 1. x <= typeUpperBound(RHS.getUnspecifiedType())
1507-
// 2. x >= typeLowerBound(RHS.getUnspecifiedType())
1508-
//
1509-
exists(Expr lhs, Expr rhs, boolean isEQ |
1526+
exists(Expr lhs, Expr rhs |
15101527
linearAccess(lhs, v, p, q) and
1511-
eqOpWithSwapAndNegate(guard, lhs, rhs, isEQ, branch) and
1528+
eqOpWithSwapAndNegate(guard, lhs, rhs, true, branch) and
1529+
getBounds(rhs, boundValue, isLowerBound) and
15121530
strictness = Nonstrict()
1513-
|
1514-
// True branch
1515-
isEQ = true and getBounds(rhs, boundValue, isLowerBound)
1516-
or
1517-
// False branch: set the bounds to the min/max for the type.
1518-
isEQ = false and exprTypeBounds(rhs, boundValue, isLowerBound)
15191531
)
1532+
// x != RHS and !x are handled elsewhere
15201533
}
15211534

15221535
/** Utility for `linearBoundFromGuard`. */
@@ -1533,6 +1546,42 @@ private predicate exprTypeBounds(Expr expr, float boundValue, boolean isLowerBou
15331546
isLowerBound = false and boundValue = exprMaxVal(expr.getFullyConverted())
15341547
}
15351548

1549+
/**
1550+
* Holds if `(v, phi)` ensures that `access` is not equal to `neConstant`. For
1551+
* example, the condition `if (x + 1 != 3)` ensures that `x` is not equal to 2.
1552+
* Only integral types are supported.
1553+
*/
1554+
private predicate isNEPhi(
1555+
Variable v, RangeSsaDefinition phi, VariableAccess access, float neConstant
1556+
) {
1557+
exists(
1558+
ComparisonOperation cmp, boolean branch, Expr linearExpr, Expr rExpr, float p, float q, float r
1559+
|
1560+
access.getTarget() = v and
1561+
phi.isGuardPhi(access, cmp, branch) and
1562+
eqOpWithSwapAndNegate(cmp, linearExpr, rExpr, false, branch) and
1563+
v.getUnspecifiedType() instanceof IntegralOrEnumType and // Float `!=` is too imprecise
1564+
r = getValue(rExpr).toFloat() and
1565+
linearAccess(linearExpr, access, p, q) and
1566+
neConstant = (r - q) / p
1567+
)
1568+
}
1569+
1570+
/**
1571+
* Holds if `(v, phi)` constrains the value of `access` but in a way that
1572+
* doesn't allow this library to constrain the upper or lower bounds of
1573+
* `access`. An example is `if (x != y)` if neither `x` nor `y` is a
1574+
* compile-time constant.
1575+
*/
1576+
private predicate isUnsupportedGuardPhi(Variable v, RangeSsaDefinition phi, VariableAccess access) {
1577+
exists(ComparisonOperation cmp, boolean branch |
1578+
access.getTarget() = v and
1579+
phi.isGuardPhi(access, cmp, branch) and
1580+
eqOpWithSwapAndNegate(cmp, _, _, false, branch) and
1581+
not isNEPhi(v, phi, access, _)
1582+
)
1583+
}
1584+
15361585
cached
15371586
private module SimpleRangeAnalysisCached {
15381587
/**

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -536,33 +536,33 @@
536536
| test.c:541:7:541:7 | n | 0 |
537537
| test.c:542:9:542:9 | n | 1 |
538538
| test.c:545:7:545:7 | n | 0 |
539-
| test.c:546:9:546:9 | n | 0 |
539+
| test.c:546:9:546:9 | n | 1 |
540540
| test.c:548:9:548:9 | n | 0 |
541541
| test.c:551:8:551:8 | n | 0 |
542542
| test.c:552:9:552:9 | n | 0 |
543543
| test.c:554:9:554:9 | n | 0 |
544544
| test.c:557:10:557:10 | n | 0 |
545-
| test.c:558:5:558:5 | n | 0 |
545+
| test.c:558:5:558:5 | n | 1 |
546546
| test.c:561:7:561:7 | n | 0 |
547547
| test.c:565:7:565:7 | n | -32768 |
548548
| test.c:568:7:568:7 | n | 0 |
549549
| test.c:569:9:569:9 | n | 0 |
550-
| test.c:571:9:571:9 | n | 0 |
550+
| test.c:571:9:571:9 | n | 1 |
551551
| test.c:574:7:574:7 | n | 0 |
552552
| test.c:575:9:575:9 | n | 0 |
553553
| test.c:577:9:577:9 | n | 0 |
554-
| test.c:580:10:580:10 | n | -32768 |
555-
| test.c:581:5:581:5 | n | -32768 |
554+
| test.c:580:10:580:10 | n | 0 |
555+
| test.c:581:5:581:5 | n | 1 |
556556
| test.c:584:7:584:7 | n | 0 |
557557
| test.c:588:7:588:7 | n | -32768 |
558558
| test.c:589:9:589:9 | n | -32768 |
559559
| test.c:590:11:590:11 | n | 0 |
560560
| test.c:594:7:594:7 | n | -32768 |
561561
| test.c:595:13:595:13 | n | 5 |
562-
| test.c:598:9:598:9 | n | 5 |
562+
| test.c:598:9:598:9 | n | 6 |
563563
| test.c:601:7:601:7 | n | -32768 |
564-
| test.c:601:22:601:22 | n | -32768 |
565-
| test.c:602:9:602:9 | n | -32768 |
564+
| test.c:601:22:601:22 | n | -32767 |
565+
| test.c:602:9:602:9 | n | -32766 |
566566
| test.cpp:10:7:10:7 | b | -2147483648 |
567567
| test.cpp:11:5:11:5 | x | -2147483648 |
568568
| test.cpp:13:10:13:10 | x | -2147483648 |

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ int notequal_type_endpoint(unsigned n) {
543543
}
544544

545545
if (n != 0) {
546-
out(n); // 1 .. [BUG: lower bound is deduced to be 0]
546+
out(n); // 1 ..
547547
} else {
548548
out(n); // 0 .. 0
549549
}
@@ -555,7 +555,7 @@ int notequal_type_endpoint(unsigned n) {
555555
}
556556

557557
while (n != 0) {
558-
n--; // 1 .. [BUG: lower bound is deduced to be 0]
558+
n--; // 1 ..
559559
}
560560

561561
out(n); // 0 .. 0
@@ -568,7 +568,7 @@ void notequal_refinement(short n) {
568568
if (n == 0) {
569569
out(n); // 0 .. 0
570570
} else {
571-
out(n); // 1 .. [BUG: lower bound is deduced to be 0]
571+
out(n); // 1 ..
572572
}
573573

574574
if (n) {
@@ -578,7 +578,7 @@ void notequal_refinement(short n) {
578578
}
579579

580580
while (n != 0) {
581-
n--; // 1 .. [BUG: lower bound is deduced to be -32768]
581+
n--; // 1 ..
582582
}
583583

584584
out(n); // 0 .. 0
@@ -595,10 +595,10 @@ void notequal_variations(short n, float f) {
595595
if (2 * n - 10 == 0) { // Same as `n == 10/2` (modulo overflow)
596596
return;
597597
}
598-
out(n); // 6 .. [BUG: lower bound is deduced to be 5]
598+
out(n); // 6 ..
599599
}
600600

601601
if (n != -32768 && n != -32767) {
602-
out(n); // -32766 .. [BUG: lower bound is deduced to be -32768]
602+
out(n); // -32766 ..
603603
}
604604
}

0 commit comments

Comments
 (0)