[WinASan] Improve exception reporting accuracy
authorReid Kleckner <rnk@google.com>
Mon, 30 Nov 2020 18:21:14 +0000 (10:21 -0800)
committerReid Kleckner <rnk@google.com>
Tue, 1 Dec 2020 00:39:22 +0000 (16:39 -0800)
Previously, ASan would produce reports like this:
ERROR: AddressSanitizer: breakpoint on unknown address 0x000000000000 (pc 0x7fffdd7c5e86 ...)

This is unhelpful, because the developer may think this is a null
pointer dereference, and not a breakpoint exception on some PC.

The cause was that SignalContext::GetAddress would read the
ExceptionInformation array to retreive an address for any kind of
exception. That data is only available for access violation exceptions.
This changes it to be conditional on the exception type, and to use the
PC otherwise.

I added a variety of tests for common exception types:
- int div zero
- breakpoint
- ud2a / illegal instruction
- SSE misalignment

I also tightened up IsMemoryAccess and GetWriteFlag to check the
ExceptionCode rather than looking at ExceptionInformation[1] directly.

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

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
compiler-rt/test/asan/TestCases/Windows/breakpoint.cpp [new file with mode: 0644]
compiler-rt/test/asan/TestCases/Windows/illegal_instruction.cpp [new file with mode: 0644]
compiler-rt/test/asan/TestCases/Windows/integer_divide_by_zero.cpp [new file with mode: 0644]
compiler-rt/test/asan/TestCases/Windows/sse_misalignment.cpp [new file with mode: 0644]

