r334636 - [analyzer] Fix offset overflow check in MemRegion

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Wed Jun 13 11:32:19 2018
New Revision: 334636

URL: http://llvm.org/viewvc/llvm-project?rev=334636&view=rev
Log:
[analyzer] Fix offset overflow check in MemRegion

rdar://39593879
https://bugs.llvm.org/show_bug.cgi?id=37142

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

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
    cfe/trunk/test/Analysis/region_store_overflow.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=334636&r1=334635&r2=334636&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Wed Jun 13 11:32:19 2018
@@ -41,6 +41,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/CheckedArithmetic.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -1181,38 +1182,8 @@ const SymbolicRegion *MemRegion::getSymb
   return nullptr;
 }
 
-/// Perform a given operation on two integers, return whether it overflows.
-/// Optionally write the resulting output into \p Res.
-static bool checkedOp(
-    int64_t LHS,
-    int64_t RHS,
-    std::function<llvm::APInt(llvm::APInt *, const llvm::APInt &, bool &)> Op,
-    int64_t *Res = nullptr) {
-  llvm::APInt ALHS(/*BitSize=*/64, LHS, /*Signed=*/true);
-  llvm::APInt ARHS(/*BitSize=*/64, RHS, /*Signed=*/true);
-  bool Overflow;
-  llvm::APInt Out = Op(&ALHS, ARHS, Overflow);
-  if (!Overflow && Res)
-    *Res = Out.getSExtValue();
-  return Overflow;
-}
-
-static bool checkedAdd(
-    int64_t LHS,
-    int64_t RHS,
-    int64_t *Res=nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::sadd_ov, Res);
-}
-
-static bool checkedMul(
-    int64_t LHS,
-    int64_t RHS,
-    int64_t *Res=nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::smul_ov, Res);
-}
-
 RegionRawOffset ElementRegion::getAsArrayOffset() const {
-  CharUnits offset = CharUnits::Zero();
+  int64_t offset = 0;
   const ElementRegion *ER = this;
   const MemRegion *superR = nullptr;
   ASTContext &C = getContext();
@@ -1224,7 +1195,7 @@ RegionRawOffset ElementRegion::getAsArra
 
     // FIXME: generalize to symbolic offsets.
     SVal index = ER->getIndex();
-    if (Optional<nonloc::ConcreteInt> CI = index.getAs<nonloc::ConcreteInt>()) {
+    if (auto CI = index.getAs<nonloc::ConcreteInt>()) {
       // Update the offset.
       int64_t i = CI->getValue().getSExtValue();
 
@@ -1237,20 +1208,15 @@ RegionRawOffset ElementRegion::getAsArra
           break;
         }
 
-        CharUnits size = C.getTypeSizeInChars(elemType);
-
-        int64_t Mult;
-        bool Overflow = checkedAdd(i, size.getQuantity(), &Mult);
-        if (!Overflow)
-          Overflow = checkedMul(Mult, offset.getQuantity());
-        if (Overflow) {
+        int64_t size = C.getTypeSizeInChars(elemType).getQuantity();
+        if (auto NewOffset = llvm::checkedMulAdd(i, size, offset)) {
+          offset = *NewOffset;
+        } else {
           LLVM_DEBUG(llvm::dbgs() << "MemRegion::getAsArrayOffset: "
                                   << "offset overflowing, returning unknown\n");
 
           return nullptr;
         }
-
-        offset += (i * size);
       }
 
       // Go to the next ElementRegion (if any).
@@ -1262,7 +1228,7 @@ RegionRawOffset ElementRegion::getAsArra
   }
 
   assert(superR && "super region cannot be NULL");
-  return RegionRawOffset(superR, offset);
+  return RegionRawOffset(superR, CharUnits::fromQuantity(offset));
 }
 
 /// Returns true if \p Base is an immediate base class of \p Child

Modified: cfe/trunk/test/Analysis/region_store_overflow.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/region_store_overflow.c?rev=334636&r1=334635&r2=334636&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/region_store_overflow.c (original)
+++ cfe/trunk/test/Analysis/region_store_overflow.c Wed Jun 13 11:32:19 2018
@@ -1,6 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core -verify %s
 
-// expected-no-diagnostics
 int **h;
 int overflow_in_memregion(long j) {
   for (int l = 0;; ++l) {
@@ -9,3 +8,9 @@ int overflow_in_memregion(long j) {
   }
   return 0;
 }
+
+void rdar39593879(long long *d) {
+  long e, f;
+  e = f = d[1]; // no-crash
+  for (; d[e];) f-- > 0; // expected-warning{{relational comparison result unused}}; 
+}