[Lldb-commits] [lldb] [lldb] Improve error reporting in GetLocation_DW_OP_addr (PR #120162)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Dec 16 15:58:37 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/120162.diff
3 Files Affected:
- (modified) lldb/include/lldb/Expression/DWARFExpression.h (+4-7)
- (modified) lldb/source/Expression/DWARFExpression.cpp (+12-10)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+16-12)
``````````diff
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;
``````````
</details>
https://github.com/llvm/llvm-project/pull/120162
More information about the lldb-commits
mailing list