[PATCH] D105522: [llvm-readobj][XCOFF] Fix the error dumping for the first item of StringTable.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 15 01:06:29 PDT 2021


jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/string-table.yaml:1
+## Test that the string table is dumped correctly.
+
----------------
Apaprently I missed in the other patch that there wasn't an llvm-readobj test case, even though there should have been.

Can you extend this test file to include a case where there is more than one symbol name, please?


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/string-table.yaml:7
+
+# SINGLE-BYTE: AddressSize: 64bit
+# SINGLE-BYTE: StringTable {
----------------
I don't think you need this line. Same goes below.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/string-table.yaml:9
+# SINGLE-BYTE: StringTable {
+# SINGLE-BYTE:   [     4]   n  
+# SINGLE-BYTE: }
----------------
Use -NEXT suffixes on your check patterns. This test in its current form will also pass with the following output:
```
StringTable {
<some garbage>
   [     4]   n
<more garbage>
}
```

Also, delete trailing spaces.

Same comments below.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/string-table.yaml:30
+
+## There is an empty string table with just the length field.
+## TODO: Print the string table size to check the length field.
----------------
Why does 32-bit XCOFF generate an empty string table if it isn't used? Is that a bug in the emitter, or a spec detail?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105522



More information about the llvm-commits mailing list