This is an archive of the discontinued LLVM Phabricator instance.

[libunwind] Inform ASan that resumption is noreturn
ClosedPublic

Authored by smeenai on May 23 2021, 8:59 PM.

Details

Summary

If you're building libunwind instrumented with ASan, _Unwind_RaiseException
will poison the stack and then transfer control in a manner which isn't
understood by ASan, so the stack will remain poisoned. This can cause
false positives, e.g. if you call an uninstrumented function (so it
doesn't re-poison the stack) after catching an exception. Add a call to
__asan_handle_no_return inside __unw_resume to get ASan to unpoison
the stack and avoid this.

__unw_resume seems like the appropriate place to make this call, since
it's used for resumption by all unwind implementations except SJLJ. SJLJ
uses __builtin_longjmp to handle resumption, which is already
recognized as noreturn (and therefore ASan adds the __asan_handle_no_return
call itself), so it doesn't need any special handling.

PR32434 is somewhat similar (in particular needing a component built
without ASan to trigger the bug), and rG781ef03e1012, the fix for that
bug, adds an interceptor for _Unwind_RaiseException. This interceptor
won't always be triggered though, e.g. if you statically link the
unwinder into libc++abi in a way that prevents interposing the unwinder
functions (e.g. marking the symbols as hidden, using --exclude-libs,
or using -Bsymbolic). rG53335d6d86d5 makes __cxa_throw call
__asan_handle_no_return explicitly, to similarly avoid relying on
interception.

Diff Detail

Event Timeline

smeenai created this revision.May 23 2021, 8:59 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2021, 8:59 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
smeenai requested review of this revision.May 23 2021, 8:59 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2021, 8:59 PM
compnerd accepted this revision.May 26 2021, 8:37 AM
compnerd added a subscriber: compnerd.

Thanks for adding that comment - I think that the cleanup routine may be confusing otherwise.

This revision is now accepted and ready to land.May 26 2021, 8:37 AM
This revision was automatically updated to reflect the committed changes.