[PATCH] D78782: Add .debug_ranges support to the DWARF YAML.

[Greg Clayton via Phabricator via llvm-commits]

clayborg added inline comments.


================
Comment at: llvm/test/tools/obj2yaml/macho-DWARF-debug-ranges.yaml:163
+    - ''
+    - ''
+DWARF:
----------------
Seems like another bug in the existing mach-o YAML stuff out of the scope of this patch. Don't want to remove in case it causes problems  with the current existing code as this was created by the current obj2yaml.


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:95-96
+    else if (CUAddrSize != AddrSize)
+      return createStringError(std::errc::invalid_argument,
+                               "address sizes vary in different compile units");
+  }
----------------
jhenderson wrote:
> Test case?
I tried to make a test case and ran into multiple issues:
- I modified my DWARF generator to emit two different compile units with different address sizes but obj2yaml won't load the DWARF as the DWARF parser asserts when it tries to make the YAML by loading the DWARF
- if I hand edit the YAML, section sizes change and all of the data is off since and there are the existing yaml2obj ignores a different AddrSize

This really never happens in reality. I had this as an assert before, but you requested the error. Since the DWARF parsing code already asserts on this issue, I would be fine if we did too here?


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:110
+      return E;
+    for (const auto &RLE: DwarfRanges.getEntries())
+      YamlRanges.Entries.push_back({RLE.StartAddress, RLE.EndAddress});
----------------
jhenderson wrote:
> The linter is still complaining about clang-format on this line.
I kept seeing this in reverse thinking it didn't want the space... After looking around it seems that a space _is_ supposed to be added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78782