[asan] Fix nonsensical reports of partial right OOB.

In case of partial right OOB, ASan was reporting
  X is located 0 bytes to the right of [A, B)
where X was actually inside [A, B).

With this change, ASan will report B as the error address in such case.

llvm-svn: 174373

9 files changed, 62 insertions, 37 deletions

diff --git a/compiler-rt/lib/asan/asan_allocator.cc b/compiler-rt/lib/asan/asan_allocator.cc
index 189b079fd56..f6483d793da 100644
--- a/compiler-rt/lib/asan/asan_allocator.cc
+++ b/compiler-rt/lib/asan/asan_allocator.cc
@@ -369,7 +369,7 @@ class MallocInfo {
         left_chunk->chunk_state != CHUNK_AVAILABLE)
       return left_chunk;
     // Choose based on offset.
-    uptr l_offset = 0, r_offset = 0;
+    sptr l_offset = 0, r_offset = 0;
     CHECK(AsanChunkView(left_chunk).AddrIsAtRight(addr, 1, &l_offset));
     CHECK(AsanChunkView(right_chunk).AddrIsAtLeft(addr, 1, &r_offset));
     if (l_offset < r_offset)
@@ -389,7 +389,7 @@ class MallocInfo {
     CHECK(m->chunk_state == CHUNK_ALLOCATED ||
           m->chunk_state == CHUNK_AVAILABLE ||
           m->chunk_state == CHUNK_QUARANTINE);
-    uptr offset = 0;
+    lptr offset = 0;
     AsanChunkView m_view(m);
     if (m_view.AddrIsInside(addr, 1, &offset))
       return m;
diff --git a/compiler-rt/lib/asan/asan_allocator.h b/compiler-rt/lib/asan/asan_allocator.h
index 2de6b982ac7..d8cef68a0fb 100644
--- a/compiler-rt/lib/asan/asan_allocator.h
+++ b/compiler-rt/lib/asan/asan_allocator.h
@@ -55,14 +55,14 @@ class AsanChunkView {
   uptr FreeTid();
   void GetAllocStack(StackTrace *stack);
   void GetFreeStack(StackTrace *stack);
-  bool AddrIsInside(uptr addr, uptr access_size, uptr *offset) {
+  bool AddrIsInside(uptr addr, uptr access_size, sptr *offset) {
     if (addr >= Beg() && (addr + access_size) <= End()) {
       *offset = addr - Beg();
       return true;
     }
     return false;
   }
-  bool AddrIsAtLeft(uptr addr, uptr access_size, uptr *offset) {
+  bool AddrIsAtLeft(uptr addr, uptr access_size, sptr *offset) {
     (void)access_size;
     if (addr < Beg()) {
       *offset = Beg() - addr;
@@ -70,12 +70,9 @@ class AsanChunkView {
     }
     return false;
   }
-  bool AddrIsAtRight(uptr addr, uptr access_size, uptr *offset) {
+  bool AddrIsAtRight(uptr addr, uptr access_size, sptr *offset) {
     if (addr + access_size >= End()) {
-      if (addr <= End())
-        *offset = 0;
-      else
-        *offset = addr - End();
+      *offset = addr - End();
       return true;
     }
     return false;
diff --git a/compiler-rt/lib/asan/asan_allocator2.cc b/compiler-rt/lib/asan/asan_allocator2.cc
index bd717ee5b87..45655ff0535 100644
--- a/compiler-rt/lib/asan/asan_allocator2.cc
+++ b/compiler-rt/lib/asan/asan_allocator2.cc
@@ -547,7 +547,7 @@ AsanChunk *ChooseChunk(uptr addr,
       return right_chunk;
   }
   // Same chunk_state: choose based on offset.
-  uptr l_offset = 0, r_offset = 0;
+  sptr l_offset = 0, r_offset = 0;
   CHECK(AsanChunkView(left_chunk).AddrIsAtRight(addr, 1, &l_offset));
   CHECK(AsanChunkView(right_chunk).AddrIsAtLeft(addr, 1, &r_offset));
   if (l_offset < r_offset)
@@ -558,7 +558,7 @@ AsanChunk *ChooseChunk(uptr addr,
 AsanChunkView FindHeapChunkByAddress(uptr addr) {
   AsanChunk *m1 = GetAsanChunkByAddr(addr);
   if (!m1) return AsanChunkView(m1);
-  uptr offset = 0;
+  sptr offset = 0;
   if (AsanChunkView(m1).AddrIsAtLeft(addr, 1, &offset)) {
     // The address is in the chunk's left redzone, so maybe it is actually
     // a right buffer overflow from the other chunk to the left.
diff --git a/compiler-rt/lib/asan/asan_globals.cc b/compiler-rt/lib/asan/asan_globals.cc
index ab013d519ad..3101f163a54 100644
--- a/compiler-rt/lib/asan/asan_globals.cc
+++ b/compiler-rt/lib/asan/asan_globals.cc
@@ -48,7 +48,7 @@ void PoisonRedZones(const Global &g)  {
   }
 }
 
-bool DescribeAddressIfGlobal(uptr addr) {
+bool DescribeAddressIfGlobal(uptr addr, uptr size) {
   if (!flags()->report_globals) return false;
   BlockingMutexLock lock(&mu_for_globals);
   bool res = false;
@@ -57,7 +57,7 @@ bool DescribeAddressIfGlobal(uptr addr) {
     if (flags()->report_globals >= 2)
       Report("Search Global: beg=%p size=%zu name=%s\n",
              (void*)g.beg, g.size, (char*)g.name);
-    res |= DescribeAddressRelativeToGlobal(addr, g);
+    res |= DescribeAddressRelativeToGlobal(addr, size, g);
   }
   return res;
 }
diff --git a/compiler-rt/lib/asan/asan_interceptors.cc b/compiler-rt/lib/asan/asan_interceptors.cc
index 281a13d302e..b1bdf0acb98 100644
--- a/compiler-rt/lib/asan/asan_interceptors.cc
+++ b/compiler-rt/lib/asan/asan_interceptors.cc
@@ -31,12 +31,14 @@ namespace __asan {
 // that no extra frames are created, and stack trace contains
 // relevant information only.
 // We check all shadow bytes.
-#define ACCESS_MEMORY_RANGE(offset, size, isWrite) do {                  \
-  if (uptr __ptr = __asan_region_is_poisoned((uptr)(offset), size)) {    \
-    GET_CURRENT_PC_BP_SP;                                                \
-    __asan_report_error(pc, bp, sp, __ptr, isWrite, /* access_size */1); \
-  }                                                                      \
-} while (0)
+#define ACCESS_MEMORY_RANGE(offset, size, isWrite) do {                 \
+    uptr __offset = (uptr)(offset);                                     \
+    uptr __size = (uptr)(size);                                         \
+    if (__asan_region_is_poisoned(__offset, __size)) {                  \
+      GET_CURRENT_PC_BP_SP;                                             \
+      __asan_report_error(pc, bp, sp, __offset, isWrite, __size);       \
+    }                                                                   \
+  } while (0)
 
 #define ASAN_READ_RANGE(offset, size) ACCESS_MEMORY_RANGE(offset, size, false)
 #define ASAN_WRITE_RANGE(offset, size) ACCESS_MEMORY_RANGE(offset, size, true);
diff --git a/compiler-rt/lib/asan/asan_report.cc b/compiler-rt/lib/asan/asan_report.cc
index c381f4f010f..e687b57b7a6 100644
--- a/compiler-rt/lib/asan/asan_report.cc
+++ b/compiler-rt/lib/asan/asan_report.cc
@@ -190,19 +190,23 @@ static void PrintGlobalNameIfASCII(const __asan_global &g) {
   Printf("  '%s' is ascii string '%s'\n", g.name, (char*)g.beg);
 }
 
-bool DescribeAddressRelativeToGlobal(uptr addr, const __asan_global &g) {
+bool DescribeAddressRelativeToGlobal(uptr addr, uptr size,
+                                     const __asan_global &g) {
   static const uptr kMinimalDistanceFromAnotherGlobal = 64;
   if (addr <= g.beg - kMinimalDistanceFromAnotherGlobal) return false;
   if (addr >= g.beg + g.size_with_redzone) return false;
   Decorator d;
   Printf("%s", d.Location());
-  Printf("%p is located ", (void*)addr);
   if (addr < g.beg) {
-    Printf("%zd bytes to the left", g.beg - addr);
-  } else if (addr >= g.beg + g.size) {
-    Printf("%zd bytes to the right", addr - (g.beg + g.size));
+    Printf("%p is located %zd bytes to the left", (void*)addr, g.beg - addr);
+  } else if (addr + size > g.beg + g.size) {
+    if (addr < g.beg + g.size)
+      addr = g.beg + g.size;
+    Printf("%p is located %zd bytes to the right", (void*)addr,
+           addr - (g.beg + g.size));
   } else {
-    Printf("%zd bytes inside", addr - g.beg);  // Can it happen?
+    // Can it happen?
+    Printf("%p is located %zd bytes inside", (void*)addr, addr - g.beg);
   }
   Printf(" of global variable '%s' (0x%zx) of size %zu\n",
              g.name, g.beg, g.size);
@@ -288,18 +292,22 @@ bool DescribeAddressIfStack(uptr addr, uptr access_size) {
 
 static void DescribeAccessToHeapChunk(AsanChunkView chunk, uptr addr,
                                       uptr access_size) {
-  uptr offset;
+  sptr offset;
   Decorator d;
   Printf("%s", d.Location());
-  Printf("%p is located ", (void*)addr);
-  if (chunk.AddrIsInside(addr, access_size, &offset)) {
-    Printf("%zu bytes inside of", offset);
-  } else if (chunk.AddrIsAtLeft(addr, access_size, &offset)) {
-    Printf("%zu bytes to the left of", offset);
+  if (chunk.AddrIsAtLeft(addr, access_size, &offset)) {
+    Printf("%p is located %zd bytes to the left of", (void*)addr, offset);
   } else if (chunk.AddrIsAtRight(addr, access_size, &offset)) {
-    Printf("%zu bytes to the right of", offset);
+    if (offset < 0) {
+      addr -= offset;
+      offset = 0;
+    }
+    Printf("%p is located %zd bytes to the right of", (void*)addr, offset);
+  } else if (chunk.AddrIsInside(addr, access_size, &offset)) {
+    Printf("%p is located %zd bytes inside of", (void*)addr, offset);
   } else {
-    Printf(" somewhere around (this is AddressSanitizer bug!)");
+    Printf("%p is located somewhere around (this is AddressSanitizer bug!)",
+           (void*)addr);
   }
   Printf(" %zu-byte region [%p,%p)\n", chunk.UsedSize(),
          (void*)(chunk.Beg()), (void*)(chunk.End()));
@@ -372,7 +380,7 @@ void DescribeAddress(uptr addr, uptr access_size) {
   if (DescribeAddressIfShadow(addr))
     return;
   CHECK(AddrIsInMem(addr));
-  if (DescribeAddressIfGlobal(addr))
+  if (DescribeAddressIfGlobal(addr, access_size))
     return;
   if (DescribeAddressIfStack(addr, access_size))
     return;
diff --git a/compiler-rt/lib/asan/asan_report.h b/compiler-rt/lib/asan/asan_report.h
index dd991229c01..55a8039dc99 100644
--- a/compiler-rt/lib/asan/asan_report.h
+++ b/compiler-rt/lib/asan/asan_report.h
@@ -21,8 +21,9 @@ namespace __asan {
 // The following functions prints address description depending
 // on the memory type (shadow/heap/stack/global).
 void DescribeHeapAddress(uptr addr, uptr access_size);
-bool DescribeAddressIfGlobal(uptr addr);
-bool DescribeAddressRelativeToGlobal(uptr addr, const __asan_global &g);
+bool DescribeAddressIfGlobal(uptr addr, uptr access_size);
+bool DescribeAddressRelativeToGlobal(uptr addr, uptr access_size,
+                                     const __asan_global &g);
 bool DescribeAddressIfShadow(uptr addr);
 bool DescribeAddressIfStack(uptr addr, uptr access_size);
 // Determines memory type on its own.
diff --git a/compiler-rt/lib/asan/lit_tests/partial_right.cc b/compiler-rt/lib/asan/lit_tests/partial_right.cc
new file mode 100644
index 00000000000..c579262726f
--- /dev/null
+++ b/compiler-rt/lib/asan/lit_tests/partial_right.cc
@@ -0,0 +1,17 @@
+// RUN: %clangxx_asan -m64 -O0 %s -o %t && %t 2>&1 | %symbolize | FileCheck %s
+// RUN: %clangxx_asan -m64 -O1 %s -o %t && %t 2>&1 | %symbolize | FileCheck %s
+// RUN: %clangxx_asan -m64 -O2 %s -o %t && %t 2>&1 | %symbolize | FileCheck %s
+// RUN: %clangxx_asan -m64 -O3 %s -o %t && %t 2>&1 | %symbolize | FileCheck %s
+// RUN: %clangxx_asan -m32 -O0 %s -o %t && %t 2>&1 | %symbolize | FileCheck %s
+// RUN: %clangxx_asan -m32 -O1 %s -o %t && %t 2>&1 | %symbolize | FileCheck %s
+// RUN: %clangxx_asan -m32 -O2 %s -o %t && %t 2>&1 | %symbolize | FileCheck %s
+// RUN: %clangxx_asan -m32 -O3 %s -o %t && %t 2>&1 | %symbolize | FileCheck %s
+
+#include <stdlib.h>
+int main(int argc, char **argv) {
+  volatile int *x = (int*)malloc(2*sizeof(int) + 2);
+  int res = x[2];  // BOOOM
+  // CHECK: {{READ of size 4 at 0x.* thread T0}}
+  // CHECK: [[ADDR:0x[01-9a-fa-f]+]] is located 0 bytes to the right of {{.*}}-byte region [{{.*}},{{.*}}[[ADDR]])
+  return res;
+}
diff --git a/compiler-rt/lib/asan/lit_tests/strncpy-overflow.cc b/compiler-rt/lib/asan/lit_tests/strncpy-overflow.cc
index 2d79c7a2d97..5133b5c1653 100644
--- a/compiler-rt/lib/asan/lit_tests/strncpy-overflow.cc
+++ b/compiler-rt/lib/asan/lit_tests/strncpy-overflow.cc
@@ -22,7 +22,7 @@ int main(int argc, char **argv) {
   strcpy(hello, "hello");
   char *short_buffer = (char*)malloc(9);
   strncpy(short_buffer, hello, 10);  // BOOM
-  // CHECK: {{WRITE of size 1 at 0x.* thread T0}}
+  // CHECK: {{WRITE of size 10 at 0x.* thread T0}}
   // CHECK-Linux: {{    #0 0x.* in .*strncpy}}
   // CHECK-Darwin: {{    #0 0x.* in _?wrap_strncpy}}
   // CHECK: {{    #1 0x.* in _?main .*strncpy-overflow.cc:}}[[@LINE-4]]