Generalize SHF_LINK_ORDER to user defined sections

[• rafael]

Summary

The discussion about gcing metadata sections at [e-mail] currently favours having a section flag that would indicate that a one section contains metadata to the section linked to via .sh_link. There is also the desire to reuse SHF_LINK_ORDER and rename it.

This patch implements that idea while still using the SHF_LINK_ORDER name. This is a valid extension of SHF_LINK_ORDER is used for.

[rnk]

lgtm

lld/ELF/MarkLive.cpp

170

Do we already have the dependency from the associated section? It looks like LLD has some logic for it, but I can't tell if it's enough.

[• rafael]

Rui, are you OK with this?

[ruiu]

LGTM

[• espindola]

294592

lld/ELF/MarkLive.cpp

 template <class ELFT> static bool isReserved(InputSectionBase<ELFT> *Sec) {
   switch (Sec->Type) {
   case SHT_FINI_ARRAY:
   case SHT_INIT_ARRAY:
   case SHT_NOTE:
   case SHT_PREINIT_ARRAY:
     return true;
   default:
+    if (Sec->Flags & SHF_LINK_ORDER)
+      return false;
+
     if (!(Sec->Flags & SHF_ALLOC))
       return true;
 
     // We do not want to reclaim sections if they can be referred
     // by __start_* and __stop_* symbols.
     StringRef S = Sec->Name;
     if (isValidCIdentifier(S))
       return true;

lld/test/ELF/gc-sections-metadata.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: ld.lld --gc-sections %t.o -o %t
+# RUN: llvm-objdump -section-headers %t | FileCheck  %s
+
+# CHECK:      1 .foo1
+# CHECK-NEXT:   bar1
+# CHECK-NEXT:   .zed1
+# CHECK-NEXT:   .text
+# CHECK-NEXT:   .comment
+# CHECK-NEXT:   .symtab
+# CHECK-NEXT:   .shstrtab
+# CHECK-NEXT:   .strtab
+
+.global _start
+_start:
+.quad .foo1
+
+.section .foo1,"a"
+.quad 0
+
+.section .foo2,"a"
+.quad 0
+
+.section bar1,"am",@progbits,.foo1
+.quad .zed1
+.quad .foo1
+
+.section bar2,"am",@progbits,.foo2
+.quad .zed2
+.quad .foo2
+
+.section .zed1,"a"
+.quad 0
+
+.section .zed2,"a"
+.quad 0

llvm/docs/Extensions.rst

.. code-block:: gas
 
   .section  .text,"ax",@progbits,unique,2
         nop
 
 
 The unique number is not present in the resulting object at all. It is just used
 in the assembler to differentiate the sections.
 
+The 'm' flag is mapped to SHF_LINK_ORDER. If it is present, a symbol
+must be given that identifies the section to be placed is the
+.sh_link.
+
+.. code-block:: gas
+
+        .section .foo,"a",@progbits
+        .Ltmp:
+        .section .bar,"am",@progbits,.Ltmp
+
+which is equivalent to just
+
+.. code-block:: gas
+
+        .section .foo,"a",@progbits
+        .section .bar,"am",@progbits,.foo
+
+
 Target Specific Behaviour
 =========================
 
 X86
 ---
 
 Relocations
 ^^^^^^^^^^^

llvm/include/llvm/MC/MCContext.h

public:
     MCSectionELF *getELFSection(const Twine &Section, unsigned Type,
                                 unsigned Flags, unsigned EntrySize,
                                 const Twine &Group) {
       return getELFSection(Section, Type, Flags, EntrySize, Group, ~0);
     }
 
     MCSectionELF *getELFSection(const Twine &Section, unsigned Type,
                                 unsigned Flags, unsigned EntrySize,
-                                const Twine &Group, unsigned UniqueID);
+                                const Twine &Group, unsigned UniqueID) {
+      return getELFSection(Section, Type, Flags, EntrySize, Group, UniqueID,
+                           nullptr);
+    }
+
+    MCSectionELF *getELFSection(const Twine &Section, unsigned Type,
+                                unsigned Flags, unsigned EntrySize,
+                                const Twine &Group, unsigned UniqueID,
+                                const MCSectionELF *Associated);
 
     MCSectionELF *getELFSection(const Twine &Section, unsigned Type,
                                 unsigned Flags, unsigned EntrySize,
                                 const MCSymbolELF *Group, unsigned UniqueID,
                                 const MCSectionELF *Associated);
 
     /// Get a section with the provided group identifier. This section is
     /// named by concatenating \p Prefix with '.' then \p Suffix. The \p Type

llvm/lib/MC/ELFObjectWriter.cpp

case ELF::SHT_SYMTAB_SHNDX:
     break;
 
   case ELF::SHT_GROUP:
     sh_link = SymbolTableIndex;
     sh_info = GroupSymbolIndex;
     break;
   }
 
