[lldb/DWARF] Switch to llvm location list parser

[labath]

Summary

This patch deletes the lldb location list parser and teaches the
DWARFExpression class to use the parser in llvm instead. I have
centralized all the places doing the parsing into a single
GetLocationExpression function.

In theory the the actual location list parsing should be covered by llvm
tests, and this glue code by our existing location list tests, but since
we don't have that many location list tests, I've tried to extend the
coverage a bit by adding some explicit dwarf5 loclist handling and a
test of the dumping code.

For DWARF4 location lists this should be NFC (modulo small differences
in error handling which should only show up on invalid inputs). In case
of DWARF5, this fixes various missing bits of functionality, most
notably, the lack of support for DW_LLE_offset_pair.

[dblaikie]

lldb/source/Expression/DWARFExpression.cpp

96

I think usually the unreachable comment that's used in this sort of case is "Invalid LocationListFormat!" or similar, to give a bit more context about the situation.

642

I'd probably write this as "... != None" rather than "bool(...)" - would make it clearer what the return type of GetLocationExpression is/what conversion is being done here.

2827

Does LLDB_LOG_ERROR stop the program? If not, then the next line ("if (loc->Range)" will invoke UB when it tries to dereference the "loc" when it is unset.

[labath]

lldb/source/Expression/DWARFExpression.cpp

2827

It doesn't. You're right, there should be a return here, just let me see if I can tickle this into exploding.

[labath]

Feedback @dblaikie.

lldb/source/Expression/DWARFExpression.cpp

2827

It turns out triggering this was pretty hard, because the main source of these errors is a failed indirect address resolution, but our callback never returned any errors. I've now fixed it to return errors and included an additional test.

[dblaikie]

lldb/source/Expression/DWARFExpression.cpp

2827

Looks great!

That's about all the thoughts I have on the patch - will leave the rest to LLDB developers.

[JDevlieghere]

lldb/include/lldb/Expression/DWARFExpression.h

259–260

Should this be part of the DWARFExpression API? It feels like a static function might be sufficient. Maybe it could take a range directly?

lldb/source/Expression/DWARFExpression.cpp

53

Why the unchecked?

165

llvm::MCRegisterInfo *MRI = abi ? &abi->GetMCRegisterInfo() : nullptr;

2804

Should this be a doxygen comment on the function itself?

[aprantl]

Nice!

lldb/source/Expression/DWARFExpression.cpp

57

Doxygen comment?

136

Doxygen comment: what is this used for?

[labath]

• "inline" RelocateLowHighPC
• add some comments

[labath]

lldb/include/lldb/Expression/DWARFExpression.h

259–260

It's a private member function, so it's not really a part of any API. However, given that now it's called from only a single place, it's not actually useful. It served a purpose in the preceding patch (I'm going to need a stamp on that too BTW), since there it had like five callers, but now that this patch centralizes them, I can just inline it.

lldb/source/Expression/DWARFExpression.cpp

53

Because I've already checked that the offset is valid (that's the difference between the checked and unchecked versions, though I'm not really sure if we need them).

57

I've added /something/, though I think the purpose is mostly obvious here.

Btw, this part is going to get refactored further, since the current design doesn't really permit mixing DWARF 4&5 location lists within a single object file.

[clayborg]

lgtm

lldb/include/lldb/Expression/DWARFExpression.h

private:
   ///     The length in bytes of the opcodes to be printed.
   ///
   /// \param[in] level
   ///     The level of detail to use in pretty-printing.
   ///
   /// \param[in] abi
   ///     An optional ABI plug-in that can be used to resolve register
   ///     names.
-  void DumpLocation(Stream *s, lldb::offset_t offset, lldb::offset_t length,
+  void DumpLocation(Stream *s, const DataExtractor &data,
                     lldb::DescriptionLevel level, ABI *abi) const;
 
-  bool GetLocation(lldb::addr_t func_load_addr, lldb::addr_t pc,
-                   lldb::offset_t &offset, lldb::offset_t &len);
-
-  static bool AddressRangeForLocationListEntry(
-      const DWARFUnit *dwarf_cu, const DataExtractor &debug_loc_data,
-      lldb::offset_t *offset_ptr, lldb::addr_t &low_pc, lldb::addr_t &high_pc);
-
-  bool GetOpAndEndOffsets(StackFrame &frame, lldb::offset_t &op_offset,
-                          lldb::offset_t &end_offset);
-
-  void RelocateLowHighPC(lldb::addr_t base_address, lldb::addr_t func_load_addr,
-                         lldb::addr_t &low_pc, lldb::addr_t &high_pc) const;
+  llvm::Optional<DataExtractor>
+  GetLocationExpression(lldb::addr_t load_function_start,
+                        lldb::addr_t addr) const;
 
   /// Module which defined this expression.
   lldb::ModuleWP m_module_wp;
 
   /// A data extractor capable of reading opcode bytes
   DataExtractor m_data;
 
   /// The DWARF compile unit this expression belongs to. It is used to evaluate

lldb/source/Expression/DWARFExpression.cpp

 using namespace lldb_private;
 
 static lldb::addr_t
 ReadAddressFromDebugAddrSection(const DWARFUnit *dwarf_cu,
                                 uint32_t index) {
   uint32_t index_size = dwarf_cu->GetAddressByteSize();
   dw_offset_t addr_base = dwarf_cu->GetAddrBase();
   lldb::offset_t offset = addr_base + index * index_size;
-  return dwarf_cu->GetSymbolFileDWARF()
-      .GetDWARFContext()
-      .getOrLoadAddrData()
-      .GetMaxU64(&offset, index_size);
+  const DWARFDataExtractor &data =
+      dwarf_cu->GetSymbolFileDWARF().GetDWARFContext().getOrLoadAddrData();
+  if (data.ValidOffsetForDataOfSize(offset, index_size))
+    return data.GetMaxU64_unchecked(&offset, index_size);
+  return LLDB_INVALID_ADDRESS;
+}
+
+/// Return the location list parser for the given format.
+static std::unique_ptr<llvm::DWARFLocationTable>
+GetLocationTable(DWARFExpression::LocationListFormat format, const DataExtractor &data) {
+  llvm::DWARFDataExtractor llvm_data(
+      toStringRef(data.GetData()),
+      data.GetByteOrder() == lldb::eByteOrderLittle, data.GetAddressByteSize());
+
+  switch (format) {
+  case DWARFExpression::NonLocationList:
+    return nullptr;
+  // DWARF<=4 .debug_loc
+  case DWARFExpression::RegularLocationList:
+    return std::make_unique<llvm::DWARFDebugLoc>(llvm_data);
+  // Non-standard DWARF 4 extension (fission) .debug_loc.dwo
+  case DWARFExpression::SplitDwarfLocationList:
+  // DWARF 5 .debug_loclists(.dwo)
+  case DWARFExpression::LocLists:
+    return std::make_unique<llvm::DWARFDebugLoclists>(
+        llvm_data, format == DWARFExpression::LocLists ? 5 : 4);
+  }
+  llvm_unreachable("Invalid LocationListFormat!");
 }
 
 // DWARFExpression constructor
 DWARFExpression::DWARFExpression()
     : m_module_wp(), m_data(), m_dwarf_cu(nullptr),
       m_reg_kind(eRegisterKindDWARF) {}
 
 DWARFExpression::DWARFExpression(lldb::ModuleSP module_sp,
                                  const DataExtractor &data,
                                  const DWARFUnit *dwarf_cu)
     : m_module_wp(), m_data(data), m_dwarf_cu(dwarf_cu),
       m_reg_kind(eRegisterKindDWARF) {
   if (module_sp)
     m_module_wp = module_sp;
 }
 
 // Destructor
 DWARFExpression::~DWARFExpression() {}
 
 bool DWARFExpression::IsValid() const { return m_data.GetByteSize() > 0; }
 
 void DWARFExpression::UpdateValue(uint64_t const_value,
                                   lldb::offset_t const_value_byte_size,
                                   uint8_t addr_byte_size) {
   if (!const_value_byte_size)
     return;
 
   m_data.SetData(
       DataBufferSP(new DataBufferHeap(&const_value, const_value_byte_size)));
   m_data.SetByteOrder(endian::InlHostByteOrder());
   m_data.SetAddressByteSize(addr_byte_size);
 }
 
-void DWARFExpression::DumpLocation(Stream *s, lldb::offset_t offset,
-                                   lldb::offset_t length,
+void DWARFExpression::DumpLocation(Stream *s, const DataExtractor &data,
                                    lldb::DescriptionLevel level,
                                    ABI *abi) const {
-  llvm::DWARFExpression(DataExtractor(m_data, offset, length).GetAsLLVM(),
-                        llvm::dwarf::DWARF_VERSION, m_data.GetAddressByteSize())
+  llvm::DWARFExpression(data.GetAsLLVM(), llvm::dwarf::DWARF_VERSION,
+                        data.GetAddressByteSize())
       .print(s->AsRawOstream(), abi ? &abi->GetMCRegisterInfo() : nullptr,
              nullptr);
 }
 
 void DWARFExpression::SetLocationListAddresses(addr_t cu_file_addr,
                                                addr_t func_file_addr) {
   m_loclist_addresses = LoclistAddresses{cu_file_addr, func_file_addr};
 }
 
 int DWARFExpression::GetRegisterKind() { return m_reg_kind; }
 
 void DWARFExpression::SetRegisterKind(RegisterKind reg_kind) {
   m_reg_kind = reg_kind;
 }
 
 bool DWARFExpression::IsLocationList() const {
   return bool(m_loclist_addresses);
 }
 
+namespace {
+/// Implement enough of the DWARFObject interface in order to be able to call
+/// DWARFLocationTable::dumpLocationList. We don't have access to a real
+/// DWARFObject here because DWARFExpression is used in non-DWARF scenarios too.
+class DummyDWARFObject final: public llvm::DWARFObject {
+public:
+  DummyDWARFObject(bool IsLittleEndian) : IsLittleEndian(IsLittleEndian) {}
+
+  bool isLittleEndian() const override { return IsLittleEndian; }
+
+  llvm::Optional<llvm::RelocAddrEntry> find(const llvm::DWARFSection &Sec,
+                                            uint64_t Pos) const override {
+    return llvm::None;
+  }
+private:
+  bool IsLittleEndian;
+};
+}
+
 void DWARFExpression::GetDescription(Stream *s, lldb::DescriptionLevel level,
                                      addr_t location_list_base_addr,
                                      ABI *abi) const {
   if (IsLocationList()) {
     // We have a location list
     lldb::offset_t offset = 0;
-    uint32_t count = 0;
-    addr_t curr_base_addr = location_list_base_addr;
-    while (m_data.ValidOffset(offset)) {
-      addr_t begin_addr_offset = LLDB_INVALID_ADDRESS;
-      addr_t end_addr_offset = LLDB_INVALID_ADDRESS;
-      if (!AddressRangeForLocationListEntry(m_dwarf_cu, m_data, &offset,
-                                            begin_addr_offset, end_addr_offset))
-        break;
-
-      if (begin_addr_offset == 0 && end_addr_offset == 0)
-        break;
-
-      if (begin_addr_offset < end_addr_offset) {
-        if (count > 0)
-          s->PutCString(", ");
-        VMRange addr_range(curr_base_addr + begin_addr_offset,
-                           curr_base_addr + end_addr_offset);
-        addr_range.Dump(s->AsRawOstream(), 0, 8);
-        s->PutChar('{');
-        lldb::offset_t location_length = m_data.GetU16(&offset);
-        DumpLocation(s, offset, location_length, level, abi);
-        s->PutChar('}');
-        offset += location_length;
-      } else {
-        if ((m_data.GetAddressByteSize() == 4 &&
-             (begin_addr_offset == UINT32_MAX)) ||
-            (m_data.GetAddressByteSize() == 8 &&
-             (begin_addr_offset == UINT64_MAX))) {
-          curr_base_addr = end_addr_offset + location_list_base_addr;
-          // We have a new base address
-          if (count > 0)
-            s->PutCString(", ");
-          s->Format("base_addr = {0:x}", end_addr_offset);
-        }
-      }
+    std::unique_ptr<llvm::DWARFLocationTable> loctable_up = GetLocationTable(
+        m_dwarf_cu->GetSymbolFileDWARF().GetLocationListFormat(), m_data);
+    if (!loctable_up)
+      return;
 
-      count++;
-    }
+    llvm::MCRegisterInfo *MRI = abi ? &abi->GetMCRegisterInfo() : nullptr;
+
+    loctable_up->dumpLocationList(
+        &offset, s->AsRawOstream(),
+        llvm::object::SectionedAddress{m_loclist_addresses->cu_file_addr}, MRI,
+        DummyDWARFObject(m_data.GetByteOrder() == eByteOrderLittle), nullptr,
+        llvm::DIDumpOptions(), s->GetIndentLevel() + 2);
   } else {
     // We have a normal location that contains DW_OP location opcodes
-    DumpLocation(s, 0, m_data.GetByteSize(), level, abi);
+    DumpLocation(s, m_data, level, abi);
   }
 }
 
 static bool ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
                                       lldb::RegisterKind reg_kind,
                                       uint32_t reg_num, Status *error_ptr,
                                       Value &value) {
   if (reg_ctx == nullptr) {

 bool DWARFExpression::LocationListContainsAddress(addr_t func_load_addr,
                                                   lldb::addr_t addr) const {
   if (func_load_addr == LLDB_INVALID_ADDRESS || addr == LLDB_INVALID_ADDRESS)
     return false;
 
   if (!IsLocationList())
     return false;
 
-  lldb::offset_t offset = 0;
-  lldb::addr_t base_address = m_loclist_addresses->cu_file_addr;
-  while (m_data.ValidOffset(offset)) {
-    // We need to figure out what the value is for the location.
-    addr_t lo_pc = LLDB_INVALID_ADDRESS;
-    addr_t hi_pc = LLDB_INVALID_ADDRESS;
-    if (!AddressRangeForLocationListEntry(m_dwarf_cu, m_data, &offset, lo_pc,
-                                          hi_pc))
-      break;
-
-    if (lo_pc == 0 && hi_pc == 0)
-      break;
-
-    if ((m_data.GetAddressByteSize() == 4 && (lo_pc == UINT32_MAX)) ||
-        (m_data.GetAddressByteSize() == 8 && (lo_pc == UINT64_MAX))) {
-      base_address = hi_pc;
-      continue;
-    }
-    RelocateLowHighPC(base_address, func_load_addr, lo_pc, hi_pc);
-
-    if (lo_pc <= addr && addr < hi_pc)
-      return true;
-
-    offset += m_data.GetU16(&offset);
-  }
-  return false;
-}
-
-bool DWARFExpression::GetLocation(addr_t func_load_addr, addr_t pc,
-                                  lldb::offset_t &offset,
-                                  lldb::offset_t &length) {
-  offset = 0;
-  if (!IsLocationList()) {
-    length = m_data.GetByteSize();
-    return true;
-  }
-
-  if (func_load_addr != LLDB_INVALID_ADDRESS && pc != LLDB_INVALID_ADDRESS) {
-    addr_t base_address = m_loclist_addresses->cu_file_addr;
-    while (m_data.ValidOffset(offset)) {
-      // We need to figure out what the value is for the location.
-      addr_t lo_pc = LLDB_INVALID_ADDRESS;
-      addr_t hi_pc = LLDB_INVALID_ADDRESS;
-      if (!AddressRangeForLocationListEntry(m_dwarf_cu, m_data, &offset, lo_pc,
-                                            hi_pc))
-        break;
-
-      if (lo_pc == 0 && hi_pc == 0)
-        break;
-
-      if ((m_data.GetAddressByteSize() == 4 && (lo_pc == UINT32_MAX)) ||
-          (m_data.GetAddressByteSize() == 8 && (lo_pc == UINT64_MAX))) {
-        base_address = hi_pc;
-        continue;
-      }
-
-      RelocateLowHighPC(base_address, func_load_addr, lo_pc, hi_pc);
-      length = m_data.GetU16(&offset);
-
-      if (length > 0 && lo_pc <= pc && pc < hi_pc)
-        return true;
-
-      offset += length;
-    }
-  }
-  offset = LLDB_INVALID_OFFSET;
-  length = 0;
-  return false;
+  return GetLocationExpression(func_load_addr, addr) != llvm::None;
 }
 
 bool DWARFExpression::DumpLocationForAddress(Stream *s,
                                              lldb::DescriptionLevel level,
                                              addr_t func_load_addr,
                                              addr_t address, ABI *abi) {
-  lldb::offset_t offset = 0;
-  lldb::offset_t length = 0;
-
-  if (GetLocation(func_load_addr, address, offset, length)) {
-    if (length > 0) {
-      DumpLocation(s, offset, length, level, abi);
-      return true;
-    }
+  if (!IsLocationList()) {
+    DumpLocation(s, m_data, level, abi);
+  if (llvm::Optional<DataExtractor> expr =
+          GetLocationExpression(func_load_addr, address)) {
+    DumpLocation(s, *expr, level, abi);
+    return true;
   }
   return false;
 }
 
 static bool Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
                                        ExecutionContext *exe_ctx,
                                        RegisterContext *reg_ctx,
                                        const DataExtractor &opcodes,
bool DWARFExpression::Evaluate(ExecutionContext *exe_ctx,
                                RegisterContext *reg_ctx,
                                lldb::addr_t func_load_addr,
                                const Value *initial_value_ptr,
                                const Value *object_address_ptr, Value &result,
                                Status *error_ptr) const {
   ModuleSP module_sp = m_module_wp.lock();
 
   if (IsLocationList()) {
-    lldb::offset_t offset = 0;
     addr_t pc;
     StackFrame *frame = nullptr;
     if (reg_ctx)
       pc = reg_ctx->GetPC();
     else {
       frame = exe_ctx->GetFramePtr();
       if (!frame)
         return false;
       RegisterContextSP reg_ctx_sp = frame->GetRegisterContext();
       if (!reg_ctx_sp)
         return false;
       pc = reg_ctx_sp->GetPC();
     }
 
     if (func_load_addr != LLDB_INVALID_ADDRESS) {
       if (pc == LLDB_INVALID_ADDRESS) {
         if (error_ptr)
           error_ptr->SetErrorString("Invalid PC in frame.");
         return false;
       }
 
-      addr_t base_address = m_loclist_addresses->cu_file_addr;
-      while (m_data.ValidOffset(offset)) {
-        // We need to figure out what the value is for the location.
-        addr_t lo_pc = LLDB_INVALID_ADDRESS;
-        addr_t hi_pc = LLDB_INVALID_ADDRESS;
-        if (!AddressRangeForLocationListEntry(m_dwarf_cu, m_data, &offset,
-                                              lo_pc, hi_pc))
-          break;
-
-        if (lo_pc == 0 && hi_pc == 0)
-          break;
-
-        if ((m_data.GetAddressByteSize() == 4 &&
-             (lo_pc == UINT32_MAX)) ||
-            (m_data.GetAddressByteSize() == 8 &&
-             (lo_pc == UINT64_MAX))) {
-          base_address = hi_pc;
-          continue;
-        }
-
-        RelocateLowHighPC(base_address, func_load_addr, lo_pc, hi_pc);
-        uint16_t length = m_data.GetU16(&offset);
-
-        if (length > 0 && lo_pc <= pc && pc < hi_pc) {
-          return DWARFExpression::Evaluate(
-              exe_ctx, reg_ctx, module_sp,
-              DataExtractor(m_data, offset, length), m_dwarf_cu, m_reg_kind,
-              initial_value_ptr, object_address_ptr, result, error_ptr);
-        }
-        offset += length;
-      }
+      if (llvm::Optional<DataExtractor> expr =
+              GetLocationExpression(func_load_addr, pc)) {
+            exe_ctx, reg_ctx, module_sp, *expr, m_dwarf_cu, m_reg_kind,
     }
     if (error_ptr)
       error_ptr->SetErrorString("variable not available");
     return false;
   }
 
   // Not a location list, just a single expression.
   return DWARFExpression::Evaluate(exe_ctx, reg_ctx, module_sp, m_data,
if (log && log->GetVerbose()) {
         LLDB_LOGF(log, "  %s", new_value.GetData());
       }
     }
     result = stack.back();
   }
   return true; // Return true on success
 }
 
-bool DWARFExpression::AddressRangeForLocationListEntry(
-    const DWARFUnit *dwarf_cu, const DataExtractor &debug_loc_data,
-    lldb::offset_t *offset_ptr, lldb::addr_t &low_pc, lldb::addr_t &high_pc) {
-  if (!debug_loc_data.ValidOffset(*offset_ptr))
-    return false;
-
-  DWARFExpression::LocationListFormat format =
-      dwarf_cu->GetSymbolFileDWARF().GetLocationListFormat();
-  switch (format) {
-  case NonLocationList:
-    return false;
-  case RegularLocationList:
-    low_pc = debug_loc_data.GetAddress(offset_ptr);
-    high_pc = debug_loc_data.GetAddress(offset_ptr);
-    return true;
-  case SplitDwarfLocationList:
-  case LocLists:
-    switch (debug_loc_data.GetU8(offset_ptr)) {
-    case DW_LLE_end_of_list:
-      return false;
-    case DW_LLE_startx_endx: {
-      uint64_t index = debug_loc_data.GetULEB128(offset_ptr);
-      low_pc = ReadAddressFromDebugAddrSection(dwarf_cu, index);
-      index = debug_loc_data.GetULEB128(offset_ptr);
-      high_pc = ReadAddressFromDebugAddrSection(dwarf_cu, index);
-      return true;
-    }
-    case DW_LLE_startx_length: {
-      uint64_t index = debug_loc_data.GetULEB128(offset_ptr);
-      low_pc = ReadAddressFromDebugAddrSection(dwarf_cu, index);
-      uint64_t length = (format == LocLists)
-                            ? debug_loc_data.GetULEB128(offset_ptr)
-                            : debug_loc_data.GetU32(offset_ptr);
-      high_pc = low_pc + length;
-      return true;
-    }
-    case DW_LLE_start_length: {
-      low_pc = debug_loc_data.GetAddress(offset_ptr);
-      high_pc = low_pc + debug_loc_data.GetULEB128(offset_ptr);
-      return true;
-    }
-    case DW_LLE_start_end: {
-      low_pc = debug_loc_data.GetAddress(offset_ptr);
-      high_pc = debug_loc_data.GetAddress(offset_ptr);
-      return true;
-    }
-    default:
-      // Not supported entry type
-      lldbassert(false && "Not supported location list type");
-      return false;
-    }
-  }
-  assert(false && "Not supported location list type");
-  return false;
-}
-
 static bool print_dwarf_exp_op(Stream &s, const DataExtractor &data,
                                lldb::offset_t *offset_ptr, int address_size,
                                int dwarf_ref_size) {
   uint8_t opcode = data.GetU8(offset_ptr);
   DRC_class opcode_class;
   uint64_t uint;
   int64_t sint;
 
while (debug_loc_data.ValidOffset(offset)) {
     uint32_t loc_length = debug_loc_data.GetU16(&offset);
 
     DataExtractor locationData(debug_loc_data, offset, loc_length);
     PrintDWARFExpression(s, locationData, addr_size, 4, false);
     offset += loc_length;
   }
 }
 
-bool DWARFExpression::GetOpAndEndOffsets(StackFrame &frame,
-                                         lldb::offset_t &op_offset,
-                                         lldb::offset_t &end_offset) {
-  SymbolContext sc = frame.GetSymbolContext(eSymbolContextFunction);
-  if (!sc.function) {
-    return false;
-  }
-
-  addr_t loclist_base_file_addr =
-      sc.function->GetAddressRange().GetBaseAddress().GetFileAddress();
-  if (loclist_base_file_addr == LLDB_INVALID_ADDRESS) {
-    return false;
-  }
-
-  addr_t pc_file_addr = frame.GetFrameCodeAddress().GetFileAddress();
-  lldb::offset_t opcodes_offset, opcodes_length;
-  if (!GetLocation(loclist_base_file_addr, pc_file_addr, opcodes_offset,
-                   opcodes_length)) {
-    return false;
-  }
-
-  if (opcodes_length == 0) {
-    return false;
-  }
-
-  op_offset = opcodes_offset;
-  end_offset = opcodes_offset + opcodes_length;
-  return true;
+static DataExtractor ToDataExtractor(const llvm::DWARFLocationExpression &loc,
+                                     ByteOrder byte_order, uint32_t addr_size) {
+  auto buffer_sp =
+      std::make_shared<DataBufferHeap>(loc.Expr.data(), loc.Expr.size());
+  return DataExtractor(buffer_sp, byte_order, addr_size);
+}
+
+llvm::Optional<DataExtractor>
+DWARFExpression::GetLocationExpression(addr_t load_function_start,
+                                       addr_t addr) const {
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+
+  std::unique_ptr<llvm::DWARFLocationTable> loctable_up = GetLocationTable(
+      m_dwarf_cu->GetSymbolFileDWARF().GetLocationListFormat(), m_data);
+  if (!loctable_up)
+    return llvm::None;
+  llvm::Optional<DataExtractor> result;
+  uint64_t offset = 0;
+  auto lookup_addr =
+      [&](uint32_t index) -> llvm::Optional<llvm::object::SectionedAddress> {
+    addr_t address = ReadAddressFromDebugAddrSection(m_dwarf_cu, index);
+    if (address == LLDB_INVALID_ADDRESS)
+      return llvm::None;
+    return llvm::object::SectionedAddress{address};
+  };
+  auto process_list = [&](llvm::Expected<llvm::DWARFLocationExpression> loc) {
+    if (!loc) {
+      LLDB_LOG_ERROR(log, loc.takeError(), "{0}");
+      return true;
+    }
+    if (loc->Range) {
+      // This relocates low_pc and high_pc by adding the difference between the
+      // function file address, and the actual address it is loaded in memory.
+      addr_t slide = load_function_start - m_loclist_addresses->func_file_addr;
+      loc->Range->LowPC += slide;
+      loc->Range->HighPC += slide;
+
+      if (loc->Range->LowPC <= addr && addr < loc->Range->HighPC)
+        result = ToDataExtractor(*loc, m_data.GetByteOrder(),
+                                 m_data.GetAddressByteSize());
     }
-
-void DWARFExpression::RelocateLowHighPC(addr_t base_address,
-                                        addr_t func_load_addr, addr_t &low_pc,
-                                        addr_t &high_pc) const {
-  // How this works:
-  // base_address is the current base address, as known in the file. low_pc and
-  // high_pc are relative to that. First, we relocate the base address by
-  // applying the load bias (the difference between an address in the file and
-  // the actual address in memory). Then we relocate low_pc and high_pc based on
-  // that.
-  base_address += func_load_addr - m_loclist_addresses->func_file_addr;
-  low_pc += base_address;
-  high_pc += base_address;
+    return !result;
+  };
+  llvm::Error E = loctable_up->visitAbsoluteLocationList(
+      offset, llvm::object::SectionedAddress{m_loclist_addresses->cu_file_addr},
+      lookup_addr, process_list);
+  if (E)
+    LLDB_LOG_ERROR(log, std::move(E), "{0}");
+  return result;
 }
 
 bool DWARFExpression::MatchesOperand(StackFrame &frame,
                                      const Instruction::Operand &operand) {
   using namespace OperandMatchers;
 
-  lldb::offset_t op_offset;
-  lldb::offset_t end_offset;
-  if (!GetOpAndEndOffsets(frame, op_offset, end_offset)) {
+  RegisterContextSP reg_ctx_sp = frame.GetRegisterContext();
+  if (!reg_ctx_sp) {
     return false;
   }
 
-  if (!m_data.ValidOffset(op_offset) || op_offset >= end_offset) {
+  DataExtractor opcodes;
+  if (IsLocationList()) {
+    SymbolContext sc = frame.GetSymbolContext(eSymbolContextFunction);
+    if (!sc.function)
       return false;
-  }
 
-  RegisterContextSP reg_ctx_sp = frame.GetRegisterContext();
-  if (!reg_ctx_sp) {
+    addr_t load_function_start =
+        sc.function->GetAddressRange().GetBaseAddress().GetFileAddress();
+    if (load_function_start == LLDB_INVALID_ADDRESS)
       return false;
-  }
 
-  DataExtractor opcodes = m_data;
+    addr_t pc = frame.GetFrameCodeAddress().GetLoadAddress(
+        frame.CalculateTarget().get());
+
+    if (llvm::Optional<DataExtractor> expr = GetLocationExpression(load_function_start, pc))
+      opcodes = std::move(*expr);
+    else
+      return false;
+  } else
+    opcodes = m_data;
+
+
+  lldb::offset_t op_offset = 0;
   uint8_t opcode = opcodes.GetU8(&op_offset);
 
   if (opcode == DW_OP_fbreg) {
     int64_t offset = opcodes.GetSLEB128(&op_offset);
 
     DWARFExpression *fb_expr = frame.GetFrameBaseExpression(nullptr);
     if (!fb_expr) {
       return false;

lldb/test/Shell/SymbolFile/DWARF/debug_loc.s

-# Test debug_loc parsing, including the cases of invalid input. The exact
+# Test location list handling, including the cases of invalid input. The exact
 # behavior in the invalid cases is not particularly important, but it should be
 # "reasonable".
 
 # REQUIRES: x86
 
-# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
-# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" -o exit \
-# RUN:   | FileCheck %s
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym LOC=0 > %t
+# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" \
+# RUN:   -o "image dump symfile" -o exit | FileCheck %s
+
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym LOCLISTS=0 > %t
+# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" \
+# RUN:   -o "image dump symfile" -o exit | FileCheck %s --check-prefix=CHECK --check-prefix=LOCLISTS
 
 # CHECK-LABEL: image lookup -v -a 0
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg5 RDI,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
-# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 
 # CHECK-LABEL: image lookup -v -a 2
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg0 RAX,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
-# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 # CHECK: Variable: {{.*}}, name = "x3", type = "int", location = DW_OP_reg1 RDX,
 
+# CHECK-LABEL: image dump symfile
+# CHECK: CompileUnit{0x00000000}
+# CHECK:   Function{
+# CHECK:     Variable{{.*}}, name = "x0", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  [0x0000000000000000, 0x0000000000000001): DW_OP_reg5 RDI
+# CHECK-NEXT:  [0x0000000000000001, 0x0000000000000006): DW_OP_reg0 RAX
+# CHECK:     Variable{{.*}}, name = "x1", {{.*}}, scope = parameter
+# CHECK:     Variable{{.*}}, name = "x2", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  error: unexpected end of data
+# CHECK:     Variable{{.*}}, name = "x3", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  [0x0000000000000002, 0x0000000000000003): DW_OP_reg1 RDX
+# LOCLISTS:  Variable{{.*}}, name = "x4", {{.*}}, scope = parameter, location =
+# LOCLISTS-NEXT: DW_LLE_startx_length   (0x000000000000dead, 0x0000000000000001): DW_OP_reg2 RCX
+
+.ifdef LOC
+.macro OFFSET_PAIR lo hi
+        .quad \lo
+        .quad \hi
+.endm
+
+.macro BASE_ADDRESS base
+        .quad -1
+        .quad \base
+.endm
+
+.macro EXPR_SIZE sz
+        .short \sz
+.endm
+
+.macro END_OF_LIST
+        .quad 0
+        .quad 0
+.endm
+.endif
+
+.ifdef LOCLISTS
+.macro OFFSET_PAIR lo hi
+        .byte   4                       # DW_LLE_offset_pair
+        .uleb128 \lo
+        .uleb128 \hi
+.endm
+
+.macro BASE_ADDRESS base
+        .byte   6                       # DW_LLE_base_address
+        .quad \base
+.endm
+
+.macro EXPR_SIZE sz
+        .uleb128 \sz
+.endm
+
+.macro END_OF_LIST
+        .byte   0                       # DW_LLE_end_of_list
+.endm
+.endif
+
         .type   f,@function
 f:                                      # @f
 .Lfunc_begin0:
         nop
 .Ltmp0:
         nop
 .Ltmp1:
         nop
.Lfunc_end0:
         .section        .debug_str,"MS",@progbits,1
 .Linfo_string0:
         .asciz  "Hand-written DWARF"
 .Linfo_string3:
         .asciz  "f"
 .Linfo_string4:
         .asciz  "int"
 
+.ifdef LOC
         .section        .debug_loc,"",@progbits
+.endif
+.ifdef LOCLISTS
+        .section        .debug_loclists,"",@progbits
+        .long   .Ldebug_loclist_table_end0-.Ldebug_loclist_table_start0 # Length
+.Ldebug_loclist_table_start0:
+        .short  5                       # Version
+        .byte   8                       # Address size
+        .byte   0                       # Segment selector size
+        .long   0                       # Offset entry count
+.endif
 .Ldebug_loc0:
-        .quad   .Lfunc_begin0-.Lfunc_begin0
-        .quad   .Ltmp0-.Lfunc_begin0
-        .short  1                       # Loc expr size
+        OFFSET_PAIR .Lfunc_begin0-.Lfunc_begin0, .Ltmp0-.Lfunc_begin0
+        EXPR_SIZE 1
         .byte   85                      # super-register DW_OP_reg5
-        .quad   .Ltmp0-.Lfunc_begin0
-        .quad   .Lfunc_end0-.Lfunc_begin0
-        .short  1                       # Loc expr size
+        OFFSET_PAIR   .Ltmp0-.Lfunc_begin0, .Lfunc_end0-.Lfunc_begin0
+        EXPR_SIZE 1
         .byte   80                      # super-register DW_OP_reg0
-        .quad   0
-        .quad   0
+        END_OF_LIST
 
 .Ldebug_loc3:
-        .quad   -1                      # Select base address
-        .quad   .Ltmp1
-        .quad   .Ltmp1-.Ltmp1
-        .quad   .Ltmp2-.Ltmp1
-        .short  1                       # Loc expr size
+        BASE_ADDRESS .Ltmp1
+        OFFSET_PAIR .Ltmp1-.Ltmp1, .Ltmp2-.Ltmp1
+        EXPR_SIZE 1
         .byte   81                      # super-register DW_OP_reg1
-        .quad   0
-        .quad   0
+        END_OF_LIST
+
+.ifdef LOCLISTS
+.Ldebug_loc4:
+        .byte   3                       # DW_LLE_startx_length
+        .uleb128 0xdead
+        .uleb128 1
+        EXPR_SIZE 1
+        .byte   82                      # super-register DW_OP_reg2
+        END_OF_LIST
+.endif
 
 .Ldebug_loc2:
-        .quad   .Lfunc_begin0-.Lfunc_begin0
-        .quad   .Lfunc_end0-.Lfunc_begin0
-        .short  0xdead                  # Loc expr size
+        OFFSET_PAIR .Lfunc_begin0-.Lfunc_begin0, .Lfunc_end0-.Lfunc_begin0
+        EXPR_SIZE  0xdead
+.Ldebug_loclist_table_end0:
 
         .section        .debug_abbrev,"",@progbits
         .byte   1                       # Abbreviation Code
         .byte   17                      # DW_TAG_compile_unit
         .byte   1                       # DW_CHILDREN_yes
         .byte   37                      # DW_AT_producer
         .byte   14                      # DW_FORM_strp
         .byte   19                      # DW_AT_language
.Ldebug_loclist_table_end0:
         .byte   0                       # EOM(1)
         .byte   0                       # EOM(2)
         .byte   0                       # EOM(3)
 
         .section        .debug_info,"",@progbits
 .Lcu_begin0:
         .long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
 .Ldebug_info_start0:
+.ifdef LOC
         .short  4                       # DWARF version number
         .long   .debug_abbrev           # Offset Into Abbrev. Section
         .byte   8                       # Address Size (in bytes)
+.endif
+.ifdef LOCLISTS
+        .short  5                       # DWARF version number
+        .byte   1                       # DWARF Unit Type
+        .byte   8                       # Address Size (in bytes)
+        .long   .debug_abbrev           # Offset Into Abbrev. Section
+.endif
         .byte   1                       # Abbrev [1] 0xb:0x50 DW_TAG_compile_unit
         .long   .Linfo_string0          # DW_AT_producer
         .short  12                      # DW_AT_language
         .byte   2                       # Abbrev [2] 0x2a:0x29 DW_TAG_subprogram
         .quad   .Lfunc_begin0           # DW_AT_low_pc
         .long   .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc
         .long   .Linfo_string3          # DW_AT_name
         .long   .Lint                   # DW_AT_type
         .byte   3                       # Abbrev [3] DW_TAG_formal_parameter
         .long   .Ldebug_loc0            # DW_AT_location
         .asciz  "x0"                    # DW_AT_name
         .long   .Lint-.Lcu_begin0       # DW_AT_type
         .byte   3                       # Abbrev [3] DW_TAG_formal_parameter
         .long   0xdeadbeef              # DW_AT_location
         .asciz  "x1"                    # DW_AT_name
         .long   .Lint-.Lcu_begin0       # DW_AT_type
         .byte   3                       # Abbrev [3] DW_TAG_formal_parameter
         .long   .Ldebug_loc2            # DW_AT_location
         .asciz  "x2"                    # DW_AT_name
         .long   .Lint-.Lcu_begin0       # DW_AT_type
         .byte   3                       # Abbrev [3] DW_TAG_formal_parameter
         .long   .Ldebug_loc3            # DW_AT_location
         .asciz  "x3"                    # DW_AT_name
         .long   .Lint-.Lcu_begin0       # DW_AT_type
+.ifdef LOCLISTS
+        .byte   3                       # Abbrev [3] DW_TAG_formal_parameter
+        .long   .Ldebug_loc4            # DW_AT_location
+        .asciz  "x4"                    # DW_AT_name
+        .long   .Lint-.Lcu_begin0       # DW_AT_type
+.endif
         .byte   0                       # End Of Children Mark
 .Lint:
         .byte   4                       # Abbrev [4] 0x53:0x7 DW_TAG_base_type
         .long   .Linfo_string4          # DW_AT_name
         .byte   5                       # DW_AT_encoding
         .byte   4                       # DW_AT_byte_size
         .byte   0                       # End Of Children Mark
 .Ldebug_info_end0: