From 75aa1c144f4fbd5497aa95b51074e0ba45fa05f3 Mon Sep 17 00:00:00 2001 From: Kristof Umann Date: Fri, 16 Nov 2018 01:00:55 +0000 Subject: [PATCH] [analyzer] ConversionChecker: handle floating point MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Extend the alpha.core.Conversion checker to handle implicit converions where a too large integer value is converted to a floating point type. Each floating point type has a range where it can exactly represent all integers; we emit a warning when the integer value is above this range. Although it is possible to exactly represent some integers which are outside of this range (those that are divisible by a large enough power of 2); we still report cast involving those, because their usage may lead to bugs. (For example, if 1<<24 is stored in a float variable x, then x==x+1 holds.) Patch by: Donát Nagy! Differential Revision: https://reviews.llvm.org/D52730 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@347006 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/ConversionChecker.cpp | 55 +++++++++++++++---- test/Analysis/conversion.c | 31 ++++++++++- 2 files changed, 75 insertions(+), 11 deletions(-) diff --git a/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp b/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp index 17ec2c2887..208f94451c 100644 --- a/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -14,8 +14,10 @@ // 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 +// * assignment / initialization when the source value is greater than the max +// value of the target integer type +// * assignment / initialization when the source integer is above the range +// where the target floating point type can represent all integers // // Many compilers and tools have similar checks that are based on semantic // analysis. Those checks are sound but have poor precision. ConversionChecker @@ -28,6 +30,9 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "llvm/ADT/APFloat.h" + +#include using namespace clang; using namespace ento; @@ -40,11 +45,9 @@ public: private: mutable std::unique_ptr BT; - // Is there loss of precision bool isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType, 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; @@ -132,19 +135,51 @@ bool ConversionChecker::isLossOfPrecision(const ImplicitCastExpr *Cast, QualType SubType = Cast->IgnoreParenImpCasts()->getType(); - if (!DestType->isIntegerType() || !SubType->isIntegerType()) + if (!DestType->isRealType() || !SubType->isIntegerType()) return false; - if (C.getASTContext().getIntWidth(DestType) >= - C.getASTContext().getIntWidth(SubType)) + const bool isFloat = DestType->isFloatingType(); + + const auto &AC = C.getASTContext(); + + // We will find the largest RepresentsUntilExp value such that the DestType + // can exactly represent all nonnegative integers below 2^RepresentsUntilExp. + unsigned RepresentsUntilExp; + + if (isFloat) { + const llvm::fltSemantics &Sema = AC.getFloatTypeSemantics(DestType); + RepresentsUntilExp = llvm::APFloat::semanticsPrecision(Sema); + } else { + RepresentsUntilExp = AC.getIntWidth(DestType); + if (RepresentsUntilExp == 1) { + // This is just casting a number to bool, probably not a bug. + return false; + } + if (DestType->isSignedIntegerType()) + RepresentsUntilExp--; + } + + if (RepresentsUntilExp >= sizeof(unsigned long long) * CHAR_BIT) { + // Avoid overflow in our later calculations. return false; + } + + unsigned CorrectedSrcWidth = AC.getIntWidth(SubType); + if (SubType->isSignedIntegerType()) + CorrectedSrcWidth--; - unsigned W = C.getASTContext().getIntWidth(DestType); - if (W == 1 || W >= 64U) + if (RepresentsUntilExp >= CorrectedSrcWidth) { + // Simple case: the destination can store all values of the source type. return false; + } - unsigned long long MaxVal = 1ULL << W; + unsigned long long MaxVal = 1ULL << RepresentsUntilExp; + if (isFloat) { + // If this is a floating point type, it can also represent MaxVal exactly. + MaxVal++; + } return C.isGreaterOrEqual(Cast->getSubExpr(), MaxVal); + // TODO: maybe also check negative values with too large magnitude. } bool ConversionChecker::isLossOfSign(const ImplicitCastExpr *Cast, diff --git a/test/Analysis/conversion.c b/test/Analysis/conversion.c index 03cc78e0c1..8b77e25358 100644 --- a/test/Analysis/conversion.c +++ b/test/Analysis/conversion.c @@ -137,6 +137,12 @@ void dontwarn5() { U8 = S + 10; } +char dontwarn6(long long x) { + long long y = 42; + y += x; + return y == 42; +} + // C library functions, handled via apiModeling.StdCLibraryFunctions @@ -154,7 +160,7 @@ typedef struct FILE {} FILE; int getc(FILE *stream); # define EOF (-1) char reply_string[8192]; FILE *cin; -extern int dostuff (void); +extern int dostuff(void); int libraryFunction2() { int c, n; int dig; @@ -179,3 +185,26 @@ int libraryFunction2() { } } +double floating_point(long long a, int b) { + if (a > 1LL << 55) { + double r = a; // expected-warning {{Loss of precision}} + return r; + } else if (b > 1 << 25) { + float f = b; // expected-warning {{Loss of precision}} + return f; + } + return 137; +} + +double floating_point2() { + int a = 1 << 24; + long long b = 1LL << 53; + float f = a; // no-warning + double d = b; // no-warning + return d - f; +} + +int floating_point_3(unsigned long long a) { + double b = a; // no-warning + return 42; +} -- 2.40.0