Skip to content

Commit 25b9696

Browse files
NagyDonatEndre Fulop
andcommitted
[analyzer] Upstream BitwiseShiftChecker
This commit releases a checker that was developed to a stable level in the Ericsson-internal fork of Clang Static Analyzer. Note that the functionality of this checker overlaps with core.UndefinedBinaryOperatorResult ("UBOR"), but there are several differences between them: (1) UBOR is only triggered when the constant folding performed by the Clang Static Analyzer engine determines that the value of a binary operator expression is undefined; this checker can report issues where the operands are not constants. (2) UBOR has unrelated checks for handling other binary operators, this checker only examines bitwise shifts. (3) This checker has a Pedantic flag and by default does not report expressions (e.g. -2 << 2) that're undefined by the standard but consistently supported in practice. (4) UBOR exhibits buggy behavior in code that involves cast expressions, e.g. void foo(unsigned short s) { if (s == 2) { (void) ((unsigned int) s) << 16; } } Later it would be good to eliminate this overlap (perhaps by deprecating and then eliminating the bitwise shift handling in UBOR), but in my opinion that belongs to separate commits. Differential Revision: https://reviews.llvm.org/D156312 Co-authored-by: Endre Fulop <[email protected]>
1 parent a37e8a4 commit 25b9696

18 files changed

+950
-18
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,10 @@ libclang
283283
Static Analyzer
284284
---------------
285285

286+
- Added a new checker ``core.BitwiseShift`` which reports situations where
287+
bitwise shift operators produce undefined behavior (because some operand is
288+
negative or too large).
289+
286290
.. _release-notes-sanitizers:
287291

288292
Sanitizers

clang/docs/analyzer/checkers.rst

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,56 @@ Models core language features and contains general-purpose checkers such as divi
2929
null pointer dereference, usage of uninitialized values, etc.
3030
*These checkers must be always switched on as other checker rely on them.*
3131

32+
.. _core-BitwiseShift:
33+
34+
core.BitwiseShift (C, C++)
35+
""""""""""""""""""""""""""
36+
37+
Finds undefined behavior caused by the bitwise left- and right-shift operator
38+
operating on integer types.
39+
40+
By default, this checker only reports situations when the right operand is
41+
either negative or larger than the bit width of the type of the left operand;
42+
these are logically unsound.
43+
44+
Moreover, if the pedantic mode is activated by
45+
``-analyzer-config core.BitwiseShift:Pedantic=true``, then this checker also
46+
reports situations where the _left_ operand of a shift operator is negative or
47+
overflow occurs during the right shift of a signed value. (Most compilers
48+
handle these predictably, but the C standard and the C++ standards before C++20
49+
say that they're undefined behavior. In the C++20 standard these constructs are
50+
well-defined, so activating pedantic mode in C++20 has no effect.)
51+
52+
**Examples**
53+
54+
.. code-block:: cpp
55+
56+
static_assert(sizeof(int) == 4, "assuming 32-bit int")
57+
58+
void basic_examples(int a, int b) {
59+
if (b < 0) {
60+
b = a << b; // warn: right operand is negative in left shift
61+
} else if (b >= 32) {
62+
b = a >> b; // warn: right shift overflows the capacity of 'int'
63+
}
64+
}
65+
66+
int pedantic_examples(int a, int b) {
67+
if (a < 0) {
68+
return a >> b; // warn: left operand is negative in right shift
69+
}
70+
a = 1000u << 31; // OK, overflow of unsigned value is well-defined, a == 0
71+
if (b > 10) {
72+
a = b << 31; // this is undefined before C++20, but the checker doesn't
73+
// warn because it doesn't know the exact value of b
74+
}
75+
return 1000 << 31; // warn: this overflows the capacity of 'int'
76+
}
77+
78+
**Solution**
79+
80+
Ensure the shift operands are in proper range before shifting.
81+
3282
.. _core-CallAndMessage:
3383

3484
core.CallAndMessage (C, C++, ObjC)

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,19 @@ def WebKitAlpha : Package<"webkit">, ParentPackage<Alpha>;
126126

127127
let ParentPackage = Core in {
128128

129+
def BitwiseShiftChecker : Checker<"BitwiseShift">,
130+
HelpText<"Finds cases where bitwise shift operation causes undefined behaviour.">,
131+
CheckerOptions<[
132+
CmdLineOption<Boolean,
133+
"Pedantic",
134+
"If set to true, the checker reports undefined behavior even "
135+
"if it is supported by most compilers. (This flag has no "
136+
"effect in C++20 where these constructs are legal.)",
137+
"false",
138+
Released>,
139+
]>,
140+
Documentation<HasDocumentation>;
141+
129142
def CallAndMessageModeling : Checker<"CallAndMessageModeling">,
130143
HelpText<"Responsible for essential modeling and assumptions after a "
131144
"function/method call. For instance, if we can't reason about the "

0 commit comments

Comments
 (0)