[JITLink][ELF] Support duplicated section names from object file

[steven_wu]

Summary

ELF object files can contain duplicated sections (thus section symbols
as well), espeically when comdats/section groups are present. This patch
adds support for generating LinkGraph from object files that have
duplicated section names. This is the first step to properly model
comdats/section groups.

[lhames]

I think you can probably switch from GraphSections to GraphBlocks here, rather than keeping the pair -- you can always get back to the Section from the Block (via getSection()).

[steven_wu]

Address review feedback

[steven_wu]

Fix the rebase issue that mixed up with D114749

[steven_wu]

@lhames ping

[jrtc27]

In D114753#3164838, @steven_wu wrote:

@lhames ping

From the LLVM Code-Review Policy and Practices (https://llvm.org/docs/CodeReview.html):

If it is urgent, provide reasons why it is important to you to get this patch landed and ping it every couple of days. If it is not urgent, the common courtesy ping rate is one week. Remember that you’re asking for valuable time from other professional developers.

It's only been 39 hours, so not even short enough for if it were urgent.

[lhames]

LGTM -- Thanks Steven!

[steven_wu]

Rebase the change to TOT

llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h

 protected:
   using ELFSectionIndex = unsigned;
   using ELFSymbolIndex = unsigned;
 
   bool isRelocatable() const {
     return Obj.getHeader().e_type == llvm::ELF::ET_REL;
   }
 
-  void setGraphSection(ELFSectionIndex SecIndex, Section &Sec) {
-    assert(!GraphSections.count(SecIndex) && "Duplicate section at index");
-    GraphSections[SecIndex] = &Sec;
+  void setGraphBlock(ELFSectionIndex SecIndex, Block *B) {
+    assert(!GraphBlocks.count(SecIndex) && "Duplicate section at index");
+    GraphBlocks[SecIndex] = B;
   }
 
-  Section *getGraphSection(ELFSectionIndex SecIndex) {
-    auto I = GraphSections.find(SecIndex);
-    if (I == GraphSections.end())
+  Block *getGraphBlock(ELFSectionIndex SecIndex) {
+    auto I = GraphBlocks.find(SecIndex);
+    if (I == GraphBlocks.end())
       return nullptr;
     return I->second;
   }
 
   void setGraphSymbol(ELFSymbolIndex SymIndex, Symbol &Sym) {
     assert(!GraphSymbols.count(SymIndex) && "Duplicate symbol at index");
     GraphSymbols[SymIndex] = &Sym;
   }
protected:
   }
 
   const ELFFile &Obj;
 
   typename ELFFile::Elf_Shdr_Range Sections;
   const typename ELFFile::Elf_Shdr *SymTabSec = nullptr;
   StringRef SectionStringTab;
 
-  // Maps ELF section indexes to LinkGraph Sections.
-  // Only SHF_ALLOC sections will have graph sections.
-  DenseMap<ELFSectionIndex, Section *> GraphSections;
+  // Maps ELF section indexes to LinkGraph Blocks.
+  // Only SHF_ALLOC sections will have graph blocks.
+  DenseMap<ELFSectionIndex, Block *> GraphBlocks;
   DenseMap<ELFSymbolIndex, Symbol *> GraphSymbols;
   DenseMap<const typename ELFFile::Elf_Shdr *,
            ArrayRef<typename ELFFile::Elf_Word>>
       ShndxTables;
 };
 
 template <typename ELFT>
 ELFLinkGraphBuilder<ELFT>::ELFLinkGraphBuilder(
for (ELFSectionIndex SecIndex = 0; SecIndex != Sections.size(); ++SecIndex) {
 
     // Get the section's memory protection flags.
     MemProt Prot;
     if (Sec.sh_flags & ELF::SHF_EXECINSTR)
       Prot = MemProt::Read | MemProt::Exec;
     else
       Prot = MemProt::Read | MemProt::Write;
 
-    auto &GraphSec = G->createSection(*Name, Prot);
+    // Look for existing sections first.
+    auto *GraphSec = G->findSectionByName(*Name);
+    if (!GraphSec)
+      GraphSec = &G->createSection(*Name, Prot);
+    assert(GraphSec->getMemProt() == Prot && "MemProt should match");
+
+    Block *B = nullptr;
     if (Sec.sh_type != ELF::SHT_NOBITS) {
       auto Data = Obj.template getSectionContentsAsArray<char>(Sec);
       if (!Data)
         return Data.takeError();
 
-      G->createContentBlock(GraphSec, *Data, Sec.sh_addr, Sec.sh_addralign, 0);
+      B = &G->createContentBlock(*GraphSec, *Data, Sec.sh_addr,
+                                 Sec.sh_addralign, 0);
     } else
-      G->createZeroFillBlock(GraphSec, Sec.sh_size, Sec.sh_addr,
+      B = &G->createZeroFillBlock(*GraphSec, Sec.sh_size, Sec.sh_addr,
                                   Sec.sh_addralign, 0);
 
-    setGraphSection(SecIndex, GraphSec);
+    setGraphBlock(SecIndex, B);
   }
 
   return Error::success();
 }
 
 template <typename ELFT> Error ELFLinkGraphBuilder<ELFT>::graphifySymbols() {
   LLVM_DEBUG(dbgs() << "  Creating graph symbols...\n");
 
if (Sym.isDefined() &&
         if (ShndxTable == ShndxTables.end())
           continue;
         auto NdxOrErr = object::getExtendedSymbolTableIndex<ELFT>(
             Sym, SymIndex, ShndxTable->second);
         if (!NdxOrErr)
           return NdxOrErr.takeError();
         Shndx = *NdxOrErr;
       }
-      if (auto *GraphSec = getGraphSection(Shndx)) {
-        Block *B = nullptr;
-        {
-          auto Blocks = GraphSec->blocks();
-          assert(Blocks.begin() != Blocks.end() && "No blocks for section");
-          assert(std::next(Blocks.begin()) == Blocks.end() &&
-                 "Multiple blocks for section");
-          B = *Blocks.begin();
-        }
-
+      if (auto *B = getGraphBlock(Shndx)) {
         LLVM_DEBUG({
           dbgs() << "      " << SymIndex
                  << ": Creating defined graph symbol for ELF symbol \"" << *Name
                  << "\"\n";
         });
 
-        if (Sym.getType() == ELF::STT_SECTION)
-          *Name = GraphSec->getName();
-
+        // Model the section symbols as anonymous symbol.
         auto &GSym =
-            G->addDefinedSymbol(*B, Sym.getValue(), *Name, Sym.st_size, L, S,
-                                Sym.getType() == ELF::STT_FUNC, false);
+            Sym.getType() == ELF::STT_SECTION
+                ? G->addAnonymousSymbol(*B, Sym.getValue(), Sym.st_size, false,
+                                        false)
+                : G->addDefinedSymbol(*B, Sym.getValue(), *Name, Sym.st_size, L,
+                                      S, Sym.getType() == ELF::STT_FUNC, false);
         setGraphSymbol(SymIndex, GSym);
       }
     } else if (Sym.isUndefined() && Sym.isExternal()) {
       LLVM_DEBUG({
         dbgs() << "      " << SymIndex
                << ": Creating external graph symbol for ELF symbol \"" << *Name
                << "\"\n";
       });
Error ELFLinkGraphBuilder<ELFT>::forEachRelocation(
 
   // Consider skipping these relocations.
   if (!ProcessDebugSections && isDwarfSection(*Name)) {
     LLVM_DEBUG(dbgs() << "    skipped (dwarf section)\n\n");
     return Error::success();
   }
 
   // Lookup the link-graph node corresponding to the target section name.
-  Section *GraphSect = G->findSectionByName(*Name);
-  if (!GraphSect)
+  auto *BlockToFix = getGraphBlock(RelSect.sh_info);
+  if (!BlockToFix)
     return make_error<StringError>(
         "Refencing a section that wasn't added to the graph: " + *Name,
         inconvertibleErrorCode());
 
   auto RelEntries = Obj.relas(RelSect);
   if (!RelEntries)
     return RelEntries.takeError();
 
   // Let the callee process relocation entries one by one.
   for (const typename ELFT::Rela &R : *RelEntries)
-    if (Error Err = Func(R, **FixupSection, *GraphSect))
+    if (Error Err = Func(R, **FixupSection, *BlockToFix))
       return Err;
 
   LLVM_DEBUG(dbgs() << "\n");
   return Error::success();
 }
 
 } // end namespace jitlink
 } // end namespace llvm
 
 #undef DEBUG_TYPE
 
 #endif // LIB_EXECUTIONENGINE_JITLINK_ELFLINKGRAPHBUILDER_H

llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp

for (const auto &RelSect : Base::Sections)
                                               &Self::addSingleRelocation))
         return Err;
 
     return Error::success();
   }
 
   Error addSingleRelocation(const typename ELFT::Rela &Rel,
                             const typename ELFT::Shdr &FixupSect,
-                            Section &GraphSection) {
+                            Block &BlockToFix) {
     using Base = ELFLinkGraphBuilder<ELFT>;
 
     uint32_t SymbolIndex = Rel.getSymbol(false);
     auto ObjSymbol = Base::Obj.getRelocationSymbol(Rel, Base::SymTabSec);
     if (!ObjSymbol)
       return ObjSymbol.takeError();
 
     Symbol *GraphSymbol = Base::getGraphSymbol(SymbolIndex);
     if (!GraphSymbol)
       return make_error<StringError>(
           formatv("Could not find symbol at given index, did you add it to "
                   "JITSymbolTable? index: {0}, shndx: {1} Size of table: {2}",
                   SymbolIndex, (*ObjSymbol)->st_shndx,
                   Base::GraphSymbols.size()),
           inconvertibleErrorCode());
 
     uint32_t Type = Rel.getType(false);
     Expected<aarch64::EdgeKind_aarch64> Kind = getRelocationKind(Type);
     if (!Kind)
       return Kind.takeError();
 
     int64_t Addend = Rel.r_addend;
-    Block *BlockToFix = *(GraphSection.blocks().begin());
     JITTargetAddress FixupAddress = FixupSect.sh_addr + Rel.r_offset;
-    Edge::OffsetT Offset = FixupAddress - BlockToFix->getAddress();
+    Edge::OffsetT Offset = FixupAddress - BlockToFix.getAddress();
     Edge GE(*Kind, Offset, *GraphSymbol, Addend);
     LLVM_DEBUG({
       dbgs() << "    ";
-      printEdge(dbgs(), *BlockToFix, GE, aarch64::getEdgeKindName(*Kind));
+      printEdge(dbgs(), BlockToFix, GE, aarch64::getEdgeKindName(*Kind));
       dbgs() << "\n";
     });
 
-    BlockToFix->addEdge(std::move(GE));
+    BlockToFix.addEdge(std::move(GE));
     return Error::success();
   }
 
 public:
   ELFLinkGraphBuilder_aarch64(StringRef FileName,
                               const object::ELFFile<ELFT> &Obj, const Triple T)
       : ELFLinkGraphBuilder<ELFT>(Obj, std::move(T), FileName,
                                   aarch64::getEdgeKindName) {}

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

