[PATCH] D41816: [analyzer] Model and check unrepresentable left shifts

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs created this revision.
rnkovacs added reviewers: NoQ, dcoughlin, xazax.hun.
Herald added subscribers: a.sidorin, szepet, baloghadamsoftware, whisperity.

Left shifting a signed positive value is undefined if the result is not representable in the unsigned version of the return type.

The analyzer now returns an UndefVal in this case and UndefResultChecker is updated to warn about it.


https://reviews.llvm.org/D41816

Files:
  lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  test/Analysis/bitwise-ops.c


Index: test/Analysis/bitwise-ops.c
===================================================================
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -51,3 +51,9 @@
   }
   return 0;
 }
+
+int testUnrepresentableLeftShift(int a) {
+  if (a == 8)
+    return a << 30; // expected-warning{{The result of the left shift is undefined because it is not representable in the return type}}
+  return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===================================================================
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -224,7 +224,6 @@
       // FIXME: This logic should probably go higher up, where we can
       // test these conditions symbolically.
 
-      // FIXME: Expand these checks to include all undefined behavior.
       if (V1.isSigned() && V1.isNegative())
         return nullptr;
 
@@ -236,16 +235,17 @@
       if (Amt >= V1.getBitWidth())
         return nullptr;
 
+      if (V1.isSigned() && (unsigned) Amt > V1.countLeadingZeros())
+          return nullptr;
+
       return &getValue( V1.operator<<( (unsigned) Amt ));
     }
 
     case BO_Shr: {
 
       // FIXME: This logic should probably go higher up, where we can
       // test these conditions symbolically.
 
-      // FIXME: Expand these checks to include all undefined behavior.
-
       if (V2.isSigned() && V2.isNegative())
         return nullptr;
 
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -141,6 +141,15 @@
                  C.isNegative(B->getLHS())) {
         OS << "The result of the left shift is undefined because the left "
               "operand is negative";
+      } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl) {
+        SValBuilder &SB = C.getSValBuilder();
+        const llvm::APSInt *LHS =
+            SB.getKnownValue(state, C.getSVal(B->getLHS()));
+        const llvm::APSInt *RHS =
+            SB.getKnownValue(state, C.getSVal(B->getRHS()));
+        if ((unsigned) RHS->getZExtValue() > LHS->countLeadingZeros())
+          OS << "The result of the left shift is undefined because it is not "
+                "representable in the return type";
       } else {
         OS << "The result of the '"
            << BinaryOperator::getOpcodeStr(B->getOpcode())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41816.128908.patch
Type: text/x-patch
Size: 2505 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180108/923e6fd9/attachment.bin>