[PATCH] D65446: [yaml2obj/obj2yaml] - Add a basic support for extended section indexes.

Tue Aug 6 02:57:34 PDT 2019

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

================
Comment at: test/tools/obj2yaml/elf-sht-symtab-shndx.yaml:21
+# CASE1-NEXT:     EntSize: 0x0000000000000004
+# CASE1-NEXT:     Content: '000000000100000002000000'
+# CASE1-NEXT: Symbols:
----------------
jhenderson wrote:
> I think it would be clearer for obj2yaml to emit an array of entries (and yaml2obj to consume them). What do you think? (We should allow explicit EntSize/Content in the YAML to allow creation of broken stuff too).
> 
> ```
> - Name: .symtab_shndx
>   Type: SHT_SYMTAB_SHNDX
>   Link: .symtab
>   Entries:
>     - Value: 0
>     - Value: 1
>     - Value: 42
> ```
Sounds good. What about me doing this in a follow-up or do you prefer to have this from the start?

================
Comment at: tools/obj2yaml/elf2yaml.cpp:180

+  // SHT_SYMTAB_SHNDX section might be used when dumping all symbols below.
+  if (SymTabShndx) {
----------------
jhenderson wrote:
> I'm not sure what this comment is for? It doesn't seem really relevant for the code.
I am trying to say here that `SHT_SYMTAB_SHNDX` should be handled first, becase the code below (`dumpSymbols`)
might want to use `ShndxTable`. I'll reword.

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

https://reviews.llvm.org/D65446