[asan] Fix unknown-crash being reported for multi-byte errors, and incorrect memory access addresses being reported

compiler-rt/lib/asan/asan_errors.cpp

@@ -437,8 +437,16 @@ ErrorGeneric::ErrorGeneric(u32 tid, uptr pc_, uptr bp_, uptr sp_, uptr addr,
     bug_descr = "unknown-crash";
     if (AddrIsInMem(addr)) {
       u8 *shadow_addr = (u8 *)MemToShadow(addr);
-      // If we are accessing 16 bytes, look at the second shadow byte.
-      if (*shadow_addr == 0 && access_size > ASAN_SHADOW_GRANULARITY)
+      u8 *shadow_addr_upper_bound = (u8 *)MEM_TO_SHADOW(addr + access_size);
+      // We use the MEM_TO_SHADOW macro for the upper bound above instead of
+      // MemToShadow to skip the assertion that (addr + access_size) is within
+      // the valid memory range. The validity of the shadow address is checked
+      // via AddrIsInShadow in the while loop below.
+
+      // If the access could span multiple shadow bytes,
+      // do a sequential scan and look for the first bad shadow byte.
+      while (*shadow_addr == 0 && shadow_addr < shadow_addr_upper_bound &&
+             AddrIsInShadow((uptr)(shadow_addr + 1)))
         shadow_addr++;
       // If we are in the partial right redzone, look at the next shadow byte.
       if (*shadow_addr > 0 && *shadow_addr < 128) shadow_addr++;

compiler-rt/lib/asan/asan_interceptors_memintrinsics.h

@@ -52,31 +52,31 @@ struct AsanInterceptorContext {
 // that no extra frames are created, and stack trace contains
 // relevant information only.
 // We check all shadow bytes.
-#define ACCESS_MEMORY_RANGE(ctx, offset, size, isWrite)                   \
-  do {                                                                    \
-    uptr __offset = (uptr)(offset);                                       \
-    uptr __size = (uptr)(size);                                           \
-    uptr __bad = 0;                                                       \
-    if (UNLIKELY(__offset > __offset + __size)) {                         \
-      GET_STACK_TRACE_FATAL_HERE;                                         \
-      ReportStringFunctionSizeOverflow(__offset, __size, &stack);         \
-    }                                                                     \
-    if (UNLIKELY(!QuickCheckForUnpoisonedRegion(__offset, __size)) &&     \
-        (__bad = __asan_region_is_poisoned(__offset, __size))) {          \
-      AsanInterceptorContext *_ctx = (AsanInterceptorContext *)ctx;       \
-      bool suppressed = false;                                            \
-      if (_ctx) {                                                         \
-        suppressed = IsInterceptorSuppressed(_ctx->interceptor_name);     \
-        if (!suppressed && HaveStackTraceBasedSuppressions()) {           \
-          GET_STACK_TRACE_FATAL_HERE;                                     \
-          suppressed = IsStackTraceSuppressed(&stack);                    \
-        }                                                                 \
-      }                                                                   \
-      if (!suppressed) {                                                  \
-        GET_CURRENT_PC_BP_SP;                                             \
-        ReportGenericError(pc, bp, sp, __bad, isWrite, __size, 0, false); \
-      }                                                                   \
-    }                                                                     \
+#define ACCESS_MEMORY_RANGE(ctx, offset, size, isWrite)                      \
+  do {                                                                       \
+    uptr __offset = (uptr)(offset);                                          \
+    uptr __size = (uptr)(size);                                              \
+    uptr __bad = 0;                                                          \
+    if (UNLIKELY(__offset > __offset + __size)) {                            \
+      GET_STACK_TRACE_FATAL_HERE;                                            \
+      ReportStringFunctionSizeOverflow(__offset, __size, &stack);            \
+    }                                                                        \
+    if (UNLIKELY(!QuickCheckForUnpoisonedRegion(__offset, __size)) &&        \
+        (__bad = __asan_region_is_poisoned(__offset, __size))) {             \
+      AsanInterceptorContext *_ctx = (AsanInterceptorContext *)ctx;          \
+      bool suppressed = false;                                               \
+      if (_ctx) {                                                            \
+        suppressed = IsInterceptorSuppressed(_ctx->interceptor_name);        \
+        if (!suppressed && HaveStackTraceBasedSuppressions()) {              \
+          GET_STACK_TRACE_FATAL_HERE;                                        \
+          suppressed = IsStackTraceSuppressed(&stack);                       \
+        }                                                                    \
+      }                                                                      \
+      if (!suppressed) {                                                     \
+        GET_CURRENT_PC_BP_SP;                                                \
+        ReportGenericError(pc, bp, sp, __offset, isWrite, __size, 0, false); \
+      }                                                                      \
+    }                                                                        \
   } while (0)
 
 #define ASAN_READ_RANGE(ctx, offset, size) \

compiler-rt/test/asan/TestCases/wild_pointer.cpp → compiler-rt/test/asan/TestCases/heap-overflow-large-read.cpp

@@ -11,22 +11,22 @@
 int main() {
   char *p = new char;
   char *dest = new char;
-  const size_t offset = 0x4567890123456789;
+  const size_t size = 0x4567890123456789;
 
   // The output here needs to match the output from the sanitizer runtime,
   // which includes 0x and prints hex in lower case.
   //
   // On Windows, %p omits %0x and prints hex characters in upper case,
   // so we use PRIxPTR instead of %p.
   fprintf(stderr, "Expected bad addr: %#" PRIxPTR "\n",
-          reinterpret_cast<uintptr_t>(p + offset));
+          reinterpret_cast<uintptr_t>(p));
   // Flush it so the output came out before the asan report.
   fflush(stderr);
 
-  memmove(dest, p, offset);
+  memmove(dest, p, size);
   return 0;
 }
 
 // CHECK: Expected bad addr: [[ADDR:0x[0-9,a-f]+]]
-// CHECK: AddressSanitizer: unknown-crash on address [[ADDR]]
-// CHECK: Address [[ADDR]] is a wild pointer inside of access range of size 0x4567890123456789
+// CHECK: AddressSanitizer: heap-buffer-overflow on address [[ADDR]]
+// CHECK: READ of size 5001116549197948809 at [[ADDR]] thread T0

compiler-rt/test/asan/TestCases/stack-buffer-overflow-partial-1.cpp

@@ -0,0 +1,45 @@
+// RUN: %clangxx_asan %s -o %t
+// RUN: not %run %t 1 2>&1 | FileCheck %s
+// RUN: not %run %t 2 2>&1 | FileCheck %s
+// RUN: not %run %t 7 2>&1 | FileCheck %s
+
+#define STACK_ALLOC_SIZE 8
+#define READ_SIZE 8
+
+#include <stdlib.h>
+#include <assert.h>
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds(int offset) {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + offset);
+  return *x_ptr;
+}
+
+int main(int argc, char **argv) {
+  int offset = atoi(argv[1]);
+
+  // We are explicitly testing that we correctly detect and report this error
+  // as a *partial stack buffer overflow*.
+  assert(offset < STACK_ALLOC_SIZE);
+  assert(offset + READ_SIZE > STACK_ALLOC_SIZE);
+
+  struct X x = out_of_bounds(offset);
+  int y = 0;
+
+  for (int i = 0; i < READ_SIZE; i++) {
+    y ^= x.bytes[i];
+  }
+
+  return y;
+}
+
+// CHECK: ERROR: AddressSanitizer: stack-buffer-overflow on address
+// CHECK: {{READ of size 8 at 0x.* thread T0}}
+// CHECK: {{.* 0x.* in out_of_bounds.*stack-buffer-overflow-partial-1.cpp:}}
+// CHECK: {{Address 0x.* is located in stack of thread T0 at offset}}
+// CHECK: {{    #0 0x.* in out_of_bounds.*stack-buffer-overflow-partial-1.cpp:}}
+// CHECK: 'bytes'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable

compiler-rt/test/asan/TestCases/stack-buffer-overflow-partial-2.cpp

@@ -0,0 +1,47 @@
+// RUN: %clangxx_asan %s -o %t
+// RUN: not %run %t 1 2>&1 | FileCheck %s
+// RUN: not %run %t 2 2>&1 | FileCheck %s
+// RUN: not %run %t 7 2>&1 | FileCheck %s
+// RUN: not %run %t 8 2>&1 | FileCheck %s
+// RUN: not %run %t 15 2>&1 | FileCheck %s
+
+#define STACK_ALLOC_SIZE 16
+#define READ_SIZE 16
+
+#include <stdlib.h>
+#include <assert.h>
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds(int offset) {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + offset);
+  return *x_ptr;
+}
+
+int main(int argc, char **argv) {
+  int offset = atoi(argv[1]);
+
+  // We are explicitly testing that we correctly detect and report this error
+  // as a *partial stack buffer overflow*.
+  assert(offset < STACK_ALLOC_SIZE);
+  assert(offset + READ_SIZE > STACK_ALLOC_SIZE);
+
+  struct X x = out_of_bounds(offset);
+  int y = 0;
+
+  for (int i = 0; i < READ_SIZE; i++) {
+    y ^= x.bytes[i];
+  }
+
+  return y;
+}
+
+// CHECK: ERROR: AddressSanitizer: stack-buffer-overflow on address
+// CHECK: {{READ of size 16 at 0x.* thread T0}}
+// CHECK: {{.* 0x.* in out_of_bounds.*stack-buffer-overflow-partial-2.cpp:}}
+// CHECK: {{Address 0x.* is located in stack of thread T0 at offset}}
+// CHECK: {{    #0 0x.* in out_of_bounds.*stack-buffer-overflow-partial-2.cpp:}}
+// CHECK: 'bytes'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable

compiler-rt/test/asan/TestCases/stack-buffer-overflow-partial-3.cpp

@@ -0,0 +1,47 @@
+// RUN: %clangxx_asan %s -o %t
+// RUN: not %run %t 2 2>&1 | FileCheck %s
+// RUN: not %run %t 3 2>&1 | FileCheck %s
+// RUN: not %run %t 7 2>&1 | FileCheck %s
+// RUN: not %run %t 8 2>&1 | FileCheck %s
+// RUN: not %run %t 15 2>&1 | FileCheck %s
+
+#define STACK_ALLOC_SIZE 16
+#define READ_SIZE 15
+
+#include <stdlib.h>
+#include <assert.h>
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds(int offset) {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + offset);
+  return *x_ptr;
+}
+
+int main(int argc, char **argv) {
+  int offset = atoi(argv[1]);
+
+  // We are explicitly testing that we correctly detect and report this error
+  // as a *partial stack buffer overflow*.
+  assert(offset < STACK_ALLOC_SIZE);
+  assert(offset + READ_SIZE > STACK_ALLOC_SIZE);
+
+  struct X x = out_of_bounds(offset);
+  int y = 0;
+
+  for (int i = 0; i < READ_SIZE; i++) {
+    y ^= x.bytes[i];
+  }
+
+  return y;
+}
+
+// CHECK: ERROR: AddressSanitizer: stack-buffer-overflow on address
+// CHECK: {{READ of size 15 at 0x.* thread T0}}
+// CHECK: {{.* 0x.* in out_of_bounds.*stack-buffer-overflow-partial-3.cpp:}}
+// CHECK: {{Address 0x.* is located in stack of thread T0 at offset}}
+// CHECK: {{    #0 0x.* in out_of_bounds.*stack-buffer-overflow-partial-3.cpp:}}
+// CHECK: 'bytes'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable

compiler-rt/test/asan/TestCases/stack-buffer-overflow-partial-4.cpp

@@ -0,0 +1,47 @@
+// RUN: %clangxx_asan %s -o %t
+// RUN: not %run %t 3 2>&1 | FileCheck %s
+// RUN: not %run %t 7 2>&1 | FileCheck %s
+// RUN: not %run %t 10 2>&1 | FileCheck %s
+// RUN: not %run %t 13 2>&1 | FileCheck %s
+// RUN: not %run %t 19 2>&1 | FileCheck %s
+
+#define STACK_ALLOC_SIZE 20
+#define READ_SIZE 18
+
+#include <stdlib.h>
+#include <assert.h>
+
+struct X {
+  char bytes[READ_SIZE];
+};
+
+__attribute__((noinline)) struct X out_of_bounds(int offset) {
+  volatile char bytes[STACK_ALLOC_SIZE];
+  struct X* x_ptr = (struct X*)(bytes + offset);
+  return *x_ptr;
+}
+
+int main(int argc, char **argv) {
+  int offset = atoi(argv[1]);
+
+  // We are explicitly testing that we correctly detect and report this error
+  // as a *partial stack buffer overflow*.
+  assert(offset < STACK_ALLOC_SIZE);
+  assert(offset + READ_SIZE > STACK_ALLOC_SIZE);
+
+  struct X x = out_of_bounds(offset);
+  int y = 0;
+
+  for (int i = 0; i < READ_SIZE; i++) {
+    y ^= x.bytes[i];
+  }
+
+  return y;
+}
+
+// CHECK: ERROR: AddressSanitizer: stack-buffer-overflow on address
+// CHECK: {{READ of size 18 at 0x.* thread T0}}
+// CHECK: {{.* 0x.* in out_of_bounds.*stack-buffer-overflow-partial-4.cpp:}}
+// CHECK: {{Address 0x.* is located in stack of thread T0 at offset}}
+// CHECK: {{    #0 0x.* in out_of_bounds.*stack-buffer-overflow-partial-4.cpp:}}
+// CHECK: 'bytes'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable

compiler-rt/test/asan/TestCases/strcasestr-1.c

@@ -19,7 +19,7 @@ int main(int argc, char **argv) {
   char s1[4] = "abC";
   __asan_poison_memory_region ((char *)&s1[2], 2);
   r = strcasestr(s1, s2);
-  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
+  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   assert(r == s1 + 2);
   return 0;
 }

compiler-rt/test/asan/TestCases/strcasestr-2.c

@@ -20,6 +20,6 @@ int main(int argc, char **argv) {
   __asan_poison_memory_region ((char *)&s2[2], 2);
   r = strcasestr(s1, s2);
   assert(r == 0);
-  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
+  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   return 0;
 }

compiler-rt/test/asan/TestCases/strcspn-1.c

@@ -14,7 +14,7 @@ int main(int argc, char **argv) {
   char s1[4] = "caB";
   __asan_poison_memory_region ((char *)&s1[2], 2);
   r = strcspn(s1, s2);
-  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
+  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   assert(r == 1);
   return 0;
 }

compiler-rt/test/asan/TestCases/strcspn-2.c

@@ -14,7 +14,7 @@ int main(int argc, char **argv) {
   char s2[4] = "abc";
   __asan_poison_memory_region ((char *)&s2[2], 2);
   r = strcspn(s1, s2);
-  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
+  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   assert(r == 0);
   return 0;
 }

