Support for __asan_apply_to_globals in ASAN runtime library

[rgov]

Summary

These are the changes to the ASAN runtime library that are needed for D16737: Support for dead code stripping on Mach-O platforms.

[samsonov]

Please update ASan ABI version in lib/asan/asan_init_version.h. See the comment about test you might want to add in http://reviews.llvm.org/D16737

[rgov]

Update ABI version per @samsonov.

[rgov]

Add a test case.

[rgov]

Update comment in test case to be more accurate.

[glider]

LGTM

[glider]

Um, by the way, does the test pass without the LLVM-side change? I suspect no.

[rgov]

Thanks for reviewing. It does need the LLVM change first. So I will wait to commit until after D16737: Support for dead code stripping on Mach-O platforms is approved.

[samsonov]

LGTM

lib/asan/asan_init_version.h

33

This will be v8.

test/asan/TestCases/Darwin/dead-strip.c

22

You can avoid using custom pragmas by smth. like

int main(int argc, char *argv[]) {

alive[argc] = 0;
return 0;

}

[rgov]

I discovered that this change was incomplete due to duplicate registration of globals. I've changed this to use a per-library flag that indicates whether globals have been registered already, and ignores duplicate calls.

In the future I'd like to investigate simply eliding the unnecessary ctors, but this fix basically makes them harmless, in the same way that multiple calls to __asan_init do not cause problems.

This also happens to resolve the expected failure of the initialization-bug.cc test case. I do not know if a pathological binary could still execute a ctor that accesses globals before they are registered.

[samsonov]

lib/asan/asan_globals.cc

292

Let's just make two functions instead of passing one or the other callback here.
I agree that you can use the same "flag" here: first function will set it from false to true, another one - from true to false.

WDYT?

[rgov]

lib/asan/asan_globals.cc

292

I have no preference, other than that it is already written as-is :)

I'm not sure about calling it register_module_globals since module seems to mean translation unit and not shared object—unless I'm mistaken.

[samsonov]

lib/asan/asan_globals.cc

292

Yes, "module" has way too many meanings :), "library"?
I'd actually prefer to have two different functions, as this would make the intent cleaner. Sorry for bringing it up that late in the review process :/

[rgov]

Split into two functions per @samsonov's request. Just went with asan_register_module_globals and we can decide to rename it later if we care to.

[samsonov]

lib/asan/asan_init_version.h

32

Please fix a comment

lib/asan/asan_internal.h

77

It's not really a flag, it's a library/executable marker. You may clarify the way it's used here. E.g. why is it uptr*, not void*?

test/asan/TestCases/Darwin/dead-strip.c

22

So, is it possible to make this test pragma-less? In this way we will also ensure that this invalid load will not be optimized away by the compiler (i.e. run this test with -O3 in addition to -O0).

[rgov]

Committed as r264644.

lib/asan/asan_globals.cc

void StopInitOrderChecking() {
   }
 }
 
 } // namespace __asan
 
 // ---------------------- Interface ---------------- {{{1
 using namespace __asan;  // NOLINT
 
+
+// Apply __asan_register_globals to all globals found in the same loaded
+// shared object as `flag'. The flag tracks whether globals have already been
+// registered or not for this shared object.
+void __asan_register_module_globals(uptr *flag) {
+  if (*flag)
+    return;
+  AsanApplyToGlobals(__asan_register_globals, flag);
+  *flag = 1; // indicate that globals have been registered
+}
+
+// This mirrors __asan_register_module_globals.
+void __asan_unregister_module_globals(uptr *flag) {
+  if (!*flag)
+    return;
+  AsanApplyToGlobals(__asan_unregister_globals, flag);
+  *flag = 0;
+}
+
 // Register an array of globals.
 void __asan_register_globals(__asan_global *globals, uptr n) {
   if (!flags()->report_globals) return;
   GET_STACK_TRACE_MALLOC;
   u32 stack_id = StackDepotPut(stack);
   BlockingMutexLock lock(&mu_for_globals);
   if (!global_registration_site_vector)
     global_registration_site_vector =

lib/asan/asan_init_version.h

 
 extern "C" {
   // Every time the ASan ABI changes we also change the version number in the
   // __asan_init function name.  Objects built with incompatible ASan ABI
   // versions will not link with run-time.
   // Changes between ABI versions:
   // v1=>v2: added 'module_name' to __asan_global
   // v2=>v3: stack frame description (created by the compiler)
-  //         contains the function PC as the 3-rd field (see
+  //         contains the function PC as the 3rd field (see
   //         DescribeAddressIfStack).
   // v3=>v4: added '__asan_global_source_location' to __asan_global.
   // v4=>v5: changed the semantics and format of __asan_stack_malloc_ and
   //         __asan_stack_free_ functions.
   // v5=>v6: changed the name of the version check symbol
   // v6=>v7: added 'odr_indicator' to __asan_global.
-  #define __asan_version_mismatch_check __asan_version_mismatch_check_v7
+  // v7=>v8: added '__asan_apply_to_globals' function (for dead stripping
+  //         support on Mach-O platforms)
+  #define __asan_version_mismatch_check __asan_version_mismatch_check_v8
 }
 
 #endif  // ASAN_INIT_VERSION_H

lib/asan/asan_interface_internal.h

struct __asan_global {
     const char *module_name; // Module name as a C string. This pointer is a
                              // unique identifier of a module.
     uptr has_dynamic_init;   // Non-zero if the global has dynamic initializer.
     __asan_global_source_location *location;  // Source location of a global,
                                               // or NULL if it is unknown.
     uptr odr_indicator;      // The address of the ODR indicator symbol.
   };
 
+  // These functions can be called on some platforms to find globals in the same
+  // loaded shared object as `flag' and apply __asan_(un)register_globals
+  // to them, filtering out redundant calls.
+  SANITIZER_INTERFACE_ATTRIBUTE
+  void __asan_register_module_globals(uptr *flag);
+  SANITIZER_INTERFACE_ATTRIBUTE
+  void __asan_unregister_module_globals(uptr *flag);
+
   // These two functions should be called by the instrumented code.
   // 'globals' is an array of structures describing 'n' globals.
   SANITIZER_INTERFACE_ATTRIBUTE
   void __asan_register_globals(__asan_global *globals, uptr n);
   SANITIZER_INTERFACE_ATTRIBUTE
   void __asan_unregister_globals(__asan_global *globals, uptr n);
 
   // These two functions should be called before and after dynamic initializers

lib/asan/asan_internal.h

 // asan_malloc_linux.cc / asan_malloc_mac.cc
 void ReplaceSystemMalloc();
 
 // asan_linux.cc / asan_mac.cc / asan_win.cc
 void *AsanDoesNotSupportStaticLinkage();
 void AsanCheckDynamicRTPrereqs();
 void AsanCheckIncompatibleRT();
 
+typedef void (*globals_op_fptr)(__asan_global *, uptr);
+void AsanApplyToGlobals(globals_op_fptr op, uptr *flag);
+
 void AsanOnDeadlySignal(int, void *siginfo, void *context);
 
 void ReadContextStack(void *context, uptr *stack, uptr *ssize);
 void StopInitOrderChecking();
 
 // Wrapper for TLS/TSD.
 void AsanTSDInit(void (*destructor)(void *tsd));
 void *AsanTSDGet();

lib/asan/asan_linux.cc

 
 void InitializePlatformInterceptors() {}
 
 void *AsanDoesNotSupportStaticLinkage() {
   // This will fail to link with -static.
   return &_DYNAMIC;  // defined in link.h
 }
 
+void AsanApplyToGlobals(globals_op_fptr op, uptr *flag) {
+  UNIMPLEMENTED();
+}
+
 #if SANITIZER_ANDROID
 // FIXME: should we do anything for Android?
 void AsanCheckDynamicRTPrereqs() {}
 void AsanCheckIncompatibleRT() {}
 #else
 static int FindFirstDSOCallback(struct dl_phdr_info *info, size_t size,
                                 void *data) {
   // Continue until the first dynamic library is found

lib/asan/asan_mac.cc

 #include "asan_internal.h"
 #include "asan_mapping.h"
 #include "asan_stack.h"
 #include "asan_thread.h"
 #include "sanitizer_common/sanitizer_atomic.h"
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_mac.h"
 
+#include <dlfcn.h>
 #include <fcntl.h>
 #include <libkern/OSAtomic.h>
 #include <mach-o/dyld.h>
+#include <mach-o/getsect.h>
 #include <mach-o/loader.h>
 #include <pthread.h>
 #include <stdlib.h>  // for free()
 #include <sys/mman.h>
 #include <sys/resource.h>
 #include <sys/sysctl.h>
 #include <sys/ucontext.h>
 #include <unistd.h>

 }
 
 // No-op. Mac does not support static linkage anyway.
 void AsanCheckDynamicRTPrereqs() {}
 
 // No-op. Mac does not support static linkage anyway.
 void AsanCheckIncompatibleRT() {}
 
+void AsanApplyToGlobals(globals_op_fptr op, uptr *flag) {
+  // Find the Mach-O header for the image containing the flag
+  Dl_info info;
+  int err = dladdr(flag, &info);
+  if (err == 0) return;
+
+#if __LP64__
+  const struct mach_header_64 *mh = (struct mach_header_64 *)info.dli_fbase;
+#else
+  const struct mach_header *mh = (struct mach_header *)info.dli_fbase;
+#endif
+
+  // Look up the __asan_globals section in that image and register its globals
+  unsigned long size = 0;
+  __asan_global *globals = (__asan_global *)getsectiondata(
+      mh,
+      "__DATA", "__asan_globals",
+      &size);
+
+  if (!globals) return;
+  if (size % sizeof(__asan_global) != 0) return;
+  op(globals, size / sizeof(__asan_global));
+}
+
 void ReadContextStack(void *context, uptr *stack, uptr *ssize) {
   UNIMPLEMENTED();
 }
 
 // Support for the following functions from libdispatch on Mac OS:
 //   dispatch_async_f()
 //   dispatch_async()
 //   dispatch_sync_f()

lib/asan/asan_win.cc

void InitializePlatformInterceptors() {
 
   // NtWaitForWorkViaWorkerFactory is always linked dynamically.
   CHECK(::__interception::OverrideFunction(
       "NtWaitForWorkViaWorkerFactory",
       (uptr)WRAP(NtWaitForWorkViaWorkerFactory),
       (uptr *)&REAL(NtWaitForWorkViaWorkerFactory)));
 }
 
+void AsanApplyToGlobals(globals_op_fptr op, uptr *flag) {
+  UNIMPLEMENTED();
+}
+
 // ---------------------- TSD ---------------- {{{
 static bool tsd_key_inited = false;
 
 static __declspec(thread) void *fake_tsd = 0;
 
 void AsanTSDInit(void (*destructor)(void *tsd)) {
   // FIXME: we're ignoring the destructor for now.
   tsd_key_inited = true;

test/asan/TestCases/Darwin/dead-strip.c

+// Test that AddressSanitizer does not re-animate dead globals when dead
+// stripping is turned on.
+//
+// This test verifies that an out-of-bounds access on a global variable is
+// detected after dead stripping has been performed. This proves that the
+// runtime is able to register globals in the __DATA,__asan_globals section.
+
+// RUN: %clang_asan -Xlinker -dead_strip -o %t %s
+// RUN: llvm-nm -format=posix %t | FileCheck --check-prefix NM-CHECK %s
+// RUN: not %run %t 2>&1 | FileCheck --check-prefix ASAN-CHECK %s
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Warray-bounds"
+
+int alive[1] = {};
+int dead[1] = {};
+// NM-CHECK: {{^_alive }}
+// NM-CHECK-NOT: {{^_dead }}
+
+int main() {
+  alive[1] = 0;
+  // ASAN-CHECK: {{0x.* is located 0 bytes to the right of global variable}}
+  return 0;
+}
+
+#pragma clang diagnostic pop

test/asan/TestCases/initialization-bug.cc

 // Test to make sure basic initialization order errors are caught.
 
 // RUN: %clangxx_asan -O0 %s %p/Helpers/initialization-bug-extra2.cc -o %t-INIT-ORDER-EXE
 // RUN: %env_asan_opts=check_initialization_order=true not %run %t-INIT-ORDER-EXE 2>&1 | FileCheck %s
 
 // Do not test with optimization -- the error may be optimized away.
 
 // FIXME: https://code.google.com/p/address-sanitizer/issues/detail?id=186
-// XFAIL: darwin,win32
+// XFAIL: win32
 
 #include <cstdio>
 
 // The structure of the test is:
 // "x", "y", "z" are dynamically initialized globals.
 // Value of "x" depends on "y", value of "y" depends on "z".
 // "x" and "z" are defined in this TU, "y" is defined in another one.
 // Thus we shoud stably report initialization order fiasco independently of