r315462 - [Analyzer] Clarify error messages for undefined result
Daniel Marjamaki via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 11 07:49:35 PDT 2017
Author: danielmarjamaki
Date: Wed Oct 11 07:49:35 2017
New Revision: 315462
URL: http://llvm.org/viewvc/llvm-project?rev=315462&view=rev
Log:
[Analyzer] Clarify error messages for undefined result
Differential Revision: https://reviews.llvm.org/D30295
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerContext.cpp
cfe/trunk/test/Analysis/bitwise-ops.c
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=315462&r1=315461&r2=315462&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h Wed Oct 11 07:49:35 2017
@@ -196,6 +196,13 @@ public:
return getState()->getSVal(S, getLocationContext());
}
+ /// \brief Returns true if the value of \p E is greater than or equal to \p
+ /// Val under unsigned comparison
+ bool isGreaterOrEqual(const Expr *E, unsigned long long Val);
+
+ /// Returns true if the value of \p E is negative.
+ bool isNegative(const Expr *E);
+
/// \brief Generates a new transition in the program state graph
/// (ExplodedGraph). Uses the default CheckerContext predecessor node.
///
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp?rev=315462&r1=315461&r2=315462&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp Wed Oct 11 07:49:35 2017
@@ -123,57 +123,6 @@ void ConversionChecker::reportBug(Explod
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<NonLoc>() && EVal.getAs<Loc>()) {
- ProgramStateManager &Mgr = C.getStateManager();
- EVal =
- Mgr.getStoreManager().getBinding(State->getStore(), EVal.castAs<Loc>());
- }
- if (EVal.isUnknownOrUndef() || !EVal.getAs<NonLoc>())
- 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<DefinedSVal>());
- 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<NonLoc>())
- return false;
- DefinedSVal DefinedEVal = EVal.castAs<DefinedSVal>();
-
- 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<DefinedSVal>());
-
- return StNegative && !StPositive;
-}
-
bool ConversionChecker::isLossOfPrecision(const ImplicitCastExpr *Cast,
QualType DestType,
CheckerContext &C) const {
@@ -195,18 +144,18 @@ bool ConversionChecker::isLossOfPrecisio
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) {
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp?rev=315462&r1=315461&r2=315462&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp Wed Oct 11 07:49:35 2017
@@ -59,6 +59,11 @@ static bool isArrayIndexOutOfBounds(Chec
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 @@ void UndefResultChecker::checkPostStmt(c
}
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 << " greater 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<BugReport>(*BT, OS.str(), N);
if (Ex) {
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CheckerContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CheckerContext.cpp?rev=315462&r1=315461&r2=315462&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CheckerContext.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerContext.cpp Wed Oct 11 07:49:35 2017
@@ -99,3 +99,35 @@ StringRef CheckerContext::getMacroNameOr
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<NonLoc>()) {
+ LHSVal = Mgr.getStoreManager().getBinding(State->getStore(),
+ LHSVal.castAs<Loc>());
+ if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs<NonLoc>())
+ 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<DefinedSVal>());
+ 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());
+}
Modified: cfe/trunk/test/Analysis/bitwise-ops.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bitwise-ops.c?rev=315462&r1=315461&r2=315462&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/bitwise-ops.c (original)
+++ cfe/trunk/test/Analysis/bitwise-ops.c Wed Oct 11 07:49:35 2017
@@ -22,11 +22,25 @@ int testConstantShifts_PR18073(int which
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 greater 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 greater 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 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;
+}
More information about the cfe-commits
mailing list