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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/DWARFYAML.cpp:36
+    SecNames.push_back("debug_ranges");
+  if (PubNames.Entries.size() != 0)
+    SecNames.push_back("debug_pubnames");
----------------
Higuoxing wrote:
> grimar wrote:
> > Why this is `!DWARF.PubNames.Entries` and not `!DWARF.PubNames`?
> > (The same question for things around).
> Because `Data` possesses `PubNames` directly, rather than `Optional<PubNames>`.
> 
> ```
> struct PubNames {
>   ...
>   std::vector<PubEntry> Entries;
> };
> 
> struct Data {
>   PubSection PubNames;
> };
> ```
> 
> Can I fix it in the next patch?
OK. I wonder if we could start from having 2 just sections here for start: ".debug_str" and ".debug_info"
since seems other lines here arenot needed for this patch?


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:741
+
+  ELFYAML::RawContentSection *RawSec =
+      dyn_cast_or_null<ELFYAML::RawContentSection>(YAMLSec);
----------------
I'd move it below, it is a bit too far from the place where `RawSec` is used first time.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:5
+# RUN: yaml2obj --docnum=1 %s -o %t1.o
+# RUN: llvm-dwarfdump --debug-str %t1.o | FileCheck %s
+
----------------
jhenderson wrote:
> I'd like to hear other people's opinions on this:
> 
> Presumably one of the reasons we are adding this support is because in the future, we'd like to write llvm-dwarfdump tests by using yaml2obj, but if we do that, we end up in a bit of a circular dependency, as we are using llvm-dwarfdump to test yaml2obj. That could result in a bug in the common library that results in both appearing to produce the right data, but actually it not being correct.
> 
> For .debug_str, it would be trivial to use other options, like llvm-readobj's --section-data or --string-dump options. I don't think this would work as well for other sections though, so maybe the circular dependency is okay? Thoughts?
> I don't think this would work as well for other sections though, so maybe the circular dependency is okay? Thoughts?

As far I understand, we have no much options. lib/DebugInfo is underlying layer used by llvm-dwarfdump, llvm-objdump to
dump the debug data (I believe). I'd try to use llvm-readobj's --section-data or other options where possible, like you suggest,
but it seems OK to use llvm-dwarfdump for other cases to me.

Partially I guess this problem will be solved by obj2yaml when it will be able to dump debug sections.
I.e. I expect that it might be able to catch some things after dumping objects produced by yaml2obj as probably will use some
independent code for dumping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80203