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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 27 01:33:44 PDT 2020


jhenderson added a comment.

Does this patch add support in yaml2obj for debug_ranges (I get a bit lost as to what point this happens)? If it is newly supported, it should be tested in a yaml2obj test too.



================
Comment at: llvm/test/tools/obj2yaml/macho-DWARF-debug-ranges.yaml:1
+# Test that yaml2obj and obj2yaml can create mach-o files with valid
+# __debug_ranges section.
----------------
Something I'm trying to encourage where possible is to use '##' for comments in lit tests, as it makes the comments stand out from the functional parts of the test (lit and FileCheck run lines). You'll see that practice in many newer tests for obj2yaml/yaml2obj etc.


================
Comment at: llvm/test/tools/obj2yaml/macho-DWARF-debug-ranges.yaml:13
+#                  [0x0000000000000000, 0x0000000000000030)
+#                  [0x0000000000001000, 0x0000000000002000))
+#               DW_AT_stmt_list   (0x00000000)
----------------
It might be a good idea to add a test case for a base selection entry, with the first "address" being 0xffffffffffffffff, and then confirm that obj2yaml/yaml2obj can handle it and subsequent entries (or emit a sensible error if not).


================
Comment at: llvm/test/tools/obj2yaml/macho-DWARF-debug-ranges.yaml:53
+# CHECK-NEXT:           HighOffset:      0x0000000000002000
+
+
----------------
Nit: unnecessary blank line.


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:87
+Error dumpDebugRanges(DWARFContext &DCtx, DWARFYAML::Data &Y) {
+
+  // We are assuming all address byte sizes will be consistent across all
----------------
Nit: delete blank line.


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:88-89
+
+  // We are assuming all address byte sizes will be consistent across all
+  // compile units.
+  uint32_t AddrSize = 0;
----------------
Rather than this assumption, maybe it would be better to emit an error in this patch, if the sizes differ.


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:107
+      return E;
+    for (const auto &RLE: DwarfRanges.getEntries())
+      YamlRanges.Entries.push_back({RLE.StartAddress, RLE.EndAddress});
----------------
clang-format is complaining in a few places in this file. Please fix before committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78782





More information about the llvm-commits mailing list