[ASan/Win] Fix PR20918 -- SEH handler doesn't work with the MD runtime

llvm-svn: 217679

6 files changed, 103 insertions, 8 deletions

diff --git a/clang/lib/Driver/Tools.cpp b/clang/lib/Driver/Tools.cpp
index 7fb1a1a33c7..0e1dea2a97f 100644
--- a/clang/lib/Driver/Tools.cpp
+++ b/clang/lib/Driver/Tools.cpp
@@ -7822,6 +7822,9 @@ void visualstudio::Link::ConstructJob(Compilation &C, const JobAction &JA,
       addSanitizerRTWindows(getToolChain(), Args, CmdArgs, "asan_dynamic-i386");
       addSanitizerRTWindows(getToolChain(), Args, CmdArgs,
                             "asan_dynamic_runtime_thunk-i386");
+      // Make sure the dynamic runtime thunk is not optimized out at link time
+      // to ensure proper SEH handling.
+      CmdArgs.push_back(Args.MakeArgString("-include:___asan_seh_interceptor"));
     } else if (DLL) {
       addSanitizerRTWindows(getToolChain(), Args, CmdArgs,
                             "asan_dll_thunk-i386");
diff --git a/clang/test/Driver/cl-link.c b/clang/test/Driver/cl-link.c
index fa02a4cff0b..5bd20011796 100644
--- a/clang/test/Driver/cl-link.c
+++ b/clang/test/Driver/cl-link.c
@@ -25,6 +25,7 @@
 // ASAN-MD: "-incremental:no"
 // ASAN-MD: "{{.*}}clang_rt.asan_dynamic-i386.lib"
 // ASAN-MD: "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib"
+// ASAN-MD: "-include:___asan_seh_interceptor"
 // ASAN-MD: "{{.*}}cl-link{{.*}}.obj"
 
 // RUN: %clang_cl /LD -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
diff --git a/compiler-rt/lib/asan/asan_win.cc b/compiler-rt/lib/asan/asan_win.cc
index ade28c0d41b..5303d1bd1e9 100644
--- a/compiler-rt/lib/asan/asan_win.cc
+++ b/compiler-rt/lib/asan/asan_win.cc
@@ -89,7 +89,7 @@ void AsanOnSIGSEGV(int, void *siginfo, void *context) {
 
 static LPTOP_LEVEL_EXCEPTION_FILTER default_seh_handler;
 
-long WINAPI SEHHandler(EXCEPTION_POINTERS *info) {
+static long WINAPI SEHHandler(EXCEPTION_POINTERS *info) {
   EXCEPTION_RECORD *exception_record = info->ExceptionRecord;
   CONTEXT *context = info->ContextRecord;
   uptr pc = (uptr)exception_record->ExceptionAddress;
@@ -114,16 +114,39 @@ long WINAPI SEHHandler(EXCEPTION_POINTERS *info) {
   return default_seh_handler(info);
 }
 
-int SetSEHFilter() {
-  default_seh_handler = SetUnhandledExceptionFilter(SEHHandler);
+// We want to install our own exception handler (EH) to print helpful reports
+// on access violations and whatnot.  Unfortunately, the CRT initializers assume
+// they are run before any user code and drop any previously-installed EHs on
+// the floor, so we can't install our handler inside __asan_init.
+// (See crt0dat.c in the CRT sources for the details)
+//
+// Things get even more complicated with the dynamic runtime, as it finishes its
+// initialization before the .exe module CRT begins to initialize.
+//
+// For the static runtime (-MT), it's enough to put a callback to
+// __asan_set_seh_filter in the last section for C initializers.
+//
+// For the dynamic runtime (-MD), we want link the same
+// asan_dynamic_runtime_thunk.lib to all the modules, thus __asan_set_seh_filter
+// will be called for each instrumented module.  This ensures that at least one
+// __asan_set_seh_filter call happens after the .exe module CRT is initialized.
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE
+int __asan_set_seh_filter() {
+  // We should only store the previous handler if it's not our own handler in
+  // order to avoid loops in the EH chain.
+  auto prev_seh_handler = SetUnhandledExceptionFilter(SEHHandler);
+  if (prev_seh_handler != &SEHHandler)
+    default_seh_handler = prev_seh_handler;
   return 0;
 }
 
-// Put a pointer to SetSEHFilter at the end of the global list
-// of C initializers, after the default handler is set by the CRT.
-// See crt0dat.c in the CRT sources for the details.
+#if !ASAN_DYNAMIC
+// Put a pointer to __asan_set_seh_filter at the end of the global list
+// of C initializers, after the default EH is set by the CRT.
 #pragma section(".CRT$XIZ", long, read)  // NOLINT
-__declspec(allocate(".CRT$XIZ")) int (*__intercept_seh)() = SetSEHFilter;
+static __declspec(allocate(".CRT$XIZ"))
+    int (*__intercept_seh)() = __asan_set_seh_filter;
+#endif
 
 }  // namespace __asan
 
diff --git a/compiler-rt/lib/asan/asan_win_dynamic_runtime_thunk.cc b/compiler-rt/lib/asan/asan_win_dynamic_runtime_thunk.cc
index b1aec21e1fe..3a4de7dbf1f 100644
--- a/compiler-rt/lib/asan/asan_win_dynamic_runtime_thunk.cc
+++ b/compiler-rt/lib/asan/asan_win_dynamic_runtime_thunk.cc
@@ -15,7 +15,7 @@
 //
 // This includes:
 //  - forwarding the detect_stack_use_after_return runtime option
-//  - FIXME: installing a custom SEH handler (PR20918)
+//  - installing a custom SEH handler
 //
 //===----------------------------------------------------------------------===//
 
@@ -24,6 +24,7 @@
 // simplifies the build procedure.
 #ifdef ASAN_DYNAMIC_RUNTIME_THUNK
 extern "C" {
+__declspec(dllimport) int __asan_set_seh_filter();
 __declspec(dllimport) int __asan_should_detect_stack_use_after_return();
 
 // Define a copy of __asan_option_detect_stack_use_after_return that should be
@@ -38,5 +39,14 @@ __declspec(dllimport) int __asan_should_detect_stack_use_after_return();
 // constant after initialization anyways.
 int __asan_option_detect_stack_use_after_return =
     __asan_should_detect_stack_use_after_return();
+
+// Set the ASan-specific SEH handler at the end of CRT initialization of each
+// module (see asan_win.cc for the details).
+//
+// Unfortunately, putting a pointer to __asan_set_seh_filter into
+// __asan_intercept_seh gets optimized out, so we have to use an extra function.
+static int SetSEHFilter() { return __asan_set_seh_filter(); }
+#pragma section(".CRT$XIZ", long, read)  // NOLINT
+__declspec(allocate(".CRT$XIZ")) int (*__asan_seh_interceptor)() = SetSEHFilter;
 }
 #endif // ASAN_DYNAMIC_RUNTIME_THUNK
diff --git a/compiler-rt/test/asan/TestCases/Windows/dll_null_deref.cc b/compiler-rt/test/asan/TestCases/Windows/dll_null_deref.cc
new file mode 100644
index 00000000000..0fb18de2916
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/Windows/dll_null_deref.cc
@@ -0,0 +1,18 @@
+// RUN: %clang_cl_asan -O0 %p/dll_host.cc -Fe%t
+// RUN: %clang_cl_asan -LD -O0 %s -Fe%t.dll
+// RUN: not %run %t %t.dll 2>&1 | FileCheck %s
+
+__attribute__((noinline))
+static void NullDeref(int *ptr) {
+  // CHECK: ERROR: AddressSanitizer: access-violation on unknown address
+  // CHECK:   {{0x0*000.. .*pc 0x.*}}
+  ptr[10]++;  // BOOM
+}
+
+extern "C" __declspec(dllexport)
+int test_function() {
+  NullDeref((int*)0);
+  // CHECK: {{    #1 0x.* in test_function .*\dll_null_deref.cc:}}[[@LINE-1]]
+  // CHECK: AddressSanitizer can not provide additional info.
+  return 0;
+}
diff --git a/compiler-rt/test/asan/TestCases/Windows/null_deref_multiple_dlls.cc b/compiler-rt/test/asan/TestCases/Windows/null_deref_multiple_dlls.cc
new file mode 100644
index 00000000000..62fe544ae54
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/Windows/null_deref_multiple_dlls.cc
@@ -0,0 +1,40 @@
+// Make sure everything works even if the main module doesn't have any stack
+// variables, thus doesn't explicitly reference any symbol exported by the
+// runtime thunk.
+//
+// RUN: %clang_cl_asan -LD -O0 -DDLL1 %s -Fe%t1.dll
+// RUN: %clang_cl_asan -LD -O0 -DDLL2 %s -Fe%t2.dll
+// RUN: %clang_cl_asan -O0 -DEXE %s %t1.lib %t2.lib -Fe%t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+#include <malloc.h>
+#include <string.h>
+
+extern "C" {
+#if defined(EXE)
+__declspec(dllimport) void foo1();
+__declspec(dllimport) void foo2();
+
+int main() {
+  foo1();
+  foo2();
+}
+#elif defined(DLL1)
+__declspec(dllexport) void foo1() {}
+#elif defined(DLL2)
+__attribute__((noinline))
+static void NullDeref(int *ptr) {
+  // CHECK: ERROR: AddressSanitizer: access-violation on unknown address
+  // CHECK:   {{0x0*000.. .*pc 0x.*}}
+  ptr[10]++;  // BOOM
+}
+
+__declspec(dllexport) void foo2() {
+  NullDeref((int*)0);
+  // CHECK: {{    #1 0x.* in foo2.*null_deref_multiple_dlls.cc:}}[[@LINE-1]]
+  // CHECK: AddressSanitizer can not provide additional info.
+}
+#else
+# error oops!
+#endif
+}