This is an archive of the discontinued LLVM Phabricator instance.

[asan] A clang flag to enable ELF globals-gc
ClosedPublic

Authored by eugenis on May 4 2017, 5:35 PM.

Details

Reviewers
pcc
chandlerc
Summary

This feature is subtly broken when the linker is gold 2.26 or
earlier. See the following bug for details:

https://sourceware.org/bugzilla/show_bug.cgi?id=19002

Since the decision needs to be made at compilation time, we can not
test the linker version. The flag is off by default on ELF targets,
and on otherwise.

Diff Detail

Repository
rL LLVM

Event Timeline

eugenis updated this revision to Diff 97901.May 4 2017, 5:35 PM
eugenis created this revision.
chandlerc accepted this revision.May 4 2017, 5:38 PM

Generally makes sense. A question about the name. Feel free to land with a name that you and other sanitizer folks are happy with.

include/clang/Driver/Options.td
830

Would the name "-fsanitize-address-globals-dead-stripping" be a bit easier to understand? not sure.

This revision is now accepted and ready to land.May 4 2017, 5:38 PM
eugenis added inline comments.May 4 2017, 5:41 PM
include/clang/Driver/Options.td
830

It does sound a little bit better.
The current name is copied from the -mllvm flag, which was initially OSX-specific, where "live_support" is part of the section name.

eugenis updated this revision to Diff 98200.May 8 2017, 1:34 PM
eugenis marked 2 inline comments as done.May 8 2017, 1:34 PM
pcc accepted this revision.May 8 2017, 4:32 PM

LGTM

Unfortunate, but I guess this is in the same category as things like -fsized-deallocation.

lib/Driver/SanitizerArgs.cpp
572

Maybe include a binutils PR reference so that we can remove this eventually?

727

I think you don't need MakeArgString for string literals.

eugenis updated this revision to Diff 98354.May 9 2017, 2:57 PM
eugenis marked an inline comment as done.
eugenis marked an inline comment as done.

I'll fix the other unnecessary MakeArgString calls in a separate commit

eugenis closed this revision.May 9 2017, 3:11 PM

r302591, thanks for the review!