From 38c5d6f70060046bbbfec7491c7ba54a50fa5d16 Mon Sep 17 00:00:00 2001 From: Georgii Rymar Date: Thu, 14 May 2020 21:44:06 +0300 Subject: [PATCH] [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 --- llvm/lib/ObjectYAML/ELFEmitter.cpp | 46 ++++++++++++++-------- .../tools/yaml2obj/ELF/custom-null-section.yaml | 5 ++- llvm/test/tools/yaml2obj/ELF/section-link.yaml | 12 ++++-- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp index 2a38395..1ca1dfe 100644 --- a/llvm/lib/ObjectYAML/ELFEmitter.cpp +++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp @@ -218,6 +218,7 @@ template class ELFState { void assignSectionAddress(Elf_Shdr &SHeader, ELFYAML::Section *YAMLSec); + BumpPtrAllocator StringAlloc; uint64_t alignToOffset(ContiguousBlobAccumulator &CBA, uint64_t Align, llvm::Optional Offset); @@ -241,11 +242,6 @@ template ELFState::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH) : Doc(D), ErrHandler(EH) { std::vector 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::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH) std::make_unique( 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 &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 ImplicitSections; if (Doc.DynamicSymbols) ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"}); @@ -401,22 +412,28 @@ bool ELFState::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 @@ -426,7 +443,6 @@ void ELFState::initSectionHeaders(std::vector &SHeaders, // valid SHN_UNDEF entry since SHT_NULL == 0. SHeaders.resize(Doc.getSections().size()); - size_t SecNdx = -1; for (const std::unique_ptr &D : Doc.Chunks) { if (ELFYAML::Fill *S = dyn_cast(D.get())) { S->Offset = alignToOffset(CBA, /*Align=*/1, S->Offset); @@ -435,16 +451,16 @@ void ELFState::initSectionHeaders(std::vector &SHeaders, continue; } - ++SecNdx; ELFYAML::Section *Sec = cast(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::initSectionHeaders(std::vector &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 void ELFState::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 20ea73e..0ccff19 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 efe4d3f..bd82058 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. -- 2.7.4