From: Daniel Marjamaki <daniel.marjamaki@evidente.se>
Date: Wed, 5 Apr 2017 08:57:04 +0000 (+0000)
Subject: [analyzer] alpha.core.Conversion - Fix false positive for 'U32 += S16;' expression... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b49003d6f8e81c3622eb792f29375900f4978a9c;p=clang

[analyzer] alpha.core.Conversion - Fix false positive for 'U32 += S16;' expression, that is not unsafe

Summary:
The alpha.core.Conversion was too strict about compound assignments and could warn even though there is no problem.

Differential Revision: https://reviews.llvm.org/D25596



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@299523 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp b/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
index 2bb9e85873..ea894c8101 100644
--- a/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -41,7 +41,8 @@ private:
   mutable std::unique_ptr<BuiltinBug> BT;
 
   // Is there loss of precision
-  bool isLossOfPrecision(const ImplicitCastExpr *Cast, CheckerContext &C) const;
+  bool isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType,
+                         CheckerContext &C) const;
 
   // Is there loss of sign
   bool isLossOfSign(const ImplicitCastExpr *Cast, CheckerContext &C) const;
@@ -73,16 +74,30 @@ void ConversionChecker::checkPreStmt(const ImplicitCastExpr *Cast,
   // 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) {
+    if (Opc == BO_Assign) {
       LossOfSign = isLossOfSign(Cast, C);
-      LossOfPrecision = isLossOfPrecision(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);
+    } else if (Opc == BO_MulAssign) {
+      LossOfSign = isLossOfSign(Cast, C);
+      LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
+    } else if (Opc == BO_DivAssign || Opc == BO_RemAssign) {
+      LossOfSign = isLossOfSign(Cast, C);
+      // No loss of precision.
+    } else if (Opc == BO_AndAssign) {
+      LossOfSign = isLossOfSign(Cast, C);
+      // No loss of precision.
+    } else if (Opc == BO_OrAssign || Opc == BO_XorAssign) {
+      LossOfSign = isLossOfSign(Cast, C);
+      LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
     } else if (B->isRelationalOp() || B->isMultiplicativeOp()) {
       LossOfSign = isLossOfSign(Cast, C);
     }
   } else if (isa<DeclStmt>(Parent)) {
     LossOfSign = isLossOfSign(Cast, C);
-    LossOfPrecision = isLossOfPrecision(Cast, C);
+    LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
   }
 
   if (LossOfSign || LossOfPrecision) {
@@ -113,6 +128,13 @@ static bool isGreaterEqual(CheckerContext &C, const Expr *E,
                            unsigned long long Val) {
   ProgramStateRef State = C.getState();
   SVal EVal = C.getSVal(E);
+  if (EVal.isUnknownOrUndef())
+    return false;
+  if (!EVal.getAs<NonLoc>() && EVal.getAs<Loc>()) {
+    ProgramStateManager &Mgr = C.getStateManager();
+    EVal =
+        Mgr.getStoreManager().getBinding(State->getStore(), EVal.castAs<Loc>());
+  }
   if (EVal.isUnknownOrUndef() || !EVal.getAs<NonLoc>())
     return false;
 
@@ -153,22 +175,22 @@ static bool isNegative(CheckerContext &C, const Expr *E) {
 }
 
 bool ConversionChecker::isLossOfPrecision(const ImplicitCastExpr *Cast,
-                                        CheckerContext &C) const {
+                                          QualType DestType,
+                                          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())
+  if (!DestType->isIntegerType() || !SubType->isIntegerType())
     return false;
 
-  if (C.getASTContext().getIntWidth(CastType) >=
+  if (C.getASTContext().getIntWidth(DestType) >=
       C.getASTContext().getIntWidth(SubType))
     return false;
 
-  unsigned W = C.getASTContext().getIntWidth(CastType);
+  unsigned W = C.getASTContext().getIntWidth(DestType);
   if (W == 1 || W >= 64U)
     return false;
 
diff --git a/test/Analysis/conversion.c b/test/Analysis/conversion.c
index 5827794af2..a22a567654 100644
--- a/test/Analysis/conversion.c
+++ b/test/Analysis/conversion.c
@@ -9,9 +9,67 @@ void assign(unsigned U, signed S) {
   if (U > 300)
     S8 = U; // expected-warning {{Loss of precision in implicit conversion}}
   if (S > 10)
-    U8 = S;
+    U8 = S; // no-warning
   if (U < 200)
-    S8 = U;
+    S8 = U; // no-warning
+}
+
+void addAssign() {
+  unsigned long L = 1000;
+  int I = -100;
+  U8 += L; // expected-warning {{Loss of precision in implicit conversion}}
+  L += I; // no-warning
+}
+
+void subAssign() {
+  unsigned long L = 1000;
+  int I = -100;
+  U8 -= L; // expected-warning {{Loss of precision in implicit conversion}}
+  L -= I; // no-warning
+}
+
+void mulAssign() {
+  unsigned long L = 1000;
+  int I = -1;
+  U8 *= L; // expected-warning {{Loss of precision in implicit conversion}}
+  L *= I;  // expected-warning {{Loss of sign in implicit conversion}}
+  I = 10;
+  L *= I; // no-warning
+}
+
+void divAssign() {
+  unsigned long L = 1000;
+  int I = -1;
+  U8 /= L; // no-warning
+  L /= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void remAssign() {
+  unsigned long L = 1000;
+  int I = -1;
+  U8 %= L; // no-warning
+  L %= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void andAssign() {
+  unsigned long L = 1000;
+  int I = -1;
+  U8 &= L; // no-warning
+  L &= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void orAssign() {
+  unsigned long L = 1000;
+  int I = -1;
+  U8 |= L; // expected-warning {{Loss of precision in implicit conversion}}
+  L |= I;  // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void xorAssign() {
+  unsigned long L = 1000;
+  int I = -1;
+  U8 ^= L; // expected-warning {{Loss of precision in implicit conversion}}
+  L ^= I;  // expected-warning {{Loss of sign in implicit conversion}}
 }
 
 void init1() {
@@ -21,7 +79,7 @@ void init1() {
 
 void relational(unsigned U, signed S) {
   if (S > 10) {
-    if (U < S) {
+    if (U < S) { // no-warning
     }
   }
   if (S < -10) {
@@ -32,14 +90,14 @@ void relational(unsigned U, signed S) {
 
 void multiplication(unsigned U, signed S) {
   if (S > 5)
-    S = U * S;
+    S = U * S; // no-warning
   if (S < -10)
     S = U * S; // expected-warning {{Loss of sign}}
 }
 
 void division(unsigned U, signed S) {
   if (S > 5)
-    S = U / S;
+    S = U / S; // no-warning
   if (S < -10)
     S = U / S; // expected-warning {{Loss of sign}}
 }