[llvm] 38c5d6f - [yaml2obj] - Add a technical prefix for each unnamed chunk.

Georgii Rymar via llvm-commits llvm-commits at lists.llvm.org
Sat May 23 07:34:45 PDT 2020


Author: Georgii Rymar
Date: 2020-05-23T17:22:23+03:00
New Revision: 38c5d6f70060046bbbfec7491c7ba54a50fa5d16

URL: https://github.com/llvm/llvm-project/commit/38c5d6f70060046bbbfec7491c7ba54a50fa5d16
DIFF: https://github.com/llvm/llvm-project/commit/38c5d6f70060046bbbfec7491c7ba54a50fa5d16.diff

LOG: [yaml2obj] - Add a technical prefix for each unnamed chunk.

This change does not affect the produced binary.

In this patch I assign a technical suffix to each section/fill
(i.e. chunk) name when it is empty. It allows to simplify the code
slightly and improve error messages reported.

In the code we have the section to index mapping, SN2I, which is
globally used. With this change we can use it to map "empty"
names to indexes now, what is helpful.

Differential revision: https://reviews.llvm.org/D79984

Added: 
    

Modified: 
    llvm/lib/ObjectYAML/ELFEmitter.cpp
    llvm/test/tools/yaml2obj/ELF/custom-null-section.yaml
    llvm/test/tools/yaml2obj/ELF/section-link.yaml

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
index 2a3839509faf..1ca1dfe0dc4e 100644
--- a/llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -218,6 +218,7 @@ template <class ELFT> class ELFState {
 
   void assignSectionAddress(Elf_Shdr &SHeader, ELFYAML::Section *YAMLSec);
 
+  BumpPtrAllocator StringAlloc;
   uint64_t alignToOffset(ContiguousBlobAccumulator &CBA, uint64_t Align,
                          llvm::Optional<llvm::yaml::Hex64> Offset);
 
@@ -241,11 +242,6 @@ template <class ELFT>
 ELFState<ELFT>::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH)
     : Doc(D), ErrHandler(EH) {
   std::vector<ELFYAML::Section *> Sections = Doc.getSections();
-  StringSet<> DocSections;
-  for (const ELFYAML::Section *Sec : Sections)
-    if (!Sec->Name.empty())
-      DocSections.insert(Sec->Name);
-
   // Insert SHT_NULL section implicitly when it is not defined in YAML.
   if (Sections.empty() || Sections.front()->Type != ELF::SHT_NULL)
     Doc.Chunks.insert(
@@ -253,6 +249,21 @@ ELFState<ELFT>::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH)
         std::make_unique<ELFYAML::Section>(
             ELFYAML::Chunk::ChunkKind::RawContent, /*IsImplicit=*/true));
 
+  // We add a technical suffix for each unnamed section/fill. It does not affect
+  // the output, but allows us to map them by name in the code and report better
+  // error messages.
+  StringSet<> DocSections;
+  for (size_t I = 0; I < Doc.Chunks.size(); ++I) {
+    const std::unique_ptr<ELFYAML::Chunk> &C = Doc.Chunks[I];
+    if (C->Name.empty()) {
+      std::string NewName = ELFYAML::appendUniqueSuffix(
+          /*Name=*/"", "index " + Twine(I));
+      C->Name = StringRef(NewName).copy(StringAlloc);
+      assert(ELFYAML::dropUniqueSuffix(C->Name).empty());
+    }
+    DocSections.insert(C->Name);
+  }
+
   std::vector<StringRef> ImplicitSections;
   if (Doc.DynamicSymbols)
     ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"});
@@ -401,22 +412,28 @@ bool ELFState<ELFT>::initImplicitHeader(ContiguousBlobAccumulator &CBA,
   return true;
 }
 
-constexpr StringRef SuffixStart = " (";
+constexpr char SuffixStart = '(';
 constexpr char SuffixEnd = ')';
 
 std::string llvm::ELFYAML::appendUniqueSuffix(StringRef Name,
                                               const Twine &Msg) {
-  return (Name + SuffixStart + Msg + Twine(SuffixEnd)).str();
+  // Do not add a space when a Name is empty.
+  std::string Ret = Name.empty() ? "" : Name.str() + ' ';
+  return Ret + (Twine(SuffixStart) + Msg + Twine(SuffixEnd)).str();
 }
 
 StringRef llvm::ELFYAML::dropUniqueSuffix(StringRef S) {
   if (S.empty() || S.back() != SuffixEnd)
     return S;
 
+  // A special case for empty names. See appendUniqueSuffix() above.
   size_t SuffixPos = S.rfind(SuffixStart);
-  if (SuffixPos == StringRef::npos)
+  if (SuffixPos == 0)
+    return "";
+
+  if (SuffixPos == StringRef::npos || S[SuffixPos - 1] != ' ')
     return S;
-  return S.substr(0, SuffixPos);
+  return S.substr(0, SuffixPos - 1);
 }
 
 template <class ELFT>
