danielmarjamaki created this revision.
The error messages are confusing when shift result is undefined because the
shift count is negative or exceeds the bit width. I have seen that users often
draw the conclusion that Clang thinks some operand is uninitialized and
determine that Clang shows a false positive.
I also know that some users use negative shift count by intention and don't
think that would cause problems.
This patch clarify the error message and refactors the code a little.
Repository:
rL LLVM
https://reviews.llvm.org/D30295
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Core/CheckerHelpers.cpp
test/Analysis/bitwise-ops.c
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;
+}
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -94,3 +94,37 @@
return std::make_pair(VD, RHS);
}
+
+bool clang::ento::isExprResultConformsComparisonRule(CheckerContext &C,
+ BinaryOperatorKind BOK,
+ const Expr *LExpr,
+ const SVal RVal) {
+ ProgramStateRef State = C.getState();
+
+ SVal LVal = C.getSVal(LExpr);
+ if (LVal.isUnknownOrUndef() || !LVal.getAs<NonLoc>())
+ return false;
+
+ SValBuilder &Bldr = C.getSValBuilder();
+ SVal Eval = Bldr.evalBinOp(State, BOK, LVal, RVal, Bldr.getConditionType());
+ if (Eval.isUnknownOrUndef())
+ return false;
+
+ ConstraintManager &CM = C.getConstraintManager();
+ ProgramStateRef StTrue, StFalse;
+ std::tie(StTrue, StFalse) = CM.assumeDual(State, Eval.castAs<DefinedSVal>());
+ return StTrue && !StFalse;
+}
+
+// Is E value greater or equal than Val?
+bool clang::ento::isGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val) {
+ 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: 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"
@@ -35,6 +36,11 @@
};
} // end anonymous namespace
+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();
@@ -80,9 +86,24 @@
}
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<BugReport>(*BT, OS.str(), N);
if (Ex) {
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,50 +109,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() || !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,
CheckerContext &C) const {
// Don't warn about explicit loss of precision.
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -16,6 +16,7 @@
#include "clang/AST/Stmt.h"
#include <tuple>
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
namespace clang {
@@ -42,6 +43,12 @@
std::pair<const clang::VarDecl *, const clang::Expr *>
parseAssignment(const Stmt *S);
+bool isExprResultConformsComparisonRule(CheckerContext &C,
+ BinaryOperatorKind BOK,
+ const Expr *LExpr, const SVal RVal);
+bool isGreaterEqual(CheckerContext &C, const Expr *E, unsigned long long Val);
+bool isNegative(CheckerContext &C, const Expr *E);
+
} // end GR namespace
} // end clang namespace
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