compiler-rt/test/asan/TestCases/strpbrk-1.c

@@ -14,7 +14,7 @@ int main(int argc, char **argv) {
   char s1[4] = "cab";
   __asan_poison_memory_region ((char *)&s1[2], 2);
   r = strpbrk(s1, s2);
-  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
+  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   assert(r == s1 + 1);
   return 0;
 }

compiler-rt/test/asan/TestCases/strpbrk-2.c

@@ -14,7 +14,7 @@ int main(int argc, char **argv) {
   char s2[4] = "bca";
   __asan_poison_memory_region ((char *)&s2[2], 2);
   r = strpbrk(s1, s2);
-  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
+  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   assert(r == s1);
   return 0;
 }

compiler-rt/test/asan/TestCases/strspn-1.c

@@ -14,7 +14,7 @@ int main(int argc, char **argv) {
   char s1[4] = "acb";
   __asan_poison_memory_region ((char *)&s1[2], 2);
   r = strspn(s1, s2);
-  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
+  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   assert(r == 1);
   return 0;
 }

compiler-rt/test/asan/TestCases/strspn-2.c

@@ -14,7 +14,7 @@ int main(int argc, char **argv) {
   char s2[5] = "abcd";
   __asan_poison_memory_region ((char *)&s2[3], 2);
   r = strspn(s1, s2);
-  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
+  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   assert(r >= 2);
   return 0;
 }

compiler-rt/test/asan/TestCases/strstr-1.c

@@ -15,7 +15,7 @@ int main(int argc, char **argv) {
   char s1[4] = "acb";
   __asan_poison_memory_region ((char *)&s1[2], 2);
   r = strstr(s1, s2);
-  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} {{partially overflows this variable|is inside this variable}}
+  // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   assert(r == s1 + 1);
   return 0;
 }

compiler-rt/test/asan/TestCases/strstr-2.c

@@ -15,7 +15,7 @@ int main(int argc, char **argv) {
   char s2[4] = "cab";
   __asan_poison_memory_region ((char *)&s2[2], 2);
   r = strstr(s1, s2);
-  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
+  // CHECK:'s2'{{.*}} <== Memory access at offset {{[0-9]+}} is inside this variable
   assert(r == 0);
   return 0;
 }