[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan created this revision.
ebevhan added reviewers: dergachev.a, dcoughlin.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.
Herald added a reviewer: george.karpenkov.

RegionStoreManager::getSizeInElements used 'int' for size
calculations, and ProgramState::assumeInBound fell back
to 'int' as well for its index calculations. This causes
truncation for sufficiently large sizes/indexes.

Use a signed size_t and ArrayIndexTy respectively to
prevent these problems.


Repository:
  rC Clang

https://reviews.llvm.org/D46944

Files:
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/array-index.c


Index: test/Analysis/array-index.c
===================================================================
--- /dev/null
+++ test/Analysis/array-index.c
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.ArrayBound,alpha.unix.cstring.OutOfBounds -verify -Wno-implicit-function-declaration %s
+
+// expected-no-diagnostics
+
+#define SIZE 4294967296
+
+static unsigned size;
+static void * addr;
+static unsigned buf[SIZE];
+
+void fie() {
+  buf[SIZE-1] = 1;
+}
+
+void foo() {
+  memcpy(buf, addr, size);
+}
+
+void bar() {
+  memcpy(addr, buf, size);
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1341,7 +1341,7 @@
   // If a variable is reinterpreted as a type that doesn't fit into a larger
   // type evenly, round it down.
   // This is a signed value, since it's used in arithmetic with signed indices.
-  return svalBuilder.makeIntVal(RegionSize / EleSize, false);
+  return svalBuilder.makeIntVal(RegionSize / EleSize, Ctx.getSignedSizeType());
 }
 
 //===----------------------------------------------------------------------===//
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -336,9 +336,8 @@
 
   // Get the offset: the minimum value of the array index type.
   BasicValueFactory &BVF = svalBuilder.getBasicValueFactory();
-  // FIXME: This should be using ValueManager::ArrayindexTy...somehow.
   if (indexTy.isNull())
-    indexTy = Ctx.IntTy;
+    indexTy = svalBuilder.getArrayIndexType();
   nonloc::ConcreteInt Min(BVF.getMinValue(indexTy));
 
   // Adjust the index.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46944.147087.patch
Type: text/x-patch
Size: 1841 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180516/d1bd469f/attachment.bin>