This is an archive of the discontinued LLVM Phabricator instance.

RFC: Generalize inverted gc dependencies
Needs ReviewPublic

Authored by espindola on Jan 12 2017, 1:01 PM.

Details

Summary

I am mostly interested in checking that this is sufficient to implement the asan use case. If it does I will benchmark this to see if I have to move the std::vector out or not.

Diff Detail

Event Timeline

rafael updated this revision to Diff 84170.Jan 12 2017, 1:01 PM
rafael retitled this revision from to RFC: Generalize inverted gc dependencies.
rafael updated this object.
rafael added reviewers: ruiu, pcc, eugenis, rnk.
rafael added a subscriber: llvm-commits.
ruiu added inline comments.Jan 12 2017, 1:10 PM
lld/ELF/InputFiles.cpp
326

Does this mean that there is a section with SHF_LINK_ORDER bit but with a null sh_link? It seems like a broken object file to me.

lld/ELF/MarkLive.cpp
238–244

Why do you need this code? It seems you initializes DependentSections in InputFiles.

eugenis added inline comments.Jan 12 2017, 2:36 PM
lld/ELF/MarkLive.cpp
241

This does not have to be an InputSection.
I see it crashing on .rodata.str1.1 which is a MergeInputSection.

eugenis edited edge metadata.Jan 12 2017, 2:43 PM

A bigger problem is current LLD crashing on SHF_LINK_ORDER with zero sh_link, and ld.bfd complaining about that with warning messages. This means we can not safely create such sections in the compiler.

Add SHF_ASSOCIATED?

rafael updated this revision to Diff 84606.Jan 16 2017, 3:25 PM

Add and use SHF_ASSOCIATIVE. If this works for asan I will try to get the value added to the spec.

silvas added a subscriber: silvas.Jan 16 2017, 7:27 PM

Small nit.

llvm/include/llvm/Support/ELF.h
749

Explain the semantics. There is currently no spec reference to look up.

Thank you, this looks and works great!

My earlier comment still needs to be addressed though, casts to InputSection must be replaced with dyn_cast<> with null checks.

rafael updated this revision to Diff 84843.Jan 18 2017, 8:42 AM

Fix review comments.

If everyone is OK with this I will reply to the generic-abi.

pcc edited edge metadata.Jan 18 2017, 9:50 AM

The design of this feature looks good to me, thanks.

lld/ELF/InputFiles.cpp
322

I think it's fine to have this restriction, but aren't we still crashing on associated sections with relocations referring to merge sections?

ruiu edited edge metadata.Jan 18 2017, 11:03 AM

Looks good, but are you going to submit this?

Fix crash.

I will now go back to the generic-abi thread.

eugenis added inline comments.Jan 20 2017, 3:58 PM
llvm/include/llvm/Support/ELF.h
752

this is conflicting with XCORE_SHF_DP_SECTION a few lines below, which is printed in assembly as "d" flag instead of "o"

espindola commandeered this revision.Mar 15 2018, 8:55 AM
espindola added a reviewer: rafael.