[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 created this revision.
pfultz2 added reviewers: NoQ, xazax.hun, dkrupp, whisperity.
pfultz2 added a project: clang.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet.
Herald added a reviewer: george.karpenkov.

This expands checking for more expressions. This will check underflow and loss of precision when using call expressions like:

  void foo(unsigned);
  int i = -1;
  foo(i);

This also includes other expressions as well, so it can catch negative indices to `std::vector` since it uses unsigned integers for `[]` and `.at()` function.


Repository:
  rC Clang

https://reviews.llvm.org/D46081

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


Index: test/Analysis/conversion.c
===================================================================
--- test/Analysis/conversion.c
+++ test/Analysis/conversion.c
@@ -102,6 +102,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.
@@ -129,12 +146,17 @@
   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;
 }
 
 
Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -53,9 +53,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<DeclRefExpr>(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.
@@ -75,8 +74,10 @@
   if (const auto *B = dyn_cast<BinaryOperator>(Parent)) {
     BinaryOperator::Opcode Opc = B->getOpcode();
     if (Opc == BO_Assign) {
-      LossOfSign = isLossOfSign(Cast, C);
-      LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
+      if (!B->getRHS()->isIntegerConstantExpr(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);
@@ -95,7 +96,7 @@
     } else if (B->isRelationalOp() || B->isMultiplicativeOp()) {
       LossOfSign = isLossOfSign(Cast, C);
     }
-  } else if (isa<DeclStmt>(Parent)) {
+  } else {
     LossOfSign = isLossOfSign(Cast, C);
     LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46081.143996.patch
Type: text/x-patch
Size: 3116 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180425/93ca6cb4/attachment.bin>