Incorporated further review comments by Daniel.
Working copy updated, built, and tested using check-all.
Investigating individual diagnostics pointed out by Daniel.
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
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!
+}
+
Index: lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp
+++ lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp
@@ -0,0 +1,177 @@
+//== 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, which 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 St, 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 St,
+ CheckerContext &C) const {
+ if (ExplodedNode *N = C.addTransition(St)) {
+ if (!BT)
+ BT.reset(new BuiltinBug(this, "assigning negative value to "
+ "unsigned variable loses sign "
+ "and may cause undesired runtime "
+ "behavior"));
+ C.emitReport(llvm::make_unique<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 St = 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())
+ return;
+
+ //
+ // 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(St, BO_LT, CastVal, Zero, RHSTy);
+ if (Optional<DefinedSVal> LessThanZeroDVal =
+ LessThanZeroVal.getAs<DefinedSVal>()) {
+ ProgramStateRef StatePos, StateNeg;
+
+ std::tie(StateNeg, StatePos) = CM.assumeDual(St, *LessThanZeroDVal);
+ if (StateNeg && !StatePos) {
+ // Binding of a negative value to a unsigned location.
+ emitReport(StateNeg, C);
+ }
+ }
+}
+
+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())
+ return;
+
+ //
+ // 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: lib/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- lib/StaticAnalyzer/Checkers/Checkers.td
+++ lib/StaticAnalyzer/Checkers/Checkers.td
@@ -108,6 +108,10 @@
HelpText<"Check for assignment of a fixed address to a pointer">,
DescFile<"FixedAddressChecker.cpp">;
+def LossOfSignChecker : Checker<"LossOfSignAssign">,
+ HelpText<"Warn about (possible) loss of sign in assignments and initializations">,
+ DescFile<"LossOfSignChecker.cpp">;
+
def PointerArithChecker : Checker<"PointerArithm">,
HelpText<"Check for pointer arithmetic on locations other than array elements">,
DescFile<"PointerArithChecker">;
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
