donat.nagy updated this revision to Diff 550318.
donat.nagy edited the summary of this revision.
donat.nagy added a comment.
I verified that the checker handles the examples in the documentation correctly
(and added them to the test suite). However, as I was tweaking the examples in
the documentation, I accidentally found a situation where the checker produces
a very surprising and arguably incorrect error message.
After investigating this issue, I added the testcases
`signed_aritmetic_{good,bad}` which document the current sub-optimal state. The
root cause of this problem is a high-level property of the engine (that it
assumes that signed overflows are always possible and acceptable) and I don't
see a local workaround that would silence or fix these incorrect error messages.
@steakhal @NoQ What do you know about these signed overflow issues (I presume
that analogous issues also appear in other checkers)? How should we handle this
limitation of this checker?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156312/new/
https://reviews.llvm.org/D156312
Files:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/analyzer-enabled-checkers.c
clang/test/Analysis/bitwise-ops-nocrash.c
clang/test/Analysis/bitwise-ops.c
clang/test/Analysis/bitwise-shift-common.c
clang/test/Analysis/bitwise-shift-pedantic.c
clang/test/Analysis/bitwise-shift-sanity-checks.c
clang/test/Analysis/bitwise-shift-state-update.c
clang/test/Analysis/casts.c
clang/test/Analysis/diagnostics/track_subexpressions.cpp
clang/test/Analysis/left-shift-cxx2a.cpp
clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
Index: clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
===================================================================
--- clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
+++ clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
@@ -14,7 +14,7 @@
if (p) {
// no-crash
}
- if (p == (int *)0x404) {
+ if (p == (int *)0x1b) {
// no-crash
}
}
@@ -29,7 +29,7 @@
if (p) {
// no-crash
}
- if (p == (int *)0x404) {
+ if (p == (int *)0x1b) {
// no-crash
}
}
@@ -43,8 +43,6 @@
nonloc_OP_loc(p, BINOP(-)); // no-crash
// Bitwise operators:
- // expected-warning@+2 {{The result of the left shift is undefined due to shifting by '1028', which is greater or equal to the width of type 'int' [core.UndefinedBinaryOperatorResult]}}
- // expected-warning@+2 {{The result of the right shift is undefined due to shifting by '1028', which is greater or equal to the width of type 'int' [core.UndefinedBinaryOperatorResult]}}
nonloc_OP_loc(p, BINOP(<<)); // no-crash
nonloc_OP_loc(p, BINOP(>>)); // no-crash
nonloc_OP_loc(p, BINOP(&));
Index: clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -24,6 +24,7 @@
// CHECK-NEXT: apiModeling.TrustReturnsNonnull
// CHECK-NEXT: apiModeling.llvm.CastValue
// CHECK-NEXT: apiModeling.llvm.ReturnValue
+// CHECK-NEXT: core.BitwiseShift
// CHECK-NEXT: core.DivideZero
// CHECK-NEXT: core.DynamicTypePropagation
// CHECK-NEXT: core.NonnilStringConstants
Index: clang/test/Analysis/left-shift-cxx2a.cpp
===================================================================
--- clang/test/Analysis/left-shift-cxx2a.cpp
+++ clang/test/Analysis/left-shift-cxx2a.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx17 -std=c++17 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx2a -std=c++2a %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-disable-checker core.BitwiseShift -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx17 -std=c++17 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-disable-checker core.BitwiseShift -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx2a -std=c++2a %s
int testNegativeShift(int a) {
if (a == -5) {
Index: clang/test/Analysis/diagnostics/track_subexpressions.cpp
===================================================================
--- clang/test/Analysis/diagnostics/track_subexpressions.cpp
+++ clang/test/Analysis/diagnostics/track_subexpressions.cpp
@@ -12,10 +12,10 @@
void shift_by_undefined_value() {
uint8_t shift_amount = get_uint8_max(); // expected-note{{'shift_amount' initialized to 255}}
- // expected-note@-1{{Calling 'get_uint8_max'}}
- // expected-note@-2{{Returning from 'get_uint8_max'}}
- (void)(TCP_MAXWIN << shift_amount); // expected-warning{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
- // expected-note@-1{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
+ // expected-note@-1{{Calling 'get_uint8_max'}}
+ // expected-note@-2{{Returning from 'get_uint8_max'}}
+ (void)(TCP_MAXWIN << shift_amount); // expected-warning{{Left shift by '255' overflows the capacity of 'int'}}
+ // expected-note@-1{{The result of left shift is undefined because the right operand '255' is not smaller than 32, the capacity of 'int'}}
}
namespace array_index_tracking {
Index: clang/test/Analysis/casts.c
===================================================================
--- clang/test/Analysis/casts.c
+++ clang/test/Analysis/casts.c
@@ -227,7 +227,7 @@
globalA == 3;
(long)globalA << 48;
#ifdef BIT32
- // expected-warning@-2{{The result of the left shift is undefined due to shifting by '48', which is greater or equal to the width of type 'long'}}
+ // expected-warning@-2{{Left shift by '48' overflows the capacity of 'long'}}
#else
// expected-no-diagnostics
#endif
Index: clang/test/Analysis/bitwise-shift-state-update.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/bitwise-shift-state-update.c
@@ -0,0 +1,42 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \
+// RUN: -analyzer-config core.BitwiseShift:Pedantic=true \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config eagerly-assume=false \
+// RUN: -verify=expected,c \
+// RUN: -triple x86_64-pc-linux-gnu -x c %s \
+// RUN: -Wno-shift-count-negative -Wno-shift-negative-value \
+// RUN: -Wno-shift-count-overflow -Wno-shift-overflow \
+// RUN: -Wno-shift-sign-overflow
+//
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \
+// RUN: -analyzer-config core.BitwiseShift:Pedantic=true \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config eagerly-assume=false \
+// RUN: -verify=expected,cxx \
+// RUN: -triple x86_64-pc-linux-gnu -x c++ -std=c++14 %s \
+// RUN: -Wno-shift-count-negative -Wno-shift-negative-value \
+// RUN: -Wno-shift-count-overflow -Wno-shift-overflow \
+// RUN: -Wno-shift-sign-overflow
+
+// Tests for validating the state updates provided by the BitwiseShift checker.
+// These clang_analyzer_value() tests are in a separate file because
+// debug.ExprInspection repeats each 'warning' with an superfluous 'note', so
+// note level output (-analyzer-output=text) is not enabled in this file.
+
+void clang_analyzer_value(int);
+void clang_analyzer_eval(int);
+
+int state_update_generic(int left, int right) {
+ int x = left << right;
+ clang_analyzer_value(left); // expected-warning {{32s:{ [0, 2147483647] } }}
+ clang_analyzer_value(right); // expected-warning {{32s:{ [0, 31] } }}
+ return x;
+}
+
+int state_update_exact_shift(int arg) {
+ int x = 65535 << arg;
+ clang_analyzer_value(arg);
+ // c-warning@-1 {{32s:{ [0, 15] } }}
+ // cxx-warning@-2 {{32s:{ [0, 16] } }}
+ return x;
+}
Index: clang/test/Analysis/bitwise-shift-sanity-checks.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/bitwise-shift-sanity-checks.c
@@ -0,0 +1,118 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \
+// RUN: -analyzer-config core.BitwiseShift:Pedantic=true \
+// RUN: -verify=expected,pedantic \
+// RUN: -triple x86_64-pc-linux-gnu -x c %s \
+// RUN: -Wno-shift-count-negative -Wno-shift-negative-value \
+// RUN: -Wno-shift-count-overflow -Wno-shift-overflow \
+// RUN: -Wno-shift-sign-overflow
+//
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \
+// RUN: -analyzer-config core.BitwiseShift:Pedantic=true \
+// RUN: -verify=expected,pedantic \
+// RUN: -triple x86_64-pc-linux-gnu -x c++ -std=c++14 %s \
+// RUN: -Wno-shift-count-negative -Wno-shift-negative-value \
+// RUN: -Wno-shift-count-overflow -Wno-shift-overflow \
+// RUN: -Wno-shift-sign-overflow
+//
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \
+// RUN: -verify=expected \
+// RUN: -triple x86_64-pc-linux-gnu -x c++ -std=c++20 %s \
+// RUN: -Wno-shift-count-negative -Wno-shift-negative-value \
+// RUN: -Wno-shift-count-overflow -Wno-shift-overflow \
+// RUN: -Wno-shift-sign-overflow
+
+// This test file verifies that the BitwiseShift checker does not crash or
+// report false positives (at least on the cases that are listed here...)
+// For the sake of brevity, 'note' output is not checked in this file.
+
+// TEST OBVIOUSLY CORRECT CODE
+//===----------------------------------------------------------------------===//
+
+unsigned shift_unsigned(void) {
+ // Shifts of unsigned LHS may overflow, even if the RHS is signed.
+ // In shifts the type of the right operand does not affect the type of the
+ // calculation and the result.
+ return 1024u << 25ll; // no-warning
+}
+
+int shift_zeroes(void) {
+ return 0 << 0; // no-warning
+}
+
+int no_info(int left, int right) {
+ return left << right; // no-warning
+}
+
+int all_okay(int left, int right) {
+ if (left < 0 || right < 0)
+ return 42;
+ return (left << right) + (left >> right); // no-warning
+}
+
+// DOCUMENT A LIMITATION OF THE ANALYZER ENGINE
+//===----------------------------------------------------------------------===//
+
+int signed_arithmetic_good(int left, int right) {
+ if (right >= 32)
+ return 0;
+ return left << (right - 32);
+ // expected-warning@-1 {{Right operand is negative in left shift}}
+}
+
+int signed_arithmetic_bad(int left, int right) {
+ // FIXME: The analyzer engine handles overflow of signed values as if it was
+ // a valid code path, so here it thinks that (right + 32) is either at least
+ // 32 *or* very negative after an overflow. As checkOvershift() is called
+ // before checkOperandNegative(), the checker will first rule out the case
+ // when (right + 32) is larger than 32 and then it reports that it's
+ // negative. Swapping the order of the two checks would trigger an analogous
+ // fault in signed_aritmetic_good().
+ if (right < 0)
+ return 0;
+ return left << (right + 32);
+ // expected - warning@-1 {{Left shift overflows the capacity of 'int'}}
+ // expected-warning@-2 {{Right operand is negative in left shift}}
+}
+
+// TEST THE EXAMPLES FROM THE DOCUMENTATION
+//===----------------------------------------------------------------------===//
+
+void basic_examples(int a, int b) {
+ if (b < 0) {
+ b = a << b; // expected-warning {{Right operand is negative in left shift}}
+ } else if (b >= 32) {
+ b = a >> b; // expected-warning {{Right shift overflows the capacity of 'int'}}
+ }
+}
+
+int pedantic_examples(int a, int b) {
+ if (a < 0) {
+ return a >> b; // pedantic-warning {{Left operand is negative in right shift}}
+ }
+ a = 1000u << 31; // OK, overflow of unsigned shift is well-defined, a == 0
+ if (b > 10) {
+ a = b << 31; // this is UB before C++20, but the checker doesn't warn because
+ // it doesn't know the exact value of b
+ }
+ return 1000 << 31; // pedantic-warning {{The shift '1000 << 31' overflows the capacity of 'int'}}
+}
+
+// TEST UNUSUAL CODE THAT SHOULD NOT CRASH
+//===----------------------------------------------------------------------===//
+
+__int128 large_left(void) {
+ // Ensure that we do not crash when the left operand doesn't fit in 64 bits.
+ return (__int128) 1 << 63 << 10 << 10; // no-crash
+}
+
+int large_right(void) {
+ // Ensure that we do not crash when the right operand doesn't fit in 64 bits.
+ return 1 << ((__int128) 1 << 118); // no-crash
+ // expected-warning@-1 {{Left shift by '332306998946228968225951765070086144' overflows the capacity of 'int'}}
+}
+
+void doubles_cast_to_integer(int *c) {
+ *c = 1 << (int)1.5; // no-crash
+ *c = ((int)1.5) << 1; // no-crash
+ *c = ((int)1.5) << (int)1.5; // no-crash
+}
Index: clang/test/Analysis/bitwise-shift-pedantic.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/bitwise-shift-pedantic.c
@@ -0,0 +1,167 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \
+// RUN: -analyzer-config core.BitwiseShift:Pedantic=true \
+// RUN: -analyzer-output=text -verify=expected,c \
+// RUN: -triple x86_64-pc-linux-gnu -x c %s \
+// RUN: -Wno-shift-count-negative -Wno-shift-negative-value \
+// RUN: -Wno-shift-count-overflow -Wno-shift-overflow \
+// RUN: -Wno-shift-sign-overflow
+//
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \
+// RUN: -analyzer-config core.BitwiseShift:Pedantic=true \
+// RUN: -analyzer-output=text -verify=expected,cxx \
+// RUN: -triple x86_64-pc-linux-gnu -x c++ -std=c++14 %s \
+// RUN: -Wno-shift-count-negative -Wno-shift-negative-value \
+// RUN: -Wno-shift-count-overflow -Wno-shift-overflow \
+// RUN: -Wno-shift-sign-overflow
+
+// This test file verifies the pedantic mode of the BitwiseShift checker, which
+// also reports issues that are undefined behavior (according to the standard,
+// under C and in C++ before C++20), but would be accepted by many compilers.
+
+// TEST NEGATIVE LEFT OPERAND
+//===----------------------------------------------------------------------===//
+
+int negative_left_operand_literal(void) {
+ return -2 << 2;
+ // expected-warning@-1 {{Left operand is negative in left shift}}
+ // expected-note@-2 {{The result of left shift is undefined because the left operand is negative}}
+}
+
+int negative_left_operand_symbolic(int left, int right) {
+ // expected-note@+2 {{Assuming 'left' is < 0}}
+ // expected-note@+1 {{Taking false branch}}
+ if (left >= 0)
+ return 0;
+ return left >> right;
+ // expected-warning@-1 {{Left operand is negative in right shift}}
+ // expected-note@-2 {{The result of right shift is undefined because the left operand is negative}}
+}
+
+int negative_left_operand_compound(short arg) {
+ // expected-note@+2 {{Assuming 'arg' is < 0}}
+ // expected-note@+1 {{Taking false branch}}
+ if (arg >= 0)
+ return 0;
+ return (arg - 3) << 2;
+ // expected-warning@-1 {{Left operand is negative in left shift}}
+ // expected-note@-2 {{The result of left shift is undefined because the left operand is negative}}
+}
+
+int double_negative(void) {
+ // In this case we still report that the right operand is negative, because
+ // that's the more "serious" issue:
+ return -2 >> -2;
+ // expected-warning@-1 {{Right operand is negative in right shift}}
+ // expected-note@-2 {{The result of right shift is undefined because the right operand is negative}}
+}
+
+int single_unknown_negative(int arg) {
+ // In this case just one of the operands will be negative, so we end up
+ // reporting the left operand after assuming that the right operand is
+ // positive.
+ // expected-note@+2 {{Assuming 'arg' is not equal to 0}}
+ // expected-note@+1 {{Taking false branch}}
+ if (!arg)
+ return 0;
+ // We're first checking the right operand, record that it must be positive,
+ // then report that then the left argument must be negative.
+ return -arg << arg;
+ // expected-warning@-1 {{Left operand is negative in left shift}}
+ // expected-note@-2 {{The result of left shift is undefined because the left operand is negative}}
+}
+
+void shift_negative_by_zero(int c) {
+ // This seems to be innocent, but the standard (before C++20) clearly implies
+ // that this is UB, so we should report it in pedantic mode.
+ c = (-1) << 0;
+ // expected-warning@-1 {{Left operand is negative in left shift}}
+ // expected-note@-2 {{The result of left shift is undefined because the left operand is negative}}
+}
+
+// TEST OVERFLOW OF CONCRETE SIGNED LEFT OPERAND
+//===----------------------------------------------------------------------===//
+// (the most complex and least important part of the checker)
+
+int concrete_overflow_literal(void) {
+ // 27 in binary is 11011 (5 bits), when shifted by 28 bits it becomes
+ // 1_10110000_00000000_00000000_00000000
+ return 27 << 28;
+ // expected-warning@-1 {{The shift '27 << 28' overflows the capacity of 'int'}}
+ // cxx-note@-2 {{The shift '27 << 28' is undefined because 'int' can hold only 32 bits (including the sign bit), so 1 bit overflows}}
+ // c-note@-3 {{The shift '27 << 28' is undefined because 'int' can hold only 31 bits (excluding the sign bit), so 2 bits overflow}}
+}
+
+int concrete_overflow_symbolic(int arg) {
+ // 29 in binary is 11101 (5 bits), when shifted by 29 bits it becomes
+ // 11_10100000_00000000_00000000_00000000
+
+ // expected-note@+2 {{Assuming 'arg' is equal to 29}}
+ // expected-note@+1 {{Taking false branch}}
+ if (arg != 29)
+ return 0;
+ return arg << arg;
+ // expected-warning@-1 {{The shift '29 << 29' overflows the capacity of 'int'}}
+ // cxx-note@-2 {{The shift '29 << 29' is undefined because 'int' can hold only 32 bits (including the sign bit), so 2 bits overflow}}
+ // c-note@-3 {{The shift '29 << 29' is undefined because 'int' can hold only 31 bits (excluding the sign bit), so 3 bits overflow}}
+}
+
+int concrete_overflow_language_difference(void) {
+ // 21 in binary is 10101 (5 bits), when shifted by 27 bits it becomes
+ // 10101000_00000000_00000000_00000000
+ // This does not overflow the 32-bit capacity of int, but reaches the sign
+ // bit, which is undefined under C (but accepted in C++ even before C++20).
+ return 21 << 27;
+ // c-warning@-1 {{The shift '21 << 27' overflows the capacity of 'int'}}
+ // c-note@-2 {{The shift '21 << 27' is undefined because 'int' can hold only 31 bits (excluding the sign bit), so 1 bit overflows}}
+}
+
+int concrete_overflow_int_min(void) {
+ // Another case that's undefined in C but valid in all C++ versions.
+ // Note the "represented by 1 bit" special case
+ return 1 << 31;
+ // c-warning@-1 {{The shift '1 << 31' overflows the capacity of 'int'}}
+ // c-note@-2 {{The shift '1 << 31' is undefined because 'int' can hold only 31 bits (excluding the sign bit), so 1 bit overflows}}
+}
+
+int concrete_overflow_vague(int arg) {
+ // expected-note@+2 {{Assuming 'arg' is > 25}}
+ // expected-note@+1 {{Taking false branch}}
+ if (arg <= 25)
+ return 0;
+ return 1024 << arg;
+ // expected-warning@-1 {{Left shift of '1024' overflows the capacity of 'int'}}
+ // cxx-note@-2 {{Left shift of '1024' is undefined because 'int' can hold only 32 bits (including the sign bit), so some bits overflow}}
+ // c-note@-3 {{Left shift of '1024' is undefined because 'int' can hold only 31 bits (excluding the sign bit), so some bits overflow}}
+}
+
+int concrete_overflow_vague_only_c(int arg) {
+ // A third case that's undefined in C but valid in all C++ versions.
+
+ // c-note@+2 {{Assuming 'arg' is > 20}}
+ // c-note@+1 {{Taking false branch}}
+ if (arg <= 20)
+ return 0;
+ return 1024 << arg;
+ // c-warning@-1 {{Left shift of '1024' overflows the capacity of 'int'}}
+ // c-note@-2 {{Left shift of '1024' is undefined because 'int' can hold only 31 bits (excluding the sign bit), so some bits overflow}}
+}
+
+int concrete_overflow_vague_left(int arg) {
+ // This kind of overflow check only handles concrete values on the LHS. With
+ // some effort it would be possible to report errors in cases like this; but
+ // it's probably a waste of time especially considering that overflows of
+ // left shifts became well-defined in C++20.
+
+ if (arg <= 1024)
+ return 0;
+ return arg << 25; // no-warning
+}
+
+int concrete_overflow_shift_zero(void) {
+ // This is legal, even in C.
+ // The relevant rule (as paraphrased on cppreference.com) is:
+ // "For signed LHS with nonnegative values, the value of LHS << RHS is
+ // LHS * 2^RHS if it is representable in the promoted type of lhs, otherwise
+ // the behavior is undefined."
+ return 0 << 31; // no-warning
+}
Index: clang/test/Analysis/bitwise-shift-common.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/bitwise-shift-common.c
@@ -0,0 +1,160 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \
+// RUN: -analyzer-output=text -verify \
+// RUN: -triple x86_64-pc-linux-gnu -x c %s \
+// RUN: -Wno-shift-count-negative -Wno-shift-negative-value \
+// RUN: -Wno-shift-count-overflow -Wno-shift-overflow \
+// RUN: -Wno-shift-sign-overflow
+//
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \
+// RUN: -analyzer-config core.BitwiseShift:Pedantic=true \
+// RUN: -analyzer-output=text -verify \
+// RUN: -triple x86_64-pc-linux-gnu -x c++ -std=c++20 %s \
+// RUN: -Wno-shift-count-negative -Wno-shift-negative-value \
+// RUN: -Wno-shift-count-overflow -Wno-shift-overflow \
+// RUN: -Wno-shift-sign-overflow
+//
+// This test file verifies the default behavior of the BitwiseShift checker,
+// which reports the serious logical defects, but doesn't warn on code that's
+// legal under C++20 (or later) and widely accepted (but theoretically
+// undefined) in other compilation modes.
+
+// TEST NEGATIVE RIGHT OPERAND
+//===----------------------------------------------------------------------===//
+
+int negative_right_operand_literal(void) {
+ return 2 << -2;
+ // expected-warning@-1 {{Right operand is negative in left shift}}
+ // expected-note@-2 {{The result of left shift is undefined because the right operand is negative}}
+}
+
+int negative_right_operand_symbolic(int left, int right) {
+ // expected-note@+2 {{Assuming 'right' is < 0}}
+ // expected-note@+1 {{Taking false branch}}
+ if (right >= 0)
+ return 0;
+ return left >> right;
+ // expected-warning@-1 {{Right operand is negative in right shift}}
+ // expected-note@-2 {{The result of right shift is undefined because the right operand is negative}}
+}
+
+int negative_right_operand_compound(short arg) {
+ // expected-note@+2 {{Assuming 'arg' is < 2}}
+ // expected-note@+1 {{Taking false branch}}
+ if (arg >= 2 )
+ return 0;
+ return 2 << (arg - 1 - 1 - 1);
+ // expected-warning@-1 {{Right operand is negative in left shift}}
+ // expected-note@-2 {{The result of left shift is undefined because the right operand is negative}}
+}
+
+// TEST TOO LARGE RIGHT OPERAND
+//===----------------------------------------------------------------------===//
+
+int too_large_right_operand_literal(void) {
+ return 2 << 32;
+ // expected-warning@-1 {{Left shift by '32' overflows the capacity of 'int'}}
+ // expected-note@-2 {{The result of left shift is undefined because the right operand '32' is not smaller than 32, the capacity of 'int'}}
+}
+
+int too_large_right_operand_exact_symbolic(int arg) {
+ // expected-note@+4 {{Assuming 'arg' is > 33}}
+ // expected-note@+3 {{Left side of '||' is false}}
+ // expected-note@+2 {{Assuming 'arg' is < 35}}
+ // expected-note@+1 {{Taking false branch}}
+ if (arg <= 33 || arg >= 35)
+ return 0;
+ return 3 << arg;
+ // expected-warning@-1 {{Left shift by '34' overflows the capacity of 'int'}}
+ // expected-note@-2 {{The result of left shift is undefined because the right operand '34' is not smaller than 32, the capacity of 'int'}}
+}
+
+int too_large_right_operand_exact_symbolic_2(char arg) {
+ // expected-note@+2 {{Assuming the condition is false}}
+ // expected-note@+1 {{Taking false branch}}
+ if (arg != ' ')
+ return 0;
+ return 3 << arg;
+ // expected-warning@-1 {{Left shift by '32' overflows the capacity of 'int'}}
+ // expected-note@-2 {{The result of left shift is undefined because the right operand '32' is not smaller than 32, the capacity of 'int'}}
+}
+
+int too_large_right_operand_symbolic(int left, int right) {
+ // expected-note@+2 {{Assuming 'right' is > 31}}
+ // expected-note@+1 {{Taking false branch}}
+ if (right <= 31)
+ return 0;
+ return left >> right;
+ // expected-warning@-1 {{Right shift overflows the capacity of 'int'}}
+ // expected-note@-2 {{The result of right shift is undefined because the right operand is not smaller than 32, the capacity of 'int'}}
+ // NOTE: the messages of the checker are a bit vague in this case, but the
+ // tracking of the variables reveals our knowledge about them.
+}
+
+int too_large_right_operand_compound(unsigned short arg) {
+ // Note: this would be valid code with an 'unsigned int' because
+ // unsigned addition is allowed to overflow.
+ return 1 << (32 + arg);
+ // expected-warning@-1 {{Left shift overflows the capacity of 'int'}}
+ // expected-note@-2 {{The result of left shift is undefined because the right operand is not smaller than 32, the capacity of 'int'}}
+}
+
+// TEST STATE UPDATES
+//===----------------------------------------------------------------------===//
+
+void state_update(char a, int *p) {
+ // NOTE: with 'int a' this would not produce a bug report because the engine
+ // would not rule out an overflow.
+ *p += 1 << a;
+ // expected-note@-1 {{Assuming right operand of bit shift is non-negative but less than 32}}
+ *p += 1 << (a + 32);
+ // expected-warning@-1 {{Left shift overflows the capacity of 'int'}}
+ // expected-note@-2 {{The result of left shift is undefined because the right operand is not smaller than 32, the capacity of 'int'}}
+}
+
+void state_update_2(char a, int *p) {
+ *p += 1234 >> (a + 32);
+ // expected-note@-1 {{Assuming right operand of bit shift is non-negative but less than 32}}
+ *p += 1234 >> a;
+ // expected-warning@-1 {{Right operand is negative in right shift}}
+ // expected-note@-2 {{The result of right shift is undefined because the right operand is negative}}
+}
+
+// TEST EXPRESSION TRACKING
+//===----------------------------------------------------------------------===//
+// Expression tracking a "generic" tool that's used by many other checkers,
+// so this is just a minimal test to see that it's activated.
+
+void setValue(unsigned *p, unsigned newval) {
+ *p = newval;
+ // expected-note@-1 {{The value 33 is assigned to 'right'}}
+}
+
+int expression_tracked_back(void) {
+ unsigned left = 115; // expected-note {{'left' initialized to 115}}
+ unsigned right;
+ setValue(&right, 33);
+ // expected-note@-1 {{Calling 'setValue'}}
+ // expected-note@-2 {{Passing the value 33 via 2nd parameter 'newval'}}
+ // expected-note@-3 {{Returning from 'setValue'}}
+
+ return left << right;
+ // expected-warning@-1 {{Left shift by '33' overflows the capacity of 'unsigned int'}}
+ // expected-note@-2 {{The result of left shift is undefined because the right operand '33' is not smaller than 32, the capacity of 'unsigned int'}}
+}
+
+// TEST PERMISSIVENESS
+//===----------------------------------------------------------------------===//
+
+int allow_overflows_and_negative_operands(void) {
+ // These are all legal under C++ 20 and many compilers accept them under
+ // earlier standards as well.
+ int int_min = 1 << 31; // no-warning
+ int this_overflows = 1027 << 30; // no-warning
+ return (-2 << 5) + (-3 >> 4); // no-warning
+}
+
+int double_negative(void) {
+ return -2 >> -2;
+ // expected-warning@-1 {{Right operand is negative in right shift}}
+ // expected-note@-2 {{The result of right shift is undefined because the right operand is negative}}
+}
Index: clang/test/Analysis/bitwise-ops.c
===================================================================
--- clang/test/Analysis/bitwise-ops.c
+++ clang/test/Analysis/bitwise-ops.c
@@ -1,4 +1,10 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-disable-checker=core.BitwiseShift -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify %s
+
+// NOTE: This test file disables the checker core.BitwiseShift (which would
+// report all undefined behavior connected to bitwise shifts) to verify the
+// behavior of core.UndefinedBinaryOperatorResult (which resports cases when
+// the constant folding in BasicValueFactory produces an "undefined" result
+// from a shift or any other binary operator).
void clang_analyzer_eval(int);
#define CHECK(expr) if (!(expr)) return; clang_analyzer_eval(expr)
@@ -13,10 +19,6 @@
}
int testConstantShifts_PR18073(int which) {
- // FIXME: We should have a checker that actually specifically checks bitwise
- // shifts against the width of the LHS's /static/ type, rather than just
- // having BasicValueFactory return "undefined" when dealing with two constant
- // operands.
switch (which) {
case 1:
return 0ULL << 63; // no-warning
Index: clang/test/Analysis/bitwise-ops-nocrash.c
===================================================================
--- clang/test/Analysis/bitwise-ops-nocrash.c
+++ clang/test/Analysis/bitwise-ops-nocrash.c
@@ -1,4 +1,10 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=text -triple x86_64-linux-gnu -Wno-shift-count-overflow -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-disable-checker=core.BitwiseShift -analyzer-output=text -triple x86_64-linux-gnu -Wno-shift-count-overflow -verify %s
+
+// NOTE: This test file disables the checker core.BitwiseShift (which would
+// report all undefined behavior connected to bitwise shifts) to verify the
+// behavior of core.UndefinedBinaryOperatorResult (which resports cases when
+// the constant folding in BasicValueFactory produces an "undefined" result
+// from a shift or any other binary operator).
#define offsetof(type,memb) ((unsigned long)&((type*)0)->memb)
@@ -10,7 +16,7 @@
// no crash
int left_shift_overflow_no_crash(unsigned int i) {
unsigned shift = 323U; // expected-note{{'shift' initialized to 323}}
- switch (i) { // expected-note{{Control jumps to 'case 8:' at line 14}}
+ switch (i) { // expected-note{{Control jumps to 'case 8:' at line 20}}
case offsetof(S, guest_fpc):
return 3 << shift; // expected-warning{{The result of the left shift is undefined due to shifting by '323', which is greater or equal to the width of type 'int'}}
// expected-note@-1{{The result of the left shift is undefined due to shifting by '323', which is greater or equal to the width of type 'int'}}
Index: clang/test/Analysis/analyzer-enabled-checkers.c
===================================================================
--- clang/test/Analysis/analyzer-enabled-checkers.c
+++ clang/test/Analysis/analyzer-enabled-checkers.c
@@ -11,6 +11,7 @@
// CHECK-NEXT: apiModeling.TrustReturnsNonnull
// CHECK-NEXT: apiModeling.llvm.CastValue
// CHECK-NEXT: apiModeling.llvm.ReturnValue
+// CHECK-NEXT: core.BitwiseShift
// CHECK-NEXT: core.CallAndMessageModeling
// CHECK-NEXT: core.CallAndMessage
// CHECK-NEXT: core.DivideZero
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -33,6 +33,7 @@
// CHECK-NEXT: cfg-rich-constructors = true
// CHECK-NEXT: cfg-scopes = false
// CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: core.BitwiseShift:Pedantic = false
// CHECK-NEXT: core.CallAndMessage:ArgInitializedness = true
// CHECK-NEXT: core.CallAndMessage:ArgPointeeInitializedness = false
// CHECK-NEXT: core.CallAndMessage:CXXDeallocationArg = true
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -10,6 +10,7 @@
ArrayBoundChecker.cpp
ArrayBoundCheckerV2.cpp
BasicObjCFoundationChecks.cpp
+ BitwiseShiftChecker.cpp
BlockInCriticalSectionChecker.cpp
BoolAssignmentChecker.cpp
BuiltinFunctionChecker.cpp
Index: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp
@@ -0,0 +1,368 @@
+//== BitwiseShiftChecker.cpp ------------------------------------*- C++ -*--==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines BitwiseShiftChecker, which is a path-sensitive checker
+// that looks for undefined behavior when the operands of the bitwise shift
+// operators '<<' and '>>' are invalid (negative or too large).
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/CharUnits.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/Support/FormatVariadic.h"
+#include <memory>
+
+using namespace clang;
+using namespace ento;
+using llvm::formatv;
+
+namespace {
+enum class OperandSide { Left, Right };
+
+using BugReportPtr = std::unique_ptr<PathSensitiveBugReport>;
+
+struct NoteTagTemplate {
+ llvm::StringLiteral SignInfo;
+ llvm::StringLiteral UpperBoundIntro;
+};
+
+constexpr NoteTagTemplate NoteTagTemplates[] = {
+ {"", "right operand of bit shift is less than "},
+ {"left operand of bit shift is non-negative", " and right operand is less than "},
+ {"right operand of bit shift is non-negative", " but less than "},
+ {"both operands of bit shift are non-negative", " and right operand is less than "}
+};
+
+/// An implementation detail class which is introduced to split the checker
+/// logic into several methods while maintaining a consistently updated state
+/// and access to other contextual data.
+class BitwiseShiftValidator {
+ CheckerContext &Ctx;
+ ProgramStateRef FoldedState;
+ const BinaryOperator *const Op;
+ const BugType &BT;
+ const bool PedanticFlag;
+
+ // The following data members are only used for note tag creation:
+ enum { NonNegLeft = 1, NonNegRight = 2 };
+ unsigned NonNegOperands = 0;
+
+ std::optional<unsigned> UpperBoundBitCount = std::nullopt;
+
+public:
+ BitwiseShiftValidator(const BinaryOperator *O, CheckerContext &C,
+ const BugType &B, bool P)
+ : Ctx(C), FoldedState(C.getState()), Op(O), BT(B), PedanticFlag(P) {}
+ void run();
+
+private:
+ const Expr *operandExpr(OperandSide Side) const {
+ return Side == OperandSide::Left ? Op->getLHS() : Op->getRHS();
+ }
+
+ bool shouldPerformPedanticChecks() const {
+ // The pedantic flag has no effect under C++20 because the affected issues
+ // are no longer undefined under that version of the standard.
+ return PedanticFlag && !Ctx.getASTContext().getLangOpts().CPlusPlus20;
+ }
+
+ bool assumeRequirement(OperandSide Side, BinaryOperator::Opcode Cmp, unsigned Limit);
+
+ void recordAssumption(OperandSide Side, BinaryOperator::Opcode Cmp, unsigned Limit);
+ const NoteTag *createNoteTag() const;
+
+ BugReportPtr createBugReport(StringRef ShortMsg, StringRef Msg) const;
+
+ BugReportPtr checkOvershift();
+ BugReportPtr checkOperandNegative(OperandSide Side);
+ BugReportPtr checkLeftShiftOverflow();
+
+ bool isLeftShift() const { return Op->getOpcode() == BO_Shl; }
+ StringRef shiftDir() const { return isLeftShift() ? "left" : "right"; }
+ static StringRef pluralSuffix(unsigned n) { return n <= 1 ? "" : "s"; }
+ static StringRef verbSuffix(unsigned n) { return n <= 1 ? "s" : ""; }
+};
+
+void BitwiseShiftValidator::run() {
+ // Report a bug if the right operand is >= the bit width of the type of the
+ // left operand:
+ if (BugReportPtr BR = checkOvershift()) {
+ Ctx.emitReport(std::move(BR));
+ return;
+ }
+
+ // Report a bug if the right operand is negative:
+ if (BugReportPtr BR = checkOperandNegative(OperandSide::Right)) {
+ Ctx.emitReport(std::move(BR));
+ return;
+ }
+
+ if (shouldPerformPedanticChecks()) {
+ // Report a bug if the left operand is negative:
+ if (BugReportPtr BR = checkOperandNegative(OperandSide::Left)) {
+ Ctx.emitReport(std::move(BR));
+ return;
+ }
+
+ // Report a bug when left shift of a concrete signed value overflows:
+ if (BugReportPtr BR = checkLeftShiftOverflow()) {
+ Ctx.emitReport(std::move(BR));
+ return;
+ }
+ }
+
+ // No bugs detected, update the state and add a single note tag which
+ // summarizes the new assumptions.
+ Ctx.addTransition(FoldedState, createNoteTag());
+}
+
+/// This method checks a requirement that must be satisfied by the value on the
+/// given Side of a bitwise shift operator in well-defined code. If the
+/// requirement is incompatible with prior knowledge, this method reports
+/// failure by returning false.
+bool BitwiseShiftValidator::assumeRequirement(OperandSide Side,
+ BinaryOperator::Opcode Comparison,
+ unsigned Limit) {
+ SValBuilder &SVB = Ctx.getSValBuilder();
+
+ const SVal OperandVal = Ctx.getSVal(operandExpr(Side));
+ const auto LimitVal = SVB.makeIntVal(Limit, Ctx.getASTContext().IntTy);
+ // Note that the type of `LimitVal` must be a signed, because otherwise a
+ // negative `Val` could be converted to a large positive value.
+
+ auto ResultVal = SVB.evalBinOp(FoldedState, Comparison, OperandVal, LimitVal,
+ SVB.getConditionType());
+ if (auto DURes = ResultVal.getAs<DefinedOrUnknownSVal>()) {
+ auto [StTrue, StFalse] = FoldedState->assume(DURes.value());
+ if (!StTrue) {
+ // We detected undefined behavior (the caller will report it).
+ FoldedState = StFalse;
+ return false;
+ }
+ // The code may be valid, so let's assume that it's valid:
+ FoldedState = StTrue;
+ if (StFalse) {
+ // Record note tag data for the assumption that we made
+ recordAssumption(Side, Comparison, Limit);
+ }
+ }
+ return true;
+}
+
+BugReportPtr BitwiseShiftValidator::checkOvershift() {
+ const QualType LHSTy = Op->getLHS()->getType();
+ const unsigned LHSBitWidth = Ctx.getASTContext().getIntWidth(LHSTy);
+
+ if (assumeRequirement(OperandSide::Right, BO_LT, LHSBitWidth))
+ return nullptr;
+
+ const SVal Right = Ctx.getSVal(operandExpr(OperandSide::Right));
+
+ std::string RightOpStr = "";
+ if (auto ConcreteRight = Right.getAs<nonloc::ConcreteInt>())
+ RightOpStr = formatv(" '{0}'", ConcreteRight->getValue());
+
+ std::string ShortMsg = formatv(
+ "{0} shift{1}{2} overflows the capacity of '{3}'",
+ isLeftShift() ? "Left" : "Right", RightOpStr.empty() ? "" : " by",
+ RightOpStr, LHSTy.getAsString());
+ std::string Msg =
+ formatv("The result of {0} shift is undefined because the right "
+ "operand{1} is not smaller than {2}, the capacity of '{3}'",
+ shiftDir(), RightOpStr, LHSBitWidth, LHSTy.getAsString());
+ return createBugReport(ShortMsg, Msg);
+}
+
+// Before C++20, at 5.8 [expr.shift] (N4296, 2014-11-19) the standard says
+// 1. "... The behaviour is undefined if the right operand is negative..."
+// 2. "The value of E1 << E2 ...
+// if E1 has a signed type and non-negative value ...
+// otherwise, the behavior is undefined."
+// 3. "The value of E1 >> E2 ...
+// If E1 has a signed type and a negative value,
+// the resulting value is implementation-defined."
+// However, negative left arguments work in practice and the C++20 standard
+// eliminates conditions 2 and 3.
+BugReportPtr BitwiseShiftValidator::checkOperandNegative(OperandSide Side) {
+ // If the type is unsigned, it cannot be negative
+ if (!operandExpr(Side)->getType()->isSignedIntegerType())
+ return nullptr;
+
+ // Main check: determine whether the operand is constrained to be negative
+ if (assumeRequirement(Side, BO_GE, 0))
+ return nullptr;
+
+ std::string ShortMsg = formatv("{0} operand is negative in {1} shift",
+ Side == OperandSide::Left ? "Left" : "Right",
+ shiftDir())
+ .str();
+ std::string Msg = formatv("The result of {0} shift is undefined "
+ "because the {1} operand is negative",
+ shiftDir(),
+ Side == OperandSide::Left ? "left" : "right")
+ .str();
+
+ return createBugReport(ShortMsg, Msg);
+}
+
+BugReportPtr BitwiseShiftValidator::checkLeftShiftOverflow() {
+ // A right shift cannot be an overflowing left shift...
+ if (!isLeftShift())
+ return nullptr;
+
+ // In C++ it's well-defined to shift to the sign bit. In C however, it's UB.
+ // 5.8.2 [expr.shift] (N4296, 2014-11-19)
+ const bool ShouldPreserveSignBit = !Ctx.getLangOpts().CPlusPlus;
+
+ const Expr *LHS = operandExpr(OperandSide::Left);
+ const QualType LHSTy = LHS->getType();
+ const unsigned LeftBitWidth = Ctx.getASTContext().getIntWidth(LHSTy);
+ assert(LeftBitWidth > 0);
+
+ // Quote "For unsigned lhs, the value of LHS << RHS is the value of LHS *
+ // 2^RHS, reduced modulo maximum value of the return type plus 1."
+ if (LHSTy->isUnsignedIntegerType())
+ return nullptr;
+
+ // We only support concrete integers as left operand.
+ const auto Left = Ctx.getSVal(LHS).getAs<nonloc::ConcreteInt>();
+ if (!Left.has_value())
+ return nullptr;
+
+ // We should have already reported a bug if the left operand of the shift was
+ // negative, so it cannot be negative here.
+ assert(Left->getValue().isNonNegative());
+
+ const unsigned LeftAvailableBitWidth =
+ LeftBitWidth - static_cast<unsigned>(ShouldPreserveSignBit);
+ const unsigned UsedBitsInLeftOperand = Left->getValue().getActiveBits();
+ assert(LeftBitWidth >= UsedBitsInLeftOperand);
+ const unsigned MaximalAllowedShift =
+ LeftAvailableBitWidth - UsedBitsInLeftOperand;
+
+ if (assumeRequirement(OperandSide::Right, BO_LT, MaximalAllowedShift + 1))
+ return nullptr;
+
+ const std::string CapacityMsg =
+ formatv("because '{0}' can hold only {1} bits ({2} the sign bit)",
+ LHSTy.getAsString(), LeftAvailableBitWidth,
+ ShouldPreserveSignBit ? "excluding" : "including");
+
+ const SVal Right = Ctx.getSVal(Op->getRHS());
+
+ std::string ShortMsg, Msg;
+ if (const auto ConcreteRight = Right.getAs<nonloc::ConcreteInt>()) {
+ // Here ConcreteRight must contain a small non-negative integer, because
+ // otherwise one of the earlier checks should've reported a bug.
+ const unsigned RHS = ConcreteRight->getValue().getExtValue();
+ assert(RHS > MaximalAllowedShift);
+ const unsigned OverflownBits = RHS - MaximalAllowedShift;
+ ShortMsg = formatv(
+ "The shift '{0} << {1}' overflows the capacity of '{2}'",
+ Left->getValue(), ConcreteRight->getValue(), LHSTy.getAsString());
+ Msg = formatv(
+ "The shift '{0} << {1}' is undefined {2}, so {3} bit{4} overflow{5}",
+ Left->getValue(), ConcreteRight->getValue(), CapacityMsg, OverflownBits,
+ pluralSuffix(OverflownBits), verbSuffix(OverflownBits));
+ } else {
+ ShortMsg = formatv("Left shift of '{0}' overflows the capacity of '{1}'",
+ Left->getValue(), LHSTy.getAsString());
+ Msg = formatv(
+ "Left shift of '{0}' is undefined {1}, so some bits overflow",
+ Left->getValue(), CapacityMsg);
+ }
+
+ return createBugReport(ShortMsg, Msg);
+}
+
+void BitwiseShiftValidator::recordAssumption(OperandSide Side,
+ BinaryOperator::Opcode Comparison,
+ unsigned Limit) {
+ switch (Comparison) {
+ case BO_GE:
+ assert(Limit == 0);
+ NonNegOperands |= (Side == OperandSide::Left ? NonNegLeft : NonNegRight);
+ break;
+ case BO_LT:
+ assert(Side == OperandSide::Right);
+ if (!UpperBoundBitCount || Limit < UpperBoundBitCount.value())
+ UpperBoundBitCount = Limit;
+ break;
+ default:
+ llvm_unreachable("this checker does not use other comparison operators");
+ }
+}
+
+const NoteTag *BitwiseShiftValidator::createNoteTag() const {
+ if (!NonNegOperands && !UpperBoundBitCount)
+ return nullptr;
+
+ SmallString<128> Buf;
+ llvm::raw_svector_ostream Out(Buf);
+ Out << "Assuming ";
+ NoteTagTemplate Templ = NoteTagTemplates[NonNegOperands];
+ Out << Templ.SignInfo;
+ if (UpperBoundBitCount)
+ Out << Templ.UpperBoundIntro << UpperBoundBitCount.value();
+ const std::string Msg(Out.str());
+
+ return Ctx.getNoteTag(Msg, /*isPrunable=*/true);
+}
+
+std::unique_ptr<PathSensitiveBugReport>
+BitwiseShiftValidator::createBugReport(StringRef ShortMsg, StringRef Msg) const {
+ ProgramStateRef State = Ctx.getState();
+ if (ExplodedNode *ErrNode = Ctx.generateErrorNode(State)) {
+ auto BR =
+ std::make_unique<PathSensitiveBugReport>(BT, ShortMsg, Msg, ErrNode);
+ bugreporter::trackExpressionValue(ErrNode, Op->getLHS(), *BR);
+ bugreporter::trackExpressionValue(ErrNode, Op->getRHS(), *BR);
+ return BR;
+ }
+ return nullptr;
+}
+} // anonymous namespace
+
+class BitwiseShiftChecker : public Checker<check::PreStmt<BinaryOperator>> {
+ mutable std::unique_ptr<BugType> BTPtr;
+
+public:
+ void checkPreStmt(const BinaryOperator *B, CheckerContext &Ctx) const {
+ BinaryOperator::Opcode Op = B->getOpcode();
+
+ if (Op != BO_Shl && Op != BO_Shr)
+ return;
+
+ if (!BTPtr)
+ BTPtr = std::make_unique<BugType>(this, "Bitwise shift",
+ "Suspicious operation");
+
+ BitwiseShiftValidator(B, Ctx, *BTPtr, Pedantic).run();
+ }
+
+ bool Pedantic = false;
+};
+
+void ento::registerBitwiseShiftChecker(CheckerManager &Mgr) {
+ auto *Chk = Mgr.registerChecker<BitwiseShiftChecker>();
+ const AnalyzerOptions &Opts = Mgr.getAnalyzerOptions();
+ Chk->Pedantic = Opts.getCheckerBooleanOption(Chk, "Pedantic");
+}
+
+bool ento::shouldRegisterBitwiseShiftChecker(const CheckerManager &mgr) {
+ return true;
+}
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -126,6 +126,19 @@
let ParentPackage = Core in {
+def BitwiseShiftChecker : Checker<"BitwiseShift">,
+ HelpText<"Finds cases where bitwise shift operation causes undefined behaviour.">,
+ CheckerOptions<[
+ CmdLineOption<Boolean,
+ "Pedantic",
+ "If set to true, the checker reports undefined behavior even "
+ "if it is supported by most compilers. (This flag has no "
+ "effect in C++20 where these constructs are legal.)",
+ "false",
+ Released>,
+ ]>,
+ Documentation<HasDocumentation>;
+
def CallAndMessageModeling : Checker<"CallAndMessageModeling">,
HelpText<"Responsible for essential modeling and assumptions after a "
"function/method call. For instance, if we can't reason about the "
Index: clang/docs/analyzer/checkers.rst
===================================================================
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -29,6 +29,56 @@
null pointer dereference, usage of uninitialized values, etc.
*These checkers must be always switched on as other checker rely on them.*
+.. _core-BitwiseShift:
+
+core.BitwiseShift (C, C++)
+""""""""""""""""""""""""""
+
+Finds undefined behavior caused by the bitwise left- and right-shift operator
+operating on integer types.
+
+By default, this checker only reports situations when the right operand is
+either negative or larger than the bit width of the type of the left operand;
+these are logically unsound.
+
+Moreover, if the pedantic mode is activated by
+``-analyzer-config core.BitwiseShift:Pedantic=true``, then this checker also
+reports situations where the _left_ operand of a shift operator is negative or
+overflow occurs during the right shift of a signed value. (Most compilers
+handle these predictably, but the C standard and the C++ standards before C++20
+say that they're undefined behavior. In the C++20 standard these constructs are
+well-defined, so activating pedantic mode in C++20 has no effect.)
+
+**Examples**
+
+.. code-block:: cpp
+
+ static_assert(sizeof(int) == 4, "assuming 32-bit int")
+
+ void basic_examples(int a, int b) {
+ if (b < 0) {
+ b = a << b; // warn: right operand is negative in left shift
+ } else if (b >= 32) {
+ b = a >> b; // warn: right shift overflows the capacity of 'int'
+ }
+ }
+
+ int pedantic_examples(int a, int b) {
+ if (a < 0) {
+ return a >> b; // warn: left operand is negative in right shift
+ }
+ a = 1000u << 31; // OK, overflow of unsigned value is well-defined, a == 0
+ if (b > 10) {
+ a = b << 31; // this is undefined before C++20, but the checker doesn't
+ // warn because it doesn't know the exact value of b
+ }
+ return 1000 << 31; // warn: this overflows the capacity of 'int'
+ }
+
+**Solution**
+
+Ensure the shift operands are in proper range before shifting.
+
.. _core-CallAndMessage:
core.CallAndMessage (C, C++, ObjC)
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