index 85ac263..281854a 100644 (file)
@@ -956,22 +956,27 @@ void SignalContext::InitPcSpBp() {
 
 uptr SignalContext::GetAddress() const {
   EXCEPTION_RECORD *exception_record = (EXCEPTION_RECORD *)siginfo;
-  return exception_record->ExceptionInformation[1];
+  if (exception_record->ExceptionCode == EXCEPTION_ACCESS_VIOLATION)
+    return exception_record->ExceptionInformation[1];
+  return (uptr)exception_record->ExceptionAddress;
 }
 
 bool SignalContext::IsMemoryAccess() const {
-  return GetWriteFlag() != SignalContext::UNKNOWN;
+  return ((EXCEPTION_RECORD *)siginfo)->ExceptionCode ==
+         EXCEPTION_ACCESS_VIOLATION;
 }
 
-bool SignalContext::IsTrueFaultingAddress() const {
-  // FIXME: Provide real implementation for this. See Linux and Mac variants.
-  return IsMemoryAccess();
-}
+bool SignalContext::IsTrueFaultingAddress() const { return true; }
 
 SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
   EXCEPTION_RECORD *exception_record = (EXCEPTION_RECORD *)siginfo;
+
+  // The write flag is only available for access violation exceptions.
+  if (exception_record->ExceptionCode != EXCEPTION_ACCESS_VIOLATION)
+    return SignalContext::UNKNOWN;
+
   // The contents of this array are documented at
-  // https://msdn.microsoft.com/en-us/library/windows/desktop/aa363082(v=vs.85).aspx
+  // https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-exception_record
   // The first element indicates read as 0, write as 1, or execute as 8.  The
   // second element is the faulting address.
   switch (exception_record->ExceptionInformation[0]) {
diff --git a/compiler-rt/test/asan/TestCases/Windows/breakpoint.cpp b/compiler-rt/test/asan/TestCases/Windows/breakpoint.cpp
new file mode 100644 (file)
index 0000000..1c9e8c4
--- /dev/null
@@ -0,0 +1,18 @@
+// RUN: %clang_cl_asan -Od %s -Fe%t
+// RUN: %env_asan_opts=handle_sigill=1 not %run %t 2>&1 | FileCheck %s
+
+// Test the error output from a breakpoint. Assertion-like macros often end in
+// int3 on Windows.
+
+#include <stdio.h>
+
+int main() {
+  puts("before breakpoint");
+  fflush(stdout);
+  __debugbreak();
+  return 0;
+}
+// CHECK: before breakpoint
+// CHECK: ERROR: AddressSanitizer: breakpoint on unknown address [[ADDR:0x[^ ]*]]
+// CHECK-SAME: (pc [[ADDR]] {{.*}})
+// CHECK-NEXT: #0 {{.*}} in main {{.*}}breakpoint.cpp:{{.*}}
diff --git a/compiler-rt/test/asan/TestCases/Windows/illegal_instruction.cpp b/compiler-rt/test/asan/TestCases/Windows/illegal_instruction.cpp
new file mode 100644 (file)
index 0000000..2b6b05c
--- /dev/null
@@ -0,0 +1,17 @@
+// RUN: %clang_cl_asan -Od %s -Fe%t
+// RUN: %env_asan_opts=handle_sigill=1 not %run %t 2>&1 | FileCheck %s
+
+// Test the error output from an illegal instruction.
+
+#include <stdio.h>
+
+int main() {
+  puts("before ud2a");
+  fflush(stdout);
+  __builtin_trap();
+  return 0;
+}
+// CHECK: before ud2a
+// CHECK: ERROR: AddressSanitizer: illegal-instruction on unknown address [[ADDR:0x[^ ]*]]
+// CHECK-SAME: (pc [[ADDR]] {{.*}})
+// CHECK-NEXT: #0 {{.*}} in main {{.*}}illegal_instruction.cpp:{{.*}}
diff --git a/compiler-rt/test/asan/TestCases/Windows/integer_divide_by_zero.cpp b/compiler-rt/test/asan/TestCases/Windows/integer_divide_by_zero.cpp
new file mode 100644 (file)
index 0000000..9a0f97a
--- /dev/null
@@ -0,0 +1,18 @@
+// RUN: %clang_cl_asan -Od %s -Fe%t
+// RUN: %env_asan_opts=handle_sigfpe=1 not %run %t 2>&1 | FileCheck %s
+
+// Test the error output from dividing by zero.
+
+#include <stdio.h>
+
+int main() {
+  puts("before ud2a");
+  fflush(stdout);
+  volatile int numerator = 1;
+  volatile int divisor = 0;
+  return numerator / divisor;
+}
+// CHECK: before ud2a
+// CHECK: ERROR: AddressSanitizer: int-divide-by-zero on unknown address [[ADDR:0x[^ ]*]]
+// CHECK-SAME: (pc [[ADDR]] {{.*}})
+// CHECK-NEXT: #0 {{.*}} in main {{.*}}integer_divide_by_zero.cpp:{{.*}}
diff --git a/compiler-rt/test/asan/TestCases/Windows/sse_misalignment.cpp b/compiler-rt/test/asan/TestCases/Windows/sse_misalignment.cpp
new file mode 100644 (file)
index 0000000..9f7e230
--- /dev/null
@@ -0,0 +1,28 @@
+// RUN: %clang_cl_asan -Od %s -Fe%t
+// RUN: %env_asan_opts=handle_sigfpe=1 not %run %t 2>&1 | FileCheck %s
+
+// Test the error output from misaligned SSE2 memory access. This is a READ
+// memory access. Windows appears to always provide an address of -1 for these
+// types of faults, and there doesn't seem to be a way to distinguish them from
+// other types of access violations without disassembling.
+
+#include <emmintrin.h>
+#include <stdio.h>
+
+__m128i test() {
+  char buffer[17] = {};
+  __m128i a = _mm_load_si128((__m128i *)buffer);
+  __m128i b = _mm_load_si128((__m128i *)(&buffer[0] + 1));
+  return _mm_or_si128(a, b);
+}
+
+int main() {
+  puts("before alignment fault");
+  fflush(stdout);
+  volatile __m128i v = test();
+  return 0;
+}
+// CHECK: before alignment fault
+// CHECK: ERROR: AddressSanitizer: access-violation on unknown address {{0x[fF]*}}
+// CHECK-NEXT: The signal is caused by a READ memory access.
+// CHECK-NEXT: #0 {{.*}} in test(void) {{.*}}misalignment.cpp:{{.*}}