Missed updating my working copy prior to creating the patch. 
Updated, Rebuilt, and tested using check-all.


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(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 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
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to