[MC] Change ELFOSABI_NONE to ELFOSABI_GNU for SHF_GNU_RETAIN

[MaskRay]

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.

[peter.smith]

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.

[MaskRay]

In D97976#2606678, @peter.smith wrote:

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]

Add a comment

[peter.smith]

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]

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]

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.

llvm/include/llvm/MC/MCObjectWriter.h

virtual bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
                                                       bool InSet) const;
 
   virtual bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
                                                       const MCSymbol &SymA,
                                                       const MCFragment &FB,
                                                       bool InSet,
                                                       bool IsPCRel) const;
 
+  virtual void markGnuAbi() {}
+
   /// Tell the object writer to emit an address-significance table during
   /// writeObject(). If this function is not called, all symbols are treated as
   /// address-significant.
   virtual void emitAddrsigSection() {}
 
   /// Record the given symbol in the address-significance table to be written
   /// diring writeObject().
   virtual void addAddrsigSymbol(const MCSymbol *Sym) {}

llvm/lib/MC/ELFObjectWriter.cpp

 class ELFObjectWriter : public MCObjectWriter {
   /// The target specific ELF writer instance.
   std::unique_ptr<MCELFObjectTargetWriter> TargetObjectWriter;
 
   DenseMap<const MCSectionELF *, std::vector<ELFRelocationEntry>> Relocations;
 
   DenseMap<const MCSymbolELF *, const MCSymbolELF *> Renames;
 
+  bool SeenGnuAbi = false;
   bool EmitAddrsigSection = false;
   std::vector<const MCSymbol *> AddrsigSyms;
 
   bool hasRelocationAddend() const;
 
   bool shouldRelocateWithSymbol(const MCAssembler &Asm,
                                 const MCSymbolRefExpr *RefA,
                                 const MCSymbolELF *Sym, uint64_t C,
                                 unsigned Type) const;
 
 public:
   ELFObjectWriter(std::unique_ptr<MCELFObjectTargetWriter> MOTW)
       : TargetObjectWriter(std::move(MOTW)) {}
 
   void reset() override {
+    SeenGnuAbi = false;
     Relocations.clear();
     Renames.clear();
     MCObjectWriter::reset();
   }
 
   bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
                                               const MCSymbol &SymA,
                                               const MCFragment &FB, bool InSet,
                                               bool IsPCRel) const override;
 
   virtual bool checkRelocation(MCContext &Ctx, SMLoc Loc,
                                const MCSectionELF *From,
                                const MCSectionELF *To) {
     return true;
   }
 
   void recordRelocation(MCAssembler &Asm, const MCAsmLayout &Layout,
                         const MCFragment *Fragment, const MCFixup &Fixup,
                         MCValue Target, uint64_t &FixedValue) override;
 
   void executePostLayoutBinding(MCAssembler &Asm,
                                 const MCAsmLayout &Layout) override;
 
+  void markGnuAbi() override { SeenGnuAbi = true; }
+  bool seenGnuAbi() const { return SeenGnuAbi; }
   void emitAddrsigSection() override { EmitAddrsigSection = true; }
   void addAddrsigSymbol(const MCSymbol *Sym) override {
     AddrsigSyms.push_back(Sym);
   }
 
   friend struct ELFWriter;
 };
 
void ELFWriter::writeHeader(const MCAssembler &Asm) {
   W.OS << char(is64Bit() ? ELF::ELFCLASS64 : ELF::ELFCLASS32); // e_ident[EI_CLASS]
 
   // e_ident[EI_DATA]
   W.OS << char(W.Endian == support::little ? ELF::ELFDATA2LSB
                                            : ELF::ELFDATA2MSB);
 
   W.OS << char(ELF::EV_CURRENT);        // e_ident[EI_VERSION]
   // e_ident[EI_OSABI]
-  W.OS << char(OWriter.TargetObjectWriter->getOSABI());
+  uint8_t OSABI = OWriter.TargetObjectWriter->getOSABI();
+  W.OS << char(OSABI == ELF::ELFOSABI_NONE && OWriter.seenGnuAbi()
+                   ? ELF::ELFOSABI_GNU
+                   : OSABI);
   // e_ident[EI_ABIVERSION]
   W.OS << char(OWriter.TargetObjectWriter->getABIVersion());
 
   W.OS.write_zeros(ELF::EI_NIDENT - ELF::EI_PAD);
 
   W.write<uint16_t>(ELF::ET_REL);             // e_type
 
   W.write<uint16_t>(OWriter.TargetObjectWriter->getEMachine()); // e_machine = target

llvm/lib/MC/MCELFStreamer.cpp

void MCELFStreamer::changeSection(MCSection *Section,
 
   MCAssembler &Asm = getAssembler();
   // Ensure the previous section gets aligned if necessary.
   setSectionAlignmentForBundling(Asm, CurSection);
   auto *SectionELF = static_cast<const MCSectionELF *>(Section);
   const MCSymbol *Grp = SectionELF->getGroup();
   if (Grp)
     Asm.registerSymbol(*Grp);
+  if (SectionELF->getFlags() & ELF::SHF_GNU_RETAIN)
+    Asm.getWriter().markGnuAbi();
 
   changeSectionImpl(Section, Subsection);
   Asm.registerSymbol(*Section->getBeginSymbol());
 }
 
 void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {
   getAssembler().registerSymbol(*Symbol);
   const MCExpr *Value = MCSymbolRefExpr::create(

llvm/test/MC/ELF/section-gnu.s

 # RUN: llvm-mc -triple=x86_64 %s | FileCheck %s --check-prefix=ASM
-# RUN: llvm-mc -filetype=obj -triple=x86_64 %s | llvm-readobj -hS - | FileCheck %s --check-prefix=OBJ
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s | llvm-readobj -hS - | FileCheck %s --check-prefixes=GNU,OBJ
+# RUN: llvm-mc -filetype=obj -triple=aarch64-freebsd %s | llvm-readobj -hS - | FileCheck %s --check-prefixes=FREEBSD,OBJ
 
 # ASM: .section retain,"aR",@progbits
 
-## Note: GNU as sets OSABI to GNU.
-# OBJ:      OS/ABI: SystemV (0x0)
+## ELFOSABI_NONE is changed to ELFOSABI_GNU. Other OSABI values are unchanged.
+# GNU:      OS/ABI: GNU/Linux
+# FREEBSD:  OS/ABI: FreeBSD
 
 # OBJ:      Name: retain
 # OBJ-NEXT: Type: SHT_PROGBITS
 # OBJ-NEXT: Flags [
 # OBJ-NEXT:   SHF_ALLOC
 # OBJ-NEXT:   SHF_GNU_RETAIN
 # OBJ-NEXT: ]
 
 .section retain,"aR",@progbits