This is an archive of the discontinued LLVM Phabricator instance.

Differential D119410

[libc++][NFC] Work around false positive ODR violations from ASan.
ClosedPublic

Authored by var-const on Feb 9 2022, 11:07 PM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG10953974ed6b: [libc++][NFC] Work around false positive ODR violations from ASan.
Summary

This works around a known issue in ASan. ASan doesn't instrument weak
symbols. Because instrumentation increases object size, the binary can
end up with two versions of the same object, one instrumented and one
not instrumented, with different sizes, which ASan will report as an ODR
violation. In libc++, this affects typeinfo for std::bad_function_call
which is emitted as a weak symbol in the test executable and as a strong
symbol in the shared library.

The main open issue for ASan appears to be
https://github.com/google/sanitizers/issues/1017.

Diff Detail

Event Timeline

var-const created this revision.Feb 9 2022, 11:07 PM
Herald added a subscriber: arichardson. · View Herald TranscriptFeb 9 2022, 11:07 PM
var-const requested review of this revision.Feb 9 2022, 11:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2022, 11:07 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
Harbormaster completed remote builds in B148663: Diff 407400.Feb 10 2022, 12:25 AM
ldionne added subscribers: kstoimenov, nikic, ldionne.Feb 10 2022, 6:14 AM

@nikic @kstoimenov Any idea why this started happening between LLVM 13 and LLVM 14? We didn't have this issue before, and it started popping up a couple of days ago when we updated our Docker image. Sorry for pinging out of the blue, but you folks seem to have recent ASAN commits so maybe you're aware of a change that could cause this?

I'd like to avoid carrying hacks like this around, especially if virtually anyone using ASAN needs to do this. In that case, it would seem better to just make that the default ASAN behavior?

libcxx/utils/ci/run-buildbot
314 ↗(On Diff #407400)

Can you move this to the Generic-asan.cmake cache instead?

var-const updated this revision to Diff 407635.Feb 10 2022, 12:13 PM
var-const marked an inline comment as done.

Move flags to the cache.

Herald added a subscriber: mgorny. · View Herald TranscriptFeb 10 2022, 12:13 PM
var-const added inline comments.Feb 10 2022, 12:13 PM
libcxx/utils/ci/run-buildbot
314 ↗(On Diff #407400)

Done (please let me know if this is what you had in mind).

ldionne accepted this revision.Feb 10 2022, 1:44 PM

I think that should work. If the ASAN bot is happy, can you please add a comment about this being "hopefully temporary" and then commit this?

Thanks for looking into it!

This revision is now accepted and ready to land.Feb 10 2022, 1:44 PM
var-const updated this revision to Diff 407731.Feb 10 2022, 5:26 PM

Add comment.

Harbormaster completed remote builds in B148884: Diff 407731.Feb 11 2022, 1:15 AM
ldionne accepted this revision.Feb 11 2022, 6:57 AM
This revision was landed with ongoing or failed builds.Feb 11 2022, 11:57 AM
Closed by commit rG10953974ed6b: [libc++][NFC] Work around false positive ODR violations from ASan. (authored by var-const). · Explain Why
This revision was automatically updated to reflect the committed changes.
var-const added a commit: rG10953974ed6b: [libc++][NFC] Work around false positive ODR violations from ASan..
var-const added a comment.Feb 11 2022, 11:57 AM

Merging this now. generic-asan is green on the CI and the Windows failures look unrelated ("No space left on device").

Revision Contents

PathSize
libcxx/
cmake/
caches/
1 line

Diff 407635

libcxx/cmake/caches/Generic-asan.cmake

set(LLVM_USE_SANITIZER "Address" CACHE STRING "") set(LLVM_USE_SANITIZER "Address" CACHE STRING "")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -mllvm -asan-use-private-alias=1" CACHE INTERNAL "")