Skip to content

Commit 730cbfd

Browse files
committed
Add out-of-value-range-comparison.ql
1 parent d7be8d0 commit 730cbfd

File tree

1 file changed

+139
-0
lines changed

1 file changed

+139
-0
lines changed
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
/**
2+
* Finds comparisons which are always true or false because the fixed value against
3+
* which the comparison is performed is outside the value range of the other argument.
4+
* For example:
5+
* ```java
6+
* byte b = ...;
7+
* // Always true because a byte value is always smaller than Integer.MAX_VALUE
8+
* if (b < Integer.MAX_VALUE) {
9+
* ...
10+
* }
11+
* ```
12+
*
13+
* See also Error Prone pattern [ComparisonOutOfRange](https://errorprone.info/bugpattern/ComparisonOutOfRange).
14+
*
15+
* @kind problem
16+
*/
17+
18+
import java
19+
20+
// Use float as return type because CodeQL's int type is only 32 bit and therefore cannot store all Java (64-bit) long values
21+
float getFloatValue(Expr e) {
22+
result = e.(CompileTimeConstantExpr).getIntValue()
23+
or result = e.(LongLiteral).getValue().toFloat()
24+
or result = e.(CharacterLiteral).getCodePointValue()
25+
or result = e.(FloatLiteral).getFloatValue()
26+
or result = e.(DoubleLiteral).getDoubleValue()
27+
or exists(Field f, string t |
28+
f.getDeclaringType().hasQualifiedName("java.lang", t)
29+
and f.hasName("MIN_VALUE")
30+
and e = f.getAnAccess()
31+
|
32+
t = "Byte" and result = -128
33+
or t = "Short" and result = -32768
34+
or t = "Character" and result = 0
35+
or t = "Integer" and result = -2147483648
36+
// Note: Cannot be accurately represented as CodeQL float
37+
or t = "Long" and result = -9223372036854775808.0
38+
// Don't include Float and Double; unlikely that comparisons are
39+
// performed against their min values
40+
)
41+
or exists(Field f, string t |
42+
f.getDeclaringType().hasQualifiedName("java.lang", t)
43+
and f.hasName("MAX_VALUE")
44+
and e = f.getAnAccess()
45+
|
46+
t = "Byte" and result = 127
47+
or t = "Short" and result = 32767
48+
or t = "Character" and result = 65535
49+
or t = "Integer" and result = 2147483647
50+
// Note: Cannot be accurately represented as CodeQL float
51+
or t = "Long" and result = 9223372036854775807.0
52+
// Don't include Float and Double; unlikely that comparisons are
53+
// performed against their max values
54+
)
55+
// Other Character MIN and MAX values
56+
or exists(Field f, string n |
57+
f.getDeclaringType().hasQualifiedName("java.lang", "Character")
58+
and f.hasName(n)
59+
and e = f.getAnAccess()
60+
|
61+
n = "MAX_CODE_POINT" and result = 1114111
62+
or n = "MAX_HIGH_SURROGATE" and result = 56319
63+
or n = "MAX_LOW_SURROGATE" and result = 57343
64+
or n = "MAX_SURROGATE" and result = 57343
65+
or n = "MIN_CODE_POINT" and result = 0
66+
or n = "MIN_HIGH_SURROGATE" and result = 55296
67+
or n = "MIN_LOW_SURROGATE" and result = 56320
68+
or n = "MIN_SURROGATE" and result = 55296
69+
)
70+
}
71+
72+
float getMinValue(PrimitiveType t) {
73+
t.hasName("byte") and result = -128
74+
or t.hasName("short") and result = -32768
75+
or t.hasName("char") and result = 0
76+
or t.hasName("int") and result = -2147483648
77+
// Note: Cannot be accurately represented as CodeQL float
78+
or t.hasName("long") and result = -9223372036854775808.0
79+
// Don't include float and double because it is unlikely that any comparison is
80+
// performad against their min value (or smaller numbers)
81+
}
82+
83+
float getMaxValue(PrimitiveType t) {
84+
t.hasName("byte") and result = 127
85+
or t.hasName("short") and result = 32767
86+
or t.hasName("char") and result = 65535
87+
or t.hasName("int") and result = 2147483647
88+
// Note: Cannot be accurately represented as CodeQL float
89+
or t.hasName("long") and result = 9223372036854775807.0
90+
// Don't include float and double because it is unlikely that any comparison is
91+
// performad against their max value (or larger numbers)
92+
}
93+
94+
from BinaryExpr comparingExpr, Expr e, Expr fixedValueExpr, float comparedValue, PrimitiveType type, float minValue, float maxValue, string message
95+
where
96+
comparingExpr.getAnOperand() = e
97+
and comparingExpr.getAnOperand() = fixedValueExpr
98+
and comparedValue = getFloatValue(fixedValueExpr)
99+
and e != fixedValueExpr
100+
and (type = e.getType() or type = e.getType().(BoxedType).getPrimitiveType())
101+
and minValue = getMinValue(type)
102+
and maxValue = getMaxValue(type)
103+
and (
104+
exists(EqualityTest eqTest | eqTest = comparingExpr |
105+
comparedValue < minValue and (if eqTest.polarity() = true then
106+
// e == (min - x)
107+
message = "Always false because $@ is smaller than min value"
108+
// e != (min - x)
109+
else message = "Always true because $@ is smaller than max value"
110+
)
111+
or comparedValue > maxValue and (if eqTest.polarity() = true then
112+
// e == (max + x)
113+
message = "Always false because $@ is greater than max value"
114+
// e != (max + x)
115+
else message = "Always true because $@ is greater than max value"
116+
)
117+
)
118+
or exists(ComparisonExpr compExpr | compExpr = comparingExpr |
119+
comparedValue < minValue and (if e = compExpr.getGreaterOperand() then
120+
// e > (min - x) || e >= (min - x)
121+
message = "Always true because $@ is smaller than min value"
122+
// e < (min - x) || e <= (min - x)
123+
else message = "Always false because $@ is smaller than min value"
124+
)
125+
or comparedValue > maxValue and (if e = compExpr.getGreaterOperand() then
126+
// e > (max + x) || e >= (max + x)
127+
message = "Always false because $@ is greater than max value"
128+
// e < (max + x) || e <= (max + x)
129+
else message = "Always true because $@ is greater than max value"
130+
)
131+
or compExpr.isStrict() and (
132+
// e > max
133+
comparedValue >= maxValue and e = compExpr.getGreaterOperand() and message = "Always false because $@ is greater than or equal to max value"
134+
// e < min
135+
or comparedValue <= minValue and e = compExpr.getLesserOperand() and message = "Always false because $@ is smaller than or equal to min value"
136+
)
137+
)
138+
)
139+
select comparingExpr, message + " of type " + type.getName(), fixedValueExpr, "this value"

0 commit comments

Comments
 (0)