https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/120162
…addr Instead of simply raising an error flag, use an llvm::Expected to propagate a meaningful error to the caller, who can report it. rdar://139705570 >From 9637e922d0646a4008c76ac0cbf9713511bf2d3e Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Mon, 16 Dec 2024 15:08:25 -0800 Subject: [PATCH] [lldb] Improve error reporting in DWARFExpression::GetLocation_DW_OP_addr Instead of simply raising an error flag, use an llvm::Expected to propagate a meaningful error to the caller, who can report it. rdar://139705570 --- .../include/lldb/Expression/DWARFExpression.h | 11 +++----- lldb/source/Expression/DWARFExpression.cpp | 22 ++++++++------- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 28 +++++++++++-------- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/lldb/include/lldb/Expression/DWARFExpression.h b/lldb/include/lldb/Expression/DWARFExpression.h index e85ba464dea6b6..2c1e717ee32eb7 100644 --- a/lldb/include/lldb/Expression/DWARFExpression.h +++ b/lldb/include/lldb/Expression/DWARFExpression.h @@ -16,6 +16,7 @@ #include "lldb/Utility/Status.h" #include "lldb/lldb-private.h" #include "llvm/DebugInfo/DWARF/DWARFLocationExpression.h" +#include "llvm/Support/Error.h" #include <functional> namespace lldb_private { @@ -61,15 +62,11 @@ class DWARFExpression { /// The dwarf unit this expression belongs to. Only required to resolve /// DW_OP{addrx, GNU_addr_index}. /// - /// \param[out] error - /// If the location stream contains unknown DW_OP opcodes or the - /// data is missing, \a error will be set to \b true. - /// /// \return /// The address specified by the operation, if the operation exists, or - /// LLDB_INVALID_ADDRESS otherwise. - lldb::addr_t GetLocation_DW_OP_addr(const plugin::dwarf::DWARFUnit *dwarf_cu, - bool &error) const; + /// an llvm::Error otherwise. + llvm::Expected<lldb::addr_t> + GetLocation_DW_OP_addr(const plugin::dwarf::DWARFUnit *dwarf_cu) const; bool Update_DW_OP_addr(const plugin::dwarf::DWARFUnit *dwarf_cu, lldb::addr_t file_addr); diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index a7126b25c1cc38..1d826e341e2c44 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -343,30 +343,32 @@ static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data, } } -lldb::addr_t DWARFExpression::GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu, - bool &error) const { - error = false; +llvm::Expected<lldb::addr_t> +DWARFExpression::GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu) const { lldb::offset_t offset = 0; while (m_data.ValidOffset(offset)) { const uint8_t op = m_data.GetU8(&offset); if (op == DW_OP_addr) return m_data.GetAddress(&offset); + if (op == DW_OP_GNU_addr_index || op == DW_OP_addrx) { - uint64_t index = m_data.GetULEB128(&offset); + const uint64_t index = m_data.GetULEB128(&offset); if (dwarf_cu) return dwarf_cu->ReadAddressFromDebugAddrSection(index); - error = true; - break; + return llvm::createStringError("cannot evaluate %s without a DWARF unit", + DW_OP_value_to_name(op)); } + const lldb::offset_t op_arg_size = GetOpcodeDataSize(m_data, offset, op, dwarf_cu); - if (op_arg_size == LLDB_INVALID_OFFSET) { - error = true; - break; - } + if (op_arg_size == LLDB_INVALID_OFFSET) + return llvm::createStringError("cannot get opcode data size for %s", + DW_OP_value_to_name(op)); + offset += op_arg_size; } + return LLDB_INVALID_ADDRESS; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 68e50902d641a2..5bf38b332b7f34 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -14,6 +14,7 @@ #include "llvm/Support/Casting.h" #include "llvm/Support/FileUtilities.h" #include "llvm/Support/Format.h" +#include "llvm/Support/FormatAdapters.h" #include "llvm/Support/Threading.h" #include "lldb/Core/Module.h" @@ -1705,7 +1706,7 @@ SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) { // We have a SymbolFileDWARFDebugMap, so let it find the right file if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) return debug_map->GetSymbolFileByOSOIndex(*file_index); - + // Handle the .dwp file case correctly if (*file_index == DIERef::k_file_index_mask) return GetDwpSymbolFile().get(); // DWP case @@ -3539,17 +3540,20 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc, // Check if the location has a DW_OP_addr with any address value... lldb::addr_t location_DW_OP_addr = LLDB_INVALID_ADDRESS; if (!location_is_const_value_data) { - bool op_error = false; - const DWARFExpression* location = location_list.GetAlwaysValidExpr(); - if (location) - location_DW_OP_addr = - location->GetLocation_DW_OP_addr(location_form.GetUnit(), op_error); - if (op_error) { - StreamString strm; - location->DumpLocation(&strm, eDescriptionLevelFull, nullptr); - GetObjectFile()->GetModule()->ReportError( - "{0:x16}: {1} ({2}) has an invalid location: {3}", die.GetOffset(), - DW_TAG_value_to_name(die.Tag()), die.Tag(), strm.GetData()); + if (const DWARFExpression *location = + location_list.GetAlwaysValidExpr()) { + if (auto maybe_location_DW_OP_addr = + location->GetLocation_DW_OP_addr(location_form.GetUnit())) { + location_DW_OP_addr = *maybe_location_DW_OP_addr; + } else { + StreamString strm; + location->DumpLocation(&strm, eDescriptionLevelFull, nullptr); + GetObjectFile()->GetModule()->ReportError( + "{0:x16}: {1} ({2}) has an invalid location: {3}: {4}", + die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(), + llvm::fmt_consume(maybe_location_DW_OP_addr.takeError()), + strm.GetData()); + } } if (location_DW_OP_addr != LLDB_INVALID_ADDRESS) is_static_lifetime = true; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits