[yaml2obj] - Allow setting cutom Flags for implicit sections.

[grimar]

Summary

With this patch we get ability to set any flags we want
for implicit sections defined in YAML.

[jhenderson]

Do we have test cases anywhere for the sections' flags when they aren't described in the YAML?

test/tools/yaml2obj/strtab-implicit-sections-flags.yaml

1 ↗

(On Diff #204055)

Nit: add "the" before Flags.

5 ↗

(On Diff #204055)

and -> and the

32–33 ↗

(On Diff #204055)

I'd probably just remove the rest of the sentence from "and make .strtab ..."

test/tools/yaml2obj/symtab-implicit-sections-flags.yaml

1 ↗

(On Diff #204055)

Same comments apply to this test as the other test.

[grimar]

In D63136#1538159, @jhenderson wrote:

Do we have test cases anywhere for the sections' flags when they aren't described in the YAML?

When I stop adding SHF_ALLOC flag for .dynsym/.dynstr and run the tests, I see 6 are failing:

Failing Tests (6):
    LLVM :: tools/yaml2obj/dynamic-symbols.yaml
    LLVM :: tools/yaml2obj/dynsymtab-implicit-sections-size-content.yaml
    LLVM :: tools/yaml2obj/elf-symtab-shinfo.yaml
    LLVM :: tools/yaml2obj/explicit-dynsym-no-dynstr.yaml
    LLVM :: tools/yaml2obj/strtab-implicit-sections-flags.yaml
    LLVM :: tools/yaml2obj/symtab-implicit-sections-flags.yaml

The first 4 are intended to check different things and the last 2 are added by this patch.
dynamic-symbols.yaml checks that .dynsym/.dynstr has SHF_ALLOC when they are not described in it's YAML explicitly.

If I start adding SHF_ALLOC for .strtab/.symtab (in case their flags are not described explicitly)
then the following tests are failing:

Failing Tests (5):
    LLVM :: tools/yaml2obj/elf-symtab-shinfo.yaml
    LLVM :: tools/yaml2obj/strtab-implicit-sections-flags.yaml
    LLVM :: tools/yaml2obj/strtab-implicit-sections-size-content.yaml
    LLVM :: tools/yaml2obj/symtab-implicit-sections-flags.yaml
    LLVM :: tools/yaml2obj/symtab-implicit-sections-size-content.yaml

In all of these test cases these sections are described in yaml (but flags are not). I.e. seems we never test their default
flags when they aren't explicitly described in the YAML.

I'll update the strtab-implicit-sections-flags.yaml and symtab-implicit-sections-flags.yaml to fix that piece.

[grimar]

• Addressed review comments.

[jhenderson]

LGTM.

llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h

enum class SectionKind {
     Verdef,
     Verneed,
     Symver,
     MipsABIFlags
   };
   SectionKind Kind;
   StringRef Name;
   ELF_SHT Type;
-  ELF_SHF Flags;
+  Optional<ELF_SHF> Flags;
   llvm::yaml::Hex64 Address;
   StringRef Link;
   llvm::yaml::Hex64 AddressAlign;
   Optional<llvm::yaml::Hex64> EntSize;
 
   Section(SectionKind Kind) : Kind(Kind) {}
   virtual ~Section();
 };

llvm/trunk/lib/ObjectYAML/ELFYAML.cpp

StringRef MappingTraits<ELFYAML::Symbol>::validate(IO &IO,
   if (Symbol.NameIndex && !Symbol.Name.empty())
     return "Name and NameIndex cannot both be specified for Symbol";
   return StringRef();
 }
 
 static void commonSectionMapping(IO &IO, ELFYAML::Section &Section) {
   IO.mapOptional("Name", Section.Name, StringRef());
   IO.mapRequired("Type", Section.Type);
-  IO.mapOptional("Flags", Section.Flags, ELFYAML::ELF_SHF(0));
+  IO.mapOptional("Flags", Section.Flags);
   IO.mapOptional("Address", Section.Address, Hex64(0));
   IO.mapOptional("Link", Section.Link, StringRef());
   IO.mapOptional("AddressAlign", Section.AddressAlign, Hex64(0));
   IO.mapOptional("EntSize", Section.EntSize);
 }
 
 static void sectionMapping(IO &IO, ELFYAML::DynamicSection &Section) {
   commonSectionMapping(IO, Section);

llvm/trunk/test/tools/yaml2obj/strtab-implicit-sections-flags.yaml

+## For implicit string table sections, the `Flags` field can also
+## be specified in YAML. Here we test the behavior in different cases.
+
+## When flags are not explicitly specified, yaml2obj assigns no flags
+## for .strtab and the SHF_ALLOC flag for .dynstr by default.
+
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj %t1 -S | FileCheck %s --check-prefix=CASE1
+
+# CASE1:      Name: .strtab
+# CASE1-NEXT: Type: SHT_STRTAB
+# CASE1-NEXT: Flags [
+# CASE1-NEXT: ]
+# CASE1:      Name: .dynstr
+# CASE1-NEXT: Type: SHT_STRTAB
+# CASE1-NEXT: Flags [
+# CASE1-NEXT:   SHF_ALLOC
+# CASE1-NEXT: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .strtab
+    Type:    SHT_STRTAB
+  - Name:    .dynstr
+    Type:    SHT_STRTAB
+
+## Check we can set arbitrary flags for .strtab/.dynstr.
+
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: llvm-readobj %t2 -S | FileCheck %s --check-prefix=CASE2
+
+# CASE2:      Name: .strtab
+# CASE2-NEXT: Type: SHT_STRTAB
+# CASE2-NEXT: Flags [
+# CASE2-NEXT:   SHF_ALLOC
+# CASE2-NEXT:   SHF_STRINGS
+# CASE2-NEXT: ]
+# CASE2:      Name: .dynstr
+# CASE2-NEXT: Type: SHT_STRTAB
+# CASE2-NEXT: Flags [
+# CASE2-NEXT:   SHF_STRINGS
+# CASE2-NEXT: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:  .strtab
+    Type:  SHT_STRTAB
+    Flags: [ SHF_ALLOC, SHF_STRINGS ]
+  - Name:  .dynstr
+    Type:  SHT_STRTAB
+    Flags: [ SHF_STRINGS ]
+
+## Check no flags are set by default for .strtab when it is not
+## described in the YAML.
+
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: llvm-readobj %t3 -S | FileCheck %s --check-prefix=CASE3
+
+# CASE3:      Name: .strtab
+# CASE3-NEXT: Type: SHT_STRTAB
+# CASE3-NEXT: Flags [
+# CASE3-NEXT: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64

llvm/trunk/test/tools/yaml2obj/symtab-implicit-sections-flags.yaml

+## For implicit symbol table sections, the `Flags` field can also
+## be specified in YAML. Here we test the behavior in different cases.
+
+## When flags are not explicitly specified, yaml2obj assigns no flags
+## for .symtab and the SHF_ALLOC flag for .dynsym by default.
+
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj %t1 -S | FileCheck %s --check-prefix=CASE1
+
+# CASE1:      Name: .symtab
+# CASE1-NEXT: Type: SHT_SYMTAB
+# CASE1-NEXT: Flags [
+# CASE1-NEXT: ]
+# CASE1:      Name: .dynsym
+# CASE1-NEXT: Type: SHT_DYNSYM
+# CASE1-NEXT: Flags [
+# CASE1-NEXT:   SHF_ALLOC
+# CASE1-NEXT: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name: .symtab
+    Type: SHT_SYMTAB
+  - Name: .dynsym
+    Type: SHT_SYMTAB
+
+## Check we can set arbitrary flags for .symtab/.dynsym.
+
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: llvm-readobj %t2 -S | FileCheck %s --check-prefix=CASE2
+
+# CASE2:      Name: .symtab
+# CASE2-NEXT: Type: SHT_SYMTAB
+# CASE2-NEXT: Flags [
+# CASE2-NEXT:   SHF_ALLOC
+# CASE2-NEXT:   SHF_STRINGS
+# CASE2-NEXT: ]
+# CASE2:      Name: .dynsym
+# CASE2-NEXT: Type: SHT_DYNSYM
+# CASE2-NEXT: Flags [
+# CASE2-NEXT:   SHF_STRINGS
+# CASE2-NEXT: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:  .symtab
+    Type:  SHT_SYMTAB
+    Flags: [ SHF_ALLOC, SHF_STRINGS ]
+  - Name:  .dynsym
+    Type:  SHT_SYMTAB
+    Flags: [ SHF_STRINGS ]
+
+## Check no flags are set by default for .symtab when it is not
+## described in the YAML.
+
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: llvm-readobj %t3 -S | FileCheck %s --check-prefix=CASE3
+
+# CASE3:      Name: .symtab
+# CASE3-NEXT: Type: SHT_SYMTAB
+# CASE3-NEXT: Flags [
+# CASE3-NEXT: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64

llvm/trunk/tools/obj2yaml/elf2yaml.cpp

std::error_code ELFDumper<ELFT>::dumpRelocation(const RelT *Rel,
 
   return obj2yaml_error::success;
 }
 
 template <class ELFT>
 std::error_code ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
                                                    ELFYAML::Section &S) {
   S.Type = Shdr->sh_type;
-  S.Flags = Shdr->sh_flags;
+  if (Shdr->sh_flags)
+    S.Flags = static_cast<ELFYAML::ELF_SHF>(Shdr->sh_flags);
   S.Address = Shdr->sh_addr;
   S.AddressAlign = Shdr->sh_addralign;
   if (Shdr->sh_entsize)
     S.EntSize = static_cast<llvm::yaml::Hex64>(Shdr->sh_entsize);
 
   auto NameOrErr = getUniquedSectionName(Shdr);
   if (!NameOrErr)
     return errorToErrorCode(NameOrErr.takeError());

llvm/trunk/tools/yaml2obj/yaml2elf.cpp

for (size_t I = 1; I < Doc.Sections.size() + ImplicitSections.size() + 1; ++I) {
     if (initImplicitHeader(State, CBA, SHeader, SecName, Sec))
       continue;
 
     assert(Sec && "It can't be null unless it is an implicit section. But all "
                   "implicit sections should already have been handled above.");
 
     SHeader.sh_name = DotShStrtab.getOffset(SecName);
     SHeader.sh_type = Sec->Type;
-    SHeader.sh_flags = Sec->Flags;
+    if (Sec->Flags)
+      SHeader.sh_flags = *Sec->Flags;
     SHeader.sh_addr = Sec->Address;
     SHeader.sh_addralign = Sec->AddressAlign;
 
     if (!Sec->Link.empty()) {
       unsigned Index;
       if (!convertSectionIndex(SN2I, Sec->Name, Sec->Link, Index))
         return false;
       SHeader.sh_link = Index;
if (RawSec && !RawSec->Link.empty()) {
     unsigned Link = 0;
     if (IsStatic)
       Link = SN2I.get(".strtab");
     else
       SN2I.lookup(".dynstr", Link);
     SHeader.sh_link = Link;
   }
 
-  if (!IsStatic)
-    SHeader.sh_flags |= ELF::SHF_ALLOC;
+  if (YAMLSec && YAMLSec->Flags)
+    SHeader.sh_flags = *YAMLSec->Flags;
+  else if (!IsStatic)
+    SHeader.sh_flags = ELF::SHF_ALLOC;
 
   // If the symbol table section is explicitly described in the YAML
   // then we should set the fields requested.
   SHeader.sh_info =
       RawSec ? (unsigned)RawSec->Info : findFirstNonGlobal(Symbols) + 1;
   SHeader.sh_entsize = (YAMLSec && YAMLSec->EntSize)
                            ? (uint64_t)(*YAMLSec->EntSize)
                            : sizeof(Elf_Sym);
void ELFState<ELFT>::initStrtabSectionHeader(Elf_Shdr &SHeader, StringRef Name,
   } else {
     STB.write(OS);
     SHeader.sh_size = STB.getSize();
   }
 
   if (YAMLSec && YAMLSec->EntSize)
     SHeader.sh_entsize = *YAMLSec->EntSize;
 
+  if (YAMLSec && YAMLSec->Flags)
+    SHeader.sh_flags = *YAMLSec->Flags;
+  else if (Name == ".dynstr")
+    SHeader.sh_flags = ELF::SHF_ALLOC;
+
   // If .dynstr section is explicitly described in the YAML
   // then we want to use its section address.
-  if (Name == ".dynstr") {
-    if (YAMLSec)
-      SHeader.sh_addr = YAMLSec->Address;
-    // We assume that .dynstr is always allocatable.
-    SHeader.sh_flags |= ELF::SHF_ALLOC;
-  }
+  // TODO: Allow this for any explicitly described section.
+  if (YAMLSec && Name == ".dynstr")
 }
 
 template <class ELFT>
 void ELFState<ELFT>::setProgramHeaderLayout(std::vector<Elf_Phdr> &PHeaders,
                                             std::vector<Elf_Shdr> &SHeaders) {
   uint32_t PhdrIdx = 0;
   for (auto &YamlPhdr : Doc.ProgramHeaders) {
     Elf_Phdr &PHeader = PHeaders[PhdrIdx++];