This is an archive of the discontinued LLVM Phabricator instance.

[asan] Fix dead stripping of globals on Linux.
ClosedPublic

Authored by eugenis on Feb 17 2017, 3:22 PM.

Details

Summary

Use a combination of !associated, comdat, @llvm.compiler.used and
custom sections to allow dead stripping of globals and their asan
metadata. Sometimes.

Currently this works on LLD, which supports SHF_LINK_ORDER with
sh_link pointing to the associated section.

This also works on BFD, which seems to treat comdats as
all-or-nothing with respect to linker GC. There is a weird quirk
where the "first" global in each link is never GC-ed because of the
section symbols.

At this moment it does not work on Gold (as in the globals are never
stripped).

This is a re-land of r298158 rebased on D31358. This time,
asan.module_ctor is put in a comdat as well to avoid quadratic
behavior in Gold.

Diff Detail

Event Timeline

eugenis created this revision.Feb 17 2017, 3:22 PM
eugenis added a parent revision: D29104: Add !associated metadata..
eugenis updated this revision to Diff 92061.Mar 16 2017, 2:39 PM

rebased, PTAL

pcc added inline comments.Mar 16 2017, 2:53 PM
lib/Transforms/Instrumentation/AddressSanitizer.cpp
129 ↗(On Diff #92061)

What happens if you use double underscore? I would have thought that hidden visibility was enough to prevent this symbol from being exported.

lib/Transforms/Utils/ModuleUtils.cpp
233 ↗(On Diff #92061)

Can you change ThinLTOBitcodeWriter to use this function as well?

eugenis updated this revision to Diff 92067.Mar 16 2017, 3:01 PM
eugenis marked an inline comment as done.Mar 16 2017, 3:01 PM
eugenis added inline comments.
lib/Transforms/Instrumentation/AddressSanitizer.cpp
129 ↗(On Diff #92061)

/usr/bin/ld.gold: warning: Cannot export local symbol '__asan_globals_registered'

pcc accepted this revision.Mar 16 2017, 3:13 PM

LGTM

lib/Transforms/Instrumentation/AddressSanitizer.cpp
129 ↗(On Diff #92061)

So the comment should say that the triple underscore suppresses a warning rather than that it prevents exporting.

This revision is now accepted and ready to land.Mar 16 2017, 3:13 PM
eugenis updated this revision to Diff 92074.Mar 16 2017, 3:19 PM

reword a comment

This revision was automatically updated to reflect the committed changes.
eugenis reopened this revision.Mar 24 2017, 5:26 PM
This revision is now accepted and ready to land.Mar 24 2017, 5:26 PM
eugenis updated this revision to Diff 93024.Mar 24 2017, 5:27 PM
eugenis edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2019, 6:23 AM