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

[James Henderson via Phabricator via llvm-commits]

jhenderson 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:
----------------
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
```


================
Comment at: test/tools/obj2yaml/elf-sht-symtab-shndx.yaml:51
+
+## Check that yaml2obj is unable to dump the object that has
+## symbols with section index == SHN_XINDEX, but no SHT_SYMTAB_SHNDX section.
----------------
dump the object that -> dump an object, which

(same throughout the test)


================
Comment at: test/tools/obj2yaml/elf-sht-symtab-shndx.yaml:70
+## Check that yaml2obj is unable to dump the object that has symbols with
+## section index == SHN_XINDEX, but SHT_SYMTAB_SHNDX table contain invalid indices
+## that are larger than total number of the sections.
----------------
contain -> contains


================
Comment at: test/tools/obj2yaml/elf-sht-symtab-shndx.yaml:103
+
+## TODO: The error message below needs rewording. Size should not be equal to the number of symbols.
+## CASE4: Error reading file: [[FILE]]: SHT_SYMTAB_SHNDX section has sh_size (48) which is not equal to the number of symbols (3)
----------------
TODO -> FIXME


================
Comment at: test/tools/obj2yaml/elf-sht-symtab-shndx.yaml:125
+## ELF gABI allows having multiple SHT_SYMTAB_SHNDX sections.
+## We only support having one and associated with .symtab now.
+
----------------
Remove "and"


================
Comment at: test/tools/obj2yaml/elf-sht-symtab-shndx.yaml:151
+## Check that yaml2obj can't dump the object if SHT_SYMTAB_SHNDX is
+## associated not with a SHT_SYMTAB section (this case is illegal).
+
----------------
is not associated


================
Comment at: test/tools/obj2yaml/elf-sht-symtab-shndx.yaml:174
+## Check that yaml2obj can't dump the object if SHT_SYMTAB_SHNDX is
+## associated with SHT_DYNSYM section (not implemented yet).
+
----------------
with the/a


================
Comment at: test/tools/yaml2obj/elf-sht-symtab-shndx.yaml:1
+## Check that yaml2obj is able to produce the output
+## when a symbol with section index SHN_XINDEX is used,
----------------
remove "the"


================
Comment at: test/tools/yaml2obj/elf-sht-symtab-shndx.yaml:25-26
+## even when symbol's section index value is low enough to not require the extended symtab. 
+## Note: obj2yaml test suite checks that symbols still have the SHN_XINDEX index,
+##       it is not possible to check this with llvm-readobj/llvm-readelf.
+
----------------
index. It


================
Comment at: test/tools/yaml2obj/elf-sht-symtab-shndx.yaml:29
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: llvm-readobj --sections --symbols 2>&1 %t2 | FileCheck %s --check-prefix=CASE2
+
----------------
You could use --section-data to print the bytes of the symbol table and check those maybe?


================
Comment at: test/tools/yaml2obj/elf-sht-symtab-shndx.yaml:63
+
+## Check that yaml2obj allows producing the broken SHT_SYMTAB_SHNDX section
+## content (in the case below it contains 0xff as an index of section,
----------------
delete "the"


================
Comment at: test/tools/yaml2obj/elf-sht-symtab-shndx.yaml:64
+## Check that yaml2obj allows producing the broken SHT_SYMTAB_SHNDX section
+## content (in the case below it contains 0xff as an index of section,
+## which is larger than total number of sections in the file.
----------------
missing ')'

index of a section


================
Comment at: test/tools/yaml2obj/elf-sht-symtab-shndx.yaml:65
+## content (in the case below it contains 0xff as an index of section,
+## which is larger than total number of sections in the file.
+
----------------
than the total


================
Comment at: test/tools/yaml2obj/elf-sht-symtab-shndx.yaml:90
+
+## Check that yaml2obj reports an error if symbol index does not fit into 2 bytes.
+
----------------
if a symbol index


================
Comment at: tools/obj2yaml/elf2yaml.cpp:180
 
+  // SHT_SYMTAB_SHNDX section might be used when dumping all symbols below.
+  if (SymTabShndx) {
----------------
I'm not sure what this comment is for? It doesn't seem really relevant for the code.


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

https://reviews.llvm.org/D65446