[PATCH] D120184: [DWARFYAML] Add support for emitting/dumping the .debug_str.dwo section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Thanks! It's sad to see the code duplication in DWARFEmitter.cpp, but I don't have a good alternative. Just a series of nits to fix.



================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:1131
+  else if (Name == ".debug_str.dwo")
+    SHeader.sh_flags = ELF::SHF_EXCLUDE | ELF::SHF_MERGE | ELF::SHF_STRINGS;
 
----------------
I'm somewhat surrpised to see `SHF_EXCLUDE` here. Is this what the compiler generates by default?


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-str.yaml:1
-## Test how we dump the .debug_str section.
+## Test how we dump the .debug_str[.dwo] section.
 
----------------
I think we should have a test case with both .debug_str and .debug_stro.dwo (non-empty).


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-str.yaml:152
 #      NO-TERMINATOR: Sections:
 # NO-TERMINATOR-NEXT:   - Name:    .debug_str
 # NO-TERMINATOR-NEXT:     Type:    SHT_PROGBITS
----------------
Shouldn't this be parameterised so that it can be the .dwo name if appropriate?


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:41
 
-## b) Generate the .debug_str section from the raw section content.
+## b) Generate the .debug_str[.dwo] section from the raw section content.
 
----------------



================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:64
 
-## c) Generate the .debug_str section when the "Size" is specified.
+## c) Generate the .debug_str[.dwo] section when the "Size" is specified.
 
----------------



================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:100-101
+
+## d.2) Test that yaml2obj emits an error message when both the "Size" and the
+## "debug_str.dwo" entry are specified at the same time.
 
----------------
I think you can avoid needing this comment, by just doing "debug_str[.dwo]" in the previous one, like you've done elsewhere.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:126-127
+
+## e.2) Test that yaml2obj emits an error message when both the "Content" and the
+## "debug_str.dwo" entry are specified at the same time.
+
----------------
Ditto.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:240-242
+## h.2) Test that if we try to initialize the .debug_str.dwo section with a "Type" with non-standard
+## syntax, e.g., SHT_DYNAMIC, yaml2obj will treat it as a dynamic section and discard the
+## content in the "DWARF" entry.
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120184