[asan] Fix 'unknown-crash' reported for multi-byte errors

Given that a reported error by ASan spans multiple bytes, ASan may
flag the error as an 'unknown-crash' instead of the appropriate error
name.

This error can be reproduced via a partial buffer overflow (any GCC,
or after performing the patch in the previous commit). They'll report
'unknown-crash' instead of 'stack-buffer-overflow' for the below:

    # minimal reprod
    # https://godbolt.org/z/abrjrvnzj
    #
    # gcc -fsanitize=address reprod.c

    struct X {
        char bytes[16];
    };

    __attribute__((noinline)) struct X out_of_bounds() {
        volatile char bytes[16];
        struct X* x_ptr = (struct X*)(bytes + 2);
        return *x_ptr;
    }

    int main() {
        struct X x = out_of_bounds();
        return x.bytes[0];
    }

This is due to a flawed heuristic in asan_errors.cpp, which won't
always locate the appropriate shadow byte that would indicate a
corresponding error. This can happen for any reported errors which
span either: exactly 8 bytes, or 16 and more bytes.

This bug was previously hidden from Clang (but has always been present
in GCC) until the previous commit's fix on addresses. Specifically,
the fact that Clang previously reported the first poisoned byte
(instead of the start address, like in GCC) masked this bug.

This patch resolves this issue via a linear scan of applicable
shadow bytes (instead of the original heuristic, which, at best, only
increments the shadow byte address by 1 for these scenarios).

Author: wxwern

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/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