[ASan Win] Simplify and improve the way we forward ASan interface calls from DLLs

[kcc]

LGTM
Clever!

[timurrrr]

Committed as r209210, thanks!

[rnk]

lib/asan/asan_dll_thunk.cc

173

Do you guys worry about concurrent calls to __asan_init_v3, or can you always initialize before threads are created?

[timurrrr]

I've added a comment about threading in r209414.

2014-05-21 0:58 GMT+04:00 Reid Kleckner <[e-mail]>:

Comment at: lib/asan/asan_dll_thunk.cc:173
@@ +172,3 @@
+ static fntype fn = 0;
+ if (fn) return;

+

Do you guys worry about concurrent calls to __asan_init_v3, or can you always initialize before threads are created?

http://reviews.llvm.org/D3848

lib/asan/CMakeLists.txt

       add_dependencies(asan clang_rt.asan_cxx-${arch}-symbols)
       add_sanitizer_rt_symbols(clang_rt.asan-${arch} asan.syms.extra)
       add_dependencies(asan clang_rt.asan-${arch}-symbols)
     endif()
 
     if (WIN32)
       add_compiler_rt_runtime(clang_rt.asan_dll_thunk-${arch} ${arch} STATIC
         SOURCES asan_dll_thunk.cc
+                $<TARGET_OBJECTS:RTInterception.${arch}>
         CFLAGS ${ASAN_CFLAGS} -DASAN_DLL_THUNK
         DEFS ${ASAN_COMMON_DEFINITIONS})
       add_dependencies(asan clang_rt.asan_dll_thunk-${arch})
     endif()
   endforeach()
 endif()
 
 add_compiler_rt_resource_file(asan_blacklist asan_blacklist.txt)
 add_dependencies(asan asan_blacklist)
 add_dependencies(compiler-rt asan)
 
 add_subdirectory(scripts)
 
 if(COMPILER_RT_INCLUDE_TESTS)
   add_subdirectory(tests)
 endif()

lib/asan/asan_dll_thunk.cc

 // See https://code.google.com/p/address-sanitizer/issues/detail?id=209 for the
 // details.
 //===----------------------------------------------------------------------===//
 
 // Only compile this code when buidling asan_dll_thunk.lib
 // Using #ifdef rather than relying on Makefiles etc.
 // simplifies the build procedure.
 #ifdef ASAN_DLL_THUNK
+#include "sanitizer_common/sanitizer_interception.h"
 
 // ----------------- Helper functions and macros --------------------- {{{1
 extern "C" {
 void *__stdcall GetModuleHandleA(const char *module_name);
 void *__stdcall GetProcAddress(void *module, const char *proc_name);
 void abort();
 }
 