-  if (TargetObjectWriter->getEMachine() == ELF::EM_ARM &&
-      Section.getType() == ELF::SHT_ARM_EXIDX)
+  if (Section.getFlags() & ELF::SHF_LINK_ORDER)
     sh_link = SectionIndexMap.lookup(Section.getAssociatedSection());
 
   WriteSecHdrEntry(StrTabBuilder.getOffset(Section.getSectionName()),
                    Section.getType(), Section.getFlags(), 0, Offset, Size,
                    sh_link, sh_info, Section.getAlignment(),
                    Section.getEntrySize());
 }
 

llvm/lib/MC/MCContext.cpp

MCSectionELF *MCContext::getELFNamedSection(const Twine &Prefix,
                                             const Twine &Suffix, unsigned Type,
                                             unsigned Flags,
                                             unsigned EntrySize) {
   return getELFSection(Prefix + "." + Suffix, Type, Flags, EntrySize, Suffix);
 }
 
 MCSectionELF *MCContext::getELFSection(const Twine &Section, unsigned Type,
                                        unsigned Flags, unsigned EntrySize,
-                                       const Twine &Group, unsigned UniqueID) {
+                                       const Twine &Group, unsigned UniqueID,
+                                       const MCSectionELF *Associated) {
   MCSymbolELF *GroupSym = nullptr;
   if (!Group.isTriviallyEmpty() && !Group.str().empty())
     GroupSym = cast<MCSymbolELF>(getOrCreateSymbol(Group));
 
   return getELFSection(Section, Type, Flags, EntrySize, GroupSym, UniqueID,
-                       nullptr);
+                       Associated);
 }
 
 MCSectionELF *MCContext::getELFSection(const Twine &Section, unsigned Type,
                                        unsigned Flags, unsigned EntrySize,
                                        const MCSymbolELF *GroupSym,
                                        unsigned UniqueID,
                                        const MCSectionELF *Associated) {
   StringRef Group = "";

llvm/lib/MC/MCParser/ELFAsmParser.cpp

 
 private:
   bool ParseSectionName(StringRef &SectionName);
   bool ParseSectionArguments(bool IsPush, SMLoc loc);
   unsigned parseSunStyleSectionFlags();
   bool maybeParseSectionType(StringRef &TypeName);
   bool parseMergeSize(int64_t &Size);
   bool parseGroup(StringRef &GroupName);
+  bool parseMetadataSym(MCSectionELF *&Associated);
   bool maybeParseUniqueID(int64_t &UniqueID);
 };
 
 }
 
 /// ParseDirectiveSymbolAttribute
 ///  ::= { ".local", ".weak", ... } [ identifier ( , identifier )* ]
 bool ELFAsmParser::ParseDirectiveSymbolAttribute(StringRef Directive, SMLoc) {
case 'e':
       flags |= ELF::SHF_EXCLUDE;
       break;
     case 'x':
       flags |= ELF::SHF_EXECINSTR;
       break;
     case 'w':
       flags |= ELF::SHF_WRITE;
       break;
+    case 'm':
+      flags |= ELF::SHF_LINK_ORDER;
+      break;
     case 'M':
       flags |= ELF::SHF_MERGE;
       break;
     case 'S':
       flags |= ELF::SHF_STRINGS;
       break;
     case 'T':
       flags |= ELF::SHF_TLS;
if (L.is(AsmToken::Comma)) {
     if (getParser().parseIdentifier(Linkage))
       return true;
     if (Linkage != "comdat")
       return TokError("Linkage must be 'comdat'");
   }
   return false;
 }
 
