[lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 4 19:06:55 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
<details>
<summary>Changes</summary>
Change the interface of DWARFExpression::Evaluate and
DWARFExpressionList::Evaluate to return an llvm::Expected instead of a
boolean. This eliminates the Status output parameter and improves error
handling.
---
Patch is 78.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/94420.diff
12 Files Affected:
- (modified) lldb/include/lldb/Expression/DWARFExpression.h (+6-7)
- (modified) lldb/include/lldb/Expression/DWARFExpressionList.h (+6-4)
- (modified) lldb/source/Core/ValueObjectVariable.cpp (+6-2)
- (modified) lldb/source/Expression/DWARFExpression.cpp (+286-461)
- (modified) lldb/source/Expression/DWARFExpressionList.cpp (+10-18)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+12-8)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+9-7)
- (modified) lldb/source/Symbol/Function.cpp (+9-8)
- (modified) lldb/source/Target/RegisterContextUnwind.cpp (+12-11)
- (modified) lldb/source/Target/StackFrame.cpp (+7-12)
- (modified) lldb/unittests/Expression/DWARFExpressionTest.cpp (+38-40)
- (modified) llvm/include/llvm/Support/Error.h (+5)
``````````diff
diff --git a/lldb/include/lldb/Expression/DWARFExpression.h b/lldb/include/lldb/Expression/DWARFExpression.h
index 1d85308d1caa7..e85ba464dea6b 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -132,13 +132,12 @@ class DWARFExpression {
/// \return
/// True on success; false otherwise. If error_ptr is non-NULL,
/// details of the failure are provided through it.
- static bool Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
- lldb::ModuleSP module_sp, const DataExtractor &opcodes,
- const plugin::dwarf::DWARFUnit *dwarf_cu,
- const lldb::RegisterKind reg_set,
- const Value *initial_value_ptr,
- const Value *object_address_ptr, Value &result,
- Status *error_ptr);
+ static llvm::Expected<Value>
+ Evaluate(ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
+ lldb::ModuleSP module_sp, const DataExtractor &opcodes,
+ const plugin::dwarf::DWARFUnit *dwarf_cu,
+ const lldb::RegisterKind reg_set, const Value *initial_value_ptr,
+ const Value *object_address_ptr);
static bool ParseDWARFLocationList(const plugin::dwarf::DWARFUnit *dwarf_cu,
const DataExtractor &data,
diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h b/lldb/include/lldb/Expression/DWARFExpressionList.h
index c2218ad4af0a7..f711a1cbe9bbd 100644
--- a/lldb/include/lldb/Expression/DWARFExpressionList.h
+++ b/lldb/include/lldb/Expression/DWARFExpressionList.h
@@ -9,6 +9,7 @@
#ifndef LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
#define LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
+#include "lldb/Core/Value.h"
#include "lldb/Expression/DWARFExpression.h"
#include "lldb/Utility/RangeMap.h"
#include "lldb/lldb-private.h"
@@ -113,10 +114,11 @@ class DWARFExpressionList {
void SetModule(const lldb::ModuleSP &module) { m_module_wp = module; }
- bool 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;
+ llvm::Expected<Value> Evaluate(ExecutionContext *exe_ctx,
+ RegisterContext *reg_ctx,
+ lldb::addr_t func_load_addr,
+ const Value *initial_value_ptr,
+ const Value *object_address_ptr) const;
private:
// RangeDataVector requires a comparator for DWARFExpression, but it doesn't
diff --git a/lldb/source/Core/ValueObjectVariable.cpp b/lldb/source/Core/ValueObjectVariable.cpp
index 67d71c90a959d..51eb11d3a189e 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -164,8 +164,11 @@ bool ValueObjectVariable::UpdateValue() {
target);
}
Value old_value(m_value);
- if (expr_list.Evaluate(&exe_ctx, nullptr, loclist_base_load_addr, nullptr,
- nullptr, m_value, &m_error)) {
+ llvm::Expected<Value> maybe_value = expr_list.Evaluate(
+ &exe_ctx, nullptr, loclist_base_load_addr, nullptr, nullptr);
+
+ if (maybe_value) {
+ m_value = *maybe_value;
m_resolved_value = m_value;
m_value.SetContext(Value::ContextType::Variable, variable);
@@ -246,6 +249,7 @@ bool ValueObjectVariable::UpdateValue() {
SetValueIsValid(m_error.Success());
} else {
+ m_error = maybe_value.takeError();
// could not find location, won't allow editing
m_resolved_value.SetContext(Value::ContextType::Invalid, nullptr);
}
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 7473bb8255d0a..5c7ab49410efe 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -541,12 +541,12 @@ bool DWARFExpression::LinkThreadLocalStorage(
return true;
}
-static bool Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
- ExecutionContext *exe_ctx,
- RegisterContext *reg_ctx,
- const DataExtractor &opcodes,
- lldb::offset_t &opcode_offset,
- Status *error_ptr, Log *log) {
+static llvm::Error Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
+ ExecutionContext *exe_ctx,
+ RegisterContext *reg_ctx,
+ const DataExtractor &opcodes,
+ lldb::offset_t &opcode_offset,
+ Log *log) {
// DW_OP_entry_value(sub-expr) describes the location a variable had upon
// function entry: this variable location is presumed to be optimized out at
// the current PC value. The caller of the function may have call site
@@ -593,16 +593,15 @@ static bool Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
// 1. Find the function which pushed the current frame onto the stack.
if ((!exe_ctx || !exe_ctx->HasTargetScope()) || !reg_ctx) {
- LLDB_LOG(log, "Evaluate_DW_OP_entry_value: no exe/reg context");
- return false;
+ return llvm::createStringError(
+ "Evaluate_DW_OP_entry_value: no exe/reg context");
}
StackFrame *current_frame = exe_ctx->GetFramePtr();
Thread *thread = exe_ctx->GetThreadPtr();
- if (!current_frame || !thread) {
- LLDB_LOG(log, "Evaluate_DW_OP_entry_value: no current frame/thread");
- return false;
- }
+ if (!current_frame || !thread)
+ return llvm::createStringError(
+ "Evaluate_DW_OP_entry_value: no current frame/thread");
Target &target = exe_ctx->GetTargetRef();
StackFrameSP parent_frame = nullptr;
@@ -634,25 +633,23 @@ static bool Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
break;
}
if (!parent_frame || !parent_frame->GetRegisterContext()) {
- LLDB_LOG(log, "Evaluate_DW_OP_entry_value: no parent frame with reg ctx");
- return false;
+ return llvm::createStringError(
+ "Evaluate_DW_OP_entry_value: no parent frame with reg ctx");
}
Function *parent_func =
parent_frame->GetSymbolContext(eSymbolContextFunction).function;
- if (!parent_func) {
- LLDB_LOG(log, "Evaluate_DW_OP_entry_value: no parent function");
- return false;
- }
+ if (!parent_func)
+ return llvm::createStringError(
+ "Evaluate_DW_OP_entry_value: no parent function");
// 2. Find the call edge in the parent function responsible for creating the
// current activation.
Function *current_func =
current_frame->GetSymbolContext(eSymbolContextFunction).function;
- if (!current_func) {
- LLDB_LOG(log, "Evaluate_DW_OP_entry_value: no current function");
- return false;
- }
+ if (!current_func)
+ return llvm::createStringError(
+ "Evaluate_DW_OP_entry_value: no current function");
CallEdge *call_edge = nullptr;
ModuleList &modlist = target.GetImages();
@@ -663,17 +660,16 @@ static bool Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
// produced by an ambiguous tail call. In this case, refuse to proceed.
call_edge = parent_func->GetCallEdgeForReturnAddress(return_pc, target);
if (!call_edge) {
- LLDB_LOG(log,
- "Evaluate_DW_OP_entry_value: no call edge for retn-pc = {0:x} "
- "in parent frame {1}",
- return_pc, parent_func->GetName());
- return false;
+ return llvm::createStringError(llvm::formatv(
+ "Evaluate_DW_OP_entry_value: no call edge for retn-pc = {0:x} "
+ "in parent frame {1}",
+ return_pc, parent_func->GetName()));
}
Function *callee_func = call_edge->GetCallee(modlist, parent_exe_ctx);
if (callee_func != current_func) {
- LLDB_LOG(log, "Evaluate_DW_OP_entry_value: ambiguous call sequence, "
- "can't find real parent frame");
- return false;
+ return llvm::createStringError(
+ "Evaluate_DW_OP_entry_value: ambiguous call sequence, "
+ "can't find real parent frame");
}
} else {
// The StackFrameList solver machinery has deduced that an unambiguous tail
@@ -686,21 +682,19 @@ static bool Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
}
}
}
- if (!call_edge) {
- LLDB_LOG(log, "Evaluate_DW_OP_entry_value: no unambiguous edge from parent "
- "to current function");
- return false;
- }
+ if (!call_edge)
+ return llvm::createStringError(
+ "Evaluate_DW_OP_entry_value: no unambiguous edge from parent "
+ "to current function");
// 3. Attempt to locate the DW_OP_entry_value expression in the set of
// available call site parameters. If found, evaluate the corresponding
// parameter in the context of the parent frame.
const uint32_t subexpr_len = opcodes.GetULEB128(&opcode_offset);
const void *subexpr_data = opcodes.GetData(&opcode_offset, subexpr_len);
- if (!subexpr_data) {
- LLDB_LOG(log, "Evaluate_DW_OP_entry_value: subexpr could not be read");
- return false;
- }
+ if (!subexpr_data)
+ return llvm::createStringError(
+ "Evaluate_DW_OP_entry_value: subexpr could not be read");
const CallSiteParameter *matched_param = nullptr;
for (const CallSiteParameter ¶m : call_edge->GetCallSiteParameters()) {
@@ -726,28 +720,27 @@ static bool Evaluate_DW_OP_entry_value(std::vector<Value> &stack,
break;
}
}
- if (!matched_param) {
- LLDB_LOG(log,
- "Evaluate_DW_OP_entry_value: no matching call site param found");
- return false;
- }
+ if (!matched_param)
+ return llvm::createStringError(
+ "Evaluate_DW_OP_entry_value: no matching call site param found");
// TODO: Add support for DW_OP_push_object_address within a DW_OP_entry_value
// subexpresion whenever llvm does.
- Value result;
const DWARFExpressionList ¶m_expr = matched_param->LocationInCaller;
- if (!param_expr.Evaluate(&parent_exe_ctx,
- parent_frame->GetRegisterContext().get(),
- LLDB_INVALID_ADDRESS,
- /*initial_value_ptr=*/nullptr,
- /*object_address_ptr=*/nullptr, result, error_ptr)) {
+
+ llvm::Expected<Value> maybe_result = param_expr.Evaluate(
+ &parent_exe_ctx, parent_frame->GetRegisterContext().get(),
+ LLDB_INVALID_ADDRESS,
+ /*initial_value_ptr=*/nullptr,
+ /*object_address_ptr=*/nullptr);
+ if (!maybe_result) {
LLDB_LOG(log,
"Evaluate_DW_OP_entry_value: call site param evaluation failed");
- return false;
+ return maybe_result.takeError();
}
- stack.push_back(result);
- return true;
+ stack.push_back(*maybe_result);
+ return llvm::Error::success();
}
namespace {
@@ -862,19 +855,15 @@ static Scalar DerefSizeExtractDataHelper(uint8_t *addr_bytes,
return addr_data.GetAddress(&addr_data_offset);
}
-bool DWARFExpression::Evaluate(
+llvm::Expected<Value> DWARFExpression::Evaluate(
ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
lldb::ModuleSP module_sp, const DataExtractor &opcodes,
const DWARFUnit *dwarf_cu, const lldb::RegisterKind reg_kind,
- const Value *initial_value_ptr, const Value *object_address_ptr,
- Value &result, Status *error_ptr) {
+ const Value *initial_value_ptr, const Value *object_address_ptr) {
- if (opcodes.GetByteSize() == 0) {
- if (error_ptr)
- error_ptr->SetErrorString(
- "no location, value may have been optimized out");
- return false;
- }
+ if (opcodes.GetByteSize() == 0)
+ return llvm::createStringError(
+ "no location, value may have been optimized out");
std::vector<Value> stack;
Process *process = nullptr;
@@ -994,11 +983,9 @@ bool DWARFExpression::Evaluate(
// retrieved from the dereferenced address is the size of an address on the
// target machine.
case DW_OP_deref: {
- if (stack.empty()) {
- if (error_ptr)
- error_ptr->SetErrorString("Expression stack empty for DW_OP_deref.");
- return false;
- }
+ if (stack.empty())
+ return llvm::createStringError(
+ "Expression stack empty for DW_OP_deref.");
Value::ValueType value_type = stack.back().GetValueType();
switch (value_type) {
case Value::ValueType::HostAddress: {
@@ -1013,11 +1000,12 @@ bool DWARFExpression::Evaluate(
LLDB_INVALID_ADDRESS);
Address so_addr;
+ Status load_err;
auto maybe_load_addr = ResolveLoadAddress(
- exe_ctx, module_sp, error_ptr, "DW_OP_deref", file_addr, so_addr);
+ exe_ctx, module_sp, &load_err, "DW_OP_deref", file_addr, so_addr);
if (!maybe_load_addr)
- return false;
+ return load_err.ToError();
stack.back().GetScalar() = *maybe_load_addr;
// Fall through to load address promotion code below.
@@ -1041,30 +1029,22 @@ bool DWARFExpression::Evaluate(
stack.back().GetScalar() = pointer_value;
stack.back().ClearContext();
} else {
- if (error_ptr)
- error_ptr->SetErrorStringWithFormat(
- "Failed to dereference pointer from 0x%" PRIx64
- " for DW_OP_deref: %s\n",
- pointer_addr, error.AsCString());
- return false;
+ return llvm::createStringError(
+ "Failed to dereference pointer from 0x%" PRIx64
+ " for DW_OP_deref: %s\n",
+ pointer_addr, error.AsCString());
}
} else {
- if (error_ptr)
- error_ptr->SetErrorString("NULL process for DW_OP_deref.\n");
- return false;
+ return llvm::createStringError("NULL process for DW_OP_deref.\n");
}
} else {
- if (error_ptr)
- error_ptr->SetErrorString(
- "NULL execution context for DW_OP_deref.\n");
- return false;
+ return llvm::createStringError(
+ "NULL execution context for DW_OP_deref.\n");
}
break;
case Value::ValueType::Invalid:
- if (error_ptr)
- error_ptr->SetErrorString("Invalid value type for DW_OP_deref.\n");
- return false;
+ return llvm::createStringError("Invalid value type for DW_OP_deref.\n");
}
} break;
@@ -1083,18 +1063,13 @@ bool DWARFExpression::Evaluate(
// expression stack.
case DW_OP_deref_size: {
if (stack.empty()) {
- if (error_ptr)
- error_ptr->SetErrorString(
- "Expression stack empty for DW_OP_deref_size.");
- return false;
+ return llvm::createStringError(
+ "Expression stack empty for DW_OP_deref_size.");
}
uint8_t size = opcodes.GetU8(&offset);
if (size > 8) {
- if (error_ptr)
- error_ptr->SetErrorStringWithFormat(
- "Invalid address size for DW_OP_deref_size: %d\n",
- size);
- return false;
+ return llvm::createStringError(
+ "Invalid address size for DW_OP_deref_size: %d\n", size);
}
Value::ValueType value_type = stack.back().GetValueType();
switch (value_type) {
@@ -1142,13 +1117,14 @@ bool DWARFExpression::Evaluate(
auto file_addr =
stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
Address so_addr;
+ Status resolve_err;
auto maybe_load_addr =
- ResolveLoadAddress(exe_ctx, module_sp, error_ptr,
- "DW_OP_deref_size", file_addr, so_addr,
- /*check_sectionoffset=*/true);
+ ResolveLoadAddress(exe_ctx, module_sp, &resolve_err,
+ "DW_OP_deref_size", file_addr, so_addr,
+ /*check_sectionoffset=*/true);
if (!maybe_load_addr)
- return false;
+ return resolve_err.ToError();
addr_t load_addr = *maybe_load_addr;
@@ -1166,12 +1142,10 @@ bool DWARFExpression::Evaluate(
stack.back().ClearContext();
break;
} else {
- if (error_ptr)
- error_ptr->SetErrorStringWithFormat(
- "Failed to dereference pointer for DW_OP_deref_size: "
- "%s\n",
- error.AsCString());
- return false;
+ return llvm::createStringError(
+ "Failed to dereference pointer for DW_OP_deref_size: "
+ "%s\n",
+ error.AsCString());
}
}
stack.back().GetScalar() = load_addr;
@@ -1195,30 +1169,25 @@ bool DWARFExpression::Evaluate(
process->GetByteOrder(), size);
stack.back().ClearContext();
} else {
- if (error_ptr)
- error_ptr->SetErrorStringWithFormat(
- "Failed to dereference pointer from 0x%" PRIx64
- " for DW_OP_deref: %s\n",
- pointer_addr, error.AsCString());
- return false;
+ return llvm::createStringError(
+ "Failed to dereference pointer from 0x%" PRIx64
+ " for DW_OP_deref: %s\n",
+ pointer_addr, error.AsCString());
}
} else {
- if (error_ptr)
- error_ptr->SetErrorString("NULL process for DW_OP_deref_size.\n");
- return false;
+
+ return llvm::createStringError(
+ "NULL process for DW_OP_deref_size.\n");
}
} else {
- if (error_ptr)
- error_ptr->SetErrorString(
- "NULL execution context for DW_OP_deref_size.\n");
- return false;
+ return llvm::createStringError(
+ "NULL execution context for DW_OP_deref_size.\n");
}
break;
case Value::ValueType::Invalid:
- if (error_ptr)
- error_ptr->SetErrorString("Invalid value for DW_OP_deref_size.\n");
- return false;
+
+ return llvm::createStringError("Invalid value for DW_OP_deref_size.\n");
}
} break;
@@ -1239,9 +1208,9 @@ bool DWARFExpression::Evaluate(
// extended to the size of an address on the target machine before being
// pushed on the expression stack.
case DW_OP_xderef_size:
- if (error_ptr)
- error_ptr->SetErrorString("Unimplemented opcode: DW_OP_xderef_size.");
- return false;
+
+ return llvm::createStringError(
+ "Unimplemented opcode: DW_OP_xderef_size.");
// OPCODE: DW_OP_xderef
// OPERANDS: none
// DESCRIPTION: Provides an extended dereference mechanism. The entry at
@@ -1253,9 +1222,7 @@ bool DWARFExpression::Evaluate(
// retrieved from the dereferenced address is the size of an address on the
// target machine.
case DW_OP_xderef:
- if (error_ptr)
- error_ptr->SetErrorString("Unimplemented opcode: DW_OP_xderef.");
- return false;
+ return llvm::createStringError("Unimplemented opcode: DW_OP_xderef.");
// All DW_OP_constXXX opcodes have a single operand as noted below:
//
@@ -1308,9 +1275,7 @@ bool DWARFExpression::Evaluate(
// DESCRIPTION: duplicates the value at the top of the stack
case DW_OP_dup:
if (stack.empty()) {
- if (error_ptr)
- error_ptr->SetErrorString("Expression stack empty for DW_OP_dup.");
- return false;
+ return llvm::createStringError("Expression stack empty for DW_OP_dup.");
} else
stack.push_back(stack.back());
break;
@@ -1320,9 +1285,8 @@ bool DWARFExpression::Evaluate(
// DESCRIPTION: pops the value at the top of the stack
case DW_OP_drop:
if (stack.empty()) {
- if (error_ptr)
- error_ptr->SetErrorString("Expression stack empty for DW...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/94420
More information about the llvm-commits
mailing list