[PATCH] D65140: [yaml2obj] - Allow custom fields for the SHT_UNDEF sections.

[George Rimar via Phabricator via llvm-commits]

grimar marked 2 inline comments as done.
grimar added inline comments.


================
Comment at: test/tools/yaml2obj/elf-custom-null-section.yaml:127
   - Type: SHT_PROGBITS
-    Name: .foo
+    Name: 'foo'
 
----------------
MaskRay wrote:
> Is there a reason for the `.foo` -> `'foo'` change?
> 
> `SHT_PROGBITS` should be aligned
> Is there a reason for the .foo -> 'foo' change?

It's just a different test case. (I added a one or two to the begining, so had to shift down the others).

> SHT_PROGBITS should be aligned

I think it is already aligned. We align fields for each section separatelly 
taking in account the largest length of the field used.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:966
     StringRef Name = Doc.Sections[I]->Name;
+    if (Name.empty())
+      continue;
----------------
MaskRay wrote:
> What happens if there is non-SHN_UNDEF section with an empty name?
Nothing bad. We have a test case with such situation, btw, it is:
https://github.com/llvm-mirror/llvm/blob/master/test/tools/yaml2obj/unnamed-section.yaml

ELF string table starts from zero byte, i.e. it contains the empty string:
https://github.com/llvm-mirror/llvm/blob/master/lib/MC/StringTableBuilder.cpp#L168
so there is not problems to get the `""` string offset.

Initially `unnamed-section.yaml` seems tested thet we do not crash when have an unnamed section.
When I added the support for zero/undef sections I got complains in this place about "Repeated section name...<empty>"
for unnamed sections. I decided that we can probably just allow them.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65140/new/

https://reviews.llvm.org/D65140