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())


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())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to