Index: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H #include "clang/AST/Stmt.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include namespace clang { @@ -42,8 +43,14 @@ std::pair parseAssignment(const Stmt *S); -} // end GR namespace +bool isExprResultConformsComparisonRule(CheckerContext &C, + BinaryOperatorKind CompRule, + const Expr *LHSExpr, const SVal RHSVal); +bool isGreaterEqual(CheckerContext &C, const Expr *E, unsigned long long Val); +bool isNegative(CheckerContext &C, const Expr *E); -} // end clang namespace +} // namespace ento + +} // namespace clang #endif Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -28,6 +28,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" using namespace clang; using namespace ento; @@ -108,52 +109,8 @@ 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() || !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, - CheckerContext &C) const { + CheckerContext &C) const { // Don't warn about explicit loss of precision. if (Cast->isEvaluatable(C.getASTContext())) return false; Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -17,6 +17,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/raw_ostream.h" @@ -59,6 +60,11 @@ return StOutBound && !StInBound; } +static bool isShiftOverflow(CheckerContext &C, const BinaryOperator *B) { + return isGreaterEqual(C, B->getRHS(), + C.getASTContext().getIntWidth(B->getLHS()->getType())); +} + void UndefResultChecker::checkPostStmt(const BinaryOperator *B, CheckerContext &C) const { ProgramStateRef state = C.getState(); @@ -97,18 +103,31 @@ } 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) && + isNegative(C, B->getRHS())) { + OS << "The result of the '" + << BinaryOperator::getOpcodeStr(B->getOpcode()) + << "' expression is undefined due to negative value on the right " + "side of operand"; + } else if ((B->getOpcode() == BinaryOperatorKind::BO_Shl || + B->getOpcode() == BinaryOperatorKind::BO_Shr) && + isShiftOverflow(C, B)) { + OS << "The result of the '" + << BinaryOperator::getOpcodeStr(B->getOpcode()) + << "' expression is undefined due to shift count >= width of type"; + } 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/CheckerHelpers.cpp =================================================================== --- lib/StaticAnalyzer/Core/CheckerHelpers.cpp +++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp @@ -94,3 +94,40 @@ return std::make_pair(VD, RHS); } + +bool clang::ento::isExprResultConformsComparisonRule( + CheckerContext &C, BinaryOperatorKind CompRule, const Expr *LHSExpr, + const SVal RHSVal) { + ProgramStateRef State = C.getState(); + + SVal LHSVal = C.getSVal(LHSExpr); + if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs()) + return false; + + SValBuilder &Bldr = C.getSValBuilder(); + SVal Eval = + Bldr.evalBinOp(State, CompRule, LHSVal, RHSVal, Bldr.getConditionType()); + if (Eval.isUnknownOrUndef()) + return false; + ConstraintManager &CM = C.getConstraintManager(); + ProgramStateRef StTrue, StFalse; + std::tie(StTrue, StFalse) = CM.assumeDual(State, Eval.castAs()); + return StTrue && !StFalse; +} + +// Is E value greater or equal than Val? +bool clang::ento::isGreaterEqual(CheckerContext &C, const Expr *E, + unsigned long long Val) { + llvm::APSInt In; + E->EvaluateAsInt(In, C.getASTContext()); + + DefinedSVal V = + C.getSValBuilder().makeIntVal(Val, C.getASTContext().LongLongTy); + return isExprResultConformsComparisonRule(C, BO_GE, E, V); +} + +// Is E value negative? +bool clang::ento::isNegative(CheckerContext &C, const Expr *E) { + DefinedSVal V = C.getSValBuilder().makeIntVal(0, false); + return isExprResultConformsComparisonRule(C, BO_LT, E, V); +} Index: test/Analysis/bitwise-ops.c =================================================================== --- test/Analysis/bitwise-ops.c +++ test/Analysis/bitwise-ops.c @@ -29,4 +29,18 @@ 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 '<<' expression is undefined due to shift count >= width of type}} + } + return 0; +} + +int testNegativeShift(int a) { + if (a == -5) { + return 1 << a; // expected-warning{{The result of the '<<' expression is undefined due to negative value on the right side of operand}} + } + return 0; +}