Skip to content

Commit a19ea77

Browse files
committed
Add impossible-bit-check.ql
1 parent 56f3b7c commit a19ea77

File tree

1 file changed

+120
-0
lines changed

1 file changed

+120
-0
lines changed
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/**
2+
* Finds bit mask checks which seem to always be `false` because not all checked
3+
* bits can actually be set.
4+
*
5+
* Similar to SpotBugs':
6+
* - [`BIT_AND`](https://spotbugs.readthedocs.io/en/latest/bugDescriptions.html#bit-incompatible-bit-masks-bit-and)
7+
* - [`BIT_IOR`](https://spotbugs.readthedocs.io/en/latest/bugDescriptions.html#bit-incompatible-bit-masks-bit-ior)
8+
*
9+
* @id TODO
10+
* @kind problem
11+
*/
12+
13+
// TODO: Might need further refinement
14+
15+
import java
16+
17+
// Separate predicate which is not directly used by `where` clause of this query so that any equality tests which
18+
// directly compare mismatching constants (e.g. `1 == 2`, which might occur in unit tests or for debug flags which
19+
// can be enabled by changing the code), are not reported as 'bit checks' by this query
20+
int getPossibleBitsOrVar(Expr e) {
21+
result = getPossibleBits(e)
22+
or
23+
result = e.(CompileTimeConstantExpr).getIntValue()
24+
or
25+
// All bits are possible when reading non-constant var
26+
e instanceof RValue and not e instanceof CompileTimeConstantExpr and result = -1
27+
}
28+
29+
predicate isPowerOf2(int i) {
30+
i =
31+
[
32+
1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072,
33+
262144, 524288, 1048576, 2097152, 4194304, 8388608, 16777216, 33554432, 67108864, 134217728,
34+
268435456, 536870912, 1073741824
35+
]
36+
}
37+
38+
/**
39+
* For an expression, gets its int value where all bits which at runtime _might_ be set have the
40+
* value 1. And all bits which are definitely not set have the value 0.
41+
*/
42+
int getPossibleBits(Expr e) {
43+
/*
44+
* TODO: For bitwise 'not', applying `bitNot()` here can cause false positives because it is only known
45+
* here that 0 bits are guaranteed to not be set, but 1 bits might or might not be set. So using `bitNot()`
46+
* will turn those 1 bits into 'definitely not set', which is incorrect.
47+
* Would be more correct to either use -1 (all bits possible) as result, or instead additionally need to
48+
* track which bits are guaranteed to be 1, and only invert those (might also be useful for XOR).
49+
*/
50+
51+
result = getPossibleBitsOrVar(e.(BitNotExpr).getExpr()).bitNot()
52+
or
53+
exists(OrBitwiseExpr orExpr | orExpr = e |
54+
result =
55+
getPossibleBitsOrVar(orExpr.getLeftOperand())
56+
.bitOr(getPossibleBitsOrVar(orExpr.getRightOperand()))
57+
)
58+
or
59+
// Treat XOR like OR: It is only known here that 0 bits are guaranteed to not be set, but 1 bits might or
60+
// might not be set. So using `bitXor` could lead to incorrect results.
61+
exists(XorBitwiseExpr xorExpr | xorExpr = e |
62+
result =
63+
getPossibleBitsOrVar(xorExpr.getLeftOperand())
64+
.bitOr(getPossibleBitsOrVar(xorExpr.getRightOperand()))
65+
)
66+
or
67+
exists(AndBitwiseExpr andExpr | andExpr = e |
68+
result =
69+
getPossibleBitsOrVar(andExpr.getLeftOperand())
70+
.bitAnd(getPossibleBitsOrVar(andExpr.getRightOperand()))
71+
)
72+
or
73+
exists(LeftShiftExpr shiftExpr | shiftExpr = e |
74+
result =
75+
getPossibleBitsOrVar(shiftExpr.getLeftOperand())
76+
.bitShiftLeft(shiftExpr.getRightOperand().(CompileTimeConstantExpr).getIntValue())
77+
)
78+
or
79+
exists(UnsignedRightShiftExpr shiftExpr | shiftExpr = e |
80+
result =
81+
getPossibleBitsOrVar(shiftExpr.getLeftOperand())
82+
.bitShiftRight(shiftExpr.getRightOperand().(CompileTimeConstantExpr).getIntValue())
83+
)
84+
or
85+
exists(RightShiftExpr shiftExpr | shiftExpr = e |
86+
result =
87+
getPossibleBitsOrVar(shiftExpr.getLeftOperand())
88+
.bitShiftRightSigned(shiftExpr.getRightOperand().(CompileTimeConstantExpr).getIntValue())
89+
)
90+
or
91+
exists(DivExpr divExpr, int divisor | divExpr = e |
92+
divisor = divExpr.getRightOperand().(CompileTimeConstantExpr).getIntValue() and
93+
// If power of 2, then this acts like a bit shift and can be performed without knowing exact value
94+
isPowerOf2(divisor) and
95+
result = getPossibleBitsOrVar(divExpr.getLeftOperand()) / divisor
96+
)
97+
or
98+
exists(MulExpr mulExpr, Expr valueOp, CompileTimeConstantExpr multiplierOp, int multiplier |
99+
mulExpr = e and
100+
valueOp = mulExpr.getAnOperand() and
101+
multiplierOp = mulExpr.getAnOperand() and
102+
valueOp != multiplierOp
103+
|
104+
multiplier = multiplierOp.getIntValue() and
105+
// If power of 2, then this acts like a bit shift and can be performed without knowing exact value
106+
isPowerOf2(multiplier) and
107+
result = getPossibleBitsOrVar(valueOp) * multiplier
108+
)
109+
}
110+
111+
from EQExpr eqExpr, CompileTimeConstantExpr checkedExpr, int checkedValue, Expr bitExpr
112+
where
113+
checkedExpr = eqExpr.getAnOperand() and
114+
bitExpr = eqExpr.getAnOperand() and
115+
checkedExpr != bitExpr and
116+
checkedValue = checkedExpr.getIntValue() and
117+
checkedValue != 0 and
118+
// And not all checked bits can actually be set
119+
checkedValue.bitAnd(getPossibleBits(bitExpr)) != checkedValue
120+
select eqExpr, "Will never succeed because bit value will never match"

0 commit comments

Comments
 (0)