[yaml2elf] - Treat the SHN_UNDEF section as kind of regular section.

[grimar]

Summary

We have a logic that adds a few sections implicitly.
Though the SHT_NULL section with section number 0
is an exception.

In D64913 I want to teach yaml2obj to redefine the null section.
And in this patch I want to add it to the sections list,
to make it kind of regular section.

[grimar]

• A minor simplification.

[MaskRay]

[yaml2elf] - Treat the SHT_NULL section as kind of regular section.

Suggest referring to this section with SHN_UNDEF. Because a section of type SHT_NULL may exist in a different place.

Thought SHT_NULL section which is placed at index 0 is still kind of exception.

Though the SHT_NULL section with section number 0 is an exception. (Though other reviewers may have better recommendation:)

tools/yaml2obj/yaml2elf.cpp

199 ↗

(On Diff #211077)

/*IsImplicit=*/true may be a more popular choice (clang-format uses that form if you place a space in between).

328 ↗

(On Diff #211077)

You can circumvent the violation of https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop by assigning Doc.Sections.size() to a variable above (it is also used above)

[grimar]

tools/yaml2obj/yaml2elf.cpp

328 ↗

(On Diff #211077)

Doc.Sections is a vector. size() has a trivial implementation, for me it is:

	_NODISCARD size_type size() const noexcept
		{	// return length of sequence
		return (static_cast<size_type>(this->_Mylast() - this->_Myfirst()));
		}

I do not think it is very common to introduce a new variable to store size() value
returned by a vector in LLVM (well, sometimes we use (for I = 0, E =..., but not always).

And it doesn't seem worth to do. I think that coding standart assumes that re-evaluting might
be expensive. But this is not the case.

[grimar]

tools/yaml2obj/yaml2elf.cpp

328 ↗

(On Diff #211077)

Just search for ".size();" :)

[jhenderson]

LGTM, once MaskRay's comments have been addressed/agreed not to do.

[MaskRay]

LGTM once the title/description is fixed :)

llvm/trunk/test/tools/yaml2obj/duplicate-section-names.test

- Name: '.foo [1]'
     Type: SHT_PROGBITS
   - Name: .foo2
     Type: SHT_PROGBITS
 
 ## Check that yaml2obj reports an error in case we have
 ## sections with equal names and suffixes.
 
 # RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=CASE2
-# CASE2: error: Repeated section name: '.foo [1]' at YAML section number 1.
+# CASE2: error: Repeated section name: '.foo [1]' at YAML section number 2.
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name: '.foo [1]'
     Type: SHT_PROGBITS
   - Name: '.foo [1]'
     Type: SHT_PROGBITS
 
 ## Check that yaml2obj reports an error in case we have
 ## symbols without suffixes in the names and their
 ## names are equal.
 
 # RUN: not yaml2obj --docnum=3 %s 2>&1 | FileCheck %s --check-prefix=CASE3
-# CASE3: error: Repeated section name: '.foo' at YAML section number 1.
+# CASE3: error: Repeated section name: '.foo' at YAML section number 2.
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:

llvm/trunk/tools/yaml2obj/yaml2elf.cpp

 
 template <class ELFT>
 ELFState<ELFT>::ELFState(ELFYAML::Object &D) : Doc(D) {
   StringSet<> DocSections;
   for (std::unique_ptr<ELFYAML::Section> &D : Doc.Sections)
     if (!D->Name.empty())
       DocSections.insert(D->Name);
 
+  // Insert SHT_NULL section implicitly.
+  Doc.Sections.insert(
+      Doc.Sections.begin(),
+      llvm::make_unique<ELFYAML::Section>(
+          ELFYAML::Section::SectionKind::RawContent, /*IsImplicit=*/true));
+
   std::vector<StringRef> ImplicitSections = {".symtab", ".strtab", ".shstrtab"};
   if (!Doc.DynamicSymbols.empty())
     ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"});
 
   // Insert placeholders for implicit sections that are not
   // defined explicitly in YAML.
   for (StringRef SecName : ImplicitSections) {
     if (DocSections.count(SecName))

 }
 
 template <class ELFT>
 bool ELFState<ELFT>::initSectionHeaders(ELFState<ELFT> &State,
                                         std::vector<Elf_Shdr> &SHeaders,
                                         ContiguousBlobAccumulator &CBA) {
   // Ensure SHN_UNDEF entry is present. An all-zero section header is a
   // valid SHN_UNDEF entry since SHT_NULL == 0.
-  SHeaders.resize(Doc.Sections.size() + 1);
-  zero(SHeaders[0]);
+  SHeaders.resize(Doc.Sections.size());
 
-  for (size_t I = 1; I < Doc.Sections.size() + 1; ++I) {
+  for (size_t I = 1; I < Doc.Sections.size(); ++I) {
     Elf_Shdr &SHeader = SHeaders[I];
-    zero(SHeader);
-    ELFYAML::Section *Sec = Doc.Sections[I - 1].get();
+    ELFYAML::Section *Sec = Doc.Sections[I].get();
 
     // 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.
     if (initImplicitHeader(State, CBA, SHeader, Sec->Name,
                            Sec->IsImplicit ? nullptr : Sec))
       continue;
bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   }
   if (Section.Content)
     Section.Content->writeAsBinary(OS);
 
   return true;
 }
 
 template <class ELFT> bool ELFState<ELFT>::buildSectionIndex() {
-  for (unsigned i = 0, e = Doc.Sections.size(); i != e; ++i) {
-    StringRef Name = Doc.Sections[i]->Name;
+  for (unsigned I = 1, E = Doc.Sections.size(); I != E; ++I) {
+    StringRef Name = Doc.Sections[I]->Name;
     DotShStrtab.add(dropUniqueSuffix(Name));
-    // "+ 1" to take into account the SHT_NULL entry.
-    if (!SN2I.addName(Name, i + 1)) {
+    if (!SN2I.addName(Name, I)) {
       WithColor::error() << "Repeated section name: '" << Name
-                         << "' at YAML section number " << i << ".\n";
+                         << "' at YAML section number " << I << ".\n";
       return false;
     }
   }
 
   DotShStrtab.finalize();
   return true;
 }