Introduce new `disable_init` ASan option that is only supported on platforms where `SANITIZER_SUPPORTS_DISABLED_INIT` is true. Currently this is only supported on Darwin.

[delcypher]

Summary

The purpose of this option is provide a way for the ASan dylib
to be loaded via dlopen() without triggering most initialization
steps (e.g. shadow memory set up) that normally occur when the
ASan dylib is loaded.

This new functionality is exposed by

• A SANITIZER_SUPPORTS_INIT_FOR_DLOPEN macro which indicates if the feature is supported. This only true for Darwin currently.
• A HandleDlopenInit() function which should return true if the library is being loaded via dlopen() and SANITIZER_SUPPORTS_INIT_FOR_DLOPEN is supported. Platforms that support this may perform any initialization they wish inside this function.

Although disabling initialization is something that could potentially
apply to other sanitizers it appears to be unnecessary for other
sanitizers so this patch only makes the change for ASan.

rdar://problem/45284065

[krytarowski]

lib/asan/asan_flags.inc

164

Why to add a flag for it? As far as I understand it, we require this option to workaround some issue on Darwin and it's not needed so far by others. If I understanding removal of this property will break startup on Darwin.

If so, shouldn't this be just Darwin specific hardcoded property rather than generalized and unsupported option for everybody?

lib/asan/asan_internal.h

117

Shouldn't we add 'static inline'? Some compilers may complain that there is no prototype for a function.

[delcypher]

lib/asan/asan_flags.inc

164

We need a flag because we still need ASan's init to behave as it did previously in most scenarios (i.e. disable_init=0). It is only in the case where we want to perform dlopen() on the ASan dynamic library that we need to set disable_init=1.

Put another way, the problem we are trying to fix cannot be hard-coded because it is a dynamic property (how the ASan library is being loaded). I've made supporting this feature a static property guarded on SANITIZER_SUPPORTS_DISABLED_INIT which is only true on Darwin.

lib/asan/asan_internal.h

117

To be honest I'd rather stick the definition for non-Darwin platforms for in a .cc file but I wasn't sure where to put it so I just left it here.
If there isn't a better home for the non-Darwin implementation it I'll be happy to add static inline.

[krytarowski]

There is also start_deactivated in ASan.

It would help to show the crash example/log without this patch, in order to get more of the context.

[kubamracek]

Can this be triggered by a separate environment variable instead? That way we don't have to expose it in flags at all.

[delcypher]

In D54469#1296835, @krytarowski wrote:

There is also start_deactivated in ASan.

That's a different feature. That allows ASan to be partially initialized and then the rest of the initialization is delayed until later. I believe this exists for Android.

It would help to show the crash example/log without this patch, in order to get more of the context.

The test case shows how to trigger the crash today (see not --crash ... line). The context here is we are looking for a way to make it possible to dlopen() the ASan library from a non-asanified process so that specific functions inside it can be called. Today this isn't possible.

[delcypher]

In D54469#1296906, @kubamracek wrote:

Can this be triggered by a separate environment variable instead? That way we don't have to expose it in flags at all.

Sure. My original patch actually did this with a SanitizeInitIsDisabled() function which was platform specific. I thought moving it into flags was a bit cleaner and it also fixed a problem because some of the sanitizer's code doesn't behave that well if you skip parsing the flags.

[delcypher]

• Rename macro and support functions to explicitly mention dlopen.
• Introduce InitIsViaDlopen() function and remove the disable_init ASan option.

[delcypher]

@kubamracek Is this better?

[kubamracek]

lib/asan/asan_rtl.cc

403

Change this to print only under verbose mode.

lib/sanitizer_common/sanitizer_platform.h

348–352

Why do we need this? I'd remove it.

[delcypher]

Only report dlopen init when verbosity is non-zero.

[delcypher]

lib/sanitizer_common/sanitizer_platform.h

348–352

Two reasons:

1. I wanted the feature to be zero-cost for platforms that don't use it. You can see it's used in the if condition in AsanInitInternal(). It's a compile time constant so for platforms that don't support this feature the branch should just be folded away and so it won't even be checked at runtime.
2. We need to something to let us switch between the definitions of InitIsViaDlopen() and HandleDlopenInit(). This macro is used for this.

If you have a clean solution to handle the second reason without SANITIZER_SUPPORTS_INIT_FOR_DLOPEN then I can implement it.

