Improvements for asan deactivated mode: disable asan activation for runtime library on Linux, disable malloc checks.

[m.guseva]

Summary

The patch includes two changes:

1. Changed __asan_init to AsanInitInternal in AsanInitializer constructor - in order to start deactivated asan in case of asan dynamic library on Linux
2. Disabled mallock checks for deactivated mode: alloc_dealloc_mismatch, allocator_may_return_null.

[eugenis]

LGTM

lib/asan/asan_rtl.cc

713

Make it AsanInitFromRtl for clarity, and remove the check for asan_inited - it is done first thing in AsanInitInternal.

[m.guseva]

Thank you! Uploaded fixed patch.

[eugenis]

lib/asan/asan_activation.cc

52

In fact, why are you changing allocator_may_return_null? It's not a check but rather a conscious decision on malloc behavior.

[m.guseva]

lib/asan/asan_activation.cc

52

I wanted to disable ASan termination in this case. Now I see for that reason the flag should be true, not false. If it's fixed do you agree it makes sense?

[eugenis]

lib/asan/asan_activation.cc

52

Ah ok allocator_may_return_null=true is the "normal" allocator behavior.
Sounds good.

[m.guseva]

New patch uploaded.
Changed allocator_may_return_null to true for deactivated mode.

[eugenis]

LGTM

[m.guseva]

Thank you for review. So is it fine for commit now? I have no write access in llvm so could anyone please commit the patch?

[kcc]

Yes, once you have LGTM it's ok to commit.
I guess Yuri can do it (mentioning you as the author)

[glider]

LGTM

[ygribov]

Was this tested with shared DSO btw?

[glider]

Good point.
We probably also need the dynamic runtime support in Clang so that we can test the DSO regularly.
Kostya, WDYT?

[kcc]

Don't we have it already? Yury?

[ygribov]

We do but it's not enabled in default build (you need -DCOMPILER_RT_BUILD_SHARED_ASAN=ON). Perhaps we could enable it by default?

[kcc]

By default we still want to have static run-time. :)
But we may want to test this configuration on the bots too.
I wonder how much it will increase the load on our bots.

[ygribov]

By default we still want to have static run-time. :)

You will - both runtimes coexist. The shared one is only enabled with explicit -shared-libasan or something like that.

[kcc]

Ok, let's test it by default.
(oh, our poor bots...)

[eugenis]

Sounds good.
So, we need to

• Flip the default of COMPILER_RT_BUILD_SHARED_ASAN
• Add a new variant for asan unit tests (see -with-calls for example)
• Add new configurations for asan lit tests.

This way we would catch DSO issues before this code is merged to gcc, yay!

Any takers?

[samsonov]

I will work on that.

[ygribov]

Add a new variant for asan unit tests (see -with-calls for example)
Add new configurations for asan lit tests.

These two should already be working.

[ygribov]

Commited in r222732.

[samsonov]

OK, better late than never.

COMPILER_RT_BUILD_SHARED_ASAN is gone in r224470.
You can run tests (both unit and lit-tests) with shared ASan runtime as
"check-asan-dynamic".
It is not a part of "check-asan" and "check-all" to save cycles in the
default workflow.

Some of the test cases are currently failing there, I will try to fix them,
and then add "check asan-dynamic" to our bot.

lib/asan/asan_activation.cc

 
 namespace __asan {
 
 static struct AsanDeactivatedFlags {
   int quarantine_size;
   int max_redzone;
   int malloc_context_size;
   bool poison_heap;
+  bool alloc_dealloc_mismatch;
+  bool allocator_may_return_null;
 } asan_deactivated_flags;
 
 static bool asan_is_deactivated;
 
 void AsanStartDeactivated() {
   VReport(1, "Deactivating ASan\n");
   // Save flag values.
   asan_deactivated_flags.quarantine_size = flags()->quarantine_size;
   asan_deactivated_flags.max_redzone = flags()->max_redzone;
   asan_deactivated_flags.poison_heap = flags()->poison_heap;
   asan_deactivated_flags.malloc_context_size =
       common_flags()->malloc_context_size;
+  asan_deactivated_flags.alloc_dealloc_mismatch =
+      flags()->alloc_dealloc_mismatch;
+  asan_deactivated_flags.allocator_may_return_null =
+      common_flags()->allocator_may_return_null;
 
   flags()->quarantine_size = 0;
   flags()->max_redzone = 16;
   flags()->poison_heap = false;
   common_flags()->malloc_context_size = 0;
+  flags()->alloc_dealloc_mismatch = false;
+  common_flags()->allocator_may_return_null = false;
 
   asan_is_deactivated = true;
 }
 
 void AsanActivate() {
   if (!asan_is_deactivated) return;
   VReport(1, "Activating ASan\n");
 
   // Restore flag values.
   // FIXME: this is not atomic, and there may be other threads alive.
   flags()->quarantine_size = asan_deactivated_flags.quarantine_size;
   flags()->max_redzone = asan_deactivated_flags.max_redzone;
   flags()->poison_heap = asan_deactivated_flags.poison_heap;
   common_flags()->malloc_context_size =
       asan_deactivated_flags.malloc_context_size;
+  flags()->alloc_dealloc_mismatch =
+      asan_deactivated_flags.alloc_dealloc_mismatch;
+  common_flags()->allocator_may_return_null =
+      asan_deactivated_flags.allocator_may_return_null;
 
   ParseExtraActivationFlags();
 
   ReInitializeAllocator();
 
   asan_is_deactivated = false;
   VReport(
       1,
-      "quarantine_size %d, max_redzone %d, poison_heap %d, malloc_context_size "
-      "%d\n",
+      "quarantine_size %d, max_redzone %d, poison_heap %d, "
+      "malloc_context_size %d, alloc_dealloc_mismatch %d, "
+      "allocator_may_return_null %d\n",
       flags()->quarantine_size, flags()->max_redzone, flags()->poison_heap,
-      common_flags()->malloc_context_size);
+      common_flags()->malloc_context_size, flags()->alloc_dealloc_mismatch,
+      common_flags()->allocator_may_return_null);
 }
 
 }  // namespace __asan

lib/asan/asan_rtl.cc

 // (and thus normal initializer from .preinit_array haven't run).
 
 class AsanInitializer {
 public:  // NOLINT
   AsanInitializer() {
     AsanCheckIncompatibleRT();
     AsanCheckDynamicRTPrereqs();
     if (UNLIKELY(!asan_inited))
-      __asan_init();
+      AsanInitInternal();
   }
 };
 
 static AsanInitializer asan_initializer;
 #endif  // ASAN_DYNAMIC
 
 }  // namespace __asan