danielmarjamaki created this revision.
danielmarjamaki added a reviewer: zaks.anna.
danielmarjamaki added a subscriber: cfe-commits.
This is a new static analyzer checker that warns when there is loss of sign and
loss of precision.
It is similar in spirit to Wsign-compare/Wsign-conversion etc. But this checker
uses proper analysis so the output is much more meaningful.
It has been tested on debian packages.
loss of precision results:
https://drive.google.com/file/d/0BykPmWrCOxt2WFV3NVhJdE94QUE/view
2195 projects, 1026 warnings
loss of sign results:
https://drive.google.com/file/d/0BykPmWrCOxt2cUpSQVlmUVJmR0k/view
2195 projects, 3613 warnigs
It seems to me that this checker shows that Clang does not properly track
values. It seems to think that result of ! can be negative for instance.
http://reviews.llvm.org/D13126
Files:
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
test/Analysis/conversion.c
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';
+}
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<check::PreStmt<BinaryOperator>> {
+ mutable std::unique_ptr<BuiltinBug> 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<BugReport>(*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<DefinedSVal>();
+
+ 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<IntegerLiteral>(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<BinaryOperator>(E))
+ return isConstant(B->getLHS(), Ctx) && isConstant(B->getRHS(), Ctx);
+ if (const auto *C = dyn_cast<ConditionalOperator>(E))
+ return isConstant(C->getTrueExpr(), Ctx) && isConstant(C->getFalseExpr(), Ctx);
+ if (const auto *U = dyn_cast<UnaryOperator>(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<ConversionChecker>();
+}
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/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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
