[PATCH] D104613: [llvm-readobj][XCOFF] Add support for printing the String Table.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 1 03:44:39 PDT 2021


jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, pending the bug fix (but see my inline comment - feel free to land this first if it makes testing the fix easier).



================
Comment at: llvm/test/tools/yaml2obj/XCOFF/basic-doc64.yaml:137
+# CHECK64-NEXT: StringTable {
+# CHECK64-NEXT:   [     3]   ..text
+# CHECK64-NEXT:   [     a]   .abs
----------------
Esme wrote:
> jhenderson wrote:
> > Does this test pass? Why is there a second "." in this name, if so?
> > 
> > What's at bytes 0-2 in the string table?
> Yes, the test does pass. The offset is `3`, but the real offset is `4` in the Symbol entry. It is not a bug of yaml2obj or this patch, since the issue occurs when check the StringTable of a compiled binary object. This should be a bug during parsing the StringTable, and I will post another patch to fix it before the patch.
> There are '\0' at bytes 0-2. Here is an example of dumping the string data for every bytes offset of a StringTable for a compiled binary file. Where the correct result should be  `[     4]   alongname`
> ```
> StringTable {
> -every bytes
> 0: 
> 1: 
> 2: 
> 3: alongname
> 4: alongname
> 5: longname
> 6: ongname
> 7: ngname
> 8: gname
> 9: name
> 10: ame
> 11: me
> 12: e
> 13: 
> -Result
>   [     3]   .alongname  
> }
> ```
> 
Okay, thanks. Please post the other patch, then rebase this one on it. Alternatively, I don't object to you landing this one, with FIXME notes added to the test describing the problem, so that you can use the --string-table option to show the bug fix is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104613



More information about the llvm-commits mailing list