@@ -426,7 +443,6 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
   // valid SHN_UNDEF entry since SHT_NULL == 0.
   SHeaders.resize(Doc.getSections().size());
 
-  size_t SecNdx = -1;
   for (const std::unique_ptr<ELFYAML::Chunk> &D : Doc.Chunks) {
     if (ELFYAML::Fill *S = dyn_cast<ELFYAML::Fill>(D.get())) {
       S->Offset = alignToOffset(CBA, /*Align=*/1, S->Offset);
@@ -435,16 +451,16 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
       continue;
     }
 
-    ++SecNdx;
     ELFYAML::Section *Sec = cast<ELFYAML::Section>(D.get());
-    if (SecNdx == 0 && Sec->IsImplicit)
+    bool IsFirstUndefSection = D == Doc.Chunks.front();
+    if (IsFirstUndefSection && Sec->IsImplicit)
       continue;
 
     // We have a few sections like string or symbol tables that are usually
     // added implicitly to the end. However, if they are explicitly specified
     // in the YAML, we need to write them here. This ensures the file offset
     // remains correct.
-    Elf_Shdr &SHeader = SHeaders[SecNdx];
+    Elf_Shdr &SHeader = SHeaders[SN2I.get(Sec->Name)];
     if (initImplicitHeader(CBA, SHeader, Sec->Name,
                            Sec->IsImplicit ? nullptr : Sec))
       continue;
@@ -461,7 +477,6 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
 
     // Set the offset for all sections, except the SHN_UNDEF section with index
     // 0 when not explicitly requested.
-    bool IsFirstUndefSection = SecNdx == 0;
     if (!IsFirstUndefSection || Sec->Offset)
       SHeader.sh_offset = alignToOffset(CBA, SHeader.sh_addralign, Sec->Offset);
 
@@ -1427,9 +1442,6 @@ template <class ELFT> void ELFState<ELFT>::buildSectionIndex() {
     if (IsSection)
       ++SecNdx;
 
-    if (C->Name.empty())
-      continue;
-
     if (!Seen.insert(C->Name).second)
       reportError("repeated section/fill name: '" + C->Name +
                   "' at YAML section/fill number " + Twine(I));

diff  --git a/llvm/test/tools/yaml2obj/ELF/custom-null-section.yaml b/llvm/test/tools/yaml2obj/ELF/custom-null-section.yaml
index 20ea73ec834e..0ccff19eaf85 100644
--- a/llvm/test/tools/yaml2obj/ELF/custom-null-section.yaml
+++ b/llvm/test/tools/yaml2obj/ELF/custom-null-section.yaml
@@ -128,7 +128,8 @@ Sections:
 
 # RUN: not yaml2obj --docnum=6 %s -o %t6 2>&1 | FileCheck %s --check-prefix=CASE4
 
-# CASE4: error: unknown section referenced: '.foo' by YAML section ''
+# CASE4:      error: unknown section referenced: '.foo' by YAML section '(index 0)'
+# CASE4-NEXT: error: unknown section referenced: '.bar' by YAML section '(index 1)'
 
 --- !ELF
 FileHeader:
@@ -139,6 +140,8 @@ FileHeader:
 Sections:
   - Type: SHT_NULL
     Link: .foo
+  - Type: SHT_NULL
+    Link: .bar
 
 ## Check that null section fields are set to zero, if they are unspecified.
 

diff  --git a/llvm/test/tools/yaml2obj/ELF/section-link.yaml b/llvm/test/tools/yaml2obj/ELF/section-link.yaml
index efe4d3fea74d..bd82058a92ac 100644
--- a/llvm/test/tools/yaml2obj/ELF/section-link.yaml
+++ b/llvm/test/tools/yaml2obj/ELF/section-link.yaml
@@ -28,8 +28,10 @@ Sections:
 
 # RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=ERR
 
-# ERR: error: unknown section referenced: '.unknown1' by YAML section '.foo'
-# ERR: error: unknown section referenced: '.unknown2' by YAML section '.bar'
+# ERR:      error: unknown section referenced: '.unknown1' by YAML section '.foo'
+# ERR-NEXT: error: unknown section referenced: '.unknown2' by YAML section '(index 2)'
+# ERR-NEXT: error: unknown section referenced: '.unknown3' by YAML section '.bar'
+# ERR-NEXT: error: unknown section referenced: '.unknown4' by YAML section '(index 4)'
 
 --- !ELF
 FileHeader:
@@ -41,9 +43,13 @@ Sections:
   - Name: .foo
     Type: SHT_PROGBITS
     Link: .unknown1
+  - Type: SHT_PROGBITS
+    Link: .unknown2
   - Name: .bar
     Type: SHT_PROGBITS
-    Link: .unknown2
+    Link: .unknown3
+  - Type: SHT_PROGBITS
+    Link: .unknown4
 
 ## Check we link SHT_GROUP to a symbol table by default if it exists.
 ## Also, check we can set an arbitrary value for sh_link.


        


More information about the llvm-commits mailing list