-
Notifications
You must be signed in to change notification settings - Fork 15.4k
Description
Context:
ASan configuration flags can be specified, among other methods, through a user-defined function of the following form:
extern "C" __declspec(dllexport) extern const char *__asan_default_options()
{
return "allocator_may_return_null=1"; // or some other flag
}When using this method, the configuration options are not available during ASan initialization (responsible for parsing configuration flags), which occurs in AsanInitInternal:
llvm-project/compiler-rt/lib/asan/asan_rtl.cpp
Lines 393 to 402 in 175051b
| static bool AsanInitInternal() { | |
| if (LIKELY(AsanInited())) | |
| return true; | |
| SanitizerToolName = "AddressSanitizer"; | |
| CacheBinaryName(); | |
| // Initialize flags. This must be done early, because most of the | |
| // initialization steps look at flags(). | |
| InitializeFlags(); |
Instead, these flags are only parsed through a callback registered through AddRegisterWeakFunctionCallback:
llvm-project/compiler-rt/lib/asan/asan_flags.cpp
Lines 215 to 244 in 175051b
| void InitializeFlags() { | |
| InitializeDefaultFlags(); | |
| ProcessFlags(); | |
| #if SANITIZER_WINDOWS | |
| // On Windows, weak symbols are emulated by having the user program | |
| // register which weak functions are defined. | |
| // The ASAN DLL will initialize flags prior to user module initialization, | |
| // so __asan_default_options will not point to the user definition yet. | |
| // We still want to ensure we capture when options are passed via | |
| // __asan_default_options, so we add a callback to be run | |
| // when it is registered with the runtime. | |
| // There is theoretically time between the initial ProcessFlags and | |
| // registering the weak callback where a weak function could be added and we | |
| // would miss it, but in practice, InitializeFlags will always happen under | |
| // the loader lock (if built as a DLL) and so will any calls to | |
| // __sanitizer_register_weak_function. | |
| AddRegisterWeakFunctionCallback( | |
| reinterpret_cast<uptr>(__asan_default_options), []() { | |
| FlagParser asan_parser; | |
| RegisterAsanFlags(&asan_parser, flags()); | |
| RegisterCommonFlags(&asan_parser); | |
| asan_parser.ParseString(__asan_default_options()); | |
| DisplayHelpMessages(&asan_parser); | |
| ProcessFlags(); | |
| }); | |
This is further corroborated by that method's top-level comment, which reads:
On Windows, weak symbols are emulated by having the user program register which weak functions are defined. The ASAN DLL will initialize flags prior to user module initialization, so __asan_default_options will not point to the user definition yet. We still want to ensure we capture when options are passed via __asan_default_options, so we add a callback to be run when it is registered with the runtime.
Therefore, that callback should probably be checking every single ASan flag in case it has changed, and initialize them accordingly, but it's not doing so.
For example, the flag allocator_may_return_null is not being properly initialized today as it's not calling SetAllocatorMayReturnNull.
In AsanInitInternal, there are many calls to methods starting with the Set keyword, which configure ASan flags. These include:
SetCanPoisonMemorySetMallocContextSizeSetCheckUnwindCallback
and others. I expect these are additional methods that should also be invoked through the callback and that they represent other flags that are currently being missed.
Preliminary thoughts on how to address this:
I think what we want is for the flag initialization code of ASanInitInternal to be refactored into a convenient helper function that can be called multiple times, onces in ASanInitInternal, and then again in the call back within AddRegisterWeakFunctionCallback.
Of less relevance:
Hi, I'm from the msvc team, I work on ASan there. I just fixed this bug specifically for allocator_may_return_null in our internal fork, looking to contribute it back upstream shortly. Just creating this issue for reference. In general, we're interested in contributing more fixes directly upstream, so you might see more of these issues pop up, assuming that's 'ok'. Thanks!