Index: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -196,6 +196,13 @@ return getState()->getSVal(S, getLocationContext()); } + /// \brief Is value of expression greater or equal to value (unsigned + /// comparison)? + bool isGreaterOrEqual(const Expr *E, unsigned long long Val); + + /// \brief Is value of expression negative? + bool isNegative(const Expr *E); + /// \brief Generates a new transition in the program state graph /// (ExplodedGraph). Uses the default CheckerContext predecessor node. /// Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -123,57 +123,6 @@ C.emitReport(std::move(R)); } -// Is E value greater or equal than Val? -static bool isGreaterEqual(CheckerContext &C, const Expr *E, - unsigned long long Val) { - ProgramStateRef State = C.getState(); - SVal EVal = C.getSVal(E); - if (EVal.isUnknownOrUndef()) - return false; - if (!EVal.getAs() && EVal.getAs()) { - ProgramStateManager &Mgr = C.getStateManager(); - EVal = - Mgr.getStoreManager().getBinding(State->getStore(), EVal.castAs()); - } - if (EVal.isUnknownOrUndef() || !EVal.getAs()) - return false; - - SValBuilder &Bldr = C.getSValBuilder(); - DefinedSVal V = Bldr.makeIntVal(Val, C.getASTContext().LongLongTy); - - // Is DefinedEVal greater or equal with V? - SVal GE = Bldr.evalBinOp(State, BO_GE, EVal, V, Bldr.getConditionType()); - if (GE.isUnknownOrUndef()) - return false; - ConstraintManager &CM = C.getConstraintManager(); - ProgramStateRef StGE, StLT; - std::tie(StGE, StLT) = CM.assumeDual(State, GE.castAs()); - return StGE && !StLT; -} - -// Is E value negative? -static bool isNegative(CheckerContext &C, const Expr *E) { - ProgramStateRef State = C.getState(); - SVal EVal = State->getSVal(E, C.getLocationContext()); - if (EVal.isUnknownOrUndef() || !EVal.getAs()) - return false; - DefinedSVal DefinedEVal = EVal.castAs(); - - SValBuilder &Bldr = C.getSValBuilder(); - DefinedSVal V = Bldr.makeIntVal(0, false); - - SVal LT = - Bldr.evalBinOp(State, BO_LT, DefinedEVal, V, Bldr.getConditionType()); - - // Is E value greater than MaxVal? - ConstraintManager &CM = C.getConstraintManager(); - ProgramStateRef StNegative, StPositive; - std::tie(StNegative, StPositive) = - CM.assumeDual(State, LT.castAs()); - - return StNegative && !StPositive; -} - bool ConversionChecker::isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType, CheckerContext &C) const { @@ -195,18 +144,18 @@ return false; unsigned long long MaxVal = 1ULL << W; - return isGreaterEqual(C, Cast->getSubExpr(), MaxVal); + return C.isGreaterOrEqual(Cast->getSubExpr(), MaxVal); } bool ConversionChecker::isLossOfSign(const ImplicitCastExpr *Cast, - CheckerContext &C) const { + CheckerContext &C) const { QualType CastType = Cast->getType(); QualType SubType = Cast->IgnoreParenImpCasts()->getType(); if (!CastType->isUnsignedIntegerType() || !SubType->isSignedIntegerType()) return false; - return isNegative(C, Cast->getSubExpr()); + return C.isNegative(Cast->getSubExpr()); } void ento::registerConversionChecker(CheckerManager &mgr) { Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -59,6 +59,11 @@ return StOutBound && !StInBound; } +static bool isShiftOverflow(const BinaryOperator *B, CheckerContext &C) { + return C.isGreaterOrEqual( + B->getRHS(), C.getASTContext().getIntWidth(B->getLHS()->getType())); +} + void UndefResultChecker::checkPostStmt(const BinaryOperator *B, CheckerContext &C) const { ProgramStateRef state = C.getState(); @@ -97,18 +102,46 @@ } if (Ex) { - OS << "The " << (isLeft ? "left" : "right") - << " operand of '" + OS << "The " << (isLeft ? "left" : "right") << " operand of '" << BinaryOperator::getOpcodeStr(B->getOpcode()) << "' is a garbage value"; if (isArrayIndexOutOfBounds(C, Ex)) OS << " due to array index out of bounds"; - } - else { + } else { // Neither operand was undefined, but the result is undefined. - OS << "The result of the '" - << BinaryOperator::getOpcodeStr(B->getOpcode()) - << "' expression 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"; + } 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 "; + + 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 << " larger or equal to the width of type '" + << B->getLHS()->getType().getAsString() << "'."; + } else { + OS << "The result of the '" + << BinaryOperator::getOpcodeStr(B->getOpcode()) + << "' expression is undefined"; + } } auto report = llvm::make_unique(*BT, OS.str(), N); if (Ex) { Index: lib/StaticAnalyzer/Core/CheckerContext.cpp =================================================================== --- lib/StaticAnalyzer/Core/CheckerContext.cpp +++ lib/StaticAnalyzer/Core/CheckerContext.cpp @@ -99,3 +99,35 @@ return Lexer::getSpelling(Loc, buf, getSourceManager(), getLangOpts()); } +/// Evaluate comparison and return true if it's known that condition is true +static bool evalComparison(SVal LHSVal, BinaryOperatorKind ComparisonOp, + SVal RHSVal, ProgramStateRef State) { + if (LHSVal.isUnknownOrUndef()) + return false; + ProgramStateManager &Mgr = State->getStateManager(); + if (!LHSVal.getAs()) { + LHSVal = Mgr.getStoreManager().getBinding(State->getStore(), + LHSVal.castAs()); + if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs()) + return false; + } + + SValBuilder &Bldr = Mgr.getSValBuilder(); + SVal Eval = Bldr.evalBinOp(State, ComparisonOp, LHSVal, RHSVal, + Bldr.getConditionType()); + if (Eval.isUnknownOrUndef()) + return false; + ProgramStateRef StTrue, StFalse; + std::tie(StTrue, StFalse) = State->assume(Eval.castAs()); + return StTrue && !StFalse; +} + +bool CheckerContext::isGreaterOrEqual(const Expr *E, unsigned long long Val) { + DefinedSVal V = getSValBuilder().makeIntVal(Val, getASTContext().LongLongTy); + return evalComparison(getSVal(E), BO_GE, V, getState()); +} + +bool CheckerContext::isNegative(const Expr *E) { + DefinedSVal V = getSValBuilder().makeIntVal(0, false); + return evalComparison(getSVal(E), BO_LT, V, getState()); +} Index: test/Analysis/bitwise-ops.c =================================================================== --- test/Analysis/bitwise-ops.c +++ test/Analysis/bitwise-ops.c @@ -22,11 +22,25 @@ case 1: return 0ULL << 63; // no-warning case 2: - return 0ULL << 64; // expected-warning{{The result of the '<<' expression is undefined}} + return 0ULL << 64; // expected-warning{{The result of the left shift is undefined due to shifting by '64', which is larger or equal to the width of type 'unsigned long long'}} case 3: - return 0ULL << 65; // expected-warning{{The result of the '<<' expression is undefined}} + return 0ULL << 65; // expected-warning{{The result of the left shift is undefined due to shifting by '65', which is larger or equal to the width of type 'unsigned long long'}} default: return 0; } -} \ No newline at end of file +} + +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 larger 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; +}