I had missed a check for initializer on a variable in my previous revision,
resulting in a stack dump. Fixed.
Renamed the id and the test file as per Daniel's suggestion to reflect
assignment.
Validated with make check-all, which I had missed earlier.
http://reviews.llvm.org/D10634
Files:
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp
test/Analysis/LossOfSignAssign.c
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -39,6 +39,7 @@
IdenticalExprChecker.cpp
IvarInvalidationChecker.cpp
LLVMConventionsChecker.cpp
+ LossOfSignChecker.cpp
MacOSKeychainAPIChecker.cpp
MacOSXAPIChecker.cpp
MallocChecker.cpp
Index: lib/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- lib/StaticAnalyzer/Checkers/Checkers.td
+++ lib/StaticAnalyzer/Checkers/Checkers.td
@@ -88,6 +88,10 @@
let ParentPackage = CoreAlpha in {
+def LossOfSignChecker : Checker<"LossOfSignAssign">,
+ HelpText<"Warn about (possible) loss of sign in assignments and initializations">,
+ DescFile<"LossOfSignChecker.cpp">;
+
def BoolAssignmentChecker : Checker<"BoolAssignment">,
HelpText<"Warn about assigning non-{0,1} values to Boolean variables">,
DescFile<"BoolAssignmentChecker.cpp">;
Index: lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp
+++ lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp
@@ -0,0 +1,183 @@
+//== LossOfSignChecker.cpp - Loss of sign checker -----*- C/C++ -*--==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This defines LossOfSignChecker, a builtin check in ExprEngine that
+// performs checks for assignment of signed negative values to unsigned
+// variables.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+ class LossOfSignChecker : public Checker< check::Bind,
+ check::ASTDecl<VarDecl> > {
+ mutable std::unique_ptr<BuiltinBug> BT;
+
+ public:
+ void checkBind(SVal loc, SVal val, const Stmt *S, CheckerContext &C) const;
+ void checkASTDecl(const VarDecl *VD, AnalysisManager &mgr,
+ BugReporter &BR) const;
+ void doCheck(const VarDecl *VD, const Expr *RHS, SVal val, CheckerContext &C) const;
+ void emitReport(ProgramStateRef state, CheckerContext &C) const;
+ };
+} // end anonymous namespace
+
+// Strip off all intervening operations to get to the real RHS
+static const Expr* GetAbsoluteRHS(const Expr *Ex) {
+ while (Ex) {
+ Ex = Ex->IgnoreParenImpCasts();
+ if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(Ex)) {
+ if (BO->getOpcode() == BO_Assign || BO->getOpcode() == BO_Comma) {
+ Ex = BO->getRHS();
+ continue;
+ }
+ }
+ break;
+ }
+ return Ex;
+}
+
+// Report diagnostic about loss of sign
+void LossOfSignChecker::emitReport(ProgramStateRef state,
+ CheckerContext &C) const {
+ if (ExplodedNode *N = C.addTransition(state)) {
+ if (!BT)
+ BT.reset(new BuiltinBug(this, "assigning negative value to "
+ "unsigned variable loses sign "
+ "and may cause undesired runtime "
+ "behavior"));
+ C.emitReport(new BugReport(*BT, BT->getDescription(), N));
+ }
+}
+
+// Check if the said variable is unsigned and yet being assigned a negative
+// value
+void LossOfSignChecker::doCheck(const VarDecl *VD, const Expr *RHS, SVal val,
+ CheckerContext &C) const {
+ ProgramStateRef state = C.getState();
+ SValBuilder &svalBuilder = C.getSValBuilder();
+ ConstraintManager &CM = C.getConstraintManager();
+
+ // Remove extraneous wrappers from the RHS value
+ RHS = GetAbsoluteRHS(RHS);
+ if (!RHS)
+ return;
+
+ // Catch the LHS and RHS types involved
+ QualType varTy = VD->getType();
+ QualType rhsTy = RHS->getType();
+
+ // Only look at binding of signed value type to unsigned variable type
+ if (varTy->isUnsignedIntegerType() && rhsTy->isSignedIntegerType()) {
+ //
+ // check for negative value
+ //
+ DefinedOrUnknownSVal Zero = svalBuilder.makeZeroVal(rhsTy);
+
+ // Cast the RHS value to the correct (original) type that it appeared in
+ SVal castVal = svalBuilder.evalCast(val, rhsTy, varTy);
+
+ SVal LessThanZeroVal = svalBuilder.evalBinOp(state, BO_LT,
+ castVal, Zero, rhsTy);
+ if (Optional<DefinedSVal> LessThanZeroDVal =
+ LessThanZeroVal.getAs<DefinedSVal>()) {
+ ProgramStateRef StatePos, StateNeg;
+
+ std::tie(StateNeg, StatePos) = CM.assumeDual(state, *LessThanZeroDVal);
+ if (StateNeg && !StatePos) {
+ // Binding of a negative value to a unsigned location
+ emitReport(StateNeg, C);
+ return;
+ }
+
+ // If it reaches here, we cannot reason about it well
+ return;
+ }
+ }
+}
+
+void LossOfSignChecker::checkBind(SVal loc, SVal val, const Stmt *S,
+ CheckerContext &C) const {
+ // Skip statements in macros.
+ if (S->getLocStart().isMacroID())
+ return;
+
+ //
+ // "Walk" over the statement looking for possible loss of sign in assigning a
+ // known negative value to an unsigned location
+ //
+ if (const BinaryOperator* B = dyn_cast<BinaryOperator>(S)) {
+ if (!B->isAssignmentOp()) {
+ return;
+ }
+
+ if (DeclRefExpr *DR = dyn_cast<DeclRefExpr>(B->getLHS()))
+ if (VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
+ doCheck(VD, B->getRHS(), val, C);
+ }
+ }
+ else if (const DeclStmt *DS = dyn_cast<DeclStmt>(S)) {
+ // Iterate through the decls
+ for (const auto *DI : DS->decls()) {
+ if (const auto *VD = dyn_cast<VarDecl>(DI))
+ doCheck(VD, VD->getInit(), val, C);
+ }
+ }
+}
+
+void LossOfSignChecker::checkASTDecl(const VarDecl *VD, AnalysisManager &mgr,
+ BugReporter &BR) const {
+ // Only for globals
+ if (VD->isLocalVarDeclOrParm())
+ return;
+
+ // Remove extraneous wrappers from the initializer
+ const Expr *RHS = GetAbsoluteRHS(VD->getInit());
+ if (!RHS)
+ return;
+
+ // Catch the LHS and RHS types involved
+ QualType varTy = VD->getType();
+ QualType rhsTy = RHS->getType();
+
+ // Only look at binding of signed value type to unsigned variable type
+ if (varTy->isUnsignedIntegerType() && rhsTy->isSignedIntegerType()) {
+ //
+ // check for negative value
+ //
+ llvm::APSInt Result;
+ if (RHS->EvaluateAsInt(Result, BR.getContext())) {
+ if (Result.isNegative()) {
+ // Initialization of unsigned variable with signed negative value
+ SmallString<64> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << "assigning negative value to unsigned variable loses sign "
+ "and may cause undesired runtime behavior";
+
+ PathDiagnosticLocation L =
+ PathDiagnosticLocation::create(VD, BR.getSourceManager());
+ BR.EmitBasicReport(VD, this, "Loss of sign on assignment",
+ "Loss of Sign", os.str(), L);
+ }
+ }
+ }
+}
+
+void ento::registerLossOfSignChecker(CheckerManager &mgr) {
+ mgr.registerChecker<LossOfSignChecker>();
+}
+
Index: test/Analysis/LossOfSignAssign.c
===================================================================
--- test/Analysis/LossOfSignAssign.c
+++ test/Analysis/LossOfSignAssign.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core.LossOfSignAssign -verify %s
+
+unsigned char uc1 = -1; // expected-warning {{assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior}}
+signed char sc1 = -1;
+static unsigned int sui1 = -1; // expected-warning {{assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior}}
+
+int t1 () {
+ static unsigned int sui2 = -1; // expected-warning {{assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior}}
+
+ int x = -10;
+ int i = -1;
+ unsigned int j = -1; // expected-warning {{assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior}}
+
+ i = x;
+ j = i; // expected-warning {{assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior}}
+ j = (unsigned int)x; // explicit cast; don't warn
+
+ return i+j; // implicit conversion here too!
+}
+
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
