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 @@ -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/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> { + mutable std::unique_ptr 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(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(*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 LessThanZeroDVal = + LessThanZeroVal.getAs()) { + 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(S)) { + if (!B->isAssignmentOp()) { + return; + } + + if (DeclRefExpr *DR = dyn_cast(B->getLHS())) { + if (VarDecl *VD = dyn_cast(DR->getDecl())) + doCheck(VD, B->getRHS(), Val, C); + } + } else if (const DeclStmt *DS = dyn_cast(S)) { + // Iterate through the decls. + for (const auto *DI : DS->decls()) { + if (const auto *VD = dyn_cast(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(); +} 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! +} +