[PATCH] D103455: [yaml2obj] Add support for writing the long symbol name.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:277-281
+      // For XCOFF32: A value of 0 indicates that the symbol name is in the
+      // string table or .debug section.
+      if (!Is64Bit)
+        W.write<int32_t>(0);
+      W.write<uint32_t>(Strings.getOffset(YamlSym.SymbolName));
----------------
Am I right in thinking that we don't yet have XCOFF64 support in yaml2obj by the point of this patch? If not, we should probably just change this to `W.write<int32_t>(0);` and add the Is64Bit check later (optionally add an `assert(!Is64Bit && "64-bit format not yet implemented")` as a reminder.

Additionally, the comment suggests that the string can be in one of two places, but as far as this code is concerned, it is only written in one place (the string table), right? I think talking about both here can be confusing, without making it clear which is actually used.


================
Comment at: llvm/test/tools/yaml2obj/XCOFF/long-symbol-name.yaml:18
+
+--- !XCOFF
+FileHeader:
----------------
Higuoxing wrote:
> 1. Can you add a symbol with short symbol name to illustrate that long names are stored in the string table and short names are stored in the .debug section?
> 
> 2. I think it's hard to tell whether a symbol name is from the string table or the .debug section in your test since llvm-readobj only prints the symbol name without mentioning where it's from. Are there any options for llvm-readobj that is able to tell us where the symbol name is from?
+1 to @Higuoxing's comments. In particular, for testing we need to be sure that we don't accidentally start writing short names in the string table. I don't have a clear suggestion on how to do this, but one option would be to ensure we have the ability to dump the raw string table itself, and then look at the contents (similar to how you could do `llvm-readobj -p=.strtab foo.elf` to get a list of strings).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103455