[RFC] [ELF] Fix group section relocation handling

Adhemerval Zanella adhemerval.zanella at linaro.org
Thu May 14 12:23:45 PDT 2015


Hi Rui and Shankar,

While debugging some test-suite SingleSource C++ EH cases when linked with
lld [1] I noted that lld is applying wrong relocation on the text segments
if there is multiple .text segments in the same object (the COMDAT ones).

It is due in the mapping from Segments and Relocation, only the segment
name is considered and for this cases the relocation for segment '.text'
will be overwritten by last COMDAT being handled.

The patch changes this by considering the segment index references in the
group section while creating the mapping.  It also adds a small optimization
on handleSectionGroup by avoiding using the same, but rather the segment
header in the sections mapping (it should be no code generation changes for
this one).

Although I could not make the C++ EH testcases work, the relocation seems
correct and the right function are now being correctly called. I think 
lld is messing up with eh_frame and CFE generation, since the programs
now crashed in the stack unwind code from libstdc++, rather than an
illegal function.

And I putting this change on RFC because I am not sure if this is the
best strategy to handle group segments (I got the idea from my understanding
of binutils), neither how to effectively test it without restricting to
one platform.  Also, if someone could point me on how lld may be not
generating correct unwind information, I would appreciate.

[1] http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-May/085311.html

--

