[PATCH] D80203: [ObjectYAML][DWARF] Add DWARF entry in ELFYAML.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

FYI, I'm not working tomorrow, so it'll be Friday before I am able to give further comments. If @grimar approves this before Friday, I'm happy for you to commit and I'll review it post-commit after I'm back.



================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:1254
+    // For sections identified by their names, we have to set the section type
+    // for them earlier.
+    if (ELFYAML::StackSizesSection::nameMatches(Name))
----------------
I'd add the following sentence to this comment: "This means they can have other SHT_* type values, but that the special parsing of those types is not supported."

This seems like a reasonable solution to me, but I'm interested to here @grimar's thoughts.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:145
+    Offset:       0x00000050         # 0x40 for the first section.
+    Size:         6                  # Set the "Size" so that we can reuse the check tag "OVERRIEDDEN"
+
----------------
There's a typo: OVERRIEDDEN -> OVERRIDDEN


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:192
+
+## i) Test that yaml2obj will generate the .debug_str section whose sh_type is a preserved one,
+## e.g. SHT_DYNAMIC
----------------
I'm not sure "preserved" makes sense here. Probably you mean "is a type which usually uses a different syntax".


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:196
+# RUN: yaml2obj --docnum=9 %s -o %t9.o
+# RUN: llvm-readelf -S %t9.o | FileCheck %s --check-prefix=DYNAMIC-EMPTY
+
----------------
I'd avoid using -EMPTY in your suffix name, since -EMPTY is a valid FileCheck suffix. Probably it's okay just to call these cases DYNAMIC1, DYNAMIC2 etc.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:198
+
+#                 Name      Type    Address          Offset Size   ES Flg Lk Inf Al
+# DYNAMIC-EMPTY: .debug_str DYNAMIC 0000000000000000 000040 000000 01 MS  0  0   0
----------------
Did you deliberately not include the DYNAMIC-EMPTY tag here? If so, you need '##'. Same applies in all the other test cases too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80203