[PATCH] D68270: DWARFDebugLoc: Add a function to get the address range of an entry

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h:93-108
+
+    /// Return the half-open range of addresses covered by this entry.
+    /// DW_LLE_offset_pair entries are resolved using the given base address,
+    /// and the supplied DWARFUnit is used to look up any pooled (debug_addr)
+    /// addresses. In case of failure, an error is returned. If we could not get
+    /// an address range because this is a base address selection entry
+    /// (DW_LLE_base_address), then the error is of type IsBaseAddressError, and
----------------
I think using an error to express base address selection entries probably is a bit much.

I think this API would be more suited to a higher level abstraction over the whole list, rather than one entry in it. (same way we do with ranges - that API has been more fleshed out because of the presence of more users (symbolizers, etc) that want to abstract over the different representations (v4, split, v5, v5-split) & I think shows a fairly good direction this should go in too)

One possible caveat: it might make more sense to provide either a lazy iterator or a callback for entries, rather than (as the ranges API does currently) a full in-memory vector of the computed/finalized ranges, for efficiency's sake.

Also - would be super great if we could generalize both the range and loclist printing to use some common infrastructure for printing both the half-open range in non-verbose form and the verbose printing with underlying forms including RLE/LLE encodings, old-style base address selection entries (in v4), and also being able to print the section details (like we do for ranges when they're printed in the debug_info section, but we don't do it when they're printed in the actual debug_ranges/rnglists section... ). I realize this is a bit of a big feature request, but figured I'd mention it in case it helps inform your design direction/makes sense to address together, etc. (for instance, it probably means that a pair of uint64_t is insufficient, since we'll want to carry SectionIndex too - and maybe the start/end/section triple could be the same data structure (with the same printing support) as used for ranges, nested inside the data structure that includes the expression and can print that too, etc)


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:220-222
+    else
+      return createStringError(inconvertibleErrorCode(),
+                               "Reading debug_addr failed");
----------------
LLVM style suggests avoiding "else after return" ( https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return )


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68270