diff --git a/lib/ReaderWriter/ELF/ELFFile.cpp b/lib/ReaderWriter/ELF/ELFFile.cpp
index fd2ef71..9d9f588 100644
--- a/lib/ReaderWriter/ELF/ELFFile.cpp
+++ b/lib/ReaderWriter/ELF/ELFFile.cpp
@@ -10,6 +10,7 @@
 #include "ELFFile.h"
 #include "FileCommon.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
 
 namespace lld {
 namespace elf {
@@ -143,7 +144,12 @@ std::error_code ELFFile<ELFT>::createAtomizableSections() {
       auto rai(_objFile->begin_rela(&section));
       auto rae(_objFile->end_rela(&section));
 
-      _relocationAddendReferences[*sectionName] = make_range(rai, rae);
+      std::string sectionKey = *sectionName;
+      if (isSectionMemberOfGroup(&section))
+        sectionKey += llvm::utohexstr(section.sh_info);
+
+      _relocationAddendReferences[sectionKey] = make_range(rai, rae);
+
       totalRelocs += std::distance(rai, rae);
     } else if (section.sh_type == llvm::ELF::SHT_REL) {
       auto sHdr = _objFile->getSection(section.sh_info);
@@ -155,7 +161,12 @@ std::error_code ELFFile<ELFT>::createAtomizableSections() {
       auto ri(_objFile->begin_rel(&section));
       auto re(_objFile->end_rel(&section));
 
-      _relocationReferences[*sectionName] = make_range(ri, re);
+      std::string sectionKey = *sectionName;
+      if (isSectionMemberOfGroup(&section))
+        sectionKey += llvm::utohexstr(section.sh_info);
+
+      _relocationReferences[sectionKey] = make_range(ri, re);
+
       totalRelocs += std::distance(ri, re);
     } else {
       _sectionSymbols[&section];
@@ -259,12 +270,16 @@ std::error_code ELFFile<ELFT>::createSymbolsFromAtomizableSections() {
 template <class ELFT> std::error_code ELFFile<ELFT>::createAtoms() {
   // Holds all the atoms that are part of the section. They are the targets of
   // the kindGroupChild reference.
-  llvm::StringMap<std::vector<ELFDefinedAtom<ELFT> *>> atomsForSection;
+  AtomsForSectionMap atomsForSection;
 
-  // Contains a list of comdat sections for a group.
-  for (auto &i : _sectionSymbols) {
-    const Elf_Shdr *section = i.first;
-    std::vector<Elf_Sym_Iter> &symbols = i.second;
+  // Contains a list of comdat sections for a group.  The sectionIdx is used
+  // to keep track of section ordering to match group section in the References
+  // match when creating ElfDefineAtom.
+  int sectionIdx = 1;
+  for (auto i = _sectionSymbols.begin();
+       i != _sectionSymbols.end(); ++i, ++sectionIdx) {
+    const Elf_Shdr *section = (*i).first;
+    std::vector<Elf_Sym_Iter> &symbols = (*i).second;
 
     // Sort symbols by position.
     std::stable_sort(symbols.begin(), symbols.end(),
@@ -294,7 +309,7 @@ template <class ELFT> std::error_code ELFFile<ELFT>::createAtoms() {
       if (addAtoms)
         addAtom(*newAtom);
       else
-        atomsForSection[*sectionName].push_back(newAtom);
+        atomsForSection[section].push_back(newAtom);
       continue;
     }
 
@@ -347,7 +362,7 @@ template <class ELFT> std::error_code ELFFile<ELFT>::createAtoms() {
         if (addAtoms)
           addAtom(*atom);
         else
-          atomsForSection[*sectionName].push_back(atom);
+          atomsForSection[section].push_back(atom);
         continue;
       }
 
@@ -361,7 +376,7 @@ template <class ELFT> std::error_code ELFFile<ELFT>::createAtoms() {
         auto sym = new (_readerStorage) Elf_Sym(*symbol);
         sym->setBinding(llvm::ELF::STB_GLOBAL);
         anonAtom = createDefinedAtomAndAssignRelocations(
-            "", *sectionName, sym, section, symbolData, *sectionContents);
+            "", *sectionName, sym, section, sectionIdx, symbolData, *sectionContents);
         symbolData = ArrayRef<uint8_t>();
 
         // If this is the last atom, let's not create a followon reference.
@@ -373,7 +388,7 @@ template <class ELFT> std::error_code ELFFile<ELFT>::createAtoms() {
       }
 
       ELFDefinedAtom<ELFT> *newAtom = createDefinedAtomAndAssignRelocations(
-          symbolName, *sectionName, &*symbol, section, symbolData,
+          symbolName, *sectionName, &*symbol, section, sectionIdx, symbolData,
           *sectionContents);
       newAtom->setOrdinal(++_ordinal);
 
@@ -395,7 +410,7 @@ template <class ELFT> std::error_code ELFFile<ELFT>::createAtoms() {
       if (addAtoms)
         addAtom(*newAtom);
       else
-        atomsForSection[*sectionName].push_back(newAtom);
+        atomsForSection[section].push_back(newAtom);
 
       _symbolToAtomMapping.insert(std::make_pair(&*symbol, newAtom));
       if (anonAtom) {
@@ -403,7 +418,7 @@ template <class ELFT> std::error_code ELFFile<ELFT>::createAtoms() {
         if (addAtoms)
           addAtom(*anonAtom);
         else
-          atomsForSection[*sectionName].push_back(anonAtom);
+          atomsForSection[section].push_back(anonAtom);
       }
     }
   }
@@ -422,7 +437,7 @@ template <class ELFT> std::error_code ELFFile<ELFT>::createAtoms() {
 template <class ELFT>
 std::error_code ELFFile<ELFT>::handleGnuLinkOnceSection(
     const Elf_Shdr *section,
-    llvm::StringMap<std::vector<ELFDefinedAtom<ELFT> *>> &atomsForSection) {
+    AtomsForSectionMap &atomsForSection) {
   ErrorOr<StringRef> sectionName = this->getSectionName(section);
   if (std::error_code ec = sectionName.getError())
     return ec;
@@ -431,14 +446,14 @@ std::error_code ELFFile<ELFT>::handleGnuLinkOnceSection(
 
   unsigned int referenceStart = _references.size();
   std::vector<ELFReference<ELFT> *> refs;
-  for (auto ha : atomsForSection[*sectionName]) {
+  for (auto ha : atomsForSection[section]) {
     _groupChild[ha->symbol()] = std::make_pair(*sectionName, section);
     ELFReference<ELFT> *ref =
         new (_readerStorage) ELFReference<ELFT>(Reference::kindGroupChild);
     ref->setTarget(ha);
     refs.push_back(ref);
   }
-  atomsForSection[*sectionName].clear();
+  atomsForSection[section].clear();
   // Create a gnu linkonce atom.
   ELFDefinedAtom<ELFT> *atom = createDefinedAtom(
       *sectionName, *sectionName, nullptr, section, ArrayRef<uint8_t>(),
@@ -453,7 +468,7 @@ std::error_code ELFFile<ELFT>::handleGnuLinkOnceSection(
 template <class ELFT>
 std::error_code ELFFile<ELFT>::handleSectionGroup(
     const Elf_Shdr *section,
-    llvm::StringMap<std::vector<ELFDefinedAtom<ELFT> *>> &atomsForSection) {
+    AtomsForSectionMap &atomsForSection) {
   ErrorOr<StringRef> sectionName = this->getSectionName(section);
   if (std::error_code ec = sectionName.getError())
     return ec;
@@ -473,16 +488,13 @@ std::error_code ELFFile<ELFT>::handleSectionGroup(
   // member contains the symbol table index of the identifying entry.
   // The sh_flags member of the section header contains 0. The name of
   // the section (sh_name) is not specified.
-  std::vector<StringRef> sectionNames;
+  std::vector<const Elf_Shdr *> sections;
   const Elf_Word *groupMembers =
       reinterpret_cast<const Elf_Word *>(sectionContents->data());
   const size_t count = section->sh_size / sizeof(Elf_Word);
   for (size_t i = 1; i < count; i++) {
     const Elf_Shdr *shdr = _objFile->getSection(groupMembers[i]);
-    ErrorOr<StringRef> sectionName = _objFile->getSectionName(shdr);
-    if (std::error_code ec = sectionName.getError())
-      return ec;
-    sectionNames.push_back(*sectionName);
+    sections.push_back(shdr);
   }
   const Elf_Sym *symbol = _objFile->getSymbol(section->sh_info);
   const Elf_Shdr *symtab = _objFile->getSection(section->sh_link);
@@ -492,15 +504,15 @@ std::error_code ELFFile<ELFT>::handleSectionGroup(
 
   unsigned int referenceStart = _references.size();
   std::vector<ELFReference<ELFT> *> refs;
-  for (auto name : sectionNames) {
-    for (auto ha : atomsForSection[name]) {
+  for (auto sec : sections) {
+    for (auto ha : atomsForSection[sec]) {
       _groupChild[ha->symbol()] = std::make_pair(*symbolName, section);
       ELFReference<ELFT> *ref =
           new (_readerStorage) ELFReference<ELFT>(Reference::kindGroupChild);
       ref->setTarget(ha);
       refs.push_back(ref);
     }
-    atomsForSection[name].clear();
+    atomsForSection[sec].clear();
   }
 
   // Create an atom for comdat signature.
@@ -549,17 +561,21 @@ template <class ELFT> std::error_code ELFFile<ELFT>::createAtomsFromContext() {
 template <class ELFT>
 ELFDefinedAtom<ELFT> *ELFFile<ELFT>::createDefinedAtomAndAssignRelocations(
     StringRef symbolName, StringRef sectionName, const Elf_Sym *symbol,
-    const Elf_Shdr *section, ArrayRef<uint8_t> symContent,
+    const Elf_Shdr *section, int sectionIdx, ArrayRef<uint8_t> symContent,
     ArrayRef<uint8_t> secContent) {
   unsigned int referenceStart = _references.size();
 
+  std::string sectionKey = sectionName;
+  if (isSectionMemberOfGroup(section))
+    sectionKey += llvm::utohexstr(sectionIdx);
+
   // Add Rela (those with r_addend) references:
-  auto rari = _relocationAddendReferences.find(sectionName);
+  auto rari = _relocationAddendReferences.find(sectionKey);
   if (rari != _relocationAddendReferences.end())
     createRelocationReferences(symbol, symContent, rari->second);
 
   // Add Rel references.
-  auto rri = _relocationReferences.find(sectionName);
+  auto rri = _relocationReferences.find(sectionKey);
   if (rri != _relocationReferences.end())
     createRelocationReferences(symbol, symContent, secContent, rri->second);
 
@@ -691,7 +707,7 @@ ELFFile<ELFT>::createSectionAtom(const Elf_Shdr *section, StringRef sectionName,
   sym->st_value = 0;
   sym->st_size = 0;
   auto *newAtom = createDefinedAtomAndAssignRelocations(
-      "", sectionName, sym, section, content, content);
+      "", sectionName, sym, section, -1, content, content);
   newAtom->setOrdinal(++_ordinal);
   return newAtom;
 }
diff --git a/lib/ReaderWriter/ELF/ELFFile.h b/lib/ReaderWriter/ELF/ELFFile.h
index 93ade6b..3b1b94a 100644
--- a/lib/ReaderWriter/ELF/ELFFile.h
+++ b/lib/ReaderWriter/ELF/ELFFile.h
@@ -128,9 +128,12 @@ public:
   Atom *findAtom(const Elf_Sym *sourceSym, const Elf_Sym *targetSym);
 
 protected:
+  typedef llvm::DenseMap<const Elf_Shdr *, std::vector<ELFDefinedAtom<ELFT> *>>
+    AtomsForSectionMap;
+
   ELFDefinedAtom<ELFT> *createDefinedAtomAndAssignRelocations(
       StringRef symbolName, StringRef sectionName, const Elf_Sym *symbol,
-      const Elf_Shdr *section, ArrayRef<uint8_t> symContent,
+      const Elf_Shdr *sectionHdr, int sectionIdx, ArrayRef<uint8_t> symContent,
       ArrayRef<uint8_t> secContent);
 
   std::error_code doParse() override;
@@ -212,12 +215,12 @@ protected:
   /// Handle creation of atoms for .gnu.linkonce sections.
   std::error_code handleGnuLinkOnceSection(
       const Elf_Shdr *section,
-      llvm::StringMap<std::vector<ELFDefinedAtom<ELFT> *>> &atomsForSection);
+      AtomsForSectionMap &atomsForSection);
 
   // Handle COMDAT scetions.
   std::error_code handleSectionGroup(
       const Elf_Shdr *section,
-      llvm::StringMap<std::vector<ELFDefinedAtom<ELFT> *>> &atomsForSection);
+      AtomsForSectionMap &atomsForSection);
 
   /// Process the Undefined symbol and create an atom for it.
   ELFUndefinedAtom<ELFT> *createUndefinedAtom(StringRef symName,
@@ -345,10 +348,11 @@ protected:
   /// list of relocations references.  In ELF, if a section named, ".text" has
   /// relocations will also have a section named ".rel.text" or ".rela.text"
   /// which will hold the entries.
-  std::unordered_map<StringRef, range<Elf_Rela_Iter>>
-  _relocationAddendReferences;
+  std::unordered_map<std::string, range<Elf_Rela_Iter>>
+    _relocationAddendReferences;
+  std::unordered_map<std::string, range<Elf_Rel_Iter>>
+    _relocationReferences;
   MergedSectionMapT _mergedSectionMap;
-  std::unordered_map<StringRef, range<Elf_Rel_Iter>> _relocationReferences;
   std::vector<ELFReference<ELFT> *> _references;
   llvm::DenseMap<const Elf_Sym *, Atom *> _symbolToAtomMapping;
   llvm::DenseMap<const ELFReference<ELFT> *, const Elf_Sym *>



More information about the llvm-commits mailing list