r278941 - [analyzer] Add a checker for loss of sign or precision in integral casts.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 17 09:02:45 PDT 2016
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}}
+ }
+ }
+}
+
More information about the cfe-commits
mailing list