+bool ELFAsmParser::parseMetadataSym(MCSectionELF *&Associated) {
+  MCAsmLexer &L = getLexer();
+  if (L.isNot(AsmToken::Comma))
+    return TokError("expected metadata symbol");
+  Lex();
+  StringRef Name;
+  if (getParser().parseIdentifier(Name))
+    return true;
+  MCSymbol *Sym = getContext().lookupSymbol(Name);
+  if (!Sym || !Sym->isInSection())
+    return TokError("symbol is not in a section: " + Name);
+  Associated = cast<MCSectionELF>(&Sym->getSection());
+  return false;
+}
+
 bool ELFAsmParser::maybeParseUniqueID(int64_t &UniqueID) {
   MCAsmLexer &L = getLexer();
   if (L.isNot(AsmToken::Comma))
     return false;
   Lex();
   StringRef UniqueStr;
   if (getParser().parseIdentifier(UniqueStr))
     return TokError("expected identifier in directive");
bool ELFAsmParser::ParseSectionArguments(bool IsPush, SMLoc loc) {
 
   StringRef TypeName;
   int64_t Size = 0;
   StringRef GroupName;
   unsigned Flags = 0;
   const MCExpr *Subsection = nullptr;
   bool UseLastGroup = false;
   StringRef UniqueStr;
+  MCSectionELF *Associated = nullptr;
   int64_t UniqueID = ~0;
 
   // Set the defaults first.
   if (SectionName == ".fini" || SectionName == ".init" ||
       SectionName == ".rodata")
     Flags |= ELF::SHF_ALLOC;
   if (SectionName == ".fini" || SectionName == ".init")
     Flags |= ELF::SHF_EXECINSTR;
if (getLexer().is(AsmToken::Comma)) {
     }
 
     if (Mergeable)
       if (parseMergeSize(Size))
         return true;
     if (Group)
       if (parseGroup(GroupName))
         return true;
+    if (Flags & ELF::SHF_LINK_ORDER)
+      if (parseMetadataSym(Associated))
+        return true;
     if (maybeParseUniqueID(UniqueID))
       return true;
   }
 
 EndStmt:
   if (getLexer().isNot(AsmToken::EndOfStatement))
     return TokError("unexpected token in directive");
   Lex();
if (UseLastGroup) {
     if (const MCSectionELF *Section =
             cast_or_null<MCSectionELF>(CurrentSection.first))
       if (const MCSymbol *Group = Section->getGroup()) {
         GroupName = Group->getName();
         Flags |= ELF::SHF_GROUP;
       }
   }
 
-  MCSection *ELFSection = getContext().getELFSection(SectionName, Type, Flags,
-                                                     Size, GroupName, UniqueID);
+  MCSection *ELFSection = getContext().getELFSection(
+      SectionName, Type, Flags, Size, GroupName, UniqueID, Associated);
   getStreamer().SwitchSection(ELFSection, Subsection);
 
   if (getContext().getGenDwarfForAssembly()) {
     bool InsertResult = getContext().addGenDwarfSection(ELFSection);
     if (InsertResult) {
       if (getContext().getDwarfVersion() <= 2)
         Warning(loc, "DWARF2 only supports one section per compilation unit");
 

llvm/test/MC/ELF/section-metadata-err1.s

+// RUN: not llvm-mc -triple x86_64-pc-linux-gnu %s -o - 2>&1 | FileCheck %s
+
+// CHECK: error: symbol is not in a section: foo
+
+        .section .shf_metadata,"am",@progbits,foo

llvm/test/MC/ELF/section-metadata-err2.s

+// RUN: not llvm-mc -triple x86_64-pc-linux-gnu %s -o - 2>&1 | FileCheck %s
+
+// CHECK: error: symbol is not in a section: foo
+
+        .quad foo
+        .section .shf_metadata,"am",@progbits,foo

llvm/test/MC/ELF/section-metadata-err3.s

+// RUN: not llvm-mc -triple x86_64-pc-linux-gnu %s -o - 2>&1 | FileCheck %s
+
+// CHECK: error: symbol is not in a section: foo
+
+        foo = 42
+        .section .shf_metadata,"am",@progbits,foo

llvm/test/MC/ELF/section-metadata-err4.s

+// RUN: not llvm-mc -triple x86_64-pc-linux-gnu %s -o - 2>&1 | FileCheck %s
+
+// CHECK: error: expected metadata symbol
+
+        .section .shf_metadata,"am",@progbits

llvm/test/MC/ELF/section.s

 // Test that we handle the strings like gas
 .section bar-"foo"
 .section "foo"
 
 // CHECK:        Section {
 // CHECK:          Name: bar-"foo"
 // CHECK:        Section {
 // CHECK:          Name: foo
+
+// Test SHF_LINK_ORDER
+
+.section .shf_metadata_target1, "a"
+        .quad 0
+.section .shf_metadata_target2, "a", @progbits, unique, 1
+.Lshf_metadata_target2_1:
+        .quad 0
+.section .shf_metadata_target2, "a", @progbits, unique, 2
+.Lshf_metadata_target2_2:
+        .quad 0
+
+.section .shf_metadata1,"am",@progbits,.Lshf_metadata_target2_1
+.section .shf_metadata2,"am",@progbits,.Lshf_metadata_target2_2
+.section .shf_metadata3,"am",@progbits,.shf_metadata_target1
+
+// CHECK:      Section {
+// CHECK:        Index: 22
+// CHECK-NEXT:   Name: .shf_metadata_target1
+
+// CHECK:      Section {
+// CHECK:        Index: 23
+// CHECK-NEXT:   Name: .shf_metadata_target2
+
+// CHECK:      Section {
+// CHECK:        Index: 24
+// CHECK-NEXT:   Name: .shf_metadata_target2
+
+// CHECK:      Section {
+// CHECK:        Name: .shf_metadata1
+// CHECK-NEXT:   Type: SHT_PROGBITS
+// CHECK-NEXT:   Flags [
+// CHECK-NEXT:     SHF_ALLOC
+// CHECK-NEXT:     SHF_LINK_ORDER
+// CHECK-NEXT:   ]
+// CHECK-NEXT:   Address:
+// CHECK-NEXT:   Offset:
+// CHECK-NEXT:   Size:
+// CHECK-NEXT:   Link:    23
+// CHECK-NEXT:   Info:    0
+
+// CHECK:      Section {
+// CHECK:        Name: .shf_metadata2
+// CHECK-NEXT:   Type: SHT_PROGBITS
+// CHECK-NEXT:   Flags [
+// CHECK-NEXT:     SHF_ALLOC
+// CHECK-NEXT:     SHF_LINK_ORDER
+// CHECK-NEXT:   ]
+// CHECK-NEXT:   Address:
+// CHECK-NEXT:   Offset:
+// CHECK-NEXT:   Size:
+// CHECK-NEXT:   Link:    24
+// CHECK-NEXT:   Info:    0
+
+// CHECK:      Section {
+// CHECK:        Name: .shf_metadata3
+// CHECK-NEXT:   Type: SHT_PROGBITS
+// CHECK-NEXT:   Flags [
+// CHECK-NEXT:     SHF_ALLOC
+// CHECK-NEXT:     SHF_LINK_ORDER
+// CHECK-NEXT:   ]
+// CHECK-NEXT:   Address:
+// CHECK-NEXT:   Offset:
+// CHECK-NEXT:   Size:
+// CHECK-NEXT:   Link:    22
+// CHECK-NEXT:   Info:    0