This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by m.guseva on Nov 14 2014, 1:43 AM.

Details

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.

Diff Detail

Event Timeline

m.guseva updated this revision to Diff 16200.Nov 14 2014, 1:43 AM
m.guseva retitled this revision from to Improvements for asan deactivated mode: disable asan activation for runtime library on Linux, disable malloc checks..
m.guseva updated this object.
m.guseva edited the test plan for this revision. (Show Details)
m.guseva added subscribers: ygribov, Unknown Object (MLST).
eugenis accepted this revision.Nov 14 2014, 4:30 AM
eugenis added a reviewer: eugenis.
eugenis added a subscriber: 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.

This revision is now accepted and ready to land.Nov 14 2014, 4:30 AM
m.guseva updated this revision to Diff 16211.Nov 14 2014, 7:30 AM
m.guseva edited edge metadata.

Thank you! Uploaded fixed patch.

eugenis added inline comments.Nov 14 2014, 7:33 AM
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 added inline comments.Nov 14 2014, 7:56 AM
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 added inline comments.Nov 14 2014, 7:57 AM
lib/asan/asan_activation.cc
52

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

m.guseva updated this revision to Diff 16214.Nov 14 2014, 8:23 AM

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

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 edited edge metadata.Nov 19 2014, 1:40 PM

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

glider accepted this revision.Nov 21 2014, 2:16 AM
glider edited edge metadata.

LGTM

Was this tested with shared DSO btw?

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

kcc added a comment.Nov 21 2014, 11:05 AM

Don't we have it already? Yury?

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 added a comment.Nov 21 2014, 11:12 AM

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.

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 added a comment.Nov 21 2014, 11:31 AM

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

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 edited edge metadata.Nov 24 2014, 8:49 AM

I will work on that.

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.

Commited in r222732.

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.

m.guseva closed this revision.May 28 2015, 6:15 AM