Diagnose the case when a negative value is assigned to an unsigned variable.
Unfortunately, there are no existing diagnostics for such assignments
that cause the loss of the sign, potentially resulting in nasty silent runtime
failures.
http://reviews.llvm.org/D10634
Files:
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp
test/Analysis/LossOfSign.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<"LossOfSign">,
+ 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,179 @@
+//== 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);
+
+ // 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());
+
+ // 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/LossOfSign.c
===================================================================
--- test/Analysis/LossOfSign.c
+++ test/Analysis/LossOfSign.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core.LossOfSign -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
