[Lldb-commits] [lldb] 69b0b7e - [lldb] Return an llvm::Error from GetFrameBaseValue (#111882)

[via lldb-commits]

Author: Jonas Devlieghere
Date: 2024-10-10T13:11:08-07:00
New Revision: 69b0b7e7ac3adc42df517c25ed7017b5af9be9f1

URL: https://github.com/llvm/llvm-project/commit/69b0b7e7ac3adc42df517c25ed7017b5af9be9f1
DIFF: https://github.com/llvm/llvm-project/commit/69b0b7e7ac3adc42df517c25ed7017b5af9be9f1.diff

LOG: [lldb] Return an llvm::Error from GetFrameBaseValue (#111882)

This fixes the following assertion: "Cannot create Expected<T> from
Error success value." The problem was that GetFrameBaseValue return
false without updating the Status argument. This patch eliminates the 
opportunity for mistakes by returning an llvm:Error.

Added: 
    

Modified: 
    lldb/include/lldb/Target/StackFrame.h
    lldb/source/Expression/DWARFExpression.cpp
    lldb/source/Target/StackFrame.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h
index 5cc0fccee03b8f..fdbe1f567eabfa 100644
--- a/lldb/include/lldb/Target/StackFrame.h
+++ b/lldb/include/lldb/Target/StackFrame.h
@@ -195,14 +195,10 @@ class StackFrame : public ExecutionContextScope,
   /// \param [out] value
   ///   The address of the CFA for this frame, if available.
   ///
-  /// \param [out] error_ptr
-  ///   If there is an error determining the CFA address, this may contain a
-  ///   string explaining the failure.
-  ///
   /// \return
-  ///   Returns true if the CFA value was successfully set in value.  Some
-  ///   frames may be unable to provide this value; they will return false.
-  bool GetFrameBaseValue(Scalar &value, Status *error_ptr);
+  ///   If there is an error determining the CFA address, return an error
+  ///   explaining the failure. Success otherwise.
+  llvm::Error GetFrameBaseValue(Scalar &value);
 
   /// Get the DWARFExpressionList corresponding to the Canonical Frame Address.
   ///

diff  --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 22d899f799d0fd..97bcd4f7eec26f 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -1780,14 +1780,12 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
       if (exe_ctx) {
         if (frame) {
           Scalar value;
-          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 fb_err.ToError();
+          if (llvm::Error err = frame->GetFrameBaseValue(value))
+            return err;
+          int64_t fbreg_offset = opcodes.GetSLEB128(&offset);
+          value += fbreg_offset;
+          stack.push_back(value);
+          stack.back().SetValueType(Value::ValueType::LoadAddress);
         } else {
           return llvm::createStringError(
               "invalid stack frame in context for DW_OP_fbreg opcode");

diff  --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index fe0d4c93c50627..ed493e35316137 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -1079,12 +1079,12 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
   return valobj_sp;
 }
 
-bool StackFrame::GetFrameBaseValue(Scalar &frame_base, Status *error_ptr) {
+llvm::Error StackFrame::GetFrameBaseValue(Scalar &frame_base) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   if (!m_cfa_is_valid) {
     m_frame_base_error = Status::FromErrorString(
         "No frame base available for this historical stack frame.");
-    return false;
+    return m_frame_base_error.ToError();
   }
 
   if (m_flags.IsClear(GOT_FRAME_BASE)) {
@@ -1113,12 +1113,11 @@ bool StackFrame::GetFrameBaseValue(Scalar &frame_base, Status *error_ptr) {
     }
   }
 
-  if (m_frame_base_error.Success())
-    frame_base = m_frame_base;
+  if (m_frame_base_error.Fail())
+    return m_frame_base_error.ToError();
 
-  if (error_ptr)
-    *error_ptr = m_frame_base_error.Clone();
-  return m_frame_base_error.Success();
+  frame_base = m_frame_base;
+  return llvm::Error::success();
 }
 
 DWARFExpressionList *StackFrame::GetFrameBaseExpression(Status *error_ptr) {