[PATCH] D42481: [DebugInfo] Add basic support for DWARF 5 .debug_rnglists dumping

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:498
+                                   isLittleEndian(), 0);
+    DWARFDebugRnglists Rnglists;
+    uint32_t Offset = 0;
----------------
Move this variable into the loop - since it's not intended to preserve state between iterations of the loop? I realize it means some memory reuse by keeping it outside the loop, but I'd probably classify that as premature optimization?


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugRnglists.cpp:19
+void DWARFDebugRnglists::clear() {
+  std::memset(&HeaderData, 0, sizeof(Header));
+  Offsets.clear();
----------------
Could this be written as "HeaderData = {}" ?


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugRnglists.cpp:24-36
+template <typename T> static Error createError(Twine Str, T Value) {
+  return make_error<StringError>(Str + std::to_string(Value),
+                                 inconvertibleErrorCode());
+}
+
+template <typename T1, typename T2>
+static Error createError(Twine LeadingStr, T1 FirstValue, Twine MiddleStr,
----------------
These seem a bit weirdly special cased to handle certain string layouts.

Could you instead use the LLVM string format library as you've used elsewhere?


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugRnglists.cpp:103
+    case dwarf::DW_RLE_base_address:
+      llvm_unreachable("unimplemented rnglists encoding");
+      break;
----------------
Probably more appropriate to use createError here - since it's certainly reachable by user-defined inputs. But up to you if you reckon this'll be around for a really short time.


================
Comment at: test/tools/llvm-dwarfdump/X86/debug_rnglists_invalid.s:9-27
+# This case shows that all sections that can be interpreted are dumped, even when some can't be.
+# RUN: llvm-mc %S/Inputs/debug_rnglists_some_invalid.s -filetype obj -triple x86_64-pc-linux -o - | \
+# RUN: llvm-dwarfdump --debug-rnglists - 2> %t.err | FileCheck %s --check-prefix=GOOD
+# RUN: FileCheck %s --input-file %t.err --check-prefix=BAD
+# GOOD: .debug_rnglists contents:
+# GOOD-NEXT: Range List Header: length = 0x0000001e, version = 0x0005, addr_size = 0x08, seg_size = 0x00, offset_entry_count = 0x00000001
+# GOOD-NEXT: Offsets: [
----------------
Looks like this should move to the some_invalid.s file? Alternatively, could that assembly be moved into this file?


Repository:
  rL LLVM

https://reviews.llvm.org/D42481