Author: dergachev Date: Wed Aug 17 11:02:45 2016 New Revision: 278941 URL: http://llvm.org/viewvc/llvm-project?rev=278941&view=rev Log: [analyzer] Add a checker for loss of sign or precision in integral casts.
This new checker tries to find execution paths on which implicit integral casts cause definite loss of information: a certainly-negative integer is converted to an unsigned integer, or an integer is definitely truncated to fit into a smaller type. Being implicit, such casts are likely to produce unexpected results. Patch by Daniel Marjamäki! Differential Revision: https://reviews.llvm.org/D13126 Added: cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp cfe/trunk/test/Analysis/conversion.c Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=278941&r1=278940&r2=278941&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Wed Aug 17 11:02:45 2016 @@ -133,6 +133,10 @@ def CastToStructChecker : Checker<"CastT HelpText<"Check for cast from non-struct pointer to struct pointer">, DescFile<"CastToStructChecker.cpp">; +def ConversionChecker : Checker<"Conversion">, + HelpText<"Loss of sign/precision in implicit conversions">, + DescFile<"ConversionChecker.cpp">; + def IdenticalExprChecker : Checker<"IdenticalExpr">, HelpText<"Warn about unintended use of identical expressions in operators">, DescFile<"IdenticalExprChecker.cpp">; Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=278941&r1=278940&r2=278941&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Wed Aug 17 11:02:45 2016 @@ -23,6 +23,7 @@ add_clang_library(clangStaticAnalyzerChe ChrootChecker.cpp ClangCheckers.cpp CloneChecker.cpp + ConversionChecker.cpp CXXSelfAssignmentChecker.cpp DeadStoresChecker.cpp DebugCheckers.cpp Added: cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp?rev=278941&view=auto ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp (added) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp Wed Aug 17 11:02:45 2016 @@ -0,0 +1,192 @@ +//=== ConversionChecker.cpp -------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Check that there is no loss of sign/precision in assignments, comparisons +// and multiplications. +// +// ConversionChecker uses path sensitive analysis to determine possible values +// of expressions. A warning is reported when: +// * a negative value is implicitly converted to an unsigned value in an +// assignment, comparison or multiplication. +// * assignment / initialization when source value is greater than the max +// value of target +// +// Many compilers and tools have similar checks that are based on semantic +// analysis. Those checks are sound but have poor precision. ConversionChecker +// is an alternative to those checks. +// +//===----------------------------------------------------------------------===// +#include "ClangSACheckers.h" +#include "clang/AST/ParentMap.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 ConversionChecker : public Checker<check::PreStmt<ImplicitCastExpr>> { +public: + void checkPreStmt(const ImplicitCastExpr *Cast, CheckerContext &C) const; + +private: + mutable std::unique_ptr<BuiltinBug> BT; + + // Is there loss of precision + bool isLossOfPrecision(const ImplicitCastExpr *Cast, CheckerContext &C) const; + + // Is there loss of sign + bool isLossOfSign(const ImplicitCastExpr *Cast, CheckerContext &C) const; + + void reportBug(ExplodedNode *N, CheckerContext &C, const char Msg[]) const; +}; +} + +void ConversionChecker::checkPreStmt(const ImplicitCastExpr *Cast, + CheckerContext &C) const { + // TODO: For now we only warn about DeclRefExpr, to avoid noise. Warn for + // calculations also. + if (!isa<DeclRefExpr>(Cast->IgnoreParenImpCasts())) + return; + + // Don't warn for loss of sign/precision in macros. + if (Cast->getExprLoc().isMacroID()) + return; + + // Get Parent. + const ParentMap &PM = C.getLocationContext()->getParentMap(); + const Stmt *Parent = PM.getParent(Cast); + if (!Parent) + return; + + bool LossOfSign = false; + bool LossOfPrecision = false; + + // Loss of sign/precision in binary operation. + if (const auto *B = dyn_cast<BinaryOperator>(Parent)) { + BinaryOperator::Opcode Opc = B->getOpcode(); + if (Opc == BO_Assign || Opc == BO_AddAssign || Opc == BO_SubAssign || + Opc == BO_MulAssign) { + LossOfSign = isLossOfSign(Cast, C); + LossOfPrecision = isLossOfPrecision(Cast, C); + } else if (B->isRelationalOp() || B->isMultiplicativeOp()) { + LossOfSign = isLossOfSign(Cast, C); + } + } else if (isa<DeclStmt>(Parent)) { + LossOfSign = isLossOfSign(Cast, C); + LossOfPrecision = isLossOfPrecision(Cast, C); + } + + if (LossOfSign || LossOfPrecision) { + // Generate an error node. + ExplodedNode *N = C.generateNonFatalErrorNode(C.getState()); + if (!N) + return; + if (LossOfSign) + reportBug(N, C, "Loss of sign in implicit conversion"); + if (LossOfPrecision) + reportBug(N, C, "Loss of precision in implicit conversion"); + } +} + +void ConversionChecker::reportBug(ExplodedNode *N, CheckerContext &C, + const char Msg[]) const { + if (!BT) + BT.reset( + new BuiltinBug(this, "Conversion", "Possible loss of sign/precision.")); + + // Generate a report for this bug. + auto R = llvm::make_unique<BugReport>(*BT, Msg, N); + C.emitReport(std::move(R)); +} + +// Is E value greater or equal than Val? +static bool isGreaterEqual(CheckerContext &C, const Expr *E, + unsigned long long Val) { + ProgramStateRef State = C.getState(); + SVal EVal = C.getSVal(E); + if (EVal.isUnknownOrUndef() || !EVal.getAs<NonLoc>()) + return false; + + SValBuilder &Bldr = C.getSValBuilder(); + DefinedSVal V = Bldr.makeIntVal(Val, C.getASTContext().LongLongTy); + + // Is DefinedEVal greater or equal with V? + SVal GE = Bldr.evalBinOp(State, BO_GE, EVal, V, Bldr.getConditionType()); + if (GE.isUnknownOrUndef()) + return false; + ConstraintManager &CM = C.getConstraintManager(); + ProgramStateRef StGE, StLT; + std::tie(StGE, StLT) = CM.assumeDual(State, GE.castAs<DefinedSVal>()); + return StGE && !StLT; +} + +// Is E value negative? +static bool isNegative(CheckerContext &C, const Expr *E) { + ProgramStateRef State = C.getState(); + SVal EVal = State->getSVal(E, C.getLocationContext()); + if (EVal.isUnknownOrUndef() || !EVal.getAs<NonLoc>()) + return false; + DefinedSVal DefinedEVal = EVal.castAs<DefinedSVal>(); + + SValBuilder &Bldr = C.getSValBuilder(); + DefinedSVal V = Bldr.makeIntVal(0, false); + + SVal LT = + Bldr.evalBinOp(State, BO_LT, DefinedEVal, V, Bldr.getConditionType()); + + // Is E value greater than MaxVal? + ConstraintManager &CM = C.getConstraintManager(); + ProgramStateRef StNegative, StPositive; + std::tie(StNegative, StPositive) = + CM.assumeDual(State, LT.castAs<DefinedSVal>()); + + return StNegative && !StPositive; +} + +bool ConversionChecker::isLossOfPrecision(const ImplicitCastExpr *Cast, + CheckerContext &C) const { + // Don't warn about explicit loss of precision. + if (Cast->isEvaluatable(C.getASTContext())) + return false; + + QualType CastType = Cast->getType(); + QualType SubType = Cast->IgnoreParenImpCasts()->getType(); + + if (!CastType->isIntegerType() || !SubType->isIntegerType()) + return false; + + if (C.getASTContext().getIntWidth(CastType) >= + C.getASTContext().getIntWidth(SubType)) + return false; + + unsigned W = C.getASTContext().getIntWidth(CastType); + if (W == 1 || W >= 64U) + return false; + + unsigned long long MaxVal = 1ULL << W; + return isGreaterEqual(C, Cast->getSubExpr(), MaxVal); +} + +bool ConversionChecker::isLossOfSign(const ImplicitCastExpr *Cast, + CheckerContext &C) const { + QualType CastType = Cast->getType(); + QualType SubType = Cast->IgnoreParenImpCasts()->getType(); + + if (!CastType->isUnsignedIntegerType() || !SubType->isSignedIntegerType()) + return false; + + return isNegative(C, Cast->getSubExpr()); +} + +void ento::registerConversionChecker(CheckerManager &mgr) { + mgr.registerChecker<ConversionChecker>(); +} Added: cfe/trunk/test/Analysis/conversion.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/conversion.c?rev=278941&view=auto ============================================================================== --- cfe/trunk/test/Analysis/conversion.c (added) +++ cfe/trunk/test/Analysis/conversion.c Wed Aug 17 11:02:45 2016 @@ -0,0 +1,125 @@ +// RUN: %clang_cc1 -Wno-conversion -analyze -analyzer-checker=core,alpha.core.Conversion -verify %s + +unsigned char U8; +signed char S8; + +void assign(unsigned U, signed S) { + if (S < -10) + U8 = S; // expected-warning {{Loss of sign in implicit conversion}} + if (U > 300) + S8 = U; // expected-warning {{Loss of precision in implicit conversion}} + if (S > 10) + U8 = S; + if (U < 200) + S8 = U; +} + +void init1() { + long long A = 1LL << 60; + short X = A; // expected-warning {{Loss of precision in implicit conversion}} +} + +void relational(unsigned U, signed S) { + if (S > 10) { + if (U < S) { + } + } + if (S < -10) { + if (U < S) { // expected-warning {{Loss of sign in implicit conversion}} + } + } +} + +void multiplication(unsigned U, signed S) { + if (S > 5) + S = U * S; + if (S < -10) + S = U * S; // expected-warning {{Loss of sign}} +} + +void division(unsigned U, signed S) { + if (S > 5) + S = U / S; + if (S < -10) + S = U / S; // expected-warning {{Loss of sign}} +} + +void dontwarn1(unsigned U, signed S) { + U8 = S; // It might be known that S is always 0x00-0xff. + S8 = U; // It might be known that U is always 0x00-0xff. + + U8 = -1; // Explicit conversion. + S8 = ~0U; // Explicit conversion. + if (U > 300) + U8 &= U; // No loss of precision since there is &=. +} + +void dontwarn2(unsigned int U) { + if (U <= 4294967295) { + } + if (U <= (2147483647 * 2U + 1U)) { + } +} + +void dontwarn3(int X) { + S8 = X ? 'a' : 'b'; +} + +// don't warn for macros +#define DOSTUFF ({ unsigned X = 1000; U8 = X; }) +void dontwarn4() { + DOSTUFF; +} + +// don't warn for calculations +// seen some fp. For instance: c2 = (c2 >= 'A' && c2 <= 'Z') ? c2 - 'A' + 'a' : c2; +// there is a todo in the checker to handle calculations +void dontwarn5() { + signed S = -32; + U8 = S + 10; +} + + +// false positives.. + +int isascii(int c); +void falsePositive1() { + char kb2[5]; + int X = 1000; + if (isascii(X)) { + // FIXME: should not warn here: + kb2[0] = X; // expected-warning {{Loss of precision}} + } +} + + +typedef struct FILE {} FILE; int getc(FILE *stream); +# define EOF (-1) +char reply_string[8192]; +FILE *cin; +extern int dostuff (void); +int falsePositive2() { + int c, n; + int dig; + char *cp = reply_string; + int pflag = 0; + int code; + + for (;;) { + dig = n = code = 0; + while ((c = getc(cin)) != '\n') { + if (dig < 4 && dostuff()) + code = code * 10 + (c - '0'); + if (!pflag && code == 227) + pflag = 1; + if (n == 0) + n = c; + if (c == EOF) + return(4); + if (cp < &reply_string[sizeof(reply_string) - 1]) + // FIXME: should not warn here: + *cp++ = c; // expected-warning {{Loss of precision}} + } + } +} + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits