This is an archive of the discontinued LLVM Phabricator instance.

[ubsan-minimal] Fix the ubsan_minimal debug build (COMPILER_RT_DEBUG=1) on macOS
ClosedPublic

Authored by delcypher on Feb 23 2018, 7:21 AM.

Details

Summary

[ubsan-minimal] Fix the ubsan_minimal debug build (COMPILER_RT_DEBUG=1) on macOS.

ubsan_minimal makes use of the _sanitizer::atomic_load function.
This function uses the DCHECK macro which in debug builds will use
the _sanitizer::CheckFailed function.

This function is part of sanitizer_common but ubsan_minimal doesn't
use this so the implementation is missing which leads to link failures
on macOS when trying to link libclang_rt.ubsan_minimal_osx_dynamic.dylib.
This is in contrast to the BFD linker on Linux which doesn't seem to care
about the missing symbol.

A basic implementation of _sanitizer::CheckFailed has been added to
the ubsan_minimal debug build to avoid the link error. The
implementation could definitely be improved but I don't know which
functions can be used in this context so I decided to restrict myself to
functions only being used in ubsan_minimal already.

Diff Detail

Event Timeline

delcypher created this revision.Feb 23 2018, 7:21 AM
Herald added a subscriber: Restricted Project. · View Herald TranscriptFeb 23 2018, 7:21 AM
eugenis accepted this revision.Feb 23 2018, 9:40 AM
This revision is now accepted and ready to land.Feb 23 2018, 9:40 AM

@eugenis Thanks for approving. I'll wait a bit longer before committing as @vsk and @kubamracek might have opinions on this.

vsk accepted this revision.Feb 23 2018, 10:53 AM

Thanks!

lib/ubsan_minimal/ubsan_minimal_handlers.cc
68

Could you omit the parameter names for the unused arguments? This should appease some picky buildbots.

kubamracek accepted this revision.Feb 23 2018, 10:59 AM

Thanks!

delcypher updated this revision to Diff 135793.Feb 24 2018, 5:12 AM

Remove names of unused arguments.

delcypher marked an inline comment as done.Feb 24 2018, 5:13 AM
delcypher added inline comments.
lib/ubsan_minimal/ubsan_minimal_handlers.cc
68

Done.

The fact that this wasn't caught before though probably means we are missing a build bot configuration for macOS.

delcypher closed this revision.Feb 24 2018, 5:18 AM
delcypher marked an inline comment as done.

Committed in r326032