[kubamracek]

lib/asan/asan_rtl.cc

404

Use VReport instead.

[kubamracek]

lib/sanitizer_common/sanitizer_platform.h

348–352

We're trying to minimize compile-time constants, as that adds complexity. Why not just have this behavior (reading an env var to skip initialization) on all platforms?

[delcypher]

Use VReport.

[delcypher]

lib/asan/asan_rtl.cc

404

Good call. I forgot about that macro.

[delcypher]

lib/sanitizer_common/sanitizer_platform.h

348–352

I don't think having this behaviour an all platforms is a good idea. Our implementation is Darwin specific and I don't think other platforms should have to care about this at all. I think it should be a platform maintainer's choice to allow/disallow disabling of the ASan init process. Given that I'm adding a new feature that is only relevant to Darwin right now I think the feature should be completely non-existent on other platforms. This is why I chose to introduce the SANITIZER_SUPPORTS_INIT_FOR_DLOPEN compile time constant.

Also if we were to make this feature work on all platforms then it would make more sense to put the option in ASAN_OPTIONS (where it was originally in this patch) which you asked me not to do.

[kcc]

I'd like Evgenii to take a look, since there is a similarity with what we do in Android.

[kcc]

I also don't understand how it works.
What happens if you do this

x = malloc()
dlopen("asan.so");
free(x)

This needs to be a) covered by test and b) explained in comments.

lib/asan/asan_internal.h

114–121

please avoid introducing new ifdefs. There are plenty of them already, but no reason to make things worse.

[eugenis]

Is this needed for the clients of that out-of-process allocator inspection thing? Could it be a separate library that links asan allocator in out-of-process mode, but not the rest? Otherwise you are adding a mode of execution to ASan when there is no shadow mapping, and now that's something that everyone else have to keep in mind.

[delcypher]

In D54469#1298802, @kcc wrote:

I also don't understand how it works.
What happens if you do this

x = malloc()
dlopen("asan.so");
free(x)

This needs to be a) covered by test and b) explained in comments.

When the ASan library gets loaded we don't want any of the interceptors to be used. The actual purpose of being to load the ASan library this way is simply so we gain access to the ASan malloc enumerator implementation.
So in the example above, both malloc() and free() are supposed to be using the same system allocator.

We want to enable doing this because this is how the memory analysis tools on macOS work. They freeze a target process, dlopen the allocator implementation that the target process is using and then call functions in the dlopen()'ed code to perform out-of-process enumeration on behalf of the analysis tool.

I can can extend the existing test to as you suggest and add comments to explain what we expect to happen here.

[kcc]

When the ASan library gets loaded we don't want any of the interceptors to be used. The actual purpose of being to load the ASan library this way is simply so we gain access to the ASan malloc enumerator implementation.

I am more confused now.
What exactly do you call the "ASan malloc enumerator"?
Why is it going to work when the allocations come from a non-asan allocator?

[delcypher]

In D54469#1298804, @eugenis wrote:

Is this needed for the clients of that out-of-process allocator inspection thing?

Yes, this is exactly what this patch is for.

Could it be a separate library that links asan allocator in out-of-process mode, but not the rest? Otherwise you are adding a mode of execution to ASan when there is no shadow mapping, and now that's something that everyone else have to keep in mind.

This is something we have planned as a later patch (once all of the enumeration pieces have landed). However we also want to support dlopen() of the full ASan shared library for various deployment scenarios that we are concerned with.
I was hoping to make this patch as non-intrusive as possible so that other platforms need not worry about the special mode that only exists on Darwin. With the patch as it stands right now other platforms just need to make sure they don't add stuff before
if (SANITIZER_SUPPORTS_INIT_FOR_DLOPEN && UNLIKELY(InitIsViaDlopen())) {... } . Is that asking too much of other platform maintainers? If yes, how would suggest we modify this patch to make it easier for other platforms to ignore the existence of this Darwin only feature?

[delcypher]

In D54469#1298884, @kcc wrote:

When the ASan library gets loaded we don't want any of the interceptors to be used. The actual purpose of being to load the ASan library this way is simply so we gain access to the ASan malloc enumerator implementation.

I am more confused now.

Sorry. I will try to explain.

What exactly do you call the "ASan malloc enumerator"?

