donat.nagy created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411,
dkrupp, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware,
xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
donat.nagy requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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
that belongs to separate commits.
Co-authored-by: Endre Fulop <endre.fu...@sigmatechnology.se>
Repository:
rG LLVM Github Monorepo
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,52 @@
+// 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_eval() 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_eval(int);
+
+int state_update_generic(int left, int right) {
+ int x = left << right;
+ clang_analyzer_eval(left >= 0); // expected-warning {{TRUE}}
+ clang_analyzer_eval(left > 0); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(right >= 0); // expected-warning {{TRUE}}
+ clang_analyzer_eval(right > 0); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(right < 31); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(right < 32); // expected-warning {{TRUE}}
+ // No similar upper bound on 'left':
+ clang_analyzer_eval(left < 32); // expected-warning {{UNKNOWN}}
+ // In fact, there is no upper bound at all:
+ clang_analyzer_eval(left < 123456); // expected-warning {{UNKNOWN}}
+ return x;
+}
+
+int state_update_exact_shift(int arg) {
+ int x = 65535 << arg;
+ clang_analyzer_eval(arg >= 0); // expected-warning {{TRUE}}
+ clang_analyzer_eval(arg > 0); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(arg < 15); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(arg < 16); // c-warning {{TRUE}} cxx-warning {{UNKNOWN}}
+ clang_analyzer_eval(arg <= 16); // expected-warning {{TRUE}}
+ return x;
+}
+
Index: clang/test/Analysis/bitwise-shift-sanity-checks.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/bitwise-shift-sanity-checks.c
@@ -0,0 +1,66 @@
+// 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 %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++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: -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 that the BitwiseShift checker does not crash or
+// report false positives (at least on the cases that are listed here...)
+
+// expected-no-diagnostics
+
+// 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
+}
+
+// TEST UNUSUAL CODE THAT SHOULD NOT CRASH
+//===----------------------------------------------------------------------===//
+
+__int128 large_int(void) {
+ // Ensure that we do not crash on values that do not fit in 64 bits.
+ return (__int128) 1 << 63 << 10 << 10; // no-crash
+}
+
+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,176 @@
+// 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}}
+ // cxx-note@-3 {{The value '27' is represented by 5 bits, allowing at most 27 bits for bitshift}}
+ // c-note@-4 {{The shift '27 << 28' is undefined because 'int' can hold only 31 bits (excluding the sign bit), so 2 bits overflow}}
+ // c-note@-5 {{The value '27' is represented by 5 bits, allowing at most 26 bits for bitshift}}
+}
+
+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}}
+ // cxx-note@-3 {{The value '29' is represented by 5 bits, allowing at most 27 bits for bitshift}}
+ // c-note@-4 {{The shift '29 << 29' is undefined because 'int' can hold only 31 bits (excluding the sign bit), so 3 bits overflow}}
+ // c-note@-5 {{The value '29' is represented by 5 bits, allowing at most 26 bits for bitshift}}
+}
+
+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}}
+ // c-note@-3 {{The value '21' is represented by 5 bits, allowing at most 26 bits for bitshift}}
+}
+
+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}}
+ // c-note@-3 {{The value '1' is represented by 1 bit, allowing at most 30 bits for bitshift}}
+}
+
+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}}
+ // cxx-note@-3 {{The value '1024' is represented by 11 bits, allowing at most 21 bits for bitshift}}
+ // c-note@-4 {{Left shift of '1024' is undefined because 'int' can hold only 31 bits (excluding the sign bit), so some bits overflow}}
+ // c-note@-5 {{The value '1024' is represented by 11 bits, allowing at most 20 bits for bitshift}}
+}
+
+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}}
+ // c-note@-3 {{The value '1024' is represented by 11 bits, allowing at most 20 bits for bitshift}}
+}
+
+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_funny_code(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,8 @@
// CHECK-NEXT: cfg-rich-constructors = true
// CHECK-NEXT: cfg-scopes = false
// CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: consider-single-element-arrays-as-flexible-array-members = 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,390 @@
+//== ArrayBoundCheckerV2.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 caused by incorrect use of the bitwise
+// shift operators '<<' and '>>'.
+//
+//===----------------------------------------------------------------------===//
+
+#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;
+
+namespace {
+enum class OperandSide { Left, Right };
+
+struct NoteTagTemplate {
+ llvm::StringLiteral SignInfo;
+ llvm::StringLiteral UpperBoundIntro;
+};
+
+constexpr NoteTagTemplate NoteTagTemplates[4] = {
+ {"", "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 {
+ const BinaryOperator *const Op;
+ CheckerContext &Ctx;
+ ProgramStateRef State;
+ const BugType &BT;
+ const bool Pedantic;
+
+ // The following fields are only used for note tag creation:
+ enum { NonNegLeft = 1, NonNegRight = 2 };
+ unsigned NonNegFlags = 0;
+ // If there is an upper bound, it is <= the size of a primitive type
+ // (expressed in bits), so this is a very safe sentinel value:
+ enum { NoUpperBound = 999 };
+ unsigned UpperBound = NoUpperBound;
+
+public:
+ BitwiseShiftValidator(const BinaryOperator *O, CheckerContext &C,
+ const BugType &B, bool P)
+ : Op(O), Ctx(C), State(C.getState()), BT(B), Pedantic(P) {}
+ void run();
+
+private:
+ const Expr *operandExpr(OperandSide Side) {
+ return Side == OperandSide::Left ? Op->getLHS() : Op->getRHS();
+ }
+
+ bool assumeRequirement(OperandSide Side, BinaryOperator::Opcode Cmp, long long Limit);
+
+ bool isValidShift();
+ bool isOvershift();
+ bool isOperandNegative(OperandSide Side);
+ bool isLeftShiftOverflow();
+
+ void recordAssumption(OperandSide Side, BinaryOperator::Opcode Cmp, long long Limit);
+ const NoteTag *createNoteTag();
+
+ std::unique_ptr<PathSensitiveBugReport> createBugReport(StringRef ShortMsg,
+ StringRef Msg);
+
+ bool isLeftShift() { return Op->getOpcode() == BO_Shl; }
+ static StringRef pluralSuffix(unsigned n) { return n <= 1 ? "" : "s"; }
+ static StringRef verbSuffix(unsigned n) { return n <= 1 ? "s" : ""; }
+};
+
+void BitwiseShiftValidator::run() {
+ if (isValidShift()) {
+ Ctx.addTransition(State, createNoteTag());
+ }
+}
+
+bool BitwiseShiftValidator::isValidShift() {
+ // Report a bug if the right operand is >= the bit width of the type of the
+ // left operand:
+ if (isOvershift())
+ return false;
+
+ // Report a bug if the right operand is negative:
+ if (isOperandNegative(OperandSide::Right))
+ return false;
+
+ // If we reached this point, then the shift is well-defined under C++20 (or
+ // at least the static analysis cannot prove that it's not well-defined):
+ if (Ctx.getASTContext().getLangOpts().CPlusPlus20)
+ return true;
+
+ // Even under C or earlier versions of C++ the compilers tend to follow the
+ // C++20 behavior and some projects rely on this, so we only report these
+ // "undefined behavior, but works in practice" situations if the user
+ // explicitly activates the pedantic mode of this checker:
+ if (!Pedantic)
+ return true;
+
+ // Report a bug if the left operand is negative:
+ if (isOperandNegative(OperandSide::Left))
+ return false;
+
+ // Report a bug if we are left shifting a concrete signed value and it
+ // would overflow (e.g. 2 << 31 with a 32-bit int) or reach the sign bit
+ // under C (e.g. 1 << 31 with a 32-bit int; under C++ this is a
+ // well-defined way to get INT_MIN):
+ if (Op->getOpcode() == BO_Shl && isLeftShiftOverflow())
+ return false;
+
+ return true;
+}
+
+/// 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,
+ long long Limit) {
+ SValBuilder &SVB = Ctx.getSValBuilder();
+
+ const auto Val = Ctx.getSVal(operandExpr(Side));
+ const auto LimitVal = SVB.makeIntVal(Limit, Ctx.getASTContext().LongLongTy);
+
+ auto ResultVal =
+ SVB.evalBinOp(State, Comparison, Val, LimitVal, SVB.getConditionType());
+ if (auto DURes = ResultVal.getAs<DefinedOrUnknownSVal>()) {
+ auto [StTrue, StFalse] = State->assume(DURes.value());
+ if (!StTrue) {
+ // We detected undefined behavior (the caller will report it).
+ State = StFalse;
+ return false;
+ }
+ // The code may be valid, so let's assume that it's valid:
+ State = StTrue;
+ if (StFalse) {
+ // Record note tag data for the assumption that we made
+ recordAssumption(Side, Comparison, Limit);
+ }
+ }
+ return true;
+}
+
+bool BitwiseShiftValidator::isOvershift() {
+ const QualType LHSTy = Op->getLHS()->getType();
+ const unsigned LHSBitWidth = Ctx.getASTContext().getIntWidth(LHSTy);
+
+ if (assumeRequirement(OperandSide::Right, BO_LT, LHSBitWidth))
+ return false;
+
+ const SVal Right = Ctx.getSVal(operandExpr(OperandSide::Right));
+
+ std::string RightOpStr = "";
+ if (auto ConcreteRight = Right.getAs<nonloc::ConcreteInt>())
+ RightOpStr = llvm::formatv(" '{0}'", ConcreteRight->getValue());
+
+ std::string ShortMsg = llvm::formatv(
+ "{0} shift{1}{2} overflows the capacity of '{3}'",
+ isLeftShift() ? "Left" : "Right", RightOpStr.empty() ? "" : " by",
+ RightOpStr, LHSTy.getAsString());
+ std::string Msg =
+ llvm::formatv("The result of {0} shift is undefined because the right "
+ "operand{1} is not smaller than {2}, the capacity of '{3}'",
+ isLeftShift() ? "left" : "right", RightOpStr, LHSBitWidth,
+ LHSTy.getAsString());
+ if (auto R = createBugReport(ShortMsg, Msg)) {
+ Ctx.emitReport(std::move(R));
+ }
+ return true;
+}
+
+// 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.
+bool BitwiseShiftValidator::isOperandNegative(OperandSide Side) {
+ // If the type is unsigned, it cannot be negative
+ if (!operandExpr(Side)->getType()->isSignedIntegerType())
+ return false;
+
+ // Main check: determine whether the operand is constrained to be negative
+ if (assumeRequirement(Side, BO_GE, 0))
+ return false;
+
+ std::string ShortMsg = llvm::formatv("{0} operand is negative in {1} shift",
+ Side == OperandSide::Left ? "Left" : "Right",
+ isLeftShift() ? "left" : "right")
+ .str();
+ std::string Msg = llvm::formatv("The result of {0} shift is undefined "
+ "because the {1} operand is negative",
+ isLeftShift() ? "left" : "right",
+ Side == OperandSide::Left ? "left" : "right")
+ .str();
+
+ if (auto R = createBugReport(ShortMsg, Msg))
+ Ctx.emitReport(std::move(R));
+ return true;
+}
+
+bool BitwiseShiftValidator::isLeftShiftOverflow() {
+ // A right shift cannot be an overflowing left shift...
+ if (!isLeftShift())
+ return false;
+
+ // 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);
+
+ // 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 false;
+
+ // We only support concrete integers as left operand.
+ const auto Left = Ctx.getSVal(LHS).getAs<nonloc::ConcreteInt>();
+ if (!Left.has_value())
+ return false;
+
+ // If the Left operand is negative, we report that and don't reach this
+ // function:
+ 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;
+
+ const SVal Right = Ctx.getSVal(Op->getRHS());
+
+ if (assumeRequirement(OperandSide::Right, BO_LT, MaximalAllowedShift + 1))
+ return false;
+
+ const std::string CapacityMsg =
+ llvm::formatv("because '{0}' can hold only {1} bits ({2} the sign bit)",
+ LHSTy.getAsString(), LeftAvailableBitWidth,
+ ShouldPreserveSignBit ? "excluding" : "including");
+
+ std::string ShortMsg, Msg;
+ if (const auto ConcreteRight = Right.getAs<nonloc::ConcreteInt>()) {
+ const std::int64_t RHS = ConcreteRight->getValue().getExtValue();
+ assert(RHS > MaximalAllowedShift);
+ const std::int64_t OverflownBits = RHS - MaximalAllowedShift;
+ ShortMsg = llvm::formatv(
+ "The shift '{0} << {1}' overflows the capacity of '{2}'",
+ Left->getValue(), ConcreteRight->getValue(), LHSTy.getAsString());
+ Msg = llvm::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 =
+ llvm::formatv("Left shift of '{0}' overflows the capacity of '{1}'",
+ Left->getValue(), LHSTy.getAsString());
+ Msg = llvm::formatv(
+ "Left shift of '{0}' is undefined {1}, so some bits overflow",
+ Left->getValue(), CapacityMsg);
+ }
+
+ if (auto R = createBugReport(ShortMsg, Msg)) {
+ const std::string LeftNote =
+ llvm::formatv("The value '{0}' is represented by {1} bit{2}, "
+ "allowing at most {3} bit{4} for bitshift",
+ Left->getValue(), UsedBitsInLeftOperand,
+ pluralSuffix(UsedBitsInLeftOperand), MaximalAllowedShift,
+ pluralSuffix(MaximalAllowedShift));
+ R->addNote(LeftNote, PathDiagnosticLocation{LHS, Ctx.getSourceManager(),
+ Ctx.getLocationContext()});
+ Ctx.emitReport(std::move(R));
+ }
+ return true;
+}
+
+void BitwiseShiftValidator::recordAssumption(OperandSide Side,
+ BinaryOperator::Opcode Comparison,
+ long long Limit) {
+ switch (Comparison) {
+ case BO_GE:
+ assert(Limit == 0);
+ if (Side == OperandSide::Left) {
+ NonNegFlags |= NonNegLeft;
+ } else {
+ NonNegFlags |= NonNegRight;
+ }
+ break;
+ case BO_LT:
+ assert(Side == OperandSide::Right);
+ if (Limit < UpperBound)
+ UpperBound = Limit;
+ break;
+ default:
+ llvm_unreachable("this checker does not use other comparison operators");
+ }
+}
+
+const NoteTag *BitwiseShiftValidator::createNoteTag() {
+ if (!NonNegFlags && UpperBound == NoUpperBound)
+ return nullptr;
+
+ SmallString<128> Buf;
+ llvm::raw_svector_ostream Out(Buf);
+ Out << "Assuming ";
+ NoteTagTemplate Templ = NoteTagTemplates[NonNegFlags];
+ Out << Templ.SignInfo;
+ if (UpperBound != NoUpperBound)
+ Out << Templ.UpperBoundIntro << UpperBound;
+ const std::string Msg(Out.str());
+
+ return Ctx.getNoteTag(Msg, /*isPrunable=*/true);
+}
+
+std::unique_ptr<PathSensitiveBugReport>
+BitwiseShiftValidator::createBugReport(StringRef ShortMsg, StringRef Msg) {
+ ProgramStateRef State = Ctx.getState();
+ if (ExplodedNode *ErrNode = Ctx.generateErrorNode(State)) {
+ auto R =
+ std::make_unique<PathSensitiveBugReport>(BT, ShortMsg, Msg, ErrNode);
+ bugreporter::trackExpressionValue(ErrNode, Op->getLHS(), *R);
+ bugreporter::trackExpressionValue(ErrNode, Op->getRHS(), *R);
+ return R;
+ }
+ 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 if 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,57 @@
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")
+
+ int basic_examples(int a, int b) {
+ if (b < 0) {
+ return a >> b; // warn: right argument is negative
+ } else if (b >= 32) {
+ return a << b; // warn: right argument >= capacity
+ }
+ }
+
+ int pedantic_examples(int a, int b) {
+ if (a < 0) {
+ return a >> b; // warn: left argument is negative
+ }
+ 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; // warn: right shift of signed value causes overflow of
+ // signed value
+ }
+
+**Solution**
+
+Ensure the shift operands are in proper range before shifting.
+
.. _core-CallAndMessage:
core.CallAndMessage (C, C++, ObjC)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
