This is an archive of the discontinued LLVM Phabricator instance.

[asan] Implement delayed activation of AddressSanitizer
ClosedPublic

Authored by eugenis on Jan 15 2014, 4:58 AM.

Details

Summary

This feature is primarily meant for Android. It makes ASan both easier to use and much faster:

  • No need to do setprop for each started application. Instead, a one-time, device-wide setup step is required.
  • Works with -userdebug builds of Android, not just -eng builds.
  • VM is not reloaded from scratch for each ASan process. Instead, zygote is run with ASan runtime (but no instrumented code) in "deactivated" mode, and the first loaded instrumented library "activates" it.

This change adds ASAN_OPTIONS=start_deactivated=1 flag. When present, ASan will start in "deactivated" mode, with no heap poisoning, no quarantine, no stack trace gathering, and minimal redzones. All this features come back when __asan_init is called for the constructor of an instrumented library.

The feature itself is not Android-specific, and this patch includes a Linux test for it.

It is not compatible with flexible shadow mapping.

Diff Detail

Event Timeline

kcc added a comment.Jan 15 2014, 5:12 AM

LGTM modulo use of namespace,
but let others (samsonov and/or glider?) have a look at this CL and at the removal of zero-based shadow.

lib/asan/asan_activation.cc
21

namespace __asan { ?

lib/asan/asan_activation.h
19

doesn't it have to be in __asan namespace?

eugenis added inline comments.Jan 15 2014, 5:22 AM
lib/asan/asan_activation.cc
21

ack

lib/asan/asan_activation.h
19

ack

I'm a bit concerned about changing the flags at activation time while someone else might be reading them.

lib/asan/asan_activation.cc
31

s/asan/ASan here and below.

As glider pointed out, we have a "bening" race in flag copy in AsanActivate(). Sounds like we need to do all flag access with atomics to avoid that. :(

I suggest we also change the ASan runtime version with this change.

Did you mean the other change (deprecation of flexible mapping / zero-base shadow) ?
This one does not seem to change the ABI in any way.

Yes, sorry. Please disregard my previous comment.

eugenis updated this revision to Unknown Object (????).Jan 16 2014, 4:16 AM

Added namespace and a FIXME for non-atomic flag update.

glider accepted this revision.Jan 16 2014, 4:48 AM

LGTM with nits.

lib/asan/asan_activation.cc
32

So what about proper naming here? ("ASan" vs. "asan")

lib/asan/asan_activation.h
21

2-spaces before the comment here and in other places.

lib/asan/asan_malloc_mac.cc
44

Shall we have a macro for this?

eugenis closed this revision.Jan 16 2014, 5:23 AM

Addressed glider's comments in r199380.

lib/asan/asan_activation.cc
32

done

lib/asan/asan_activation.h
21

done

lib/asan/asan_malloc_mac.cc
44

done