#define WRAP_W_WWWWWW(name)                                                    \
   extern "C" void *name(void *arg1, void *arg2, void *arg3, void *arg4,        \
                         void *arg5, void *arg6) {                              \
     typedef void *(*fntype)(void *, void *, void *, void *, void *, void *);   \
     static fntype fn = (fntype)getRealProcAddressOrDie(#name);                 \
     return fn(arg1, arg2, arg3, arg4, arg5, arg6);                             \
   }
 // }}}
 
+// --------- Interface interception helper functions and macros ----------- {{{1
+// We need to intercept the ASan interface exported by the DLL thunk and forward
+// all the functions to the runtime in the main module.
+// However, we don't want to keep two lists of interface functions.
+// To avoid that, the list of interface functions should be defined using the
+// INTERFACE_FUNCTION macro. Then, all the interface can be intercepted at once
+// by calling INTERCEPT_ASAN_INTERFACE().
+
+// Use macro+template magic to automatically generate the list of interface
+// functions.  Each interface function at line LINE defines a template class
+// with a static InterfaceInteceptor<LINE>::Execute() method intercepting the
+// function.  The default implementation of InterfaceInteceptor<LINE> is to call
+// the Execute() method corresponding to the previous line.
+template<int LINE>
+struct InterfaceInteceptor {
+  static void Execute() { InterfaceInteceptor<LINE-1>::Execute(); }
+};
+
+// There shouldn't be any interface function with negative line number.
+template<>
+struct InterfaceInteceptor<0> {
+  static void Execute() {}
+};
+
+#define INTERFACE_FUNCTION(name)                                               \
+  extern "C" void name() { __debugbreak(); }                                   \
+  template<> struct InterfaceInteceptor<__LINE__> {                            \
+    static void Execute() {                                                    \
+      void *wrapper = getRealProcAddressOrDie(#name);                          \
+      if (!__interception::OverrideFunction((uptr)name, (uptr)wrapper, 0))     \
+        abort();                                                               \
+      InterfaceInteceptor<__LINE__-1>::Execute();                              \
+    }                                                                          \
+  };
+
+// INTERCEPT_ASAN_INTERFACE must be used after the last INTERFACE_FUNCTION.
+#define INTERCEPT_ASAN_INTERFACE InterfaceInteceptor<__LINE__>::Execute
+
+static void InterceptASanInterface();
+// }}}
+
 // ----------------- ASan own interface functions --------------------
+// Don't use the INTERFACE_FUNCTION machinery for this function as we actually
+// want to call it in the __asan_init interceptor.
 WRAP_W_V(__asan_should_detect_stack_use_after_return)
 
 extern "C" {
   int __asan_option_detect_stack_use_after_return;
 
   // Manually wrap __asan_init as we need to initialize
   // __asan_option_detect_stack_use_after_return afterwards.
   void __asan_init_v3() {
     typedef void (*fntype)();
-    static fntype fn = (fntype)getRealProcAddressOrDie("__asan_init_v3");
+    static fntype fn = 0;
+    if (fn) return;
+
+    fn = (fntype)getRealProcAddressOrDie("__asan_init_v3");
     fn();
     __asan_option_detect_stack_use_after_return =
         (__asan_should_detect_stack_use_after_return() != 0);
+
+    InterceptASanInterface();
   }
 }
 
-WRAP_V_V(__asan_handle_no_return)
+INTERFACE_FUNCTION(__asan_handle_no_return)
 
-WRAP_V_W(__asan_report_store1)
-WRAP_V_W(__asan_report_store2)
-WRAP_V_W(__asan_report_store4)
-WRAP_V_W(__asan_report_store8)
-WRAP_V_W(__asan_report_store16)
-WRAP_V_WW(__asan_report_store_n)
-
-WRAP_V_W(__asan_report_load1)
-WRAP_V_W(__asan_report_load2)
-WRAP_V_W(__asan_report_load4)
-WRAP_V_W(__asan_report_load8)
-WRAP_V_W(__asan_report_load16)
-WRAP_V_WW(__asan_report_load_n)
-
-WRAP_W_WWW(__asan_memcpy);
-WRAP_W_WWW(__asan_memset);
-WRAP_W_WWW(__asan_memmove);
-
-WRAP_V_WW(__asan_register_globals)
-WRAP_V_WW(__asan_unregister_globals)
-
-WRAP_V_W(__asan_before_dynamic_init)
-WRAP_V_V(__asan_after_dynamic_init)
-
-WRAP_V_WW(__asan_poison_stack_memory)
-WRAP_V_WW(__asan_unpoison_stack_memory)
-
-WRAP_V_WW(__asan_poison_memory_region)
-WRAP_V_WW(__asan_unpoison_memory_region)
-
-WRAP_W_V(__asan_get_current_fake_stack)
-WRAP_W_WWWW(__asan_addr_is_in_fake_stack)
-
-WRAP_W_WW(__asan_stack_malloc_0)
-WRAP_W_WW(__asan_stack_malloc_1)
-WRAP_W_WW(__asan_stack_malloc_2)
-WRAP_W_WW(__asan_stack_malloc_3)
-WRAP_W_WW(__asan_stack_malloc_4)
-WRAP_W_WW(__asan_stack_malloc_5)
-WRAP_W_WW(__asan_stack_malloc_6)
-WRAP_W_WW(__asan_stack_malloc_7)
-WRAP_W_WW(__asan_stack_malloc_8)
-WRAP_W_WW(__asan_stack_malloc_9)
-WRAP_W_WW(__asan_stack_malloc_10)
-
-WRAP_V_WWW(__asan_stack_free_0)
-WRAP_V_WWW(__asan_stack_free_1)
-WRAP_V_WWW(__asan_stack_free_2)
-WRAP_V_WWW(__asan_stack_free_4)
-WRAP_V_WWW(__asan_stack_free_5)
-WRAP_V_WWW(__asan_stack_free_6)
-WRAP_V_WWW(__asan_stack_free_7)
-WRAP_V_WWW(__asan_stack_free_8)
-WRAP_V_WWW(__asan_stack_free_9)
-WRAP_V_WWW(__asan_stack_free_10)
+INTERFACE_FUNCTION(__asan_report_store1)
+INTERFACE_FUNCTION(__asan_report_store2)
+INTERFACE_FUNCTION(__asan_report_store4)
+INTERFACE_FUNCTION(__asan_report_store8)
+INTERFACE_FUNCTION(__asan_report_store16)
+INTERFACE_FUNCTION(__asan_report_store_n)
+
+INTERFACE_FUNCTION(__asan_report_load1)
+INTERFACE_FUNCTION(__asan_report_load2)
+INTERFACE_FUNCTION(__asan_report_load4)
+INTERFACE_FUNCTION(__asan_report_load8)
+INTERFACE_FUNCTION(__asan_report_load16)
+INTERFACE_FUNCTION(__asan_report_load_n)
+
+INTERFACE_FUNCTION(__asan_memcpy);
+INTERFACE_FUNCTION(__asan_memset);
+INTERFACE_FUNCTION(__asan_memmove);
+
+INTERFACE_FUNCTION(__asan_register_globals)
+INTERFACE_FUNCTION(__asan_unregister_globals)
+
+INTERFACE_FUNCTION(__asan_before_dynamic_init)
+INTERFACE_FUNCTION(__asan_after_dynamic_init)
+
+INTERFACE_FUNCTION(__asan_poison_stack_memory)
+INTERFACE_FUNCTION(__asan_unpoison_stack_memory)
+
+INTERFACE_FUNCTION(__asan_poison_memory_region)
+INTERFACE_FUNCTION(__asan_unpoison_memory_region)
+
+INTERFACE_FUNCTION(__asan_get_current_fake_stack)
+INTERFACE_FUNCTION(__asan_addr_is_in_fake_stack)
+
+INTERFACE_FUNCTION(__asan_stack_malloc_0)
+INTERFACE_FUNCTION(__asan_stack_malloc_1)
+INTERFACE_FUNCTION(__asan_stack_malloc_2)
+INTERFACE_FUNCTION(__asan_stack_malloc_3)
+INTERFACE_FUNCTION(__asan_stack_malloc_4)
+INTERFACE_FUNCTION(__asan_stack_malloc_5)
+INTERFACE_FUNCTION(__asan_stack_malloc_6)
+INTERFACE_FUNCTION(__asan_stack_malloc_7)
+INTERFACE_FUNCTION(__asan_stack_malloc_8)
+INTERFACE_FUNCTION(__asan_stack_malloc_9)
+INTERFACE_FUNCTION(__asan_stack_malloc_10)
+
+INTERFACE_FUNCTION(__asan_stack_free_0)
+INTERFACE_FUNCTION(__asan_stack_free_1)
+INTERFACE_FUNCTION(__asan_stack_free_2)
+INTERFACE_FUNCTION(__asan_stack_free_4)
+INTERFACE_FUNCTION(__asan_stack_free_5)
+INTERFACE_FUNCTION(__asan_stack_free_6)
+INTERFACE_FUNCTION(__asan_stack_free_7)
+INTERFACE_FUNCTION(__asan_stack_free_8)
+INTERFACE_FUNCTION(__asan_stack_free_9)
+INTERFACE_FUNCTION(__asan_stack_free_10)
 
 // TODO(timurrrr): Add more interface functions on the as-needed basis.
 
 // ----------------- Memory allocation functions ---------------------
 WRAP_V_W(free)
 WRAP_V_WW(_free_dbg)
 
 WRAP_W_W(malloc)

 WRAP_W_W(_expand)
 WRAP_W_W(_expand_dbg)
 
 // TODO(timurrrr): Might want to add support for _aligned_* allocation
 // functions to detect a bit more bugs.  Those functions seem to wrap malloc().
 
 // TODO(timurrrr): Do we need to add _Crt* stuff here? (see asan_malloc_win.cc).
 
+  void InterceptASanInterface() {
+    INTERCEPT_ASAN_INTERFACE();
+  }
+
 #endif // ASAN_DLL_THUNK

test/asan/TestCases/Windows/dll_malloc_left_oob.cc

 // RUN: %clangxx_asan -O0 %p/dll_host.cc -Fe%t
 // RUN: %clangxx_asan -LD -O0 %s -Fe%t.dll
 // FIXME: 'cat' is needed due to PR19744.
 // RUN: not %run %t %t.dll 2>&1 | cat | FileCheck %s
 
 #include <malloc.h>
 extern "C" __declspec(dllexport)
 int test_function() {
   char *buffer = (char*)malloc(42);
   buffer[-1] = 42;
 // CHECK: AddressSanitizer: heap-buffer-overflow on address [[ADDR:0x[0-9a-f]+]]
 // CHECK: WRITE of size 1 at [[ADDR]] thread T0
-// CHECK:      test_function {{.*}}dll_malloc_left_oob.cc:[[@LINE-3]]
+// CHECK-NEXT: test_function {{.*}}dll_malloc_left_oob.cc:[[@LINE-3]]
 // CHECK-NEXT: main {{.*}}dll_host.cc
+//
 // CHECK: [[ADDR]] is located 1 bytes to the left of 42-byte region
 // CHECK-LABEL: allocated by thread T0 here:
 // CHECK:        malloc
-// CHECK:        test_function {{.*}}dll_malloc_left_oob.cc:[[@LINE-9]]
+// CHECK:        test_function {{.*}}dll_malloc_left_oob.cc:[[@LINE-10]]
 // CHECK-NEXT:   main {{.*}}dll_host.cc
 // CHECK-LABEL: SUMMARY
   free(buffer);
   return 0;
 }

test/asan/TestCases/Windows/dll_malloc_uaf.cc

 // RUN: %clangxx_asan -O0 %p/dll_host.cc -Fe%t
 // RUN: %clangxx_asan -LD -O0 %s -Fe%t.dll
 // FIXME: 'cat' is needed due to PR19744.
 // RUN: not %run %t %t.dll 2>&1 | cat | FileCheck %s
 
 #include <malloc.h>
 
 extern "C" __declspec(dllexport)
 int test_function() {
-  char *buffer = (char*)malloc(42);
+  int *buffer = (int*)malloc(42);
   free(buffer);
   buffer[0] = 42;
 // CHECK: AddressSanitizer: heap-use-after-free on address [[ADDR:0x[0-9a-f]+]]
-// CHECK: WRITE of size 1 at [[ADDR]] thread T0
-// CHECK:       test_function {{.*}}dll_malloc_uaf.cc:[[@LINE-3]]
+// CHECK: WRITE of size 4 at [[ADDR]] thread T0
+// CHECK-NEXT:  test_function {{.*}}dll_malloc_uaf.cc:[[@LINE-3]]
 // CHECK-NEXT:  main {{.*}}dll_host
+//
 // CHECK: [[ADDR]] is located 0 bytes inside of 42-byte region
 // CHECK-LABEL: freed by thread T0 here:
 // CHECK:       free
-// CHECK:       test_function {{.*}}dll_malloc_uaf.cc:[[@LINE-9]]
+// CHECK:       test_function {{.*}}dll_malloc_uaf.cc:[[@LINE-10]]
 // CHECK-NEXT:  main {{.*}}dll_host
+//
 // CHECK-LABEL: previously allocated by thread T0 here:
 // CHECK:       malloc
-// CHECK:       test_function {{.*}}dll_malloc_uaf.cc:[[@LINE-14]]
+// CHECK:       test_function {{.*}}dll_malloc_uaf.cc:[[@LINE-16]]
 // CHECK-NEXT:  main {{.*}}dll_host
   return 0;
 }

test/asan/TestCases/Windows/dll_noreturn.cc

 // RUN: %clangxx_asan -O0 %p/dll_host.cc -Fe%t
 // RUN: %clangxx_asan -LD -O0 %s -Fe%t.dll
 // FIXME: 'cat' is needed due to PR19744.
 // RUN: not %run %t %t.dll 2>&1 | cat | FileCheck %s
 
 #include <process.h>
 
 void noreturn_f() {
   int subscript = -1;
   char buffer[42];
   buffer[subscript] = 42;
   _exit(1);
 // CHECK: AddressSanitizer: stack-buffer-overflow on address [[ADDR:0x[0-9a-f]+]]
 // CHECK: WRITE of size 1 at [[ADDR]] thread T0
-// CHECK:       noreturn_f {{.*}}dll_noreturn.cc:[[@LINE-4]]
+// CHECK-NEXT:  noreturn_f {{.*}}dll_noreturn.cc:[[@LINE-4]]
 // CHECK-NEXT:  test_function {{.*}}dll_noreturn.cc
 // CHECK-NEXT:  main {{.*}}dll_host.cc
+//
 // CHECK: Address [[ADDR]] is located in stack of thread T0 at offset [[OFFSET:.*]] in frame
 // CHECK-NEXT:  noreturn_f {{.*}}dll_noreturn.cc
 // CHECK: 'buffer' <== Memory access at offset [[OFFSET]] underflows this variable
 // CHECK-LABEL: SUMMARY
 }
 
 extern "C" __declspec(dllexport)
 int test_function() {
   noreturn_f();
   return 0;
 }

test/asan/TestCases/Windows/dll_poison_unpoison.cc

int test_function() {
   __asan_poison_memory_region(buffer, 128);
   should_not_crash(&buffer[192]);
   __asan_unpoison_memory_region(buffer, 64);
   should_not_crash(&buffer[32]);
 
   should_crash(&buffer[96]);
 // CHECK: AddressSanitizer: use-after-poison on address [[ADDR:0x[0-9a-f]+]]
 // CHECK-NEXT: WRITE of size 1 at [[ADDR]] thread T0
-// CHECK:      should_crash {{.*}}\dll_poison_unpoison.cc
+// CHECK-NEXT: should_crash {{.*}}\dll_poison_unpoison.cc
 // CHECK-NEXT: test_function {{.*}}\dll_poison_unpoison.cc:[[@LINE-4]]
 // CHECK-NEXT: main
+//
 // CHECK: [[ADDR]] is located in stack of thread T0 at offset [[OFFSET:.*]] in frame
 // CHECK-NEXT: test_function {{.*}}\dll_poison_unpoison.cc
 // CHECK: 'buffer' <== Memory access at offset [[OFFSET]] is inside this variable
   return 0;
 }

test/asan/TestCases/Windows/dll_stack_use_after_return.cc

 }
 
 extern "C" __declspec(dllexport)
 int test_function() {
   foo();
   *x = 42;
 // CHECK: AddressSanitizer: stack-use-after-return
 // CHECK: WRITE of size 1 at [[ADDR:.*]] thread T0
-// CHECK:       test_function {{.*}}dll_stack_use_after_return.cc:[[@LINE-3]]
+// CHECK-NEXT:  test_function {{.*}}dll_stack_use_after_return.cc:[[@LINE-3]]
 // CHECK-NEXT:  main
+//
 // CHECK: Address [[ADDR]] is located in stack of thread T0 at offset [[OFFSET:.*]] in frame
 // CHECK-NEXT: #0 {{.*}} foo {{.*}}dll_stack_use_after_return.cc
 // CHECK: 'stack_buffer' <== Memory access at offset [[OFFSET]] is inside this variable
   return 0;
 }
 

test/asan/TestCases/Windows/dll_thread_stack_array_left_oob.cc

 // RUN: %clangxx_asan -O0 %p/dll_host.cc -Fe%t
 // RUN: %clangxx_asan -LD -O0 %s -Fe%t.dll
 // FIXME: 'cat' is needed due to PR19744.
 // RUN: not %run %t %t.dll 2>&1 | cat | FileCheck %s
 
 #include <windows.h>
 #include <malloc.h>
 
 DWORD WINAPI thread_proc(void *context) {
   int subscript = -1;
   char stack_buffer[42];
   stack_buffer[subscript] = 42;
 // CHECK: AddressSanitizer: stack-buffer-overflow on address [[ADDR:0x[0-9a-f]+]]
 // CHECK: WRITE of size 1 at [[ADDR]] thread T1
-// CHECK:   thread_proc {{.*}}dll_thread_stack_array_left_oob.cc:[[@LINE-3]]
+// CHECK-NEXT:  thread_proc {{.*}}dll_thread_stack_array_left_oob.cc:[[@LINE-3]]
+//
 // CHECK: Address [[ADDR]] is located in stack of thread T1 at offset [[OFFSET:.*]] in frame
-// CHECK:   thread_proc {{.*}}dll_thread_stack_array_left_oob.cc
+// CHECK-NEXT:  thread_proc {{.*}}dll_thread_stack_array_left_oob.cc
+//
 // CHECK: 'stack_buffer' <== Memory access at offset [[OFFSET]] underflows this variable
 
   return 0;
 }
 
 extern "C" __declspec(dllexport)
 int test_function() {
   HANDLE thr = CreateThread(NULL, 0, thread_proc, NULL, 0, NULL);