DWARFDebugLoclists: add location list "interpretation" logic

[labath]

Summary

This patch extracts the logic for computing the "absolute" locations,
which was partially present in the debug_loclists dumper, completes it,
and moves it into a separate function. This makes it possible to later
reuse the same logic for uses other than dumping.

The dumper is changed to reuse the location list interpreter, and its
format is changed somewhat. In "verbose" mode it prints the "raw" value
of a location list, the interpreted location (if available) and the
expression itself. In non-verbose mode it prints only one of the
location forms: it prefers the interpreted form, but falls back to the
"raw" format if interpretation is not possible (for instance, because we
were not given a base address, or the resolution of indirect addresses
failed).

This patch also undos some of the changes made in D69672, namely the
part about making all functions static. The main reason for this is that
I learned that the original approach (dumping only fully resolved
locations) meant that it was impossible to rewrite one of the existing
tests. To make that possible (and make the "inline location" dump work
in more cases), I now reuse the same dumping mechanism as is used for
section-based dumping. As this required having more objects know about
the various location lists classes, it seemed like a good idea to create
an interface abstracting the difference between them.

Therefore, I now create a DWARFLocationTable class, which will serve as
a base class for the location list classes. DWARFDebugLoclists is made
to inherit from that. DWARFDebugLoc will follow.

Another positive effect of this change is that section-based dumping
code will not need to use templates (as originally) envisioned, and that
the argument lists of the dumping functions become shorter.

[labath]

llvm/test/DebugInfo/X86/loclists-dwp.ll

22–25

This is the test I was talking about above. It's sole purpose is to test handling of dwp files, but one cannot resolve addresses properly just by looking at the dwp file -- it doesn't have the address pool, and llvm-dwarfdump does not have the ability to provide it right now. By maintaining the ability to print _something_ in this case, we can keep the test, and make dumps of dwp/dwo files more useful.

[dblaikie]

llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp

61–63