for (const auto &RelSect : Base::Sections)
                                               &Self::addSingleRelocation))
         return Err;
 
     return Error::success();
   }
 
   Error addSingleRelocation(const typename ELFT::Rela &Rel,
                             const typename ELFT::Shdr &FixupSect,
-                            Section &GraphSection) {
+                            Block &BlockToFix) {
     using Base = ELFLinkGraphBuilder<ELFT>;
 
     uint32_t SymbolIndex = Rel.getSymbol(false);
     auto ObjSymbol = Base::Obj.getRelocationSymbol(Rel, Base::SymTabSec);
     if (!ObjSymbol)
       return ObjSymbol.takeError();
 
     Symbol *GraphSymbol = Base::getGraphSymbol(SymbolIndex);
     if (!GraphSymbol)
       return make_error<StringError>(
           formatv("Could not find symbol at given index, did you add it to "
                   "JITSymbolTable? index: {0}, shndx: {1} Size of table: {2}",
                   SymbolIndex, (*ObjSymbol)->st_shndx,
                   Base::GraphSymbols.size()),
           inconvertibleErrorCode());
 
     uint32_t Type = Rel.getType(false);
     Expected<riscv::EdgeKind_riscv> Kind = getRelocationKind(Type);
     if (!Kind)
       return Kind.takeError();
 
     int64_t Addend = Rel.r_addend;
-    Block *BlockToFix = *(GraphSection.blocks().begin());
     JITTargetAddress FixupAddress = FixupSect.sh_addr + Rel.r_offset;
-    Edge::OffsetT Offset = FixupAddress - BlockToFix->getAddress();
+    Edge::OffsetT Offset = FixupAddress - BlockToFix.getAddress();
     Edge GE(*Kind, Offset, *GraphSymbol, Addend);
     LLVM_DEBUG({
       dbgs() << "    ";
-      printEdge(dbgs(), *BlockToFix, GE, riscv::getEdgeKindName(*Kind));
+      printEdge(dbgs(), BlockToFix, GE, riscv::getEdgeKindName(*Kind));
       dbgs() << "\n";
     });
 
-    BlockToFix->addEdge(std::move(GE));
+    BlockToFix.addEdge(std::move(GE));
     return Error::success();
   }
 
 public:
   ELFLinkGraphBuilder_riscv(StringRef FileName,
                             const object::ELFFile<ELFT> &Obj, const Triple T)
       : ELFLinkGraphBuilder<ELFT>(Obj, std::move(T), FileName,
                                   riscv::getEdgeKindName) {}

llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp

for (const auto &RelSect : Base::Sections) {
         return Err;
     }
 
     return Error::success();
   }
 
   Error addSingleRelocation(const typename ELFT::Rela &Rel,
                             const typename ELFT::Shdr &FixupSection,
-                            Section &GraphSection) {
+                            Block &BlockToFix) {
     using Base = ELFLinkGraphBuilder<ELFT>;
 
     uint32_t SymbolIndex = Rel.getSymbol(false);
     auto ObjSymbol = Base::Obj.getRelocationSymbol(Rel, Base::SymTabSec);
     if (!ObjSymbol)
       return ObjSymbol.takeError();
 
     Symbol *GraphSymbol = Base::getGraphSymbol(SymbolIndex);
Error addSingleRelocation(const typename ELFT::Rela &Rel,
     }
     case Branch32: {
       Kind = x86_64::BranchPCRel32;
       Addend = 0;
       break;
     }
     }
 
-    Block *BlockToFix = *(GraphSection.blocks().begin());
     JITTargetAddress FixupAddress = FixupSection.sh_addr + Rel.r_offset;
-    Edge::OffsetT Offset = FixupAddress - BlockToFix->getAddress();
+    Edge::OffsetT Offset = FixupAddress - BlockToFix.getAddress();
     Edge GE(Kind, Offset, *GraphSymbol, Addend);
     LLVM_DEBUG({
       dbgs() << "    ";
-      printEdge(dbgs(), *BlockToFix, GE, getELFX86RelocationKindName(Kind));
+      printEdge(dbgs(), BlockToFix, GE, getELFX86RelocationKindName(Kind));
       dbgs() << "\n";
     });
 
-    BlockToFix->addEdge(std::move(GE));
+    BlockToFix.addEdge(std::move(GE));
     return Error::success();
   }
 
 public:
   ELFLinkGraphBuilder_x86_64(StringRef FileName,
                              const object::ELFFile<object::ELF64LE> &Obj)
       : ELFLinkGraphBuilder(Obj, Triple("x86_64-unknown-linux"), FileName,
                             x86_64::getEdgeKindName) {}

llvm/test/ExecutionEngine/JITLink/X86/ELF_comdat.s

+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t
+// RUN: llvm-jitlink -noexec %t
+
+.section  .foo,"axG",@progbits,g1,comdat
+.globl  g1
+g1:
+call test1
+retq
+
+.section  .baz,"axG",@progbits,g1,comdat
+test1:
+retq
+
+.section  .bar,"axG",@progbits,g2,comdat
+.globl  g2
+g2:
+call test2
+retq
+
+.section  .baz,"axG",@progbits,g2,comdat
+test2:
+retq
+
+.text
+.globl  main
+main:
+retq