[yaml2obj][ELF] Make symbols optional for relocations

[jakehehrlich]

Summary

Some kinds of relocations do not have symbols, like R_X86_64_RELATIVE for instance. I would like to test this case in D36554 but currently can't because symbols are required by yaml2obj. The other option is using the empty symbol but that doesn't seem quite right to me.

This change makes the Symbol field of Relocation optional and in the case where the user does not specify a symbol name the Symbol index is 0.

[silvas]

Seems pretty straightforward. LGTM.

Also it's probably good to add a test documenting what happens when you have a relocation that does need a symbol but you don't provide one.

[jakehehrlich]

Added a test which documents the invalid relocations that can now be produced.

[silvas]

test/tools/yaml2obj/invalid-symboless-relocation.yaml

1 ↗

(On Diff #113285)

small wording nit: "should succeed" -> "succeeds", otherwise it isn't clear whether "should" implies "we don't do it now, but it should" and requires some context to disambiguate.

[jakehehrlich]

Updated. Should be good to land.

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

static bool classof(const Section *S) {
     return S->Kind == SectionKind::Group;
   }
 };
 
 struct Relocation {
   llvm::yaml::Hex64 Offset;
   int64_t Addend;
   ELF_REL Type;
-  StringRef Symbol;
+  Optional<StringRef> Symbol;
 };
 
 struct RelocationSection : Section {
   std::vector<Relocation> Relocations;
 
   RelocationSection() : Section(SectionKind::Relocation) {}
 
   static bool classof(const Section *S) {

llvm/trunk/lib/ObjectYAML/ELFYAML.cpp

 } // end anonymous namespace
 
 void MappingTraits<ELFYAML::Relocation>::mapping(IO &IO,
                                                  ELFYAML::Relocation &Rel) {
   const auto *Object = static_cast<ELFYAML::Object *>(IO.getContext());
   assert(Object && "The IO context is not initialized");
 
   IO.mapRequired("Offset", Rel.Offset);
-  IO.mapRequired("Symbol", Rel.Symbol);
+  IO.mapOptional("Symbol", Rel.Symbol);
 
   if (Object->Header.Machine == ELFYAML::ELF_EM(ELF::EM_MIPS) &&
       Object->Header.Class == ELFYAML::ELF_ELFCLASS(ELF::ELFCLASS64)) {
     MappingNormalization<NormalizedMips64RelType, ELFYAML::ELF_REL> Key(
         IO, Rel.Type);
     IO.mapRequired("Type", Key->Type);
     IO.mapOptional("Type2", Key->Type2, ELFYAML::ELF_REL(ELF::R_MIPS_NONE));
     IO.mapOptional("Type3", Key->Type3, ELFYAML::ELF_REL(ELF::R_MIPS_NONE));

llvm/trunk/test/tools/yaml2obj/invalid-symboless-relocation.yaml

+# This test succeeds but produces an invalid relocation. This test
+# documents this behavoir.
+# RUN: yaml2obj %s > %t
+# RUN: llvm-readobj -relocations %t | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Content:         "00000000"
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Link:            .symtab
+    Info:            .text
+    Relocations:
+      - Offset: 0x1000
+        Type:   R_X86_64_PC32
+
+#CHECK:     Relocations [
+#CHECK-NEXT:  Section (2) .rel.text {
+#CHECK-NEXT:    0x1000 R_X86_64_PC32 - 0x0
+#CHECK-NEXT:  }
+#CHECK-NEXT:]

llvm/trunk/test/tools/yaml2obj/symboless-relocation.yaml

+# Just make sure this isn't an error even though it has no Symbol
+# RUN: yaml2obj %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Content:         "00000000"
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Link:            .symtab
+    Info:            .text
+    Relocations:
+      - Offset: 0x1000
+        Type:   R_X86_64_RELATIVE

llvm/trunk/tools/yaml2obj/yaml2elf.cpp

ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
 
   auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
 
   for (const auto &Rel : Section.Relocations) {
     unsigned SymIdx = 0;
     // Some special relocation, R_ARM_v4BX for instance, does not have
     // an external reference.  So it ignores the return value of lookup()
     // here.
-    SymN2I.lookup(Rel.Symbol, SymIdx);
+    if (Rel.Symbol)
+      SymN2I.lookup(*Rel.Symbol, SymIdx);
 
     if (IsRela) {
       Elf_Rela REntry;
       zero(REntry);
       REntry.r_offset = Rel.Offset;
       REntry.r_addend = Rel.Addend;
       REntry.setSymbolAndType(SymIdx, Rel.Type, isMips64EL(Doc));
       OS.write((const char *)&REntry, sizeof(REntry));