This error misses an opportunity to say why the indirect address resolution failed - was it because the section is missing entirely, or because the index provided is outside the range of the addresses present in the address pool. "nice to have/at some point" but if the mood strikes you/you see a nice way to support that better (Error<SectionedAddress> rather than Optional<SectionedAddress> from the lookup callback, I guess - it could be then stitched together - taking the stringified version of that error and adding it into a string that has the local context here, the LLE and anything else that might be useful (offset in the section, though that isn't preserved - no worries))

126

Future change: should we roll the DWARFFormValue::dumpAddressSection from DWARFDie.cpp::dumpRanges into DWARFAddressRange::dump to get the pretty printing of section name/number in here too?

(currently probably not super important - I don't think we ever produce location lists for globals, and locals are all in one function which is currently one section - but with Propeller it'd be possible to have a location list across multiple sections so dumping the section name/number would be useful then)

[SouraVX]

llvm/test/tools/llvm-dwarfdump/X86/debug_loc_dwo.s

9

Hi Pavel, I'm not sure of this, here you're using "DW_LLE_startx_length" in a debug_loc.dwo -- any ideas ?
My primary concern here, is the Debuggability impact.
Though haven't, checked this? Did you get chance to debug these binaries{Containing above style location list} with LLDB or GDB ?? since predwarf(debug_loc and debug_loc.dwo) is debuggable with both debuggers.

[labath]

llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp

61–63

I'll think about that. Right now that isn't my priority, as this error isn't even displayed in llvm-dwarfdump -- the reason for that is because it's tricky to figure out when displaying it "useful" (e.g. when dumping a dwo file, all attempts to resolve addresses would fail, and so the errors would just be noise).

126

Yes, I think we should do that. In fact, this is the reason I did not add the section index manipulation to more places yet (e.g. loclists parser) -- there's no way to test that now.

llvm/test/tools/llvm-dwarfdump/X86/debug_loc_dwo.s

9

I'm afraid I don't understand your concerns here. Can you elaborate?

This patch doesn't change how debug_loc.dwo sections are generated. It doesn't even change how they are parsed, for the most part. The only real difference in this test is that we now explicitly print the raw entry contents whereas previously we tried to "pretty print" it, which wasn't really useful without an address pool.

clang (and gcc, I believe) have been emitting this unstandardized .debug_loc.dwo format for a while. I haven't tried debugging these binaries lately, but I know that lldb has support for this format, but it is incomplete.

[SouraVX]

llvm/test/tools/llvm-dwarfdump/X86/debug_loc_dwo.s

9

Thanks Pavel, for clarifying this. Indeed it's not touching the emission / generating part.
Obviously Debuggability is fine, checked with latest master.

This test is mostly about dwarfdump. Just curious here as to why, you've chosen "DW_LLE_*" representation here ?? those are [DWARF5] part.

[dblaikie]

llvm/test/tools/llvm-dwarfdump/X86/debug_loc_dwo.s

9

The pre-standard/standard-proposal version of Fission used this same terminology: https://gcc.gnu.org/wiki/DebugFission so the names are suitable (& since they had no prior art in the standard there was no need to use an implementation namespace (DW_LLE_GNU_etc) first (well, kind of/might've been good to do that))

[SouraVX]

Now, I'm assuming "DW_LLE*" dumping/other stuff works fine with llvm-objdump and objdump GDB. since they share the same encoding's {pre/post DWARF5}?
But thier's one more slight glitch, for case of debug_loc{No split} section. We are still emitting raw ranges in a sense --

llvm-dwarfdump -debug-loc -v a.out   --using latest master
.debug_loc contents:
0x00000000:
            [0x0000000000000000,  0x0000000000000004): DW_OP_reg5 RDI
            [0x0000000000000004,  0x0000000000000018): DW_OP_reg3 RBX

Now I understand it's not priority, but atleast for sake of uniformity and saving some confusion. We should use it in both ways or completely drop it.
Minor addition -- objdump is still broken to handle [DWARF5] stuff. for debug_loc and loc.dwo it dumps raw ranges.

[labath]

In D70081#1743535, @SouraVX wrote:

Now, I'm assuming "DW_LLE*" dumping/other stuff works fine with llvm-objdump and objdump GDB. since they share the same encoding's {pre/post DWARF5}?
But thier's one more slight glitch, for case of debug_loc{No split} section. We are still emitting raw ranges in a sense --

llvm-dwarfdump -debug-loc -v a.out   --using latest master
.debug_loc contents:
0x00000000:
            [0x0000000000000000,  0x0000000000000004): DW_OP_reg5 RDI
            [0x0000000000000004,  0x0000000000000018): DW_OP_reg3 RBX

Now I understand it's not priority, but atleast for sake of uniformity and saving some confusion. We should use it in both ways or completely drop it.
Minor addition -- objdump is still broken to handle [DWARF5] stuff. for debug_loc and loc.dwo it dumps raw ranges.

You're absolutely right, and I am planning to do that. I already have a patch which does just that -- I just need to clean it up before submission (I'll do that today or tomorrow).

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h

 //===- DWARFDebugLoc.h ------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_DEBUGINFO_DWARF_DWARFDEBUGLOC_H
 #define LLVM_DEBUGINFO_DWARF_DWARFDEBUGLOC_H
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/DebugInfo/DIContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
+#include "llvm/DebugInfo/DWARF/DWARFLocationExpression.h"
 #include "llvm/DebugInfo/DWARF/DWARFRelocMap.h"
 #include <cstdint>
 
 namespace llvm {
 class DWARFUnit;
 class MCRegisterInfo;
 class raw_ostream;
 
+/// A single location within a location list. Entries are stored in the DWARF5
+/// form even if they originally come from a DWARF<=4 location list.
+struct DWARFLocationEntry {
+  /// The entry kind (DW_LLE_***).
+  uint8_t Kind;
+
+  /// The first value of the location entry (if applicable).
+  uint64_t Value0;
+
+  /// The second value of the location entry (if applicable).
+  uint64_t Value1;
+
+  /// The location expression itself (if applicable).
+  SmallVector<uint8_t, 4> Loc;
+};
+
+/// An abstract base class for various kinds of location tables (.debug_loc,
+/// .debug_loclists, and their dwo variants).
+class DWARFLocationTable {
+public:
+  DWARFLocationTable(DWARFDataExtractor Data) : Data(std::move(Data)) {}
+  virtual ~DWARFLocationTable() = default;
+
+  /// Call the user-provided callback for each entry (including the end-of-list
+  /// entry) in the location list starting at \p Offset. The callback can return
+  /// false to terminate the iteration early. Returns an error if it was unable
+  /// to parse the entire location list correctly. Upon successful termination
+  /// \p Offset will be updated point past the end of the list.
+  virtual Error
+  visitLocationList(uint64_t *Offset,
+                    function_ref<bool(const DWARFLocationEntry &)> F) const = 0;
+
+  /// Dump the location list at the given \p Offset. The function returns true
+  /// iff it has successfully reched the end of the list. This means that one
+  /// can attempt to parse another list after the current one (\p Offset will be
+  /// updated to point past the end of the current list).
+  bool dumpLocationList(uint64_t *Offset, raw_ostream &OS, uint64_t BaseAddr,
+                        const MCRegisterInfo *MRI, DWARFUnit *U,
+                        DIDumpOptions DumpOpts, unsigned Indent) const;
+
+protected:
+  DWARFDataExtractor Data;
+
+  virtual void dumpRawEntry(const DWARFLocationEntry &Entry, raw_ostream &OS,
+                            unsigned Indent) const = 0;
+};
+
 class DWARFDebugLoc {
 public:
   /// A single location within a location list.
   struct Entry {
     /// The beginning address of the instruction range.
     uint64_t Begin;
     /// The ending address of the instruction range.
     uint64_t End;
public:
 
   /// Return the location list at the given offset or nullptr.
   LocationList const *getLocationListAtOffset(uint64_t Offset) const;
 
   Expected<LocationList>
   parseOneLocationList(const DWARFDataExtractor &Data, uint64_t *Offset);
 };
 
-class DWARFDebugLoclists {
+class DWARFDebugLoclists final : public DWARFLocationTable {
 public:
-  // Unconstructible.
-  DWARFDebugLoclists() = delete;
-
-  struct Entry {
-    uint8_t Kind;
-    uint64_t Offset;
-    uint64_t Value0;
-    uint64_t Value1;
-    SmallVector<uint8_t, 4> Loc;
-    void dump(raw_ostream &OS, uint64_t &BaseAddr, bool IsLittleEndian,
-              unsigned AddressSize, const MCRegisterInfo *MRI, DWARFUnit *U,
-              DIDumpOptions DumpOpts, unsigned Indent, size_t MaxEncodingStringLength) const;
-  };
-
-  /// Call the user-provided callback for each entry (including the end-of-list
-  /// entry) in the location list starting at \p Offset. The callback can return
-  /// false to terminate the iteration early. Returns an error if it was unable
-  /// to parse the entire location list correctly. Upon successful termination
-  /// \p Offset will be updated point past the end of the list.
-  static Error visitLocationList(const DWARFDataExtractor &Data,
-                                 uint64_t *Offset, uint16_t Version,
-                                 llvm::function_ref<bool(const Entry &)> F);
+  DWARFDebugLoclists(DWARFDataExtractor Data, uint16_t Version)
+      : DWARFLocationTable(std::move(Data)), Version(Version) {}
 
-  /// Dump the location list at the given \p Offset. The function returns true
-  /// iff it has successfully reched the end of the list. This means that one
-  /// can attempt to parse another list after the current one (\p Offset will be
-  /// updated to point past the end of the current list).
-  static bool dumpLocationList(const DWARFDataExtractor &Data, uint64_t *Offset,
-                               uint16_t Version, raw_ostream &OS,
-                               uint64_t BaseAddr, const MCRegisterInfo *MRI,
-                               DWARFUnit *U, DIDumpOptions DumpOpts,
-                               unsigned Indent);
+  Error visitLocationList(
+      uint64_t *Offset,
+      function_ref<bool(const DWARFLocationEntry &)> F) const override;
 
   /// Dump all location lists within the given range.
-  static void dumpRange(const DWARFDataExtractor &Data, uint64_t StartOffset,
-                        uint64_t Size, uint16_t Version, raw_ostream &OS,
-                        uint64_t BaseAddr, const MCRegisterInfo *MRI,
-                        DIDumpOptions DumpOpts);
+  void dumpRange(uint64_t StartOffset, uint64_t Size, raw_ostream &OS,
+
+protected:
+  void dumpRawEntry(const DWARFLocationEntry &Entry, raw_ostream &OS,
+                    unsigned Indent) const override;
+
+private:
+  uint16_t Version;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_DWARF_DWARFDEBUGLOC_H

llvm/include/llvm/DebugInfo/DWARF/DWARFLocationExpression.h

+//===- DWARFLocationExpression.h --------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DEBUGINFO_DWARF_DWARFLOCATIONEXPRESSION_H
+#define LLVM_DEBUGINFO_DWARF_DWARFLOCATIONEXPRESSION_H
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
+
+namespace llvm {
+
+/// Represents a single DWARF expression, whose value is location-dependent.
+/// Typically used in DW_AT_location attributes to describe the location of
+/// objects.
+struct DWARFLocationExpression {
+  /// The address range in which this expression is valid. None denotes a
+  /// default entry which is valid in addresses not covered by other location
+  /// expressions, or everywhere if there are no other expressions.
+  Optional<DWARFAddressRange> Range;
+
+  /// The expression itself.
+  SmallVector<uint8_t, 4> Expr;
+};
+
+} // end namespace llvm
+
+#endif // LLVM_DEBUGINFO_DWARF_DWARFLOCATIONEXPRESSION_H

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

if (Error E = Header.extract(Data, &Offset)) {
       WithColor::error() << toString(std::move(E)) << '\n';
       return;
     }
 
     Header.dump(OS, DumpOpts);
 
     uint64_t EndOffset = Header.length() + Header.getHeaderOffset();
     Data.setAddressSize(Header.getAddrSize());
+    DWARFDebugLoclists Loc(Data, Header.getVersion());
     if (DumpOffset) {
       if (DumpOffset >= Offset && DumpOffset < EndOffset) {
         Offset = *DumpOffset;
-        DWARFDebugLoclists::dumpLocationList(Data, &Offset, Header.getVersion(),
-                                             OS, /*BaseAddr=*/0, MRI, nullptr,
-                                             DumpOpts, /*Indent=*/0);
+        Loc.dumpLocationList(&Offset, OS, /*BaseAddr=*/0, MRI, nullptr,
         OS << "\n";
         return;
       }
     } else {
-      DWARFDebugLoclists::dumpRange(Data, Offset, EndOffset - Offset,
-                                    Header.getVersion(), OS, 0, MRI, DumpOpts);
+      Loc.dumpRange(Offset, EndOffset - Offset, OS, /*BaseAddr=*/0, MRI,
+                    DumpOpts);
     }
     Offset = EndOffset;
   }
 }
 
 void DWARFContext::dump(
     raw_ostream &OS, DIDumpOptions DumpOpts,
     std::array<Optional<uint64_t>, DIDT_ID_Count> DumpOffsets) {
DWARFDataExtractor Data(*DObj, DObj->getLoclistsSection(), isLittleEndian(),
                             0);
     dumpLoclistsSection(OS, DumpOpts, Data, getRegisterInfo(), *Off);
   }
   if (const auto *Off =
           shouldDump(ExplicitDWO, ".debug_loc.dwo", DIDT_ID_DebugLoc,
                      DObj->getLocDWOSection().Data)) {
     DWARFDataExtractor Data(*DObj, DObj->getLocDWOSection(), isLittleEndian(),
                             4);
+    DWARFDebugLoclists Loc(Data, /*Version=*/4);
     if (*Off) {
       uint64_t Offset = **Off;
-      DWARFDebugLoclists::dumpLocationList(Data, &Offset, /*Version=*/4, OS,
-                                           /*BaseAddr=*/0, getRegisterInfo(),
-                                           nullptr, DumpOpts, /*Indent=*/0);
+      Loc.dumpLocationList(&Offset, OS,
+                           /*BaseAddr=*/0, getRegisterInfo(), nullptr, DumpOpts,
+                           /*Indent=*/0);
       OS << "\n";
     } else {
-      DWARFDebugLoclists::dumpRange(Data, 0, Data.getData().size(),
-                                    /*Version=*/4, OS, /*BaseAddr=*/0,
-                                    getRegisterInfo(), DumpOpts);
+      Loc.dumpRange(0, Data.getData().size(), OS, /*BaseAddr=*/0,
     }
   }
 
   if (const auto *Off = shouldDump(Explicit, ".debug_frame", DIDT_ID_DebugFrame,
                                    DObj->getFrameSection().Data))
     getDebugFrame()->dump(OS, getRegisterInfo(), *Off);
 
   if (const auto *Off = shouldDump(Explicit, ".eh_frame", DIDT_ID_DebugFrame,

llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp

 #include "llvm/Support/Format.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cinttypes>
 #include <cstdint>
 
 using namespace llvm;
+using object::SectionedAddress;
+
+namespace {
+class DWARFLocationInterpreter {
+  Optional<object::SectionedAddress> Base;
+  std::function<Optional<object::SectionedAddress>(uint32_t)> LookupAddr;
+
+public:
+  DWARFLocationInterpreter(
+      Optional<object::SectionedAddress> Base,
+      std::function<Optional<object::SectionedAddress>(uint32_t)> LookupAddr)
+      : Base(Base), LookupAddr(std::move(LookupAddr)) {}
+
+  Expected<Optional<DWARFLocationExpression>>
+  Interpret(const DWARFLocationEntry &E);
+};
+} // namespace
+
+static Error createResolverError(uint32_t Index, unsigned Kind) {
+  return createStringError(errc::invalid_argument,
+                           "Unable to resolve indirect address %u for: %s",
+                           Index, dwarf::LocListEncodingString(Kind).data());
+}
+
+Expected<Optional<DWARFLocationExpression>>
+DWARFLocationInterpreter::Interpret(const DWARFLocationEntry &E) {
+  switch (E.Kind) {
+  case dwarf::DW_LLE_end_of_list:
+    return None;
+  case dwarf::DW_LLE_base_addressx: {
+    Base = LookupAddr(E.Value0);
+    if (!Base)
+      return createResolverError(E.Value0, E.Kind);
+    return None;
+  }
+  case dwarf::DW_LLE_startx_length: {
+    Optional<SectionedAddress> LowPC = LookupAddr(E.Value0);
+    if (!LowPC)
+      return createResolverError(E.Value0, E.Kind);
+    return DWARFLocationExpression{DWARFAddressRange{LowPC->Address,
+                                                     LowPC->Address + E.Value1,
+                                                     LowPC->SectionIndex},
+                                   E.Loc};
+  }
+  case dwarf::DW_LLE_offset_pair:
+    if (!Base) {
+      return createStringError(
+          inconvertibleErrorCode(),
+          "Unable to resolve DW_LLE_offset_pair: base address unknown");
+    }
+    return DWARFLocationExpression{DWARFAddressRange{Base->Address + E.Value0,
+                                                     Base->Address + E.Value1,
+                                                     Base->SectionIndex},
+                                   E.Loc};
+  case dwarf::DW_LLE_base_address:
+    Base = SectionedAddress{E.Value0, SectionedAddress::UndefSection};
+    return None;
+  case dwarf::DW_LLE_start_length:
+    return DWARFLocationExpression{
+        DWARFAddressRange{E.Value0, E.Value0 + E.Value1,
+                          SectionedAddress::UndefSection},
+        E.Loc};
+  default:
+    llvm_unreachable("unreachable locations list kind");
+  }
+}
 
 // When directly dumping the .debug_loc without a compile unit, we have to guess
 // at the DWARF version. This only affects DW_OP_call_ref, which is a rare
 // expression that LLVM doesn't produce. Guessing the wrong version means we
 // won't be able to pretty print expressions in DWARF2 binaries produced by
 // non-LLVM tools.
 static void dumpExpression(raw_ostream &OS, ArrayRef<uint8_t> Data,
                            bool IsLittleEndian, unsigned AddressSize,
                            const MCRegisterInfo *MRI, DWARFUnit *U) {
   DWARFDataExtractor Extractor(toStringRef(Data), IsLittleEndian, AddressSize);
   DWARFExpression(Extractor, dwarf::DWARF_VERSION, AddressSize).print(OS, MRI, U);
 }
 
+bool DWARFLocationTable::dumpLocationList(uint64_t *Offset, raw_ostream &OS,
+                                          uint64_t BaseAddr,
+                                          const MCRegisterInfo *MRI,
+                                          DWARFUnit *U, DIDumpOptions DumpOpts,
+                                          unsigned Indent) const {
+  DWARFLocationInterpreter Interp(
+      SectionedAddress{BaseAddr, SectionedAddress::UndefSection},
+      [U](uint32_t Index) -> Optional<SectionedAddress> {
+        if (U)
+          return U->getAddrOffsetSectionItem(Index);
+        return None;
+      });
+  OS << format("0x%8.8" PRIx64 ": ", *Offset);
+  Error E = visitLocationList(Offset, [&](const DWARFLocationEntry &E) {
+    Expected<Optional<DWARFLocationExpression>> Loc = Interp.Interpret(E);
+    if (!Loc || DumpOpts.Verbose)
+      dumpRawEntry(E, OS, Indent);
+    if (Loc && *Loc) {
+      OS << "\n";
+      OS.indent(Indent);
+      if (DumpOpts.Verbose)
+        OS << "          => ";
+      Loc.get()->Range->dump(OS, Data.getAddressSize(), DumpOpts);
+    }
+    if (!Loc)
+      consumeError(Loc.takeError());
+
+    if (E.Kind != dwarf::DW_LLE_base_address &&
+        E.Kind != dwarf::DW_LLE_base_addressx &&
+        E.Kind != dwarf::DW_LLE_end_of_list) {
+      OS << ": ";
+      dumpExpression(OS, E.Loc, Data.isLittleEndian(), Data.getAddressSize(),
+                     MRI, U);
+    }
+    return true;
+  });
+  if (E) {
+    OS << "\n";
+    OS.indent(Indent);
+    OS << "error: " << toString(std::move(E));
+    return false;
+  }
+  return true;
+}
+
 void DWARFDebugLoc::LocationList::dump(raw_ostream &OS, uint64_t BaseAddress,
                                        bool IsLittleEndian,
                                        unsigned AddressSize,
                                        const MCRegisterInfo *MRI, DWARFUnit *U,
                                        DIDumpOptions DumpOpts,
                                        unsigned Indent) const {
   for (const Entry &E : Entries) {
     OS << '\n';
while (Offset < data.getData().size()) {
     else {
       logAllUnhandledErrors(LL.takeError(), WithColor::error());
       break;
     }
   }
 }
 
 Error DWARFDebugLoclists::visitLocationList(
-    const DWARFDataExtractor &Data, uint64_t *Offset, uint16_t Version,
-    llvm::function_ref<bool(const Entry &)> F) {
+    uint64_t *Offset, function_ref<bool(const DWARFLocationEntry &)> F) const {
 
   DataExtractor::Cursor C(*Offset);
   bool Continue = true;
   while (Continue) {
-    Entry E;
-    E.Offset = C.tell();
+    DWARFLocationEntry E;
     E.Kind = Data.getU8(C);
     switch (E.Kind) {
     case dwarf::DW_LLE_end_of_list:
       break;
     case dwarf::DW_LLE_base_addressx:
       E.Value0 = Data.getULEB128(C);
       break;
     case dwarf::DW_LLE_startx_length:
while (Continue) {
     if (!C)
       return C.takeError();
     Continue = F(E) && E.Kind != dwarf::DW_LLE_end_of_list;
   }
   *Offset = C.tell();
   return Error::success();
 }
 
-bool DWARFDebugLoclists::dumpLocationList(const DWARFDataExtractor &Data,
-                                          uint64_t *Offset, uint16_t Version,
-                                          raw_ostream &OS, uint64_t BaseAddr,
-                                          const MCRegisterInfo *MRI,
-                                          DWARFUnit *U, DIDumpOptions DumpOpts,
-                                          unsigned Indent) {
+void DWARFDebugLoclists::dumpRawEntry(const DWARFLocationEntry &Entry,
+                                      raw_ostream &OS, unsigned Indent) const {
   size_t MaxEncodingStringLength = 0;
-  if (DumpOpts.Verbose) {
 #define HANDLE_DW_LLE(ID, NAME)                                                \
   MaxEncodingStringLength = std::max(MaxEncodingStringLength,                  \
                                      dwarf::LocListEncodingString(ID).size());
 #include "llvm/BinaryFormat/Dwarf.def"
-  }
-
-  OS << format("0x%8.8" PRIx64 ": ", *Offset);
-  Error E = visitLocationList(Data, Offset, Version, [&](const Entry &E) {
-    E.dump(OS, BaseAddr, Data.isLittleEndian(), Data.getAddressSize(), MRI, U,
-           DumpOpts, Indent, MaxEncodingStringLength);
-    return true;
-  });
-  if (E) {
-    OS << "\n";
-    OS.indent(Indent);
-    OS << "error: " << toString(std::move(E));
-    return false;
-  }
-  return true;
-}
 
-void DWARFDebugLoclists::Entry::dump(raw_ostream &OS, uint64_t &BaseAddr,
-                                     bool IsLittleEndian, unsigned AddressSize,
-                                     const MCRegisterInfo *MRI, DWARFUnit *U,
-                                     DIDumpOptions DumpOpts, unsigned Indent,
-                                     size_t MaxEncodingStringLength) const {
-  if (DumpOpts.Verbose) {
-    OS << "\n";
-    OS.indent(Indent);
-    auto EncodingString = dwarf::LocListEncodingString(Kind);
-    // Unsupported encodings should have been reported during parsing.
-    assert(!EncodingString.empty() && "Unknown loclist entry encoding");
-    OS << format("%s%*c", EncodingString.data(),
-                 MaxEncodingStringLength - EncodingString.size() + 1, '(');
-    switch (Kind) {
-    case dwarf::DW_LLE_startx_length:
-    case dwarf::DW_LLE_start_length:
-    case dwarf::DW_LLE_offset_pair:
-      OS << format("0x%*.*" PRIx64 ", 0x%*.*" PRIx64, AddressSize * 2,
-                 AddressSize * 2, Value0, AddressSize * 2, AddressSize * 2,
-                 Value1);
-      break;
-    case dwarf::DW_LLE_base_addressx:
-    case dwarf::DW_LLE_base_address:
-      OS << format("0x%*.*" PRIx64, AddressSize * 2, AddressSize * 2,
-                   Value0);
-      break;
-    case dwarf::DW_LLE_end_of_list:
-      break;
-    }
-    OS << ')';
-  }
-  auto PrintPrefix = [&] {
-    OS << "\n";
-    OS.indent(Indent);
-    if (DumpOpts.Verbose)
-      OS << format("%*s", MaxEncodingStringLength, (const char *)"=> ");
-  };
-  switch (Kind) {
-  case dwarf::DW_LLE_startx_length:
-    PrintPrefix();
-    OS << "Addr idx " << Value0 << " (w/ length " << Value1 << "): ";
-    break;
-  case dwarf::DW_LLE_start_length:
-    PrintPrefix();
-    DWARFAddressRange(Value0, Value0 + Value1)
-        .dump(OS, AddressSize, DumpOpts);
-    OS << ": ";
-    break;
-  case dwarf::DW_LLE_offset_pair:
-    PrintPrefix();
-    DWARFAddressRange(BaseAddr + Value0, BaseAddr + Value1)
-        .dump(OS, AddressSize, DumpOpts);
-    OS << ": ";
-    break;
-  case dwarf::DW_LLE_base_addressx:
-    if (!DumpOpts.Verbose)
-      return;
-    break;
-  case dwarf::DW_LLE_end_of_list:
-    if (!DumpOpts.Verbose)
-      return;
-    break;
-  case dwarf::DW_LLE_base_address:
-    BaseAddr = Value0;
-    if (!DumpOpts.Verbose)
-      return;
-    break;
-  default:
-    llvm_unreachable("unreachable locations list kind");
-  }
-
-  dumpExpression(OS, Loc, IsLittleEndian, AddressSize, MRI, U);
-}
+  StringRef EncodingString = dwarf::LocListEncodingString(Entry.Kind);
+  OS << format("%-*s(", MaxEncodingStringLength, EncodingString.data());
+  unsigned FieldSize = 2 + 2 * Data.getAddressSize();
+  switch (Entry.Kind) {
+    OS << format_hex(Entry.Value0, FieldSize) << ", "
+       << format_hex(Entry.Value1, FieldSize);
+    OS << format_hex(Entry.Value0, FieldSize);
 
-void DWARFDebugLoclists::dumpRange(const DWARFDataExtractor &Data,
-                                   uint64_t StartOffset, uint64_t Size,
-                                   uint16_t Version, raw_ostream &OS,
-                                   uint64_t BaseAddr, const MCRegisterInfo *MRI,
+void DWARFDebugLoclists::dumpRange(uint64_t StartOffset, uint64_t Size,
+                                   raw_ostream &OS, uint64_t BaseAddr,
+                                   const MCRegisterInfo *MRI,
                                    DIDumpOptions DumpOpts) {
   if (!Data.isValidOffsetForDataOfSize(StartOffset, Size))  {
     OS << "Invalid dump range\n";
     return;
   }
   uint64_t Offset = StartOffset;
   StringRef Separator;
   bool CanContinue = true;
   while (CanContinue && Offset < StartOffset + Size) {
     OS << Separator;
     Separator = "\n";
 
-    CanContinue = dumpLocationList(Data, &Offset, Version, OS, BaseAddr, MRI,
-                                   nullptr, DumpOpts, /*Indent=*/12);
+    CanContinue = dumpLocationList(&Offset, OS, BaseAddr, MRI, nullptr,
+                                   DumpOpts, /*Indent=*/12);
     OS << '\n';
   }
 }

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

auto Data =
             : DWARFDataExtractor(U->getLocSectionData(), Ctx.isLittleEndian(),
                                  Obj.getAddressSize());
 
     if (!Data.getData().empty()) {
       // Old-style location list were used in DWARF v4 (.debug_loc.dwo section).
       // Modern locations list (.debug_loclists) are used starting from v5.
       // Ideally we should take the version from the .debug_loclists section
       // header, but using CU's version for simplicity.
-      DWARFDebugLoclists::dumpLocationList(
-          Data, &Offset, UseLocLists ? U->getVersion() : 4, OS, BaseAddr, MRI,
-          U, LLDumpOpts, Indent);
+      DWARFDebugLoclists(Data, UseLocLists ? U->getVersion() : 4)
+          .dumpLocationList(&Offset, OS, BaseAddr, MRI, U, LLDumpOpts, Indent);
     }
     return;
   }
 
   FormValue.dump(OS, DumpOpts);
 }
 
 /// Dump the name encoded in the type tag.

llvm/test/CodeGen/X86/debug-loclists.ll

 ; RUN: llc -mtriple=x86_64-pc-linux -filetype=obj -function-sections -o %t < %s
 ; RUN: llvm-dwarfdump -v -debug-info -debug-loclists %t | FileCheck %s
 
 ; CHECK:      DW_TAG_variable
 ; FIXME: Use DW_FORM_loclistx to reduce relocations
 ; CHECK-NEXT:   DW_AT_location [DW_FORM_sec_offset]   (0x0000000c
 ; CHECK-NEXT:     [0x0000000000000000, 0x0000000000000003): DW_OP_consts +3, DW_OP_stack_value
 ; CHECK-NEXT:     [0x0000000000000003, 0x0000000000000004): DW_OP_consts +4, DW_OP_stack_value)
 ; CHECK-NEXT:   DW_AT_name {{.*}} "y"
 
 ; CHECK:      DW_TAG_variable
 ; FIXME: Use DW_FORM_loclistx to reduce relocations
 ; CHECK-NEXT:   DW_AT_location [DW_FORM_sec_offset]   (0x0000001d
-; CHECK-NEXT:     Addr idx 0 (w/ length 3): DW_OP_consts +5, DW_OP_stack_value)
+; CHECK-NEXT:     [0x0000000000000000, 0x0000000000000003): DW_OP_consts +5, DW_OP_stack_value)
 ; CHECK-NEXT:   DW_AT_name {{.*}} "x"
 
 ; CHECK:      DW_TAG_variable
 ; FIXME: Use DW_FORM_loclistx to reduce relocations
 ; CHECK-NEXT:   DW_AT_location [DW_FORM_sec_offset]   (0x00000025
 ; CHECK-NEXT:     [0x0000000000000003, 0x0000000000000004): DW_OP_reg0 RAX)
 ; CHECK-NEXT:   DW_AT_name {{.*}} "r"
 
 ; CHECK:      .debug_loclists contents:
 ; CHECK-NEXT: 0x00000000: locations list header: length = 0x00000029, version = 0x0005, addr_size = 0x08, seg_size = 0x00, offset_entry_count = 0x00000000
 
 ; Don't use startx_length if there's more than one entry, because the shared
 ; base address will be useful for both the range that does start at the start of
 ; the function, and the one that doesn't.
 
 ; CHECK-NEXT: 0x0000000c:
 ; CHECK-NEXT:             DW_LLE_base_addressx (0x0000000000000000)
-; CHECK-NEXT:             DW_LLE_offset_pair   (0x0000000000000000, 0x0000000000000003)
-; CHECK-NEXT:                              => [0x0000000000000000, 0x0000000000000003): DW_OP_consts +3, DW_OP_stack_value
-; CHECK-NEXT:             DW_LLE_offset_pair   (0x0000000000000003, 0x0000000000000004)
-; CHECK-NEXT:                              => [0x0000000000000003, 0x0000000000000004): DW_OP_consts +4, DW_OP_stack_value
+; CHECK-NEXT:             DW_LLE_offset_pair   (0x0000000000000000, 0x0000000000000003): DW_OP_consts +3, DW_OP_stack_value
+; CHECK-NEXT:             DW_LLE_offset_pair   (0x0000000000000003, 0x0000000000000004): DW_OP_consts +4, DW_OP_stack_value
 ; CHECK-NEXT:             DW_LLE_end_of_list   ()
 
 ; Show that startx_length can be used when the address range starts at the start of the function.
 
 ; CHECK:      0x0000001d:
-; CHECK-NEXT:             DW_LLE_startx_length (0x0000000000000000, 0x0000000000000003)
-; CHECK-NEXT:                              => Addr idx 0 (w/ length 3): DW_OP_consts +5, DW_OP_stack_value
+; CHECK-NEXT:             DW_LLE_startx_length (0x0000000000000000, 0x0000000000000003): DW_OP_consts +5, DW_OP_stack_value
 ; CHECK-NEXT:             DW_LLE_end_of_list   ()
 
 ; And use a base address when the range doesn't start at an existing/useful
 ; address in the pool.
 
 ; CHECK:      0x00000025:
 ; CHECK-NEXT:             DW_LLE_base_addressx (0x0000000000000000)
-; CHECK-NEXT:             DW_LLE_offset_pair   (0x0000000000000003, 0x0000000000000004)
-; CHECK-NEXT:                              => [0x0000000000000003, 0x0000000000000004): DW_OP_reg0 RAX
+; CHECK-NEXT:             DW_LLE_offset_pair   (0x0000000000000003, 0x0000000000000004): DW_OP_reg0 RAX
 ; CHECK-NEXT:             DW_LLE_end_of_list   ()
 
 ; Built with clang -O3 -ffunction-sections from source:
 ; 
 ; int f1(int i, int j) {
 ;   int x = 5;
 ;   int y = 3;
 ;   int r = i + j;

llvm/test/DebugInfo/X86/fission-ranges.ll

 ; CHECK-NOT: .debug_loc contents:
 ; CHECK-NOT: Beginning address offset
 ; CHECK: .debug_loc.dwo contents:
 
 ; Don't assume these locations are entirely correct - feel free to update them
 ; if they've changed due to a bugfix, change in register allocation, etc.
 
 ; CHECK:      [[A]]:
-; CHECK-NEXT:   DW_LLE_startx_length (0x00000002, 0x0000000f)
-; CHECK-NEXT:                    => Addr idx 2 (w/ length 15): DW_OP_consts +0, DW_OP_stack_value
-; CHECK-NEXT:   DW_LLE_startx_length (0x00000003, 0x0000000f)
-; CHECK-NEXT:                    => Addr idx 3 (w/ length 15): DW_OP_reg0 RAX
-; CHECK-NEXT:   DW_LLE_startx_length (0x00000004, 0x00000012)
-; CHECK-NEXT:                    => Addr idx 4 (w/ length 18): DW_OP_breg7 RSP-8
+; CHECK-NEXT:   DW_LLE_startx_length (0x00000002, 0x0000000f): DW_OP_consts +0, DW_OP_stack_value
+; CHECK-NEXT:   DW_LLE_startx_length (0x00000003, 0x0000000f): DW_OP_reg0 RAX
+; CHECK-NEXT:   DW_LLE_startx_length (0x00000004, 0x00000012): DW_OP_breg7 RSP-8
 ; CHECK-NEXT:   DW_LLE_end_of_list   ()
 ; CHECK:      [[E]]:
-; CHECK-NEXT:   DW_LLE_startx_length (0x00000005, 0x00000009)
-; CHECK-NEXT:                    => Addr idx 5 (w/ length 9): DW_OP_reg0 RAX
-; CHECK-NEXT:   DW_LLE_startx_length (0x00000006, 0x00000062)
-; CHECK-NEXT:                    => Addr idx 6 (w/ length 98): DW_OP_breg7 RSP-44
+; CHECK-NEXT:   DW_LLE_startx_length (0x00000005, 0x00000009): DW_OP_reg0 RAX
+; CHECK-NEXT:   DW_LLE_startx_length (0x00000006, 0x00000062): DW_OP_breg7 RSP-44
 ; CHECK-NEXT:   DW_LLE_end_of_list   ()
 ; CHECK:      [[B]]:
-; CHECK-NEXT:   DW_LLE_startx_length (0x00000007, 0x0000000f)
-; CHECK-NEXT:                    => Addr idx 7 (w/ length 15): DW_OP_reg0 RAX
-; CHECK-NEXT:   DW_LLE_startx_length (0x00000008, 0x00000042)
-; CHECK-NEXT:                    => Addr idx 8 (w/ length 66): DW_OP_breg7 RSP-32
+; CHECK-NEXT:   DW_LLE_startx_length (0x00000007, 0x0000000f): DW_OP_reg0 RAX
+; CHECK-NEXT:   DW_LLE_startx_length (0x00000008, 0x00000042): DW_OP_breg7 RSP-32
 ; CHECK-NEXT:   DW_LLE_end_of_list   ()
 ; CHECK:      [[D]]:
-; CHECK-NEXT:   DW_LLE_startx_length (0x00000009, 0x0000000f)
-; CHECK-NEXT:                    => Addr idx 9 (w/ length 15): DW_OP_reg0 RAX
-; CHECK-NEXT:   DW_LLE_startx_length (0x0000000a, 0x0000002a)
-; CHECK-NEXT:                    => Addr idx 10 (w/ length 42): DW_OP_breg7 RSP-20
+; CHECK-NEXT:   DW_LLE_startx_length (0x00000009, 0x0000000f): DW_OP_reg0 RAX
+; CHECK-NEXT:   DW_LLE_startx_length (0x0000000a, 0x0000002a): DW_OP_breg7 RSP-20
 ; CHECK-NEXT:   DW_LLE_end_of_list   ()
 
 ; Make sure we don't produce any relocations in any .dwo section (though in particular, debug_info.dwo)
 ; HDR-NOT: .rela.{{.*}}.dwo
 
 ; Make sure we have enough stuff in the debug_addr to cover the address indexes
 ; (10 is the last index in debug_loc.dwo, making 11 entries of 8 bytes each,
 ; 11 * 8 == 88 base 10 == 58 base 16)

llvm/test/DebugInfo/X86/loclists-dwp.ll

 ;   y();
 ;   asm("" : : : "rdi");
 ; }
 ;
 ; b.cpp:
 ; void b(int i) { asm("" : : : "rdi"); }
 
 ; CHECK:      DW_AT_location [DW_FORM_sec_offset]   (0x00000000
-; CHECK-NEXT: Addr idx 0 (w/ length 6): DW_OP_reg5 RDI)
+; CHECK-NEXT: DW_LLE_startx_length (0x0000000000000000, 0x0000000000000006): DW_OP_reg5 RDI)
 
 ; CHECK:      DW_AT_location [DW_FORM_sec_offset]   (0x00000000
-; CHECK-NEXT: Addr idx 0 (w/ length 0): DW_OP_reg5 RDI)
+; CHECK-NEXT: DW_LLE_startx_length (0x0000000000000000, 0x0000000000000000): DW_OP_reg5 RDI)
 
 target triple = "x86_64-unknown-linux-gnu"
 
 define dso_local void @_Z1ai(i32 %i) local_unnamed_addr !dbg !7 {
 entry:
   call void @llvm.dbg.value(metadata i32 %i, metadata !12, metadata !DIExpression()), !dbg !13
   tail call void @_Z1yv(), !dbg !14
   tail call void asm sideeffect "", "~{rdi},~{dirflag},~{fpsr},~{flags}"(), !dbg !15, !srcloc !16

llvm/test/tools/llvm-dwarfdump/X86/debug_loc_dwo.s

 # RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux -o %t.o
 # RUN: llvm-dwarfdump --debug-loc %t.o | FileCheck %s
 
 # We make sure that llvm-dwarfdump can dump the .debug_loc.dwo section
 # without requiring a compilation unit in the .debug_info.dwo section.
 
 # CHECK:         .debug_loc.dwo contents:
 # CHECK-NEXT:    0x00000000:
-# CHECK-NEXT:    Addr idx 1 (w/ length 16): DW_OP_reg5 RDI
+# CHECK-NEXT:    DW_LLE_startx_length (0x00000001, 0x00000010): DW_OP_reg5 RDI
 
 .section .debug_loc.dwo,"",@progbits
 # One location list. The pre-DWARF v5 implementation only recognizes
 # DW_LLE_startx_length as an entry kind in .debug_loc.dwo (besides
 # end_of_list), which is what llvm generates as well.
 .byte 3          # DW_LLE_startx_length
 .byte 0x01       # Index
 .long 0x10       # Length
 .short 1         # Loc expr size
 .byte 85         # DW_OP_reg5
 .byte 0          # DW_LLE_end_of_list

llvm/test/tools/llvm-dwarfdump/X86/debug_loclists.s

 # RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux -o %t
 # RUN: llvm-dwarfdump %t | FileCheck %s
 
 
 # CHECK:          DW_AT_location        (0x0000000c
-# CHECK-NEXT:        Addr idx 0 (w/ length 1): DW_OP_reg0 RAX
+# CHECK-NEXT:        [0x0000000000000000, 0x0000000000000001): DW_OP_reg0 RAX
 # CHECK-NEXT:        [0x0000000000000001, 0x0000000000000002): DW_OP_reg1 RDX
 # CHECK-NEXT:        [0x0000000000000002, 0x0000000000000003): DW_OP_reg2 RCX
 # CHECK-NEXT:        [0x0000000000000003, 0x0000000000000004): DW_OP_reg3 RBX
-# CHECK-NEXT:        Addr idx 57005 (w/ length 1): DW_OP_reg4 RSI)
+# CHECK-NEXT:        DW_LLE_startx_length (0x000000000000dead, 0x0000000000000001): DW_OP_reg4 RSI)
 
 
         .text
 f:                                      # @f
 .Lf0:
         nop
 .Lf1:
         nop

llvm/test/tools/llvm-dwarfdump/X86/debug_loclists_startx_length.s

 # RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux -o %t.o
 # RUN: llvm-dwarfdump -v %t.o | FileCheck %s
 
 # DW_LLE_startx_length has different `length` encoding in pre-DWARF 5
 # and final DWARF 5 versions. This test checks we are able to parse
 # the final version which uses ULEB128 and not the U32.
 
 # CHECK:         .debug_loclists contents:
 # CHECK-NEXT:    0x00000000: locations list header: length = 0x0000000e, version = 0x0005, addr_size = 0x08, seg_size = 0x00, offset_entry_count = 0x00000000
 # CHECK-NEXT:    0x0000000c:
-# CHECK-NEXT:                DW_LLE_startx_length (0x0000000000000001, 0x0000000000000010)
-# CHECK-NEXT:                                 => Addr idx 1 (w/ length 16): DW_OP_reg5 RDI
+# CHECK-NEXT:                DW_LLE_startx_length (0x0000000000000001, 0x0000000000000010): DW_OP_reg5 RDI
 # CHECK-NEXT:                DW_LLE_end_of_list   ()
 
 .section .debug_loclists,"",@progbits
  .long  .Ldebug_loclist_table_end0-.Ldebug_loclist_table_start0
 .Ldebug_loclist_table_start0:
  .short 5         # Version.
  .byte 8          # Address size.
  .byte 0          # Segmen selector size.