Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -26,6 +26,7 @@ CheckSizeofPointer.cpp CheckerDocumentation.cpp ChrootChecker.cpp + ConversionChecker.cpp ClangCheckers.cpp DeadStoresChecker.cpp DebugCheckers.cpp Index: lib/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- lib/StaticAnalyzer/Checkers/Checkers.td +++ lib/StaticAnalyzer/Checkers/Checkers.td @@ -101,6 +101,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,169 @@ +//=== ConversionChecker.cpp -------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This defines ConversionChecker that warns about dangerous conversions where +// there is possible loss of sign or loss of precision. +// +//===----------------------------------------------------------------------===// +#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 ConversionChecker : public Checker> { + mutable std::unique_ptr BT; + +public: + void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const { + BinaryOperator::Opcode Opc = B->getOpcode(); + if (Opc == BO_Assign || Opc == BO_MulAssign || Opc == BO_DivAssign) { + diagnoseLossOfSign(B, C); + diagnoseLossOfPrecision(B, C); + } else if (B->isRelationalOp() || B->isMultiplicativeOp()) { + diagnoseLossOfSign(B, C); + } + } + +private: + void diagnoseLossOfSign(const BinaryOperator *B, CheckerContext &C) const; + void diagnoseLossOfPrecision(const BinaryOperator *B, + CheckerContext &C) const; + + void 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", "Loss of sign/precision.")); + + // Generate a report for this bug. + auto R = llvm::make_unique(*BT, Msg, N); + C.emitReport(std::move(R)); + } +}; +} + +static bool isUnsigned(const Expr *E) { + const Type *T = E ? E->getType().getTypePtr() : nullptr; + return T && T->isUnsignedIntegerType(); +} + +static bool isSigned(const Expr *E) { + const Type *T = E ? E->getType().getTypePtr() : nullptr; + return T && T->isSignedIntegerType(); +} + +// Get max value E can store. +static unsigned long long getMaxVal(CheckerContext &C, const Expr *E) { + unsigned Width = C.getASTContext().getIntWidth(E->getType()); + return (1ULL << (Width - 1U)) - 1ULL; +} + +// Can E value be greater than Val? +static bool canBeGreater(CheckerContext &C, const Expr *E, + unsigned long long Val) { + ProgramStateRef State = C.getState(); + SVal EVal = State->getSVal(E, C.getLocationContext()); + if (EVal.isUnknownOrUndef()) + return false; + DefinedSVal DefinedEVal = EVal.castAs(); + + SValBuilder &Bldr = C.getSValBuilder(); + DefinedSVal V = Bldr.makeIntVal(Val, true); + + // Is RHS value greater than MaxVal? + ProgramStateRef StInBound = State->assumeInBound(DefinedEVal, V, true); + ProgramStateRef StOutBound = State->assumeInBound(DefinedEVal, V, false); + return (!StInBound && StOutBound); +} + +// Can E have negative value? +static bool canBeNegative(CheckerContext &C, const Expr *E) { + if (const auto *I = dyn_cast(E->IgnoreParenCasts())) + return I->getValue().isNegative(); + return canBeGreater(C, E, getMaxVal(C, E)); +} + +bool isConstant(const Expr *E, const ASTContext &Ctx) { + E = E->IgnoreParenCasts(); + if (const auto *B = dyn_cast(E)) + return isConstant(B->getLHS(), Ctx) && isConstant(B->getRHS(), Ctx); + if (const auto *C = dyn_cast(E)) + return isConstant(C->getTrueExpr(), Ctx) && isConstant(C->getFalseExpr(), Ctx); + if (const auto *U = dyn_cast(E)) + return isConstant(U->getSubExpr(), Ctx); + return E->isEvaluatable(Ctx); +} + +void ConversionChecker::diagnoseLossOfSign(const BinaryOperator *B, + CheckerContext &C) const { + // Don't warn about explicit conversions in assignments. + if (B->isAssignmentOp() && isConstant(B->getRHS(), C.getASTContext())) + return; + + // Get signed operand. + const Expr *SignedExpr; + if (isUnsigned(B->getLHS()->IgnoreParenImpCasts())) + SignedExpr = B->getRHS(); + else if (isUnsigned(B->getRHS()->IgnoreParenImpCasts())) + SignedExpr = B->getLHS(); + else + return; + if (!isSigned(SignedExpr->IgnoreParenImpCasts())) + return; + + // For assignments there is only loss of sign if RHS is signed. + if (B->isAssignmentOp() && B->getRHS() != SignedExpr) + return; + + // Loss of sign if signed value can be negative. + if (canBeNegative(C, SignedExpr)) + reportBug(C, "Implicit conversion changes signedness (negative value)"); +} + +void ConversionChecker::diagnoseLossOfPrecision(const BinaryOperator *B, + CheckerContext &C) const { + // Don't warn about explicit conversions in assignments. + if (isConstant(B->getRHS(), C.getASTContext())) + return; + + QualType LT = B->getLHS()->IgnoreParenCasts()->getType(); + QualType RT = B->getRHS()->IgnoreParenCasts()->getType(); + + if (!LT.getTypePtr()->isIntegerType() || !RT.getTypePtr()->isIntegerType()) + return; + + if (C.getASTContext().getIntWidth(LT) >= C.getASTContext().getIntWidth(RT)) + return; + + unsigned LW = C.getASTContext().getIntWidth(LT); + if (LW >= 64U) + return; + + unsigned long long MaxVal = (1ULL << LW) - 1ULL; + + // TODO: Handle loss of precision for negative values. + if (isSigned(B->getRHS()->IgnoreParenImpCasts()) && + canBeNegative(C, B->getRHS())) + return; + + if (canBeGreater(C, B->getRHS(), MaxVal)) + reportBug(C, "Loss of precision, value too high"); +} + +void ento::registerConversionChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} Index: test/Analysis/conversion.c =================================================================== --- test/Analysis/conversion.c +++ test/Analysis/conversion.c @@ -0,0 +1,65 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core.Conversion -Wno-constant-conversion -verify %s + +unsigned char U8; +signed char S8; + +void assign(unsigned U, signed S) { + if (S < -10) + U8 = S; // expected-warning {{Implicit conversion changes signedness (negative value)}} + if (U > 300) + S8 = U; // expected-warning {{Loss of precision, value too high}} + if (S > 10) + U8 = S; + if (U < 200) + S8 = U; +} + +void relational(unsigned U, signed S) { + if (S > 10) { + if (U < S) {} + } + if (S < -10) { + if (U < S) {} // expected-warning {{Implicit conversion changes signedness (negative value)}} + } +} + +void multiplication(unsigned U, signed S) { + if (S > 5) + S = U * S; + if (S < -10) + S = U * S; // expected-warning {{Implicit conversion changes signedness (negative value)}} +} + +void division(unsigned U, signed S) { + if (S > 5) + S = U / S; + if (S < -10) + S = U / S; // expected-warning {{Implicit conversion changes signedness (negative value)}} +} + +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(int i) { + static unsigned char Buf[200] = {200,100,200}; + int S; + for (int i = 0; i < 200; i++) { + S = Buf[i]; // RHS is smaller than LHS + } +} + +void dontwarn3(unsigned int U) { + if (U <= 4294967295) {} + if (U <= (2147483647 * 2U + 1U)) {} +} + +void dontwarn4(int X) { + S8 = X ? 'a' : 'b'; +}