diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp index a2b5e2987db59..5f37b915deb8a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -115,59 +115,9 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B, OS << " due to array index out of bounds"; } else { // Neither operand was undefined, but the result is undefined. - if ((B->getOpcode() == BinaryOperatorKind::BO_Shl || - B->getOpcode() == BinaryOperatorKind::BO_Shr) && - C.isNegative(B->getRHS())) { - OS << "The result of the " - << ((B->getOpcode() == BinaryOperatorKind::BO_Shl) ? "left" - : "right") - << " shift is undefined because the right operand is negative"; - Ex = B->getRHS(); - } else if ((B->getOpcode() == BinaryOperatorKind::BO_Shl || - B->getOpcode() == BinaryOperatorKind::BO_Shr) && - isShiftOverflow(B, C)) { - - OS << "The result of the " - << ((B->getOpcode() == BinaryOperatorKind::BO_Shl) ? "left" - : "right") - << " shift is undefined due to shifting by "; - Ex = B->getRHS(); - - SValBuilder &SB = C.getSValBuilder(); - const llvm::APSInt *I = - SB.getKnownValue(C.getState(), C.getSVal(B->getRHS())); - if (!I) - OS << "a value that is"; - else if (I->isUnsigned()) - OS << '\'' << I->getZExtValue() << "\', which is"; - else - OS << '\'' << I->getSExtValue() << "\', which is"; - - OS << " greater or equal to the width of type '" - << B->getLHS()->getType() << "'."; - } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl && - C.isNegative(B->getLHS())) { - OS << "The result of the left shift is undefined because the left " - "operand is negative"; - Ex = B->getLHS(); - } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl && - isLeftShiftResultUnrepresentable(B, C)) { - ProgramStateRef State = C.getState(); - SValBuilder &SB = C.getSValBuilder(); - const llvm::APSInt *LHS = - SB.getKnownValue(State, C.getSVal(B->getLHS())); - const llvm::APSInt *RHS = - SB.getKnownValue(State, C.getSVal(B->getRHS())); - OS << "The result of the left shift is undefined due to shifting \'" - << LHS->getSExtValue() << "\' by \'" << RHS->getZExtValue() - << "\', which is unrepresentable in the unsigned version of " - << "the return type \'" << B->getLHS()->getType() << "\'"; - Ex = B->getLHS(); - } else { - OS << "The result of the '" - << BinaryOperator::getOpcodeStr(B->getOpcode()) - << "' expression is undefined"; - } + OS << "The result of the '" + << BinaryOperator::getOpcodeStr(B->getOpcode()) + << "' expression is undefined"; } auto report = std::make_unique(*BT, OS.str(), N); if (Ex) { diff --git a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp index 5924f6a671c2a..e8d74b40c6fd8 100644 --- a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp +++ b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp @@ -280,14 +280,6 @@ BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op, if (Amt >= V1.getBitWidth()) return nullptr; - if (!Ctx.getLangOpts().CPlusPlus20) { - if (V1.isSigned() && V1.isNegative()) - return nullptr; - - if (V1.isSigned() && Amt > V1.countl_zero()) - return nullptr; - } - return &getValue( V1.operator<<( (unsigned) Amt )); } diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp index 58d360a2e2dbd..aa2c8bbd15d42 100644 --- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -529,8 +529,21 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state, const llvm::APSInt *Result = BasicVals.evalAPSInt(op, LHSValue, RHSValue); - if (!Result) + if (!Result) { + if (op == BO_Shl || op == BO_Shr) { + // FIXME: At this point the constant folding claims that the result + // of a bitwise shift is undefined. However, constant folding + // relies on the inaccurate type information that is stored in the + // bit size of APSInt objects, and if we reached this point, then + // the checker core.BitwiseShift already determined that the shift + // is valid (in a PreStmt callback, by querying the real type from + // the AST node). + // To avoid embarrassing false positives, let's just say that we + // don't know anything about the result of the shift. + return UnknownVal(); + } return UndefinedVal(); + } return nonloc::ConcreteInt(*Result); } diff --git a/clang/test/Analysis/bitwise-ops-nocrash.c b/clang/test/Analysis/bitwise-ops-nocrash.c deleted file mode 100644 index 4c4900bc96a18..0000000000000 --- a/clang/test/Analysis/bitwise-ops-nocrash.c +++ /dev/null @@ -1,28 +0,0 @@ -// 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) - -typedef struct { - unsigned long guest_counter; - unsigned int guest_fpc; -} S; - -// 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 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'}} - default: - break; - } - - return 0; -} diff --git a/clang/test/Analysis/bitwise-ops.c b/clang/test/Analysis/bitwise-ops.c deleted file mode 100644 index 7ff7490ba80cf..0000000000000 --- a/clang/test/Analysis/bitwise-ops.c +++ /dev/null @@ -1,60 +0,0 @@ -// 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) - -void testPersistentConstraints(int x, int y) { - // Basic check - CHECK(x); // expected-warning{{TRUE}} - CHECK(x & 1); // expected-warning{{TRUE}} - - CHECK(1 - x); // expected-warning{{TRUE}} - CHECK(x & y); // expected-warning{{TRUE}} -} - -int testConstantShifts_PR18073(int which) { - switch (which) { - case 1: - return 0ULL << 63; // no-warning - case 2: - return 0ULL << 64; // expected-warning{{The result of the left shift is undefined due to shifting by '64', which is greater or equal to the width of type 'unsigned long long'}} - case 3: - return 0ULL << 65; // expected-warning{{The result of the left shift is undefined due to shifting by '65', which is greater or equal to the width of type 'unsigned long long'}} - - default: - return 0; - } -} - -int testOverflowShift(int a) { - if (a == 323) { - return 1 << a; // 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'}} - } - return 0; -} - -int testNegativeShift(int a) { - if (a == -5) { - return 1 << a; // expected-warning{{The result of the left shift is undefined because the right operand is negative}} - } - return 0; -} - -int testNegativeLeftShift(int a) { - if (a == -3) { - return a << 1; // expected-warning{{The result of the left shift is undefined because the left operand is negative}} - } - return 0; -} - -int testUnrepresentableLeftShift(int a) { - if (a == 8) - return a << 30; // expected-warning{{The result of the left shift is undefined due to shifting '8' by '30', which is unrepresentable in the unsigned version of the return type 'int'}} - return 0; -} diff --git a/clang/test/Analysis/bitwise-shift-sanity-checks.c b/clang/test/Analysis/bitwise-shift-sanity-checks.c index 1e20d5df3567e..a31424f18818c 100644 --- a/clang/test/Analysis/bitwise-shift-sanity-checks.c +++ b/clang/test/Analysis/bitwise-shift-sanity-checks.c @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \ +// RUN: %clang_analyze_cc1 -analyzer-checker=core \ // RUN: -analyzer-config core.BitwiseShift:Pedantic=true \ // RUN: -verify=expected,pedantic \ // RUN: -triple x86_64-pc-linux-gnu -x c %s \ @@ -6,7 +6,7 @@ // RUN: -Wno-shift-count-overflow -Wno-shift-overflow \ // RUN: -Wno-shift-sign-overflow // -// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \ +// RUN: %clang_analyze_cc1 -analyzer-checker=core \ // RUN: -analyzer-config core.BitwiseShift:Pedantic=true \ // RUN: -verify=expected,pedantic \ // RUN: -triple x86_64-pc-linux-gnu -x c++ -std=c++14 %s \ @@ -14,7 +14,7 @@ // RUN: -Wno-shift-count-overflow -Wno-shift-overflow \ // RUN: -Wno-shift-sign-overflow // -// RUN: %clang_analyze_cc1 -analyzer-checker=core.BitwiseShift \ +// RUN: %clang_analyze_cc1 -analyzer-checker=core \ // 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 \ @@ -23,6 +23,8 @@ // This test file verifies that the BitwiseShift checker does not crash or // report false positives (at least on the cases that are listed here...) +// Other core checkers are also enabled to see interactions with e.g. +// core.UndefinedBinaryOperatorResult. // For the sake of brevity, 'note' output is not checked in this file. // TEST OBVIOUSLY CORRECT CODE @@ -116,3 +118,17 @@ void doubles_cast_to_integer(int *c) { *c = ((int)1.5) << 1; // no-crash *c = ((int)1.5) << (int)1.5; // no-crash } + +// TEST CODE THAT WAS TRIGGERING BUGS IN EARLIER REVISIONS +//===----------------------------------------------------------------------===// + +unsigned int strange_cast(unsigned short sh) { + // This testcase triggers a bug in the constant folding (it "forgets" the + // cast), which is silenced in SimpleSValBuilder::evalBinOpNN() with an ugly + // workaround, because otherwise it would lead to a false positive from + // core.UndefinedBinaryOperatorResult. + unsigned int i; + sh++; + for (i=0; i