https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/94420
>From 22bb28a5e3fc84c75e1013c3b0c15654e7b786da Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Tue, 4 Jun 2024 17:04:15 -0700 Subject: [PATCH 1/5] [Support] Add variadic createStringError overload (NFC) --- llvm/include/llvm/Support/Error.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h index 662c3ea46e3c1..1fa0d8cb709cc 100644 --- a/llvm/include/llvm/Support/Error.h +++ b/llvm/include/llvm/Support/Error.h @@ -1277,6 +1277,11 @@ inline Error createStringError(const Twine &S) { return createStringError(llvm::inconvertibleErrorCode(), S); } +template <typename... Ts> +inline Error createStringError(char const *Fmt, const Ts &...Vals) { + return createStringError(llvm::inconvertibleErrorCode(), Fmt, Vals...); +} + template <typename... Ts> inline Error createStringError(std::errc EC, char const *Fmt, const Ts &... Vals) { >From d5c288ddb9473a52a8635a32d3a5cc057c9cb986 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Tue, 4 Jun 2024 19:04:07 -0700 Subject: [PATCH 2/5] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) 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. --- .../include/lldb/Expression/DWARFExpression.h | 13 +- .../lldb/Expression/DWARFExpressionList.h | 10 +- lldb/source/Core/ValueObjectVariable.cpp | 8 +- lldb/source/Expression/DWARFExpression.cpp | 747 +++++++----------- .../source/Expression/DWARFExpressionList.cpp | 28 +- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 20 +- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 16 +- lldb/source/Symbol/Function.cpp | 17 +- lldb/source/Target/RegisterContextUnwind.cpp | 23 +- lldb/source/Target/StackFrame.cpp | 19 +- .../Expression/DWARFExpressionTest.cpp | 78 +- 11 files changed, 401 insertions(+), 578 deletions(-) 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_OP_drop."); - return false; + return llvm::createStringError( + "Expression stack empty for DW_OP_drop."); } else stack.pop_back(); break; @@ -1333,10 +1297,8 @@ bool DWARFExpression::Evaluate( // the top of the stack. case DW_OP_over: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_over."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_over."); } else stack.push_back(stack[stack.size() - 2]); break; @@ -1350,10 +1312,8 @@ bool DWARFExpression::Evaluate( if (pick_idx < stack.size()) stack.push_back(stack[stack.size() - 1 - pick_idx]); else { - if (error_ptr) - error_ptr->SetErrorStringWithFormat( - "Index %u out of range for DW_OP_pick.\n", pick_idx); - return false; + return llvm::createStringError( + "Index %u out of range for DW_OP_pick.\n", pick_idx); } } break; @@ -1364,10 +1324,8 @@ bool DWARFExpression::Evaluate( // becomes the top of the stack case DW_OP_swap: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_swap."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_swap."); } else { tmp = stack.back(); stack.back() = stack[stack.size() - 2]; @@ -1383,10 +1341,8 @@ bool DWARFExpression::Evaluate( // entry. case DW_OP_rot: if (stack.size() < 3) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 3 items for DW_OP_rot."); - return false; + return llvm::createStringError( + "Expression stack needs at least 3 items for DW_OP_rot."); } else { size_t last_idx = stack.size() - 1; Value old_top = stack[last_idx]; @@ -1403,15 +1359,11 @@ bool DWARFExpression::Evaluate( // represented, the result is undefined. case DW_OP_abs: if (stack.empty()) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 1 item for DW_OP_abs."); - return false; + return llvm::createStringError( + "Expression stack needs at least 1 item for DW_OP_abs."); } else if (!stack.back().ResolveValue(exe_ctx).AbsoluteValue()) { - if (error_ptr) - error_ptr->SetErrorString( - "Failed to take the absolute value of the first stack item."); - return false; + return llvm::createStringError( + "Failed to take the absolute value of the first stack item."); } break; @@ -1421,10 +1373,8 @@ bool DWARFExpression::Evaluate( // operation on the two, and pushes the result. case DW_OP_and: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_and."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_and."); } else { tmp = stack.back(); stack.pop_back(); @@ -1440,16 +1390,12 @@ bool DWARFExpression::Evaluate( // the result. case DW_OP_div: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_div."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_div."); } else { tmp = stack.back(); if (tmp.ResolveValue(exe_ctx).IsZero()) { - if (error_ptr) - error_ptr->SetErrorString("Divide by zero."); - return false; + return llvm::createStringError("Divide by zero."); } else { stack.pop_back(); Scalar divisor, dividend; @@ -1459,9 +1405,7 @@ bool DWARFExpression::Evaluate( dividend.MakeSigned(); stack.back() = dividend / divisor; if (!stack.back().ResolveValue(exe_ctx).IsValid()) { - if (error_ptr) - error_ptr->SetErrorString("Divide failed."); - return false; + return llvm::createStringError("Divide failed."); } } } @@ -1473,10 +1417,8 @@ bool DWARFExpression::Evaluate( // of the stack from the former second entry, and pushes the result. case DW_OP_minus: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_minus."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_minus."); } else { tmp = stack.back(); stack.pop_back(); @@ -1492,10 +1434,8 @@ bool DWARFExpression::Evaluate( // stack. case DW_OP_mod: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_mod."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_mod."); } else { tmp = stack.back(); stack.pop_back(); @@ -1510,10 +1450,8 @@ bool DWARFExpression::Evaluate( // together, and pushes the result. case DW_OP_mul: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_mul."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_mul."); } else { tmp = stack.back(); stack.pop_back(); @@ -1527,16 +1465,11 @@ bool DWARFExpression::Evaluate( // DESCRIPTION: pops the top stack entry, and pushes its negation. case DW_OP_neg: if (stack.empty()) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 1 item for DW_OP_neg."); - return false; + return llvm::createStringError( + "Expression stack needs at least 1 item for DW_OP_neg."); } else { - if (!stack.back().ResolveValue(exe_ctx).UnaryNegate()) { - if (error_ptr) - error_ptr->SetErrorString("Unary negate failed."); - return false; - } + if (!stack.back().ResolveValue(exe_ctx).UnaryNegate()) + return llvm::createStringError("Unary negate failed."); } break; @@ -1546,15 +1479,11 @@ bool DWARFExpression::Evaluate( // complement case DW_OP_not: if (stack.empty()) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 1 item for DW_OP_not."); - return false; + return llvm::createStringError( + "Expression stack needs at least 1 item for DW_OP_not."); } else { if (!stack.back().ResolveValue(exe_ctx).OnesComplement()) { - if (error_ptr) - error_ptr->SetErrorString("Logical NOT failed."); - return false; + return llvm::createStringError("Logical NOT failed."); } } break; @@ -1565,10 +1494,8 @@ bool DWARFExpression::Evaluate( // operation on the two, and pushes the result. case DW_OP_or: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_or."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_or."); } else { tmp = stack.back(); stack.pop_back(); @@ -1583,10 +1510,8 @@ bool DWARFExpression::Evaluate( // pushes the result. case DW_OP_plus: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_plus."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_plus."); } else { tmp = stack.back(); stack.pop_back(); @@ -1600,18 +1525,15 @@ bool DWARFExpression::Evaluate( // constant operand and pushes the result. case DW_OP_plus_uconst: if (stack.empty()) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 1 item for DW_OP_plus_uconst."); - return false; + return llvm::createStringError( + "Expression stack needs at least 1 item for DW_OP_plus_uconst."); } else { const uint64_t uconst_value = opcodes.GetULEB128(&offset); // Implicit conversion from a UINT to a Scalar... stack.back().GetScalar() += uconst_value; if (!stack.back().GetScalar().IsValid()) { - if (error_ptr) - error_ptr->SetErrorString("DW_OP_plus_uconst failed."); - return false; + + return llvm::createStringError("DW_OP_plus_uconst failed."); } } break; @@ -1623,10 +1545,8 @@ bool DWARFExpression::Evaluate( // the stack, and pushes the result. case DW_OP_shl: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_shl."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_shl."); } else { tmp = stack.back(); stack.pop_back(); @@ -1641,18 +1561,14 @@ bool DWARFExpression::Evaluate( // specified by the former top of the stack, and pushes the result. case DW_OP_shr: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_shr."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_shr."); } else { tmp = stack.back(); stack.pop_back(); if (!stack.back().ResolveValue(exe_ctx).ShiftRightLogical( tmp.ResolveValue(exe_ctx))) { - if (error_ptr) - error_ptr->SetErrorString("DW_OP_shr failed."); - return false; + return llvm::createStringError("DW_OP_shr failed."); } } break; @@ -1665,10 +1581,8 @@ bool DWARFExpression::Evaluate( // of the stack, and pushes the result. case DW_OP_shra: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_shra."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_shra."); } else { tmp = stack.back(); stack.pop_back(); @@ -1682,10 +1596,8 @@ bool DWARFExpression::Evaluate( // exclusive-or operation on the two, and pushes the result. case DW_OP_xor: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_xor."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_xor."); } else { tmp = stack.back(); stack.pop_back(); @@ -1709,11 +1621,9 @@ bool DWARFExpression::Evaluate( if (new_offset <= opcodes.GetByteSize()) offset = new_offset; else { - if (error_ptr) - error_ptr->SetErrorStringWithFormatv( - "Invalid opcode offset in DW_OP_skip: {0}+({1}) > {2}", offset, - skip_offset, opcodes.GetByteSize()); - return false; + return llvm::createStringError(llvm::formatv( + "Invalid opcode offset in DW_OP_skip: {0}+({1}) > {2}", offset, + skip_offset, opcodes.GetByteSize())); } } break; @@ -1726,10 +1636,8 @@ bool DWARFExpression::Evaluate( // the current operation, beginning after the 2-byte constant. case DW_OP_bra: if (stack.empty()) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 1 item for DW_OP_bra."); - return false; + return llvm::createStringError( + "Expression stack needs at least 1 item for DW_OP_bra."); } else { tmp = stack.back(); stack.pop_back(); @@ -1743,11 +1651,9 @@ bool DWARFExpression::Evaluate( if (new_offset <= opcodes.GetByteSize()) offset = new_offset; else { - if (error_ptr) - error_ptr->SetErrorStringWithFormatv( - "Invalid opcode offset in DW_OP_bra: {0}+({1}) > {2}", offset, - bra_offset, opcodes.GetByteSize()); - return false; + return llvm::createStringError(llvm::formatv( + "Invalid opcode offset in DW_OP_bra: {0}+({1}) > {2}", offset, + bra_offset, opcodes.GetByteSize())); } } } @@ -1762,10 +1668,8 @@ bool DWARFExpression::Evaluate( // operation is false. case DW_OP_eq: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_eq."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_eq."); } else { tmp = stack.back(); stack.pop_back(); @@ -1783,10 +1687,8 @@ bool DWARFExpression::Evaluate( // operation is false. case DW_OP_ge: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_ge."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_ge."); } else { tmp = stack.back(); stack.pop_back(); @@ -1804,10 +1706,8 @@ bool DWARFExpression::Evaluate( // operation is false. case DW_OP_gt: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_gt."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_gt."); } else { tmp = stack.back(); stack.pop_back(); @@ -1825,10 +1725,8 @@ bool DWARFExpression::Evaluate( // operation is false. case DW_OP_le: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_le."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_le."); } else { tmp = stack.back(); stack.pop_back(); @@ -1846,10 +1744,8 @@ bool DWARFExpression::Evaluate( // operation is false. case DW_OP_lt: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_lt."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_lt."); } else { tmp = stack.back(); stack.pop_back(); @@ -1867,10 +1763,8 @@ bool DWARFExpression::Evaluate( // operation is false. case DW_OP_ne: if (stack.size() < 2) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 2 items for DW_OP_ne."); - return false; + return llvm::createStringError( + "Expression stack needs at least 2 items for DW_OP_ne."); } else { tmp = stack.back(); stack.pop_back(); @@ -1957,10 +1851,11 @@ bool DWARFExpression::Evaluate( dwarf4_location_description_kind = Register; reg_num = op - DW_OP_reg0; - if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, error_ptr, tmp)) + Status read_err; + if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, &read_err, tmp)) stack.push_back(tmp); else - return false; + return read_err.ToError(); } break; // OPCODE: DW_OP_regx // OPERANDS: @@ -1969,10 +1864,11 @@ bool DWARFExpression::Evaluate( case DW_OP_regx: { dwarf4_location_description_kind = Register; reg_num = opcodes.GetULEB128(&offset); - if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, error_ptr, tmp)) + Status read_err; + if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, &read_err, tmp)) stack.push_back(tmp); else - return false; + return read_err.ToError(); } break; // OPCODE: DW_OP_bregN @@ -2014,7 +1910,8 @@ bool DWARFExpression::Evaluate( case DW_OP_breg31: { reg_num = op - DW_OP_breg0; - if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, error_ptr, + Status read_err; + if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, &read_err, tmp)) { int64_t breg_offset = opcodes.GetSLEB128(&offset); tmp.ResolveValue(exe_ctx) += (uint64_t)breg_offset; @@ -2022,7 +1919,7 @@ bool DWARFExpression::Evaluate( stack.push_back(tmp); stack.back().SetValueType(Value::ValueType::LoadAddress); } else - return false; + return read_err.ToError(); } break; // OPCODE: DW_OP_bregx // OPERANDS: 2 @@ -2033,7 +1930,8 @@ bool DWARFExpression::Evaluate( case DW_OP_bregx: { reg_num = opcodes.GetULEB128(&offset); - if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, error_ptr, + Status read_err; + if (ReadRegisterValueAsScalar(reg_ctx, reg_kind, reg_num, &read_err, tmp)) { int64_t breg_offset = opcodes.GetSLEB128(&offset); tmp.ResolveValue(exe_ctx) += (uint64_t)breg_offset; @@ -2041,31 +1939,28 @@ bool DWARFExpression::Evaluate( stack.push_back(tmp); stack.back().SetValueType(Value::ValueType::LoadAddress); } else - return false; + return read_err.ToError(); } break; case DW_OP_fbreg: if (exe_ctx) { if (frame) { Scalar value; - if (frame->GetFrameBaseValue(value, error_ptr)) { + Status fb_err; + if (frame->GetFrameBaseValue(value, &fb_err)) { int64_t fbreg_offset = opcodes.GetSLEB128(&offset); value += fbreg_offset; stack.push_back(value); stack.back().SetValueType(Value::ValueType::LoadAddress); } else - return false; + return fb_err.ToError(); } else { - if (error_ptr) - error_ptr->SetErrorString( - "Invalid stack frame in context for DW_OP_fbreg opcode."); - return false; + return llvm::createStringError( + "Invalid stack frame in context for DW_OP_fbreg opcode."); } } else { - if (error_ptr) - error_ptr->SetErrorString( - "NULL execution context for DW_OP_fbreg.\n"); - return false; + return llvm::createStringError( + "NULL execution context for DW_OP_fbreg.\n"); } break; @@ -2127,7 +2022,7 @@ bool DWARFExpression::Evaluate( const lldb::addr_t addr = scalar.ULongLong(LLDB_INVALID_ADDRESS); switch (curr_piece_source_value_type) { case Value::ValueType::Invalid: - return false; + return llvm::createStringError("invalid value type"); case Value::ValueType::LoadAddress: case Value::ValueType::FileAddress: { if (target) { @@ -2136,35 +2031,28 @@ bool DWARFExpression::Evaluate( piece_byte_size, error, /*force_live_memory=*/false) != piece_byte_size) { - if (error_ptr) { - const char *addr_type = (curr_piece_source_value_type == - Value::ValueType::LoadAddress) - ? "load" - : "file"; - error_ptr->SetErrorStringWithFormat( - "failed to read memory DW_OP_piece(%" PRIu64 - ") from %s address 0x%" PRIx64, - piece_byte_size, addr_type, addr); - } - return false; + const char *addr_type = (curr_piece_source_value_type == + Value::ValueType::LoadAddress) + ? "load" + : "file"; + return llvm::createStringError( + "failed to read memory DW_OP_piece(%" PRIu64 + ") from %s address 0x%" PRIx64, + piece_byte_size, addr_type, addr); } } else { - if (error_ptr) - error_ptr->SetErrorStringWithFormat( - "failed to resize the piece memory buffer for " - "DW_OP_piece(%" PRIu64 ")", - piece_byte_size); - return false; + return llvm::createStringError( + "failed to resize the piece memory buffer for " + "DW_OP_piece(%" PRIu64 ")", + piece_byte_size); } } } break; case Value::ValueType::HostAddress: { - if (error_ptr) - error_ptr->SetErrorStringWithFormat( - "failed to read memory DW_OP_piece(%" PRIu64 - ") from host address 0x%" PRIx64, - piece_byte_size, addr); - return false; + return llvm::createStringError( + "failed to read memory DW_OP_piece(%" PRIu64 + ") from host address 0x%" PRIx64, + piece_byte_size, addr); } break; case Value::ValueType::Scalar: { @@ -2172,14 +2060,11 @@ bool DWARFExpression::Evaluate( uint32_t bit_offset = 0; if (!scalar.ExtractBitfield( bit_size, bit_offset)) { - if (error_ptr) - error_ptr->SetErrorStringWithFormat( - "unable to extract %" PRIu64 " bytes from a %" PRIu64 - " byte scalar value.", - piece_byte_size, - (uint64_t)curr_piece_source_value.GetScalar() - .GetByteSize()); - return false; + return llvm::createStringError( + "unable to extract %" PRIu64 " bytes from a %" PRIu64 + " byte scalar value.", + piece_byte_size, + (uint64_t)curr_piece_source_value.GetScalar().GetByteSize()); } // Create curr_piece with bit_size. By default Scalar // grows to the nearest host integer type. @@ -2198,27 +2083,20 @@ bool DWARFExpression::Evaluate( // so subsequent pieces will be able to access this piece and add // to it. if (pieces.AppendDataToHostBuffer(curr_piece) == 0) { - if (error_ptr) - error_ptr->SetErrorString("failed to append piece data"); - return false; + return llvm::createStringError("failed to append piece data"); } } else { // If this is the second or later piece there should be a value on // the stack. if (pieces.GetBuffer().GetByteSize() != op_piece_offset) { - if (error_ptr) - error_ptr->SetErrorStringWithFormat( - "DW_OP_piece for offset %" PRIu64 - " but top of stack is of size %" PRIu64, - op_piece_offset, pieces.GetBuffer().GetByteSize()); - return false; + return llvm::createStringError( + "DW_OP_piece for offset %" PRIu64 + " but top of stack is of size %" PRIu64, + op_piece_offset, pieces.GetBuffer().GetByteSize()); } - if (pieces.AppendDataToHostBuffer(curr_piece) == 0) { - if (error_ptr) - error_ptr->SetErrorString("failed to append piece data"); - return false; - } + if (pieces.AppendDataToHostBuffer(curr_piece) == 0) + return llvm::createStringError("failed to append piece data"); } } op_piece_offset += piece_byte_size; @@ -2231,10 +2109,8 @@ bool DWARFExpression::Evaluate( LocationDescriptionKind::Empty); // Reset for the next piece. dwarf4_location_description_kind = Memory; - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 1 item for DW_OP_bit_piece."); - return false; + return llvm::createStringError( + "Expression stack needs at least 1 item for DW_OP_bit_piece."); } else { UpdateValueTypeFromLocationDescription( log, dwarf_cu, dwarf4_location_description_kind, &stack.back()); @@ -2244,30 +2120,26 @@ bool DWARFExpression::Evaluate( const uint64_t piece_bit_offset = opcodes.GetULEB128(&offset); switch (stack.back().GetValueType()) { case Value::ValueType::Invalid: - return false; + return llvm::createStringError( + "unable to extract bit value from invalid value."); case Value::ValueType::Scalar: { if (!stack.back().GetScalar().ExtractBitfield(piece_bit_size, piece_bit_offset)) { - if (error_ptr) - error_ptr->SetErrorStringWithFormat( - "unable to extract %" PRIu64 " bit value with %" PRIu64 - " bit offset from a %" PRIu64 " bit scalar value.", - piece_bit_size, piece_bit_offset, - (uint64_t)(stack.back().GetScalar().GetByteSize() * 8)); - return false; + return llvm::createStringError( + "unable to extract %" PRIu64 " bit value with %" PRIu64 + " bit offset from a %" PRIu64 " bit scalar value.", + piece_bit_size, piece_bit_offset, + (uint64_t)(stack.back().GetScalar().GetByteSize() * 8)); } } break; case Value::ValueType::FileAddress: case Value::ValueType::LoadAddress: case Value::ValueType::HostAddress: - if (error_ptr) { - error_ptr->SetErrorStringWithFormat( - "unable to extract DW_OP_bit_piece(bit_size = %" PRIu64 - ", bit_offset = %" PRIu64 ") from an address value.", - piece_bit_size, piece_bit_offset); - } - return false; + return llvm::createStringError( + "unable to extract DW_OP_bit_piece(bit_size = %" PRIu64 + ", bit_offset = %" PRIu64 ") from an address value.", + piece_bit_size, piece_bit_offset); } } break; @@ -2287,9 +2159,8 @@ bool DWARFExpression::Evaluate( if (!data) { LLDB_LOG(log, "Evaluate_DW_OP_implicit_value: could not be read data"); - LLDB_ERRORF(error_ptr, "Could not evaluate %s.", - DW_OP_value_to_name(op)); - return false; + return llvm::createStringError("Could not evaluate %s.", + DW_OP_value_to_name(op)); } Value result(data, len); @@ -2299,8 +2170,8 @@ bool DWARFExpression::Evaluate( case DW_OP_implicit_pointer: { dwarf4_location_description_kind = Implicit; - LLDB_ERRORF(error_ptr, "Could not evaluate %s.", DW_OP_value_to_name(op)); - return false; + return llvm::createStringError("Could not evaluate %s.", + DW_OP_value_to_name(op)); } // OPCODE: DW_OP_push_object_address @@ -2315,10 +2186,8 @@ bool DWARFExpression::Evaluate( if (object_address_ptr) stack.push_back(*object_address_ptr); else { - if (error_ptr) - error_ptr->SetErrorString("DW_OP_push_object_address used without " - "specifying an object address"); - return false; + return llvm::createStringError("DW_OP_push_object_address used without " + "specifying an object address"); } break; @@ -2341,9 +2210,7 @@ bool DWARFExpression::Evaluate( // the stack by the called expression may be used as return values by prior // agreement between the calling and called expressions. case DW_OP_call2: - if (error_ptr) - error_ptr->SetErrorString("Unimplemented opcode DW_OP_call2."); - return false; + return llvm::createStringError("Unimplemented opcode DW_OP_call2."); // OPCODE: DW_OP_call4 // OPERANDS: 1 // uint32_t compile unit relative offset of a DIE @@ -2364,9 +2231,8 @@ bool DWARFExpression::Evaluate( // the stack by the called expression may be used as return values by prior // agreement between the calling and called expressions. case DW_OP_call4: - if (error_ptr) - error_ptr->SetErrorString("Unimplemented opcode DW_OP_call4."); - return false; + + return llvm::createStringError("Unimplemented opcode DW_OP_call4."); // OPCODE: DW_OP_stack_value // OPERANDS: None @@ -2376,10 +2242,8 @@ bool DWARFExpression::Evaluate( case DW_OP_stack_value: dwarf4_location_description_kind = Implicit; if (stack.empty()) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 1 item for DW_OP_stack_value."); - return false; + return llvm::createStringError( + "Expression stack needs at least 1 item for DW_OP_stack_value."); } stack.back().SetValueType(Value::ValueType::Scalar); break; @@ -2393,10 +2257,8 @@ bool DWARFExpression::Evaluate( // different type, and push the result. case DW_OP_convert: { if (stack.size() < 1) { - if (error_ptr) - error_ptr->SetErrorString( - "Expression stack needs at least 1 item for DW_OP_convert."); - return false; + return llvm::createStringError( + "Expression stack needs at least 1 item for DW_OP_convert."); } const uint64_t die_offset = opcodes.GetULEB128(&offset); uint64_t bit_size; @@ -2405,39 +2267,29 @@ bool DWARFExpression::Evaluate( // The generic type has the size of an address on the target // machine and an unspecified signedness. Scalar has no // "unspecified signedness", so we use unsigned types. - if (!module_sp) { - if (error_ptr) - error_ptr->SetErrorString("No module"); - return false; - } + if (!module_sp) + return llvm::createStringError("No module"); sign = false; bit_size = module_sp->GetArchitecture().GetAddressByteSize() * 8; - if (!bit_size) { - if (error_ptr) - error_ptr->SetErrorString("unspecified architecture"); - return false; - } + if (!bit_size) + return llvm::createStringError("unspecified architecture"); } else { // Retrieve the type DIE that the value is being converted to. This // offset is compile unit relative so we need to fix it up. const uint64_t abs_die_offset = die_offset + dwarf_cu->GetOffset(); // FIXME: the constness has annoying ripple effects. DWARFDIE die = const_cast<DWARFUnit *>(dwarf_cu)->GetDIE(abs_die_offset); - if (!die) { - if (error_ptr) - error_ptr->SetErrorString("Cannot resolve DW_OP_convert type DIE"); - return false; - } + if (!die) + return llvm::createStringError( + "Cannot resolve DW_OP_convert type DIE"); uint64_t encoding = die.GetAttributeValueAsUnsigned(DW_AT_encoding, DW_ATE_hi_user); bit_size = die.GetAttributeValueAsUnsigned(DW_AT_byte_size, 0) * 8; if (!bit_size) bit_size = die.GetAttributeValueAsUnsigned(DW_AT_bit_size, 0); - if (!bit_size) { - if (error_ptr) - error_ptr->SetErrorString("Unsupported type size in DW_OP_convert"); - return false; - } + if (!bit_size) + return llvm::createStringError( + "Unsupported type size in DW_OP_convert"); switch (encoding) { case DW_ATE_signed: case DW_ATE_signed_char: @@ -2448,9 +2300,8 @@ bool DWARFExpression::Evaluate( sign = false; break; default: - if (error_ptr) - error_ptr->SetErrorString("Unsupported encoding in DW_OP_convert"); - return false; + return llvm::createStringError( + "Unsupported encoding in DW_OP_convert"); } } Scalar &top = stack.back().ResolveValue(exe_ctx); @@ -2472,15 +2323,15 @@ bool DWARFExpression::Evaluate( if (cfa != LLDB_INVALID_ADDRESS) { stack.push_back(Scalar(cfa)); stack.back().SetValueType(Value::ValueType::LoadAddress); - } else if (error_ptr) - error_ptr->SetErrorString("Stack frame does not include a canonical " - "frame address for DW_OP_call_frame_cfa " - "opcode."); + } else { + return llvm::createStringError( + "Stack frame does not include a canonical " + "frame address for DW_OP_call_frame_cfa " + "opcode."); + } } else { - if (error_ptr) - error_ptr->SetErrorString("Invalid stack frame in context for " - "DW_OP_call_frame_cfa opcode."); - return false; + return llvm::createStringError("Invalid stack frame in context for " + "DW_OP_call_frame_cfa opcode."); } break; @@ -2493,29 +2344,20 @@ bool DWARFExpression::Evaluate( case DW_OP_form_tls_address: case DW_OP_GNU_push_tls_address: { if (stack.size() < 1) { - if (error_ptr) { - if (op == DW_OP_form_tls_address) - error_ptr->SetErrorString( - "DW_OP_form_tls_address needs an argument."); - else - error_ptr->SetErrorString( - "DW_OP_GNU_push_tls_address needs an argument."); - } - return false; + if (op == DW_OP_form_tls_address) + return llvm::createStringError( + "DW_OP_form_tls_address needs an argument."); + else + return llvm::createStringError( + "DW_OP_GNU_push_tls_address needs an argument."); } - if (!exe_ctx || !module_sp) { - if (error_ptr) - error_ptr->SetErrorString("No context to evaluate TLS within."); - return false; - } + if (!exe_ctx || !module_sp) + return llvm::createStringError("No context to evaluate TLS within."); Thread *thread = exe_ctx->GetThreadPtr(); - if (!thread) { - if (error_ptr) - error_ptr->SetErrorString("No thread to evaluate TLS within."); - return false; - } + if (!thread) + return llvm::createStringError("No thread to evaluate TLS within."); // Lookup the TLS block address for this thread and module. const addr_t tls_file_addr = @@ -2523,12 +2365,9 @@ bool DWARFExpression::Evaluate( const addr_t tls_load_addr = thread->GetThreadLocalData(module_sp, tls_file_addr); - if (tls_load_addr == LLDB_INVALID_ADDRESS) { - if (error_ptr) - error_ptr->SetErrorString( - "No TLS data currently exists for this thread."); - return false; - } + if (tls_load_addr == LLDB_INVALID_ADDRESS) + return llvm::createStringError( + "No TLS data currently exists for this thread."); stack.back().GetScalar() = tls_load_addr; stack.back().SetValueType(Value::ValueType::LoadAddress); @@ -2542,12 +2381,9 @@ bool DWARFExpression::Evaluate( // and the 0 based index is the ULEB128 encoded index. case DW_OP_addrx: case DW_OP_GNU_addr_index: { - if (!dwarf_cu) { - if (error_ptr) - error_ptr->SetErrorString("DW_OP_GNU_addr_index found without a " - "compile unit being specified"); - return false; - } + if (!dwarf_cu) + return llvm::createStringError("DW_OP_GNU_addr_index found without a " + "compile unit being specified"); uint64_t index = opcodes.GetULEB128(&offset); lldb::addr_t value = dwarf_cu->ReadAddressFromDebugAddrSection(index); stack.push_back(Scalar(value)); @@ -2570,10 +2406,8 @@ bool DWARFExpression::Evaluate( // encoded index. case DW_OP_GNU_const_index: { if (!dwarf_cu) { - if (error_ptr) - error_ptr->SetErrorString("DW_OP_GNU_const_index found without a " - "compile unit being specified"); - return false; + return llvm::createStringError("DW_OP_GNU_const_index found without a " + "compile unit being specified"); } uint64_t index = opcodes.GetULEB128(&offset); lldb::addr_t value = dwarf_cu->ReadAddressFromDebugAddrSection(index); @@ -2582,12 +2416,9 @@ bool DWARFExpression::Evaluate( case DW_OP_GNU_entry_value: case DW_OP_entry_value: { - if (!Evaluate_DW_OP_entry_value(stack, exe_ctx, reg_ctx, opcodes, offset, - error_ptr, log)) { - LLDB_ERRORF(error_ptr, "Could not evaluate %s.", - DW_OP_value_to_name(op)); - return false; - } + if (llvm::Error err = Evaluate_DW_OP_entry_value(stack, exe_ctx, reg_ctx, + opcodes, offset, log)) + return err; break; } @@ -2598,23 +2429,18 @@ bool DWARFExpression::Evaluate( break; } } - if (error_ptr) - error_ptr->SetErrorStringWithFormatv( - "Unhandled opcode {0} in DWARFExpression", LocationAtom(op)); - return false; + return llvm::createStringError(llvm::formatv( + "Unhandled opcode {0} in DWARFExpression", LocationAtom(op))); } } if (stack.empty()) { // Nothing on the stack, check if we created a piece value from DW_OP_piece // or DW_OP_bit_piece opcodes - if (pieces.GetBuffer().GetByteSize()) { - result = pieces; - return true; - } - if (error_ptr) - error_ptr->SetErrorString("Stack empty after evaluation."); - return false; + if (pieces.GetBuffer().GetByteSize()) + return pieces; + + return llvm::createStringError("Stack empty after evaluation."); } UpdateValueTypeFromLocationDescription( @@ -2631,8 +2457,7 @@ bool DWARFExpression::Evaluate( LLDB_LOGF(log, " %s", new_value.GetData()); } } - result = stack.back(); - return true; // Return true on success + return stack.back(); } bool DWARFExpression::ParseDWARFLocationList( diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index cba4e4e5858ac..7a5cf9f9a0be4 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -198,12 +198,10 @@ void DWARFExpressionList::GetDescription(Stream *s, } } -bool DWARFExpressionList::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> DWARFExpressionList::Evaluate( + ExecutionContext *exe_ctx, RegisterContext *reg_ctx, + lldb::addr_t func_load_addr, const Value *initial_value_ptr, + const Value *object_address_ptr) const { ModuleSP module_sp = m_module_wp.lock(); DataExtractor data; RegisterKind reg_kind; @@ -217,32 +215,26 @@ bool DWARFExpressionList::Evaluate(ExecutionContext *exe_ctx, if (exe_ctx) frame = exe_ctx->GetFramePtr(); if (!frame) - return false; + return llvm::createStringError("no frame"); RegisterContextSP reg_ctx_sp = frame->GetRegisterContext(); if (!reg_ctx_sp) - return false; + return llvm::createStringError("no register context"); reg_ctx_sp->GetPCForSymbolication(pc); } if (!pc.IsValid()) { - if (error_ptr) - error_ptr->SetErrorString("Invalid PC in frame."); - return false; + return llvm::createStringError("Invalid PC in frame."); } addr_t pc_load_addr = pc.GetLoadAddress(exe_ctx->GetTargetPtr()); const DWARFExpression *entry = GetExpressionAtAddress(func_load_addr, pc_load_addr); - if (!entry) { - if (error_ptr) { - error_ptr->SetErrorString("variable not available"); - } - return false; - } + if (!entry) + return llvm::createStringError("variable not available"); expr = *entry; } expr.GetExpressionData(data); reg_kind = expr.GetRegisterKind(); return DWARFExpression::Evaluate(exe_ctx, reg_ctx, module_sp, data, m_dwarf_cu, reg_kind, initial_value_ptr, - object_address_ptr, result, error_ptr); + object_address_ptr); } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index dc4cfc96b86f0..f0e1e0560aa9a 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -572,6 +572,8 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc, static std::optional<uint32_t> ExtractDataMemberLocation(DWARFDIE const &die, DWARFFormValue const &form_value, ModuleSP module_sp) { + Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); + // With DWARF 3 and later, if the value is an integer constant, // this form value is the offset in bytes from the beginning of // the containing entity. @@ -579,21 +581,23 @@ ExtractDataMemberLocation(DWARFDIE const &die, DWARFFormValue const &form_value, return form_value.Unsigned(); Value initialValue(0); - Value memberOffset(0); const DWARFDataExtractor &debug_info_data = die.GetData(); uint32_t block_length = form_value.Unsigned(); uint32_t block_offset = form_value.BlockData() - debug_info_data.GetDataStart(); - if (!DWARFExpression::Evaluate( - nullptr, // ExecutionContext * - nullptr, // RegisterContext * - module_sp, DataExtractor(debug_info_data, block_offset, block_length), - die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr, memberOffset, - nullptr)) { + + llvm::Expected<Value> memberOffset = DWARFExpression::Evaluate( + nullptr, // ExecutionContext * + nullptr, // RegisterContext * + module_sp, DataExtractor(debug_info_data, block_offset, block_length), + die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr); + if (!memberOffset) { + LLDB_LOG_ERROR(log, memberOffset.takeError(), + "ExtractDataMemberLocation failed: {0}"); return {}; } - return memberOffset.ResolveValue(nullptr).UInt(); + return memberOffset->ResolveValue(nullptr).UInt(); } static TypePayloadClang GetPtrAuthMofidierPayload(const DWARFDIE &die) { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 661e4a78a0215..f4556925f5d68 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2105,22 +2105,24 @@ SymbolFileDWARF::GlobalVariableMap &SymbolFileDWARF::GetGlobalAranges() { if (var_sp && !var_sp->GetLocationIsConstantValueData()) { const DWARFExpressionList &location = var_sp->LocationExpressionList(); - Value location_result; - Status error; ExecutionContext exe_ctx; - if (location.Evaluate(&exe_ctx, nullptr, LLDB_INVALID_ADDRESS, - nullptr, nullptr, location_result, - &error)) { - if (location_result.GetValueType() == + llvm::Expected<Value> location_result = location.Evaluate( + &exe_ctx, nullptr, LLDB_INVALID_ADDRESS, nullptr, nullptr); + if (location_result) { + if (location_result->GetValueType() == Value::ValueType::FileAddress) { lldb::addr_t file_addr = - location_result.GetScalar().ULongLong(); + location_result->GetScalar().ULongLong(); lldb::addr_t byte_size = 1; if (var_sp->GetType()) byte_size = var_sp->GetType()->GetByteSize(nullptr).value_or(0); m_global_aranges_up->Append(GlobalVariableMap::Entry( file_addr, byte_size, var_sp.get())); + } else { + LLDB_LOG_ERROR( + GetLog(LLDBLog::Symbols), location_result.takeError(), + "location expression failed to execute: {0}"); } } } diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp index 194f89bc51d80..96d8322b43d84 100644 --- a/lldb/source/Symbol/Function.cpp +++ b/lldb/source/Symbol/Function.cpp @@ -220,17 +220,18 @@ Function *IndirectCallEdge::GetCallee(ModuleList &images, ExecutionContext &exe_ctx) { Log *log = GetLog(LLDBLog::Step); Status error; - Value callee_addr_val; - if (!call_target.Evaluate( - &exe_ctx, exe_ctx.GetRegisterContext(), LLDB_INVALID_ADDRESS, - /*initial_value_ptr=*/nullptr, - /*object_address_ptr=*/nullptr, callee_addr_val, &error)) { - LLDB_LOGF(log, "IndirectCallEdge: Could not evaluate expression: %s", - error.AsCString()); + llvm::Expected<Value> callee_addr_val = call_target.Evaluate( + &exe_ctx, exe_ctx.GetRegisterContext(), LLDB_INVALID_ADDRESS, + /*initial_value_ptr=*/nullptr, + /*object_address_ptr=*/nullptr); + if (!callee_addr_val) { + LLDB_LOG_ERROR(log, callee_addr_val.takeError(), + "IndirectCallEdge: Could not evaluate expression: {0}"); return nullptr; } - addr_t raw_addr = callee_addr_val.GetScalar().ULongLong(LLDB_INVALID_ADDRESS); + addr_t raw_addr = + callee_addr_val->GetScalar().ULongLong(LLDB_INVALID_ADDRESS); if (raw_addr == LLDB_INVALID_ADDRESS) { LLDB_LOG(log, "IndirectCallEdge: Could not extract address from scalar"); return nullptr; diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp index e2d712cb72eae..95e8abd763d53 100644 --- a/lldb/source/Target/RegisterContextUnwind.cpp +++ b/lldb/source/Target/RegisterContextUnwind.cpp @@ -1661,12 +1661,14 @@ RegisterContextUnwind::SavedLocationForRegister( unwindplan_registerkind); Value cfa_val = Scalar(m_cfa); cfa_val.SetValueType(Value::ValueType::LoadAddress); - Value result; - Status error; - if (dwarfexpr.Evaluate(&exe_ctx, this, 0, &cfa_val, nullptr, result, - &error)) { + llvm::Expected<Value> result = + dwarfexpr.Evaluate(&exe_ctx, this, 0, &cfa_val, nullptr); + if (!result) { + LLDB_LOG_ERROR(log, result.takeError(), + "DWARF expression failed to evaluate: {0}"); + } else { addr_t val; - val = result.GetScalar().ULongLong(); + val = result->GetScalar().ULongLong(); if (unwindplan_regloc.IsDWARFExpression()) { regloc.type = UnwindLLDB::RegisterLocation::eRegisterValueInferred; regloc.location.inferred_value = val; @@ -2029,11 +2031,10 @@ bool RegisterContextUnwind::ReadFrameAddress( DWARFExpressionList dwarfexpr(opcode_ctx, dwarfdata, nullptr); dwarfexpr.GetMutableExpressionAtAddress()->SetRegisterKind( row_register_kind); - Value result; - Status error; - if (dwarfexpr.Evaluate(&exe_ctx, this, 0, nullptr, nullptr, result, - &error)) { - address = result.GetScalar().ULongLong(); + llvm::Expected<Value> result = + dwarfexpr.Evaluate(&exe_ctx, this, 0, nullptr, nullptr); + if (result) { + address = result->GetScalar().ULongLong(); if (ABISP abi_sp = m_thread.GetProcess()->GetABI()) address = abi_sp->FixCodeAddress(address); @@ -2042,7 +2043,7 @@ bool RegisterContextUnwind::ReadFrameAddress( return true; } UnwindLogMsg("Failed to set CFA value via DWARF expression: %s", - error.AsCString()); + llvm::toString(result.takeError()).c_str()); break; } case UnwindPlan::Row::FAValue::isRaSearch: { diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index 246871d5abaa5..3a2b4d05b2881 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -1091,24 +1091,19 @@ bool StackFrame::GetFrameBaseValue(Scalar &frame_base, Status *error_ptr) { m_flags.Set(GOT_FRAME_BASE); ExecutionContext exe_ctx(shared_from_this()); - Value expr_value; addr_t loclist_base_addr = LLDB_INVALID_ADDRESS; if (!m_sc.function->GetFrameBaseExpression().IsAlwaysValidSingleExpr()) loclist_base_addr = m_sc.function->GetAddressRange().GetBaseAddress().GetLoadAddress( exe_ctx.GetTargetPtr()); - if (!m_sc.function->GetFrameBaseExpression().Evaluate( - &exe_ctx, nullptr, loclist_base_addr, nullptr, nullptr, - expr_value, &m_frame_base_error)) { - // We should really have an error if evaluate returns, but in case we - // don't, lets set the error to something at least. - if (m_frame_base_error.Success()) - m_frame_base_error.SetErrorString( - "Evaluation of the frame base expression failed."); - } else { - m_frame_base = expr_value.ResolveValue(&exe_ctx); - } + llvm::Expected<Value> expr_value = + m_sc.function->GetFrameBaseExpression().Evaluate( + &exe_ctx, nullptr, loclist_base_addr, nullptr, nullptr); + if (!expr_value) + m_frame_base_error = expr_value.takeError(); + else + m_frame_base = expr_value->ResolveValue(&exe_ctx); } else { m_frame_base_error.SetErrorString("No function in symbol context."); } diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp index 602bd19ceecf8..f9e0605fce29d 100644 --- a/lldb/unittests/Expression/DWARFExpressionTest.cpp +++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp @@ -33,23 +33,23 @@ static llvm::Expected<Scalar> Evaluate(llvm::ArrayRef<uint8_t> expr, ExecutionContext *exe_ctx = nullptr) { DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle, /*addr_size*/ 4); - Value result; - Status status; - if (!DWARFExpression::Evaluate(exe_ctx, /*reg_ctx*/ nullptr, module_sp, - extractor, unit, lldb::eRegisterKindLLDB, - /*initial_value_ptr*/ nullptr, - /*object_address_ptr*/ nullptr, result, - &status)) - return status.ToError(); - - switch (result.GetValueType()) { + + llvm::Expected<Value> result = + DWARFExpression::Evaluate(exe_ctx, /*reg_ctx*/ nullptr, module_sp, + extractor, unit, lldb::eRegisterKindLLDB, + /*initial_value_ptr*/ nullptr, + /*object_address_ptr*/ nullptr); + if (!result) + return result.takeError(); + + switch (result->GetValueType()) { case Value::ValueType::Scalar: - return result.GetScalar(); + return result->GetScalar(); case Value::ValueType::LoadAddress: return LLDB_INVALID_ADDRESS; case Value::ValueType::HostAddress: { // Convert small buffers to scalars to simplify the tests. - DataBufferHeap &buf = result.GetBuffer(); + DataBufferHeap &buf = result->GetBuffer(); if (buf.GetByteSize() <= 8) { uint64_t val = 0; memcpy(&val, buf.GetBytes(), buf.GetByteSize()); @@ -58,8 +58,9 @@ static llvm::Expected<Scalar> Evaluate(llvm::ArrayRef<uint8_t> expr, } [[fallthrough]]; default: - return status.ToError(); + break; } + return llvm::createStringError("unsupported value type"); } class DWARFExpressionTester : public YAMLModuleTester { @@ -454,16 +455,15 @@ TEST_F(DWARFExpressionMockProcessTest, WASM_DW_OP_addr) { uint8_t expr[] = {DW_OP_addr, 0x40, 0x0, 0x0, 0x0}; DataExtractor extractor(expr, sizeof(expr), lldb::eByteOrderLittle, /*addr_size*/ 4); - Value result; - Status status; - ASSERT_TRUE(DWARFExpression::Evaluate( + + llvm::Expected<Value> result = DWARFExpression::Evaluate( &exe_ctx, /*reg_ctx*/ nullptr, /*module_sp*/ {}, extractor, /*unit*/ nullptr, lldb::eRegisterKindLLDB, /*initial_value_ptr*/ nullptr, - /*object_address_ptr*/ nullptr, result, &status)) - << status.ToError(); + /*object_address_ptr*/ nullptr); - ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress); + ASSERT_THAT_EXPECTED(result, llvm::Succeeded()); + ASSERT_EQ(result->GetValueType(), Value::ValueType::LoadAddress); } TEST_F(DWARFExpressionMockProcessTest, WASM_DW_OP_addr_index) { @@ -530,14 +530,14 @@ TEST_F(DWARFExpressionMockProcessTest, WASM_DW_OP_addr_index) { ExecutionContext exe_ctx(target_sp, false); - auto evaluate = [&](DWARFExpression &expr, Status &status, Value &result) { + auto evaluate = [&](DWARFExpression &expr) -> llvm::Expected<Value> { DataExtractor extractor; expr.GetExpressionData(extractor); - return DWARFExpression::Evaluate( - &exe_ctx, /*reg_ctx*/ nullptr, /*module_sp*/ {}, extractor, dwarf_cu, - lldb::eRegisterKindLLDB, - /*initial_value_ptr*/ nullptr, - /*object_address_ptr*/ nullptr, result, &status); + return DWARFExpression::Evaluate(&exe_ctx, /*reg_ctx*/ nullptr, + /*module_sp*/ {}, extractor, dwarf_cu, + lldb::eRegisterKindLLDB, + /*initial_value_ptr*/ nullptr, + /*object_address_ptr*/ nullptr); }; // DW_OP_addrx takes a single leb128 operand, the index in the addr table: @@ -546,16 +546,16 @@ TEST_F(DWARFExpressionMockProcessTest, WASM_DW_OP_addr_index) { /*addr_size*/ 4); DWARFExpression expr(extractor); - Status status; - Value result; - ASSERT_TRUE(evaluate(expr, status, result)) << status.ToError(); - ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress); - ASSERT_EQ(result.GetScalar().UInt(), 0x5678u); + llvm::Expected<Value> result = evaluate(expr); + ASSERT_THAT_EXPECTED(result, llvm::Succeeded()); + ASSERT_EQ(result->GetValueType(), Value::ValueType::LoadAddress); + ASSERT_EQ(result->GetScalar().UInt(), 0x5678u); ASSERT_TRUE(expr.Update_DW_OP_addr(dwarf_cu, 0xdeadbeef)); - ASSERT_TRUE(evaluate(expr, status, result)) << status.ToError(); - ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress); - ASSERT_EQ(result.GetScalar().UInt(), 0xdeadbeefu); + result = evaluate(expr); + ASSERT_THAT_EXPECTED(result, llvm::Succeeded()); + ASSERT_EQ(result->GetValueType(), Value::ValueType::LoadAddress); + ASSERT_EQ(result->GetScalar().UInt(), 0xdeadbeefu); } class CustomSymbolFileDWARF : public SymbolFileDWARF { @@ -825,15 +825,13 @@ TEST_F(DWARFExpressionMockProcessTest, DW_OP_piece_file_addr) { DW_OP_addr, 0x50, 0x0, 0x0, 0x0, DW_OP_piece, 1}; DataExtractor extractor(expr, sizeof(expr), lldb::eByteOrderLittle, /*addr_size*/ 4); - Value result; - Status status; - ASSERT_TRUE(DWARFExpression::Evaluate( + llvm::Expected<Value> result = DWARFExpression::Evaluate( &exe_ctx, /*reg_ctx*/ nullptr, /*module_sp*/ {}, extractor, /*unit*/ nullptr, lldb::eRegisterKindLLDB, /*initial_value_ptr*/ nullptr, - /*object_address_ptr*/ nullptr, result, &status)) - << status.ToError(); + /*object_address_ptr*/ nullptr); - ASSERT_EQ(result.GetValueType(), Value::ValueType::HostAddress); - ASSERT_THAT(result.GetBuffer().GetData(), ElementsAre(0x11, 0x22)); + ASSERT_THAT_EXPECTED(result, llvm::Succeeded()); + ASSERT_EQ(result->GetValueType(), Value::ValueType::HostAddress); + ASSERT_THAT(result->GetBuffer().GetData(), ElementsAre(0x11, 0x22)); } >From b6e28ab5747c277429fb17dafc331bd87d34569b Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Tue, 4 Jun 2024 19:28:40 -0700 Subject: [PATCH 3/5] [lldb] Move LLDB_LOG_ERROR to correct else block --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index f4556925f5d68..af3ba2cd5b39d 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2119,11 +2119,11 @@ SymbolFileDWARF::GlobalVariableMap &SymbolFileDWARF::GetGlobalAranges() { var_sp->GetType()->GetByteSize(nullptr).value_or(0); m_global_aranges_up->Append(GlobalVariableMap::Entry( file_addr, byte_size, var_sp.get())); - } else { - LLDB_LOG_ERROR( - GetLog(LLDBLog::Symbols), location_result.takeError(), - "location expression failed to execute: {0}"); } + } else { + LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), + location_result.takeError(), + "location expression failed to execute: {0}"); } } } >From aa51bea351e0fdcf1626ca545fa8c4523d9b5f8a Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Tue, 4 Jun 2024 19:48:26 -0700 Subject: [PATCH 4/5] [lldb] Improve DW_OP_entry_value error reporting --- lldb/source/Expression/DWARFExpression.cpp | 44 +++++++------------ .../basic_entry_values/main.cpp | 7 +-- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 5c7ab49410efe..80d03c84fadbd 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -593,15 +593,13 @@ static llvm::Error 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) { - return llvm::createStringError( - "Evaluate_DW_OP_entry_value: no exe/reg context"); + return llvm::createStringError("no exe/reg context"); } StackFrame *current_frame = exe_ctx->GetFramePtr(); Thread *thread = exe_ctx->GetThreadPtr(); if (!current_frame || !thread) - return llvm::createStringError( - "Evaluate_DW_OP_entry_value: no current frame/thread"); + return llvm::createStringError("no current frame/thread"); Target &target = exe_ctx->GetTargetRef(); StackFrameSP parent_frame = nullptr; @@ -619,9 +617,7 @@ static llvm::Error Evaluate_DW_OP_entry_value(std::vector<Value> &stack, // parent frame. if (return_pc == LLDB_INVALID_ADDRESS) { return_pc = parent_frame->GetFrameCodeAddress().GetLoadAddress(&target); - LLDB_LOG(log, - "Evaluate_DW_OP_entry_value: immediate ancestor with pc = {0:x}", - return_pc); + LLDB_LOG(log, "immediate ancestor with pc = {0:x}", return_pc); } // If we've found an inlined frame, skip it (these have no call site @@ -633,23 +629,20 @@ static llvm::Error Evaluate_DW_OP_entry_value(std::vector<Value> &stack, break; } if (!parent_frame || !parent_frame->GetRegisterContext()) { - return llvm::createStringError( - "Evaluate_DW_OP_entry_value: no parent frame with reg ctx"); + return llvm::createStringError("no parent frame with reg ctx"); } Function *parent_func = parent_frame->GetSymbolContext(eSymbolContextFunction).function; if (!parent_func) - return llvm::createStringError( - "Evaluate_DW_OP_entry_value: no parent function"); + return llvm::createStringError("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) - return llvm::createStringError( - "Evaluate_DW_OP_entry_value: no current function"); + return llvm::createStringError("no current function"); CallEdge *call_edge = nullptr; ModuleList &modlist = target.GetImages(); @@ -660,16 +653,14 @@ static llvm::Error 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) { - 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())); + return llvm::createStringError( + llvm::formatv("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) { return llvm::createStringError( - "Evaluate_DW_OP_entry_value: ambiguous call sequence, " - "can't find real parent frame"); + "ambiguous call sequence, can't find real parent frame"); } } else { // The StackFrameList solver machinery has deduced that an unambiguous tail @@ -683,9 +674,8 @@ static llvm::Error Evaluate_DW_OP_entry_value(std::vector<Value> &stack, } } if (!call_edge) - return llvm::createStringError( - "Evaluate_DW_OP_entry_value: no unambiguous edge from parent " - "to current function"); + return llvm::createStringError("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 @@ -693,8 +683,7 @@ static llvm::Error Evaluate_DW_OP_entry_value(std::vector<Value> &stack, const uint32_t subexpr_len = opcodes.GetULEB128(&opcode_offset); const void *subexpr_data = opcodes.GetData(&opcode_offset, subexpr_len); if (!subexpr_data) - return llvm::createStringError( - "Evaluate_DW_OP_entry_value: subexpr could not be read"); + return llvm::createStringError("subexpr could not be read"); const CallSiteParameter *matched_param = nullptr; for (const CallSiteParameter ¶m : call_edge->GetCallSiteParameters()) { @@ -721,8 +710,7 @@ static llvm::Error Evaluate_DW_OP_entry_value(std::vector<Value> &stack, } } if (!matched_param) - return llvm::createStringError( - "Evaluate_DW_OP_entry_value: no matching call site param found"); + return llvm::createStringError("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. @@ -2418,7 +2406,9 @@ llvm::Expected<Value> DWARFExpression::Evaluate( case DW_OP_entry_value: { if (llvm::Error err = Evaluate_DW_OP_entry_value(stack, exe_ctx, reg_ctx, opcodes, offset, log)) - return err; + return llvm::createStringError( + "could not evaluate DW_OP_entry_value: %s", + llvm::toString(std::move(err)).c_str()); break; } diff --git a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp index 91769e8768017..2b28d0357e0cc 100644 --- a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp +++ b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp @@ -52,9 +52,10 @@ __attribute__((noinline)) void func4_amb(int &sink, int x) { //% expect_cmd_failure=True) //% self.filecheck("expr sink", "main.cpp","-check-prefix=FUNC4-EXPR", //% expect_cmd_failure=True) - // FUNC4-EXPR-FAIL: couldn't get the value of variable x: Could not evaluate - // DW_OP_entry_value. FUNC4-EXPR: couldn't get the value of variable sink: - // Could not evaluate DW_OP_entry_value. + // FUNC4-EXPR-FAIL: couldn't get the value of variable x: could not evaluate + // DW_OP_entry_value: no matching call site param found FUNC4-EXPR: couldn't + // get the value of variable sink: could not evaluate DW_OP_entry_value: no + // matching call site param found } __attribute__((noinline)) void func5_amb() {} >From 9b69aeec6207dac23c402d4dc074056883d1f16e Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Wed, 5 Jun 2024 09:30:47 -0700 Subject: [PATCH 5/5] Use inline comments --- .../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index f0e1e0560aa9a..7d7e835c3d732 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -587,10 +587,10 @@ ExtractDataMemberLocation(DWARFDIE const &die, DWARFFormValue const &form_value, form_value.BlockData() - debug_info_data.GetDataStart(); llvm::Expected<Value> memberOffset = DWARFExpression::Evaluate( - nullptr, // ExecutionContext * - nullptr, // RegisterContext * - module_sp, DataExtractor(debug_info_data, block_offset, block_length), - die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr); + /*ExecutionContext=*/nullptr, + /*RegisterContext=*/nullptr, module_sp, + DataExtractor(debug_info_data, block_offset, block_length), die.GetCU(), + eRegisterKindDWARF, &initialValue, nullptr); if (!memberOffset) { LLDB_LOG_ERROR(log, memberOffset.takeError(), "ExtractDataMemberLocation failed: {0}"); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits