[yaml2obj] Introduce symbol version section to yaml2obj

[Higuoxing]

Summary

I am working on D54697, which needs creating .gnu.version_r section. Its entries are defined in sh_info field. So, I think it's reasonable to make sh_info field editable in raw content section.

[Higuoxing]

format codes

[jhenderson]

Whilst I'm not opposed to this in principle, I do wonder if the more correct thing to do is to make .gnu.version_r sections (which I assume have a distinctive ELF type) another kind of ELFYAML section (e.g. like RelocationSection).

I also think a refactor of the error handling is in order. Specifically, basically the same four lines of code exist any time Info or Link can be set. This should probably be pulled out into a separate function. Something like checkSectionIndex or similar.

[grimar]

I think this should be fine for now, with the refactoring change mentioned.

tools/yaml2obj/yaml2elf.cpp

264

I agree with James that this ideally should be refactored to a static helper probably.

[Higuoxing]

Thank you @jhenderson and @grimar for reviewing

Updating D54867: [yaml2obj] Introduce symbol version section to yaml2obj

I add .gnu.version_r section into yaml2obj, I don't know if this patch with such size is acceptable ... If needed, I could implement .gnu.version_d as well. Because they are similar ...

Here is my reference
https://akkadia.org/drepper/symbol-versioning

PS: I think we need a yaml2obj doc ... If this is on the todo list, I could help

[Higuoxing]

format code

[Higuoxing]

Updating D54867: [yaml2obj] Introduce symbol version section to yaml2obj

I implemented .gnu.version_d and .gnu.version_r section

[Higuoxing]

Updating D54867: [yaml2obj] Introduce symbol version section to yaml2obj

I implemented .gnu.version_d and .gnu.version_r section

[jhenderson]

Sorry, I've been a bit busy over the last couple of days. Where can I find documentation on these two sections you are adding support for?

[Higuoxing]

In D54867#1312797, @jhenderson wrote:

Sorry, I've been a bit busy over the last couple of days. Where can I find documentation on these two sections you are adding support for?

Thank you for taking time reviewing!

References:

[1] https://akkadia.org/drepper/symbol-versioning
[2] https://refspecs.linuxfoundation.org/LSB_2.1.0/LSB-Core-generic/LSB-Core-generic/symverdefs.html
[3] https://refspecs.linuxfoundation.org/LSB_2.1.0/LSB-Core-generic/LSB-Core-generic/symverrqmts.html

[Higuoxing]

test/tools/yaml2obj/symbol-version-section.yaml

74 ↗

(On Diff #175841)

According to https://www.akkadia.org/drepper/symbol-versioning

vda_next: byte offset to the next Elfxx_Verdaux entry. The first entry (pointed to by the Elfxx_Verdef entry, contains the actual defined name. The second and all later entries name predecessor versions.

What if change Auxiliaries to

Name <Required>
Predecessor/Parent <Optional> (Parent is used by `gnu-readelf` and `gnu-objdump`)

[Higuoxing]

This comment was removed by Higuoxing.

[Higuoxing]

Refactor some codes

[jhenderson]

I'm still not particularly familiar with yaml2obj, so my comments might not always make sense, so feel free to correct me.

On the two flag fields, it might be nice if the user can explicitly specify the flags they want setting (e.g. VER_FLG_BASE), as well as an arbitrary value.

I've not yet reviewed the writeSectionContent functions. I'll do those later.

include/llvm/ObjectYAML/ELFYAML.h

184 ↗

(On Diff #176096)

Is this supposed to be the Elfxx_Verdaux struct? If so, it should probably be renamed to VersionDefAux, or similar.

185 ↗

(On Diff #176096)

This should probably be a string, rather than a hex number, since it is just a name. yaml2obj should then automatically add it to a string table, referenced by the sh_link in the VerDef header.

189 ↗

(On Diff #176096)

Revision is an Elfxx_Half, so should be a uint16_t, right?

190–191 ↗

(On Diff #176096)

Ditto for these two fields. They should be Hex16.

192 ↗

(On Diff #176096)

The is an Elfxx_Word, which is always 32-bits, so should be Hex32.

193 ↗

(On Diff #176096)

What's the purpose of this field? It doesn't seem to map to any of the struct's fields.

197 ↗

(On Diff #176096)

Maybe also worth mentioning the section type here, since technically SHT_GNU_verdef sections could be called other things.

209–211 ↗

(On Diff #176096)

These are all twice the size I'd expect them to be, like in the VersionDef struct above. Why?

212 ↗

(On Diff #176096)

Similar to the VersionDefParent, this name should be a string.

216–217 ↗

(On Diff #176096)

Similar to above, these are the wrong types.

221 ↗

(On Diff #176096)

Similar to above, mention the section type here.

test/tools/yaml2obj/symbol-version-section.yaml

57 ↗

(On Diff #176096)

Nit: This should be ET_DYN or ET_EXEC, since it's got dynamic information.

74 ↗

(On Diff #175841)

Sorry, I'm not sure I understand this comment.

tools/yaml2obj/yaml2elf.cpp

229–237

Thanks for the refactor. Given the growing size of this change, would you mind doing it in a separate change and first, please, to reduce the complexity of this change?

[Higuoxing]

Abandon this patch, since @grimar has implemented .gnu.version_d section (https://reviews.llvm.org/rG623ae72ad46d378d6a2e40abf67d3c5e627bef7c)

test/tools/yaml2obj/raw-content-section-info.yaml

+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj -sections %t | FileCheck %s
+
+# CHECK:         Name: .raw.content (1)
+# CHECK-NEXT:    Type: SHT_PROGBITS (0x1)
+# CHECK-NEXT:    Flags [ (0x2)
+# CHECK-NEXT:      SHF_ALLOC (0x2)
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x0
+# CHECK-NEXT:    Offset: 0x180
+# CHECK-NEXT:    Size: 2
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 4660
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .raw.content
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Info:            0x1234
+    Content:         1234

tools/yaml2obj/yaml2elf.cpp

for (const auto &YamlPhdr : Doc.ProgramHeaders) {
     Phdr.p_type = YamlPhdr.Type;
     Phdr.p_flags = YamlPhdr.Flags;
     Phdr.p_vaddr = YamlPhdr.VAddr;
     Phdr.p_paddr = YamlPhdr.PAddr;
     PHeaders.push_back(Phdr);
   }
 }
 
 template <class ELFT>
 bool ELFState<ELFT>::initSectionHeaders(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.
   Elf_Shdr SHeader;
   zero(SHeader);
   SHeaders.push_back(SHeader);
 
   for (const auto &Sec : Doc.Sections) {
     zero(SHeader);
     SHeader.sh_name = DotShStrtab.getOffset(Sec->Name);
     SHeader.sh_type = Sec->Type;
     SHeader.sh_flags = Sec->Flags;
     SHeader.sh_addr = Sec->Address;
     SHeader.sh_addralign = Sec->AddressAlign;
 
     if (!Sec->Link.empty()) {
       unsigned Index;
       if (SN2I.lookup(Sec->Link, Index) && !to_integer(Sec->Link, Index)) {
         WithColor::error() << "Unknown section referenced: '" << Sec->Link
                            << "' at YAML section '" << Sec->Name << "'.\n";
         return false;
       }
       SHeader.sh_link = Index;
     }
 
-    if (auto S = dyn_cast<ELFYAML::RawContentSection>(Sec.get()))
+    if (auto S = dyn_cast<ELFYAML::RawContentSection>(Sec.get())) {
+      if (!Sec->Info.empty()) {
+        unsigned Index;
+        if (SN2I.lookup(Sec->Info, Index) && !to_integer(Sec->Info, Index)) {
+          WithColor::error() << "Unknown section referenced: '" << Sec->Info
+                            << "' at YAML section '" << Sec->Name << "'.\n";
+          return false;
+        }
+        SHeader.sh_info = Index;
+      }
       writeSectionContent(SHeader, *S, CBA);
-    else if (auto S = dyn_cast<ELFYAML::RelocationSection>(Sec.get())) {
+    } else if (auto S = dyn_cast<ELFYAML::RelocationSection>(Sec.get())) {
       if (S->Link.empty())
         // For relocation section set link to .symtab by default.
         SHeader.sh_link = getDotSymTabSecNo();
 
       unsigned Index;
       if (SN2I.lookup(S->Info, Index) && !to_integer(S->Info, Index)) {
         WithColor::error() << "Unknown section referenced: '" << S->Info
                            << "' at YAML section '" << S->Name << "'.\n";