Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -27,6 +27,7 @@ CheckerDocumentation.cpp ChrootChecker.cpp ClangCheckers.cpp + ConversionChecker.cpp DeadStoresChecker.cpp DebugCheckers.cpp DereferenceChecker.cpp Index: lib/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- lib/StaticAnalyzer/Checkers/Checkers.td +++ lib/StaticAnalyzer/Checkers/Checkers.td @@ -129,6 +129,10 @@ HelpText<"Check for cast from non-struct pointer to struct pointer">, DescFile<"CastToStructChecker.cpp">; +def ConversionChecker : Checker<"Conversion">, + HelpText<"Loss of sign/precision in conversions">, + DescFile<"ConversionChecker.cpp">; + def IdenticalExprChecker : Checker<"IdenticalExpr">, HelpText<"Warn about unintended use of identical expressions in operators">, DescFile<"IdenticalExprChecker.cpp">; Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -0,0 +1,183 @@ +//=== 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 not 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 has 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> { +public: + void checkPreStmt(const ImplicitCastExpr *Cast, CheckerContext &C) const; + +private: + mutable std::unique_ptr BT; + + void diagnoseLossOfPrecision(const ImplicitCastExpr *Cast, + CheckerContext &C) const; + void diagnoseLossOfSign(const ImplicitCastExpr *Cast, + CheckerContext &C) const; + + void reportBug(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(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; + + // Loss of sign/precision in binary operation.. + if (const auto *B = dyn_cast(Parent)) { + BinaryOperator::Opcode Opc = B->getOpcode(); + if (Opc == BO_Assign || Opc == BO_AddAssign || Opc == BO_SubAssign || + Opc == BO_MulAssign) { + diagnoseLossOfSign(Cast, C); + diagnoseLossOfPrecision(Cast, C); + } else if (B->isRelationalOp() || B->isMultiplicativeOp()) { + diagnoseLossOfSign(Cast, C); + } + } else if (isa(Parent)) { + diagnoseLossOfSign(Cast, C); + diagnoseLossOfPrecision(Cast, C); + } +} + +void ConversionChecker::reportBug(CheckerContext &C, const char Msg[]) const { + // Generate an error node. + ExplodedNode *N = C.generateErrorNode(C.getState()); + if (!N) + return; + + if (!BT) + BT.reset( + new BuiltinBug(this, "Conversion", "Possible loss of sign/precision.")); + + // Generate a report for this bug. + auto R = llvm::make_unique(*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()) + 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()); + 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()) + return false; + DefinedSVal DefinedEVal = EVal.castAs(); + + 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()); + + return StNegative && !StPositive; +} + +void ConversionChecker::diagnoseLossOfPrecision(const ImplicitCastExpr *Cast, + CheckerContext &C) const { + // Don't warn about explicit loss of precision. + if (Cast->isEvaluatable(C.getASTContext())) + return; + + QualType CastType = Cast->getType(); + QualType SubType = Cast->IgnoreParenImpCasts()->getType(); + + if (!CastType->isIntegerType() || !SubType->isIntegerType()) + return; + + if (C.getASTContext().getIntWidth(CastType) >= + C.getASTContext().getIntWidth(SubType)) + return; + + unsigned W = C.getASTContext().getIntWidth(CastType); + if (W == 1 || W >= 64U) + return; + + unsigned long long MaxVal = 1ULL << W; + if (isGreaterEqual(C, Cast->getSubExpr(), MaxVal)) + reportBug(C, "Loss of precision in implicit conversion."); +} + +void ConversionChecker::diagnoseLossOfSign(const ImplicitCastExpr *Cast, + CheckerContext &C) const { + QualType CastType = Cast->getType(); + QualType SubType = Cast->IgnoreParenImpCasts()->getType(); + + if (!CastType->isUnsignedIntegerType() || !SubType->isSignedIntegerType()) + return; + + if (isNegative(C, Cast->getSubExpr())) + reportBug(C, "Loss of sign in implicit conversion."); +} + +void ento::registerConversionChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} Index: test/Analysis/conversion.c =================================================================== --- test/Analysis/conversion.c +++ test/Analysis/conversion.c @@ -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}} + } + } +} +