r326122 - [analyzer] Quickfix: do not overflow in calculating offset in RegionManager

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Feb 26 13:03:06 2018
New Revision: 326122

URL: http://llvm.org/viewvc/llvm-project?rev=326122&view=rev
Log:
[analyzer] Quickfix: do not overflow in calculating offset in RegionManager

Addresses https://bugs.llvm.org/show_bug.cgi?id=36206

rdar://37159026

A proper fix would be much harder, and would involve changing the
appropriate code in ExprEngine to be aware of the size limitations of
the type used for addressing.

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

Added:
    cfe/trunk/test/Analysis/region_store_overflow.c
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
    cfe/trunk/test/Analysis/region-store.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=326122&r1=326121&r2=326122&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Mon Feb 26 13:03:06 2018
@@ -23,6 +23,11 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/Debug.h"
+
+#include<functional>
+
+#define DEBUG_TYPE "MemRegion"
 
 using namespace clang;
 using namespace ento;
@@ -1149,6 +1154,36 @@ 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();
   const ElementRegion *ER = this;
@@ -1176,6 +1211,17 @@ RegionRawOffset ElementRegion::getAsArra
         }
 
         CharUnits size = C.getTypeSizeInChars(elemType);
+
+        int64_t Mult;
+        bool Overflow = checkedAdd(i, size.getQuantity(), &Mult);
+        Overflow |= checkedMul(Mult, offset.getQuantity());
+        if (Overflow) {
+          DEBUG(llvm::dbgs() << "MemRegion::getAsArrayOffset: "
+                             << "offset overflowing, returning unknown\n");
+
+          return nullptr;
+        }
+
         offset += (i * size);
       }
 

Modified: cfe/trunk/test/Analysis/region-store.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/region-store.cpp?rev=326122&r1=326121&r2=326122&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/region-store.cpp (original)
+++ cfe/trunk/test/Analysis/region-store.cpp Mon Feb 26 13:03:06 2018
@@ -25,4 +25,4 @@ int radar13445834(Derived *Builder, Loc
   Builder->setLoc(l);
   return Builder->accessBase();
   
-}
\ No newline at end of file
+}

Added: cfe/trunk/test/Analysis/region_store_overflow.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/region_store_overflow.c?rev=326122&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/region_store_overflow.c (added)
+++ cfe/trunk/test/Analysis/region_store_overflow.c Mon Feb 26 13:03:06 2018
@@ -0,0 +1,13 @@
+// REQUIRES: asserts
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core -mllvm -debug %s 2>&1 | FileCheck %s
+
+int **h;
+int overflow_in_memregion(long j) {
+  for (int l = 0;; ++l) {
+    if (j - l > 0)
+      return h[j - l][0]; // no-crash
+  }
+  return 0;
+}
+// CHECK: {{.*}}
+// CHECK: MemRegion::getAsArrayOffset: offset overflowing, returning unknown