diff --git a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -56,9 +56,8 @@ 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())) + // Don't warn for implicit conversions to bool + if (Cast->getType()->isBooleanType()) return; // Don't warn for loss of sign/precision in macros. @@ -70,6 +69,9 @@ const Stmt *Parent = PM.getParent(Cast); if (!Parent) return; + // Dont warn if this is part of an explicit cast + if (isa(Parent)) + return; bool LossOfSign = false; bool LossOfPrecision = false; @@ -78,8 +80,10 @@ if (const auto *B = dyn_cast(Parent)) { BinaryOperator::Opcode Opc = B->getOpcode(); if (Opc == BO_Assign) { - LossOfSign = isLossOfSign(Cast, C); - LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C); + if (!Cast->IgnoreParenImpCasts()->isEvaluatable(C.getASTContext())) { + LossOfSign = isLossOfSign(Cast, C); + LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C); + } } else if (Opc == BO_AddAssign || Opc == BO_SubAssign) { // No loss of sign. LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C); @@ -98,7 +102,12 @@ } else if (B->isRelationalOp() || B->isMultiplicativeOp()) { LossOfSign = isLossOfSign(Cast, C); } - } else if (isa(Parent)) { + } else if (isa(Parent)) { + if (!Cast->IgnoreParenImpCasts()->isEvaluatable(C.getASTContext())) { + LossOfSign = isLossOfSign(Cast, C); + LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C); + } + } else { LossOfSign = isLossOfSign(Cast, C); LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C); } diff --git a/clang/test/Analysis/conversion.c b/clang/test/Analysis/conversion.c --- a/clang/test/Analysis/conversion.c +++ b/clang/test/Analysis/conversion.c @@ -105,6 +105,23 @@ S = U / S; // expected-warning {{Loss of sign}} } +void f(unsigned x) {} +void g(unsigned x) {} + +void functioncall1() { + long x = -1; + int y = 0; + f(x); // expected-warning {{Loss of sign in implicit conversion}} + f(y); +} + +void functioncall2(int x, int y) { + if (x < 0) + f(x); // expected-warning {{Loss of sign in implicit conversion}} + f(y); + f(x); // expected-warning {{Loss of sign in implicit conversion}} +} + 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. @@ -132,15 +149,38 @@ 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; + unsigned char c1 = 'A'; + c1 = (c1 >= 'A' && c1 <= 'Z') ? c1 - 'A' + 'a' : c1; + unsigned char c2 = 0; + c2 = (c2 >= 'A' && c2 <= 'Z') ? c2 - 'A' + 'a' : c2; + unsigned char c3 = 'Z'; + c3 = (c3 >= 'A' && c3 <= 'Z') ? c3 - 'A' + 'a' : c3; + unsigned char c4 = 'a'; + c4 = (c4 >= 'A' && c4 <= 'Z') ? c4 - 'A' + 'a' : c4; + unsigned char c5 = '@'; + c5 = (c5 >= 'A' && c5 <= 'Z') ? c5 - 'A' + 'a' : c5; +} + +void dontwarn6() { + int x = ~0; + unsigned y = ~0; +} + +void dontwarn7(unsigned x) { + if (x == (unsigned)-1) { + } +} + +void dontwarn8() { + unsigned x = (unsigned)-1; +} + +unsigned dontwarn9() { + return ~0; } -char dontwarn6(long long x) { +char dontwarn10(long long x) { long long y = 42; y += x; return y == 42; diff --git a/clang/test/Analysis/conversion.cpp b/clang/test/Analysis/conversion.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/conversion.cpp @@ -0,0 +1,22 @@ +// RUN: %clang_analyze_cc1 -Wno-conversion -Wno-tautological-constant-compare -analyzer-checker=core,alpha.core.Conversion -verify %s + +// expected-no-diagnostics + +void dontwarn1() { + unsigned long x = static_cast(-1); +} + +void dontwarn2(unsigned x) { + if (x == static_cast(-1)) { + } +} + +struct C { + C(unsigned x, unsigned long y) {} +}; + +void f(C) {} + +void functioncall1(long x) { + f(C(64, x)); +}