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

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki updated this revision to Diff 92810.
danielmarjamaki added a comment.

Updated the patch so all the loss of precision are detected also


Repository:
  rL LLVM

https://reviews.llvm.org/D25596

Files:
  lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  test/Analysis/conversion.c


Index: test/Analysis/conversion.c
===================================================================
--- test/Analysis/conversion.c
+++ test/Analysis/conversion.c
@@ -14,6 +14,64 @@
     S8 = U;
 }
 
+void addAssign() {
+  unsigned long L = 1000;
+  int I = -100;
+  U8 += L; // expected-warning {{Loss of precision in implicit conversion}}
+  L += I;
+}
+
+void subAssign() {
+  unsigned long L = 1000;
+  int I = -100;
+  U8 -= L; // expected-warning {{Loss of precision in implicit conversion}}
+  L -= I;
+}
+
+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;
+}
+
+void divAssign() {
+  unsigned long L = 1000;
+  int I = -1;
+  U8 /= L;
+  L /= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void remAssign() {
+  unsigned long L = 1000;
+  int I = -1;
+  U8 %= L;
+  L %= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void andAssign() {
+  unsigned long L = 1000;
+  int I = -1;
+  U8 &= L;
+  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() {
   long long A = 1LL << 60;
   short X = A; // expected-warning {{Loss of precision in implicit conversion}}
Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -41,7 +41,8 @@
   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 @@
   // 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 @@
                            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 @@
 }
 
 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;
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits