This is an archive of the discontinued LLVM Phabricator instance.

[MC] Change ELFOSABI_NONE to ELFOSABI_GNU for SHF_GNU_RETAIN
ClosedPublic

Authored by MaskRay on Mar 4 2021, 1:23 PM.

Details

Summary

GNU ld does not give SHF_GNU_RETAIN GC root semantics for ELFOSABI_NONE.
(https://sourceware.org/pipermail/binutils/2021-March/115581.html)

This allows GNU ld to interpret SHF_GNU_RETAIN and avoids a gold quirk
https://sourceware.org/bugzilla/show_bug.cgi?id=27490

Because ELFObjectWriter is in an anonymous namespace, I have to place
markGnuAbi in the parent MCObjectWriter.

Diff Detail

Unit TestsFailed

Event Timeline

MaskRay created this revision.Mar 4 2021, 1:23 PM
MaskRay requested review of this revision.Mar 4 2021, 1:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2021, 1:23 PM

No objections.

Just thinking how to avoid lifting markGnuAbi into the generic MCObjectWriter. The only immediate thing I can think of is casting the result of Asm.getWriter in

if (SectionELF->getFlags() & ELF::SHF_GNU_RETAIN)
    Asm.getWriter().markGnuAbi();

To MCELFObjectTargetWriter. I think this should be possible as I think it is the only choice from within MCELFStreamer, and I guess we could check with a dyn_cast before doing so if we were paranoid. That would allow either markGnuAbi to be put there, or with some more modifications something like setOsABI().

llvm/include/llvm/MC/MCObjectWriter.h
88

Will be worth a comment marking as ELF only.

No objections.

Just thinking how to avoid lifting markGnuAbi into the generic MCObjectWriter. The only immediate thing I can think of is casting the result of Asm.getWriter in

if (SectionELF->getFlags() & ELF::SHF_GNU_RETAIN)
    Asm.getWriter().markGnuAbi();

To MCELFObjectTargetWriter. I think this should be possible as I think it is the only choice from within MCELFStreamer, and I guess we could check with a dyn_cast before doing so if we were paranoid. That would allow either markGnuAbi to be put there, or with some more modifications something like setOsABI().

https://github.com/llvm/llvm-project/blob/main/llvm/lib/MC/ELFObjectWriter.cpp#L219 MCELFObjectTargetWriter is only accessible from the anonymous namespace ELFObjectWriter. We probably don't want to expose ELFObjectWriter in a header.

MaskRay updated this revision to Diff 328565.Mar 5 2021, 9:53 AM

Add a comment

MaskRay marked an inline comment as done.Mar 5 2021, 9:54 AM

Thanks for the explanation and the comment, I missed the part about the anonymous namespace hiding it. I'll check back next week to see if there are any other comments, if not I can set approve. Happy for someone else to do that too.

MaskRay updated this revision to Diff 329170.Mar 8 2021, 5:19 PM

Delete lld check. There is now no official way to get ELFOSABI_NONE for SHF_GNU_RETAIN, so testing EI_OSABI in lld/test/ELF is not useful.

Bdragon28 accepted this revision.Mar 8 2021, 6:59 PM

The code changes look logical and correct to me.

Marking objects with STT_GNU_IFUNC / STB_GNU_UNIQUE can be handled in a follow up.

This revision is now accepted and ready to land.Mar 8 2021, 6:59 PM