The "ASan malloc enumerator" in this context means the code that exists in the ASan library that knows how the enumerate the allocations of another process that is using ASan's allocator, i.e. this is the out-of-process allocator enumeration implementation that we are currently trying to land in https://reviews.llvm.org/D53975 .

Why is it going to work when the allocations come from a non-asan allocator?

Because the intention (when all the ASan malloc enumerator patches land) is that when we call functions in the dlopen()-ed ASan shared library, we will be asking it inspect a different process (one that is using ASan for all allocations), not ourselves.
This is the model that memory analysis tools on Darwin follow. Let's say you have a program foo_asan running that is built with ASan and you want to use the Darwin leaks programs to inspect it. When you launch leaks and tell it to inspect foo_asan it would do the following.

1. Freeze foo_asan so that all threads stop rusnning (similar to LSan's stop-the-world)
2. leaks then examines the foo_asan process and figures out what allocator implementation is being used. In this case it is ASan.
3. leaks then calls dlopen() on the ASan shared library that foo_asan is using.
4. leaks then finds a special function inside the ASan shared library that knows how to enumerate the allocations of a process using ASan.
5. leaks calls this special function, telling it to examine the frozen foo_asan program on its behalf.
6. After the special function is finished, leaks then unfreezes foo_asan allowing it to continue executing.

What this patch is doing is enabling step 3., in this list. Making it possible to dlopen() the ASan shared library without triggering its usual initialisation process because the usual initialisation process would abort the leaks process.

This patch doesn't explicitly mention any of this because the feature is adding is not specific to ASan allocator enumeration at all. It is simply trying to make it possible to load the ASan shared library via dlopen() and call some functions inside it. Our end goal is of course to get out-of-process ASan allocator enumeration working. However knowing this is not required to understand what this patch is doing. Of course knowing this answers why we are trying to add this feature.

Does this make sense now?

[delcypher]

lib/asan/asan_internal.h

114–121

Okay. I will try to just declare these functions without the macro guards and just put stub implementations in each of the platform specific files.

[delcypher]

Remove #ifdefs around introduced functions and instead implement each of them
in the platform specific files.

[delcypher]

lib/asan/asan_internal.h

114–121

@kcc I've tried to remove the #ifdef by always declaring the new functions and implementing each of them for each of the ASan supported platforms.

[delcypher]

@kubamracek @kcc It seems this review has stalled (probably due to Thanksgiving). Any more thoughts on this?

[kcc]

lib/asan/asan_internal.h

115

I wonder if you can turn these two functions into one.

[delcypher]

lib/asan/asan_internal.h

115

Sure it's possible. I'll have a go at doing this and will update the patch shortly.

[delcypher]

Merge InitIsViaDlopen() and HandleDlopenInit() into a single function
named HandleDlopenInit().

[delcypher]

Fix mistake in Darwin implementation of HandleDlopenInit() where we never actually
did the initialization.

[delcypher]

lib/asan/asan_internal.h

115

@kcc Done.

[kcc]

LGTM

lib/asan/asan_flags.inc

ASAN_FLAG(bool, use_odr_indicator, false,
           "Use special ODR indicator symbol for ODR violation detection")
 ASAN_FLAG(bool, allocator_frees_and_returns_null_on_realloc_zero, true,
           "realloc(p, 0) is equivalent to free(p) by default (Same as the "
           "POSIX standard). If set to false, realloc(p, 0) will return a "
           "pointer to an allocated space which can not be used.")
 ASAN_FLAG(bool, verify_asan_link_order, true,
           "Check position of ASan runtime in library list (needs to be disabled"
           " when other library has to be preloaded system-wide)")
+ASAN_FLAG(bool, disable_init, false,
+          "If true, disables all initialization of the "
+          "sanitizer after flags are parsed. Only supported on Darwin.")

lib/asan/asan_internal.h

 void PlatformTSDDtor(void *tsd);
 
 void AppendToErrorMessageBuffer(const char *buffer);
 
 void *AsanDlSymNext(const char *sym);
 
 void ReserveShadowMemoryRange(uptr beg, uptr end, const char *name);
 
-// Add convenient macro for interface functions that may be represented as
-// weak hooks.
+#if SANITIZER_SUPPORTS_DISABLED_INIT
+void HandleDisabledInit();
+#else
+void HandleDisabledInit() { Die(); }
+#endif
+
+// Add convenient macro for interface functions that may be represented
+// as weak hooks.
 #define ASAN_MALLOC_HOOK(ptr, size)                                   \
   do {                                                                \
     if (&__sanitizer_malloc_hook) __sanitizer_malloc_hook(ptr, size); \
     RunMallocHooks(ptr, size);                                        \
   } while (false)
 #define ASAN_FREE_HOOK(ptr)                                 \
   do {                                                      \
     if (&__sanitizer_free_hook) __sanitizer_free_hook(ptr); \

lib/asan/asan_malloc_mac.cc

#define COMMON_MALLOC_FILL_STATS(zone, stats) \
   internal_memcpy(stats, &malloc_stats, sizeof(malloc_statistics_t));
 #define COMMON_MALLOC_REPORT_UNKNOWN_REALLOC(ptr, zone_ptr, zone_name) \
   GET_STACK_TRACE_FREE; \
   ReportMacMzReallocUnknown((uptr)ptr, (uptr)zone_ptr, zone_name, &stack);
 #define COMMON_MALLOC_NAMESPACE __asan
 
 #include "sanitizer_common/sanitizer_malloc_mac.inc"
 
+namespace COMMON_MALLOC_NAMESPACE {
+void HandleDisabledInit() {
+  // On the no init path make sure we still initialize the malloc zone
+  // so Symbolication clients (e.g. `leaks`) can find an initialized
+  // malloc zone when the sanitizer dylib gets dlopen()'ed.
+  InitMallocZoneFields();
+}
+}  // namespace COMMON_MALLOC_NAMESPACE
+
 #endif

lib/asan/asan_rtl.cc

static void AsanInitInternal() {
 
   CacheBinaryName();
   CheckASLR();
 
   // Initialize flags. This must be done early, because most of the
   // initialization steps look at flags().
   InitializeFlags();
 
+  // Stop performing init if supported by the platform and it
+  // has been requested.
+  if (SANITIZER_SUPPORTS_DISABLED_INIT && UNLIKELY(flags()->disable_init)) {
+    asan_init_is_running = false;
+    Report("AddressSanitizer init has been disabled in this process.\n");
+    HandleDisabledInit();
+    return;
+  }
+
   AsanCheckIncompatibleRT();
   AsanCheckDynamicRTPrereqs();
   AvoidCVE_2016_2143();
 
   SetCanPoisonMemory(flags()->poison_heap);
   SetMallocContextSize(common_flags()->malloc_context_size);
 
   InitializePlatformExceptionHandlers();

lib/sanitizer_common/sanitizer_platform.h

 
 // Enable offline markup symbolizer for Fuchsia and RTEMS.
 #if SANITIZER_FUCHSIA || SANITIZER_RTEMS
 #define SANITIZER_SYMBOLIZER_MARKUP 1
 #else
 #define SANITIZER_SYMBOLIZER_MARKUP 0
 #endif
 
+// Enable ability to disable dynamic library initialization.
+#if SANITIZER_MAC
+#define SANITIZER_SUPPORTS_DISABLED_INIT 1
+#else
+#define SANITIZER_SUPPORTS_DISABLED_INIT 0
+#endif
+
 #endif // SANITIZER_PLATFORM_H

test/asan/TestCases/Darwin/disable_init.cc

+// RUN: %clangxx -g -O0 %s -o %t
+// RUN: not --crash env ASAN_OPTIONS=disable_init=0 %run %t %shared_libasan
+// RUN: env ASAN_OPTIONS=disable_init=1 %run %t %shared_libasan
+#include <dlfcn.h>
+#include <stdio.h>
+
+int main(int argc, char **argv) {
+  if (argc != 2) {
+    fprintf(stderr, "Usage: %s <dylib_path>\n", argv[0]);
+    return 1;
+  }
+  const char *dylib_path = argv[1];
+  void *handle = dlopen(dylib_path, RTLD_LAZY);
+  if (!handle) {
+    fprintf(stderr, "Failed to dlopen: %s\n", dlerror());
+    return 1;
+  }
+  // Make sure we can find a function we expect to be in the dylib.
+  void *fn = dlsym(handle, "__sanitizer_mz_size");
+  if (!fn) {
+    fprintf(stderr, "Failed to get symbol: %s\n", dlerror());
+    return 1;
+  }
+  // TODO(dliew): Actually call a function from the dylib that is safe to call.
+  return 0;
+}