This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Don't build ubsan cxxabi sources when unused
ClosedPublic

Authored by fjricci on Aug 17 2016, 5:39 PM.

Details

Summary

On apple targets, when SANITIZER_CAN_USE_CXXABI is false,
the ubsan cxxabi sources aren't built, since they're unused.
Do this on non-apple targets as well.

This fixes errors when linking sanitizers if c++ abi is
unavailable.

Diff Detail

Repository
rL LLVM

Event Timeline

fjricci updated this revision to Diff 68464.Aug 17 2016, 5:39 PM
fjricci retitled this revision from to [compiler-rt] Don't build ubsan cxxabi sources when unused.
fjricci updated this object.
fjricci added reviewers: pcc, rnk, samsonov, beanz.
fjricci added subscribers: compnerd, llvm-commits.
rnk edited reviewers, added: kubamracek; removed: samsonov, rnk.Aug 18 2016, 9:09 AM
rnk added a subscriber: rnk.

Can we unify things between non-Mac and Mac so that we don't need to have a dummy RTUBsan_cxx? I'm hoping Chris or Kuba can help here. I haven't heard from Kuba in months, though. =/

That was my original plan, so I was looking into why APPLE and NOT APPLE had to be different. I found this commit message from 2013 (clang r177605)

   Split ubsan runtime into three pieces (clang part):
 * libclang_rt-san-* is sanitizer_common, and is linked in only if no other
  sanitizer runtime is present.
 * libclang_rt-ubsan-* is the piece of the runtime which doesn't depend on
   a C++ ABI library, and is always linked in.
 * libclang_rt-ubsan_cxx-* is the piece of the runtime which depends on a
   C++ ABI library, and is only linked in when linking a C++ binary.

This change also switches us to using -whole-archive for the ubsan runtime
(which is made possible by the above split), and switches us to only linking
the sanitizer runtime into the main binary and not into DSOs (which is made
possible by using -whole-archive).

The motivation for this is to only link a single copy of sanitizer_common
into any binary. This is becoming important now because we want to share
more state between multiple sanitizers in the same process (for instance,
we want a single shared output mutex).

The Darwin ubsan runtime is unchanged; because we use a DSO there, we don't
need this complexity.
beanz edited edge metadata.Aug 18 2016, 10:53 AM

Why not generate static and dynamic runtimes in both cases? If we did that we could unify the build system code.

@beanz We could certainly do that, but the scope of that patch would be pretty significant - many of the sanitizers rely directly on these targets (which is why RTUbsan_cxx has to be created as a dummy target, instead of just not being created at all). It's probably the right move long term, but I think it's outside the scope of fixing the bug here.

The reason why this patch is necessary is that static linking of a sanitizer which includes RTUbsan_cxx will fail if libc++abi is unavailable, even though this code is never used or called when SANITIZER_CAN_USE_CXXABI is false.

"static linking of" meaning "statically linking to"

beanz accepted this revision.Aug 18 2016, 2:25 PM
beanz edited edge metadata.

Good enough for me. LGTM.

This revision is now accepted and ready to land.Aug 18 2016, 2:25 PM
This revision was automatically updated to reflect the committed changes.