[Lldb-commits] [lldb] [LLDB][NFC] Remove redundant target/process checks in SBFrame (PR #153258)

Felipe de Azevedo Piovezan via lldb-commits lldb-commits at lists.llvm.org
Fri Dec 12 05:07:10 PST 2025


https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/153258

>From 34ae693c7731538ec00274072c4beb3100c43848 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 12 Aug 2025 09:34:46 -0700
Subject: [PATCH 1/3] [LLDB][NFC] Remove redundant target/process checks in
 SBFrame

This is a follow up to https://github.com/llvm/llvm-project/pull/152020,
continuing the removal of now-redundant `if(process && target)` checks.
Since this causes a diff in every line of the affected functions, this
commit also uses the opportunity to create some helper functions and
reduce nesting of the affected methods by rewriting all pre-condition
checks as early returns, while remaining strictly NFC.

This has exposed some odd behaviors:

1. `SBFrame::GetVariables` has a variable `num_produced` which is
   clearly meant to be incremented on every iteration of the loop but it
   is only incremented once, after the loop. So its value is always 0 or
   1. The variable now lives in `FetchVariablesUnlessInterrupted`.
2. `SBFrame::GetVariables` has an interruption mechanism for local
   variables, but not for "recognized arguments". It's unclear if this
   is by design or not, but it is now evident that there is a
   discrepancy there.
3. In `SBFrame::EvaluateExpression` we only log some error paths, but
   not all of them.

To stick to the strictly NFC nature of this patch, it does not address
any of these issues.
---
 lldb/include/lldb/API/SBFrame.h |  19 ++
 lldb/source/API/SBFrame.cpp     | 510 ++++++++++++++++----------------
 2 files changed, 273 insertions(+), 256 deletions(-)

diff --git a/lldb/include/lldb/API/SBFrame.h b/lldb/include/lldb/API/SBFrame.h
index 5283cdfe53faa..59a7343f9eaf3 100644
--- a/lldb/include/lldb/API/SBFrame.h
+++ b/lldb/include/lldb/API/SBFrame.h
@@ -13,6 +13,8 @@
 #include "lldb/API/SBValueList.h"
 
 namespace lldb_private {
+class StackFrame;
+class Debugger;
 namespace python {
 class SWIGBridge;
 }
@@ -241,6 +243,23 @@ class LLDB_API SBFrame {
   /// not currently stopped.
   static SBValue CreateProcessIsRunningExprEvalError();
 
+  enum WasInterrupted { Yes, No };
+
+  /// Populates `value_list` with the variables from `frame` according to
+  /// `options`. This method checks whether the Debugger received an interrupt
+  /// before processing every variable, returning `WasInterrupted::yes` in that
+  /// case.
+  static WasInterrupted FetchVariablesUnlessInterrupted(
+      const lldb::SBVariablesOptions &options, lldb_private::StackFrame &frame,
+      SBValueList &value_list, lldb_private::Debugger &dbg);
+
+  /// Populates `value_list` with recognized arguments of `frame` according to
+  /// `options`.
+  static void FetchRecognizedArguments(const SBVariablesOptions &options,
+                                       lldb_private::StackFrame &frame,
+                                       SBTarget target,
+                                       SBValueList &value_list);
+
   lldb::ExecutionContextRefSP m_opaque_sp;
 };
 
diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index 42dbed490a33d..ed09fd2f2f90d 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -490,99 +490,85 @@ SBValue SBFrame::FindValue(const char *name, ValueType value_type,
   if (!exe_ctx) {
     LLDB_LOG_ERROR(GetLog(LLDBLog::API), exe_ctx.takeError(), "{0}");
     return value_sp;
-  } else {
-    Target *target = exe_ctx->GetTargetPtr();
-    Process *process = exe_ctx->GetProcessPtr();
-    if (target && process) { // FIXME: this check is redundant.
-      if (StackFrame *frame = exe_ctx->GetFramePtr()) {
-        VariableList variable_list;
-
-        switch (value_type) {
-        case eValueTypeVariableGlobal:      // global variable
-        case eValueTypeVariableStatic:      // static variable
-        case eValueTypeVariableArgument:    // function argument variables
-        case eValueTypeVariableLocal:       // function local variables
-        case eValueTypeVariableThreadLocal: // thread local variables
-        {
-          SymbolContext sc(frame->GetSymbolContext(eSymbolContextBlock));
-
-          const bool can_create = true;
-          const bool get_parent_variables = true;
-          const bool stop_if_block_is_inlined_function = true;
-
-          if (sc.block)
-            sc.block->AppendVariables(
-                can_create, get_parent_variables,
-                stop_if_block_is_inlined_function,
-                [frame](Variable *v) { return v->IsInScope(frame); },
-                &variable_list);
-          if (value_type == eValueTypeVariableGlobal 
-              || value_type == eValueTypeVariableStatic) {
-            const bool get_file_globals = true;
-            VariableList *frame_vars = frame->GetVariableList(get_file_globals,
-                                                              nullptr);
-            if (frame_vars)
-              frame_vars->AppendVariablesIfUnique(variable_list);
-          }
-          ConstString const_name(name);
-          VariableSP variable_sp(
-              variable_list.FindVariable(const_name, value_type));
-          if (variable_sp) {
-            value_sp = frame->GetValueObjectForFrameVariable(variable_sp,
-                                                             eNoDynamicValues);
-            sb_value.SetSP(value_sp, use_dynamic);
-          }
-        } break;
-
-        case eValueTypeRegister: // stack frame register value
-        {
-          RegisterContextSP reg_ctx(frame->GetRegisterContext());
-          if (reg_ctx) {
-            if (const RegisterInfo *reg_info =
-                    reg_ctx->GetRegisterInfoByName(name)) {
-              value_sp = ValueObjectRegister::Create(frame, reg_ctx, reg_info);
-              sb_value.SetSP(value_sp);
-            }
-          }
-        } break;
-
-        case eValueTypeRegisterSet: // A collection of stack frame register
-                                    // values
-        {
-          RegisterContextSP reg_ctx(frame->GetRegisterContext());
-          if (reg_ctx) {
-            const uint32_t num_sets = reg_ctx->GetRegisterSetCount();
-            for (uint32_t set_idx = 0; set_idx < num_sets; ++set_idx) {
-              const RegisterSet *reg_set = reg_ctx->GetRegisterSet(set_idx);
-              if (reg_set &&
-                  (llvm::StringRef(reg_set->name).equals_insensitive(name) ||
-                   llvm::StringRef(reg_set->short_name)
-                       .equals_insensitive(name))) {
-                value_sp =
-                    ValueObjectRegisterSet::Create(frame, reg_ctx, set_idx);
-                sb_value.SetSP(value_sp);
-                break;
-              }
-            }
-          }
-        } break;
-
-        case eValueTypeConstResult: // constant result variables
-        {
-          ConstString const_name(name);
-          ExpressionVariableSP expr_var_sp(
-              target->GetPersistentVariable(const_name));
-          if (expr_var_sp) {
-            value_sp = expr_var_sp->GetValueObject();
-            sb_value.SetSP(value_sp, use_dynamic);
-          }
-        } break;
-
-        default:
+  }
+
+  StackFrame *frame = exe_ctx->GetFramePtr();
+  if (!frame)
+    return value_sp;
+
+  VariableList variable_list;
+
+  switch (value_type) {
+  case eValueTypeVariableGlobal:        // global variable
+  case eValueTypeVariableStatic:        // static variable
+  case eValueTypeVariableArgument:      // function argument variables
+  case eValueTypeVariableLocal:         // function local variables
+  case eValueTypeVariableThreadLocal: { // thread local variables
+    SymbolContext sc(frame->GetSymbolContext(eSymbolContextBlock));
+
+    const bool can_create = true;
+    const bool get_parent_variables = true;
+    const bool stop_if_block_is_inlined_function = true;
+
+    if (sc.block)
+      sc.block->AppendVariables(
+          can_create, get_parent_variables, stop_if_block_is_inlined_function,
+          [frame](Variable *v) { return v->IsInScope(frame); }, &variable_list);
+    if (value_type == eValueTypeVariableGlobal ||
+        value_type == eValueTypeVariableStatic) {
+      const bool get_file_globals = true;
+      VariableList *frame_vars =
+          frame->GetVariableList(get_file_globals, nullptr);
+      if (frame_vars)
+        frame_vars->AppendVariablesIfUnique(variable_list);
+    }
+    ConstString const_name(name);
+    VariableSP variable_sp(variable_list.FindVariable(const_name, value_type));
+    if (variable_sp) {
+      value_sp =
+          frame->GetValueObjectForFrameVariable(variable_sp, eNoDynamicValues);
+      sb_value.SetSP(value_sp, use_dynamic);
+    }
+  } break;
+
+  case eValueTypeRegister: { // stack frame register value
+    if (RegisterContextSP reg_ctx = frame->GetRegisterContext()) {
+      if (const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoByName(name)) {
+        value_sp = ValueObjectRegister::Create(frame, reg_ctx, reg_info);
+        sb_value.SetSP(value_sp);
+      }
+    }
+  } break;
+
+  case eValueTypeRegisterSet: { // A collection of stack frame register
+                                // values
+    if (RegisterContextSP reg_ctx = frame->GetRegisterContext()) {
+      const uint32_t num_sets = reg_ctx->GetRegisterSetCount();
+      for (uint32_t set_idx = 0; set_idx < num_sets; ++set_idx) {
+        const RegisterSet *reg_set = reg_ctx->GetRegisterSet(set_idx);
+        if (reg_set &&
+            (llvm::StringRef(reg_set->name).equals_insensitive(name) ||
+             llvm::StringRef(reg_set->short_name).equals_insensitive(name))) {
+          value_sp = ValueObjectRegisterSet::Create(frame, reg_ctx, set_idx);
+          sb_value.SetSP(value_sp);
           break;
         }
       }
     }
+  } break;
+
+  case eValueTypeConstResult: { // constant result variables
+    ConstString const_name(name);
+    Target *target = exe_ctx->GetTargetPtr();
+    ExpressionVariableSP expr_var_sp(target->GetPersistentVariable(const_name));
+    if (expr_var_sp) {
+      value_sp = expr_var_sp->GetValueObject();
+      sb_value.SetSP(value_sp, use_dynamic);
+    }
+  } break;
+
+  default:
+    break;
   }
 
   return sb_value;
@@ -698,169 +684,184 @@ lldb::SBValueList SBFrame::GetVariables(bool arguments, bool locals,
   return GetVariables(options);
 }
 
+/// Returns true if the variable is in any of the requested scopes.
+static bool IsInRequestedScope(bool statics, bool arguments, bool locals,
+                               Variable &var) {
+  switch (var.GetScope()) {
+  case eValueTypeVariableGlobal:
+  case eValueTypeVariableStatic:
+  case eValueTypeVariableThreadLocal:
+    return statics;
+
+  case eValueTypeVariableArgument:
+    return arguments;
+
+  case eValueTypeVariableLocal:
+    return locals;
+
+  default:
+    break;
+  }
+  return false;
+}
+
+SBFrame::WasInterrupted SBFrame::FetchVariablesUnlessInterrupted(
+    const lldb::SBVariablesOptions &options, StackFrame &frame,
+    SBValueList &value_list, Debugger &dbg) {
+  const bool statics = options.GetIncludeStatics();
+  const bool arguments = options.GetIncludeArguments();
+  const bool locals = options.GetIncludeLocals();
+  const bool in_scope_only = options.GetInScopeOnly();
+  const bool include_runtime_support_values =
+      options.GetIncludeRuntimeSupportValues();
+  const lldb::DynamicValueType use_dynamic = options.GetUseDynamic();
+
+  Status var_error;
+  VariableList *variable_list = frame.GetVariableList(true, &var_error);
+
+  std::set<VariableSP> variable_set;
+
+  if (var_error.Fail())
+    value_list.SetError(std::move(var_error));
+
+  if (!variable_list)
+    return WasInterrupted::No;
+  const size_t num_variables = variable_list->GetSize();
+  size_t num_produced = 0;
+  for (const VariableSP &variable_sp : *variable_list) {
+    if (!variable_sp ||
+        !IsInRequestedScope(statics, arguments, locals, *variable_sp))
+      continue;
+
+    if (INTERRUPT_REQUESTED(
+            dbg,
+            "Interrupted getting frame variables with {0} of {1} "
+            "produced.",
+            num_produced, num_variables))
+      return WasInterrupted::Yes;
+
+    // Only add variables once so we don't end up with duplicates
+    if (variable_set.insert(variable_sp).second == false)
+      continue;
+    if (in_scope_only && !variable_sp->IsInScope(&frame))
+      continue;
+
+    ValueObjectSP valobj_sp(
+        frame.GetValueObjectForFrameVariable(variable_sp, eNoDynamicValues));
+
+    if (!include_runtime_support_values && valobj_sp != nullptr &&
+        valobj_sp->IsRuntimeSupportValue())
+      continue;
+
+    SBValue value_sb;
+    value_sb.SetSP(valobj_sp, use_dynamic);
+    value_list.Append(value_sb);
+  }
+  num_produced++;
+
+  return WasInterrupted::No;
+}
+
+void SBFrame::FetchRecognizedArguments(const SBVariablesOptions &options,
+                                       StackFrame &frame, SBTarget target,
+                                       SBValueList &value_list) {
+  if (!options.GetIncludeRecognizedArguments(target))
+    return;
+  RecognizedStackFrameSP recognized_frame = frame.GetRecognizedFrame();
+  if (!recognized_frame)
+    return;
+
+  ValueObjectListSP recognized_arg_list =
+      recognized_frame->GetRecognizedArguments();
+  if (!recognized_arg_list)
+    return;
+
+  const lldb::DynamicValueType use_dynamic = options.GetUseDynamic();
+  for (auto &rec_value_sp : recognized_arg_list->GetObjects()) {
+    SBValue value_sb;
+    value_sb.SetSP(rec_value_sp, use_dynamic);
+    value_list.Append(value_sb);
+  }
+}
+
 SBValueList SBFrame::GetVariables(const lldb::SBVariablesOptions &options) {
   LLDB_INSTRUMENT_VA(this, options);
 
-  SBValueList value_list;
   llvm::Expected<StoppedExecutionContext> exe_ctx =
       GetStoppedExecutionContext(m_opaque_sp);
   if (!exe_ctx) {
     LLDB_LOG_ERROR(GetLog(LLDBLog::API), exe_ctx.takeError(), "{0}");
     return SBValueList();
-  } else {
-    const bool statics = options.GetIncludeStatics();
-    const bool arguments = options.GetIncludeArguments();
-    const bool recognized_arguments =
-        options.GetIncludeRecognizedArguments(SBTarget(exe_ctx->GetTargetSP()));
-    const bool locals = options.GetIncludeLocals();
-    const bool in_scope_only = options.GetInScopeOnly();
-    const bool include_runtime_support_values =
-        options.GetIncludeRuntimeSupportValues();
-    const lldb::DynamicValueType use_dynamic = options.GetUseDynamic();
-
-    std::set<VariableSP> variable_set;
-    Process *process = exe_ctx->GetProcessPtr();
-    if (process) { // FIXME: this check is redundant.
-      if (StackFrame *frame = exe_ctx->GetFramePtr()) {
-        Debugger &dbg = process->GetTarget().GetDebugger();
-        VariableList *variable_list = nullptr;
-        Status var_error;
-        variable_list = frame->GetVariableList(true, &var_error);
-        if (var_error.Fail())
-          value_list.SetError(std::move(var_error));
-        if (variable_list) {
-          const size_t num_variables = variable_list->GetSize();
-          if (num_variables) {
-            size_t num_produced = 0;
-            for (const VariableSP &variable_sp : *variable_list) {
-              if (INTERRUPT_REQUESTED(dbg, 
-                    "Interrupted getting frame variables with {0} of {1} "
-                    "produced.", num_produced, num_variables))
-                return {};
-
-              if (variable_sp) {
-                bool add_variable = false;
-                switch (variable_sp->GetScope()) {
-                case eValueTypeVariableGlobal:
-                case eValueTypeVariableStatic:
-                case eValueTypeVariableThreadLocal:
-                  add_variable = statics;
-                  break;
-
-                case eValueTypeVariableArgument:
-                  add_variable = arguments;
-                  break;
-
-                case eValueTypeVariableLocal:
-                  add_variable = locals;
-                  break;
-
-                default:
-                  break;
-                }
-                if (add_variable) {
-                  // Only add variables once so we don't end up with duplicates
-                  if (variable_set.find(variable_sp) == variable_set.end())
-                    variable_set.insert(variable_sp);
-                  else
-                    continue;
-
-                  if (in_scope_only && !variable_sp->IsInScope(frame))
-                    continue;
-
-                  ValueObjectSP valobj_sp(frame->GetValueObjectForFrameVariable(
-                      variable_sp, eNoDynamicValues));
-
-                  if (!include_runtime_support_values && valobj_sp != nullptr &&
-                      valobj_sp->IsRuntimeSupportValue())
-                    continue;
-
-                  SBValue value_sb;
-                  value_sb.SetSP(valobj_sp, use_dynamic);
-                  value_list.Append(value_sb);
-                }
-              }
-            }
-            num_produced++;
-          }
-        }
-        if (recognized_arguments) {
-          auto recognized_frame = frame->GetRecognizedFrame();
-          if (recognized_frame) {
-            ValueObjectListSP recognized_arg_list =
-                recognized_frame->GetRecognizedArguments();
-            if (recognized_arg_list) {
-              for (auto &rec_value_sp : recognized_arg_list->GetObjects()) {
-                SBValue value_sb;
-                value_sb.SetSP(rec_value_sp, use_dynamic);
-                value_list.Append(value_sb);
-              }
-            }
-          }
-        }
-      }
-    }
   }
 
+  StackFrame *frame = exe_ctx->GetFramePtr();
+  if (!frame)
+    return SBValueList();
+
+  SBValueList value_list;
+  if (WasInterrupted::Yes ==
+      FetchVariablesUnlessInterrupted(options, *frame, value_list,
+                                      exe_ctx->GetTargetPtr()->GetDebugger()))
+    return value_list;
+
+  FetchRecognizedArguments(options, *frame, SBTarget(exe_ctx->GetTargetSP()),
+                           value_list);
   return value_list;
 }
 
 SBValueList SBFrame::GetRegisters() {
   LLDB_INSTRUMENT_VA(this);
 
-  SBValueList value_list;
   llvm::Expected<StoppedExecutionContext> exe_ctx =
       GetStoppedExecutionContext(m_opaque_sp);
   if (!exe_ctx) {
     LLDB_LOG_ERROR(GetLog(LLDBLog::API), exe_ctx.takeError(), "{0}");
     return SBValueList();
-  } else {
-    Target *target = exe_ctx->GetTargetPtr();
-    Process *process = exe_ctx->GetProcessPtr();
-    if (target && process) { // FIXME: this check is redundant.
-      if (StackFrame *frame = exe_ctx->GetFramePtr()) {
-        RegisterContextSP reg_ctx(frame->GetRegisterContext());
-        if (reg_ctx) {
-          const uint32_t num_sets = reg_ctx->GetRegisterSetCount();
-          for (uint32_t set_idx = 0; set_idx < num_sets; ++set_idx) {
-            value_list.Append(
-                ValueObjectRegisterSet::Create(frame, reg_ctx, set_idx));
-          }
-        }
-      }
-    }
   }
 
+  StackFrame *frame = exe_ctx->GetFramePtr();
+  if (!frame)
+    return SBValueList();
+
+  RegisterContextSP reg_ctx(frame->GetRegisterContext());
+  if (!reg_ctx)
+    return SBValueList();
+
+  SBValueList value_list;
+  const uint32_t num_sets = reg_ctx->GetRegisterSetCount();
+  for (uint32_t set_idx = 0; set_idx < num_sets; ++set_idx)
+    value_list.Append(ValueObjectRegisterSet::Create(frame, reg_ctx, set_idx));
+
   return value_list;
 }
 
 SBValue SBFrame::FindRegister(const char *name) {
   LLDB_INSTRUMENT_VA(this, name);
 
-  SBValue result;
   ValueObjectSP value_sp;
   llvm::Expected<StoppedExecutionContext> exe_ctx =
       GetStoppedExecutionContext(m_opaque_sp);
   if (!exe_ctx) {
     LLDB_LOG_ERROR(GetLog(LLDBLog::API), exe_ctx.takeError(), "{0}");
     return SBValue();
-  } else {
-    Target *target = exe_ctx->GetTargetPtr();
-    Process *process = exe_ctx->GetProcessPtr();
-    if (target && process) { // FIXME: this check is redundant.
-      if (StackFrame *frame = exe_ctx->GetFramePtr()) {
-        RegisterContextSP reg_ctx(frame->GetRegisterContext());
-        if (reg_ctx) {
-          if (const RegisterInfo *reg_info =
-                  reg_ctx->GetRegisterInfoByName(name)) {
-            value_sp = ValueObjectRegister::Create(frame, reg_ctx, reg_info);
-            result.SetSP(value_sp);
-          }
-        }
-      }
-    }
   }
 
+  StackFrame *frame = exe_ctx->GetFramePtr();
+  if (!frame)
+    return SBValue();
+
+  RegisterContextSP reg_ctx(frame->GetRegisterContext());
+  if (!reg_ctx)
+    return SBValue();
+
+  const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoByName(name);
+  if (!reg_info)
+    return SBValue();
+
+  SBValue result;
+  value_sp = ValueObjectRegister::Create(frame, reg_ctx, reg_info);
+  result.SetSP(value_sp);
+
   return result;
 }
 
@@ -1000,58 +1001,55 @@ lldb::SBValue SBFrame::EvaluateExpression(const char *expr,
                                           const SBExpressionOptions &options) {
   LLDB_INSTRUMENT_VA(this, expr, options);
 
-  Log *expr_log = GetLog(LLDBLog::Expressions);
-
-  SBValue expr_result;
+  auto LogResult = [](SBValue expr_result) {
+    Log *expr_log = GetLog(LLDBLog::Expressions);
+    if (expr_result.GetError().Success())
+      LLDB_LOGF(expr_log,
+                "** [SBFrame::EvaluateExpression] Expression result is "
+                "%s, summary %s **",
+                expr_result.GetValue(), expr_result.GetSummary());
+    else
+      LLDB_LOGF(
+          expr_log,
+          "** [SBFrame::EvaluateExpression] Expression evaluation failed: "
+          "%s **",
+          expr_result.GetError().GetCString());
+  };
 
   if (expr == nullptr || expr[0] == '\0') {
-    return expr_result;
+    return SBValue();
   }
 
-  ValueObjectSP expr_value_sp;
-
   llvm::Expected<StoppedExecutionContext> exe_ctx =
       GetStoppedExecutionContext(m_opaque_sp);
   if (!exe_ctx) {
     LLDB_LOG_ERROR(GetLog(LLDBLog::API), exe_ctx.takeError(), "{0}");
-    expr_result = CreateProcessIsRunningExprEvalError();
-  } else {
-    Target *target = exe_ctx->GetTargetPtr();
-    Process *process = exe_ctx->GetProcessPtr();
-    if (target && process) { // FIXME: this check is redundant.
-      if (StackFrame *frame = exe_ctx->GetFramePtr()) {
-        std::unique_ptr<llvm::PrettyStackTraceFormat> stack_trace;
-        if (target->GetDisplayExpressionsInCrashlogs()) {
-          StreamString frame_description;
-          frame->DumpUsingSettingsFormat(&frame_description);
-          stack_trace = std::make_unique<llvm::PrettyStackTraceFormat>(
-              "SBFrame::EvaluateExpression (expr = \"%s\", fetch_dynamic_value "
-              "= %u) %s",
-              expr, options.GetFetchDynamicValue(),
-              frame_description.GetData());
-        }
+    SBValue error_result = CreateProcessIsRunningExprEvalError();
+    LogResult(error_result);
+    return error_result;
+  }
 
-        target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
-        expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
-      }
-    } else {
-      Status error;
-      error = Status::FromErrorString("sbframe object is not valid.");
-      expr_value_sp = ValueObjectConstResult::Create(nullptr, std::move(error));
-      expr_result.SetSP(expr_value_sp, false);
-    }
+  StackFrame *frame = exe_ctx->GetFramePtr();
+  if (!frame)
+    return SBValue();
+
+  std::unique_ptr<llvm::PrettyStackTraceFormat> stack_trace;
+  Target *target = exe_ctx->GetTargetPtr();
+  if (target->GetDisplayExpressionsInCrashlogs()) {
+    StreamString frame_description;
+    frame->DumpUsingSettingsFormat(&frame_description);
+    stack_trace = std::make_unique<llvm::PrettyStackTraceFormat>(
+        "SBFrame::EvaluateExpression (expr = \"%s\", fetch_dynamic_value "
+        "= %u) %s",
+        expr, options.GetFetchDynamicValue(), frame_description.GetData());
   }
 
-  if (expr_result.GetError().Success())
-    LLDB_LOGF(expr_log,
-              "** [SBFrame::EvaluateExpression] Expression result is "
-              "%s, summary %s **",
-              expr_result.GetValue(), expr_result.GetSummary());
-  else
-    LLDB_LOGF(expr_log,
-              "** [SBFrame::EvaluateExpression] Expression evaluation failed: "
-              "%s **",
-              expr_result.GetError().GetCString());
+  ValueObjectSP expr_value_sp;
+  target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
+
+  SBValue expr_result;
+  expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
+  LogResult(expr_result);
 
   return expr_result;
 }

>From 7c3779816ca48a4ced20573d57052b89bc6cfa3e Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 12 Aug 2025 13:56:12 -0700
Subject: [PATCH 2/3] fixup! Move FetchRecognizedArguments out of SB header

---
 lldb/include/lldb/API/SBFrame.h |  7 -------
 lldb/source/API/SBFrame.cpp     | 31 +++++++++++++++++--------------
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/lldb/include/lldb/API/SBFrame.h b/lldb/include/lldb/API/SBFrame.h
index 59a7343f9eaf3..69636822b2c57 100644
--- a/lldb/include/lldb/API/SBFrame.h
+++ b/lldb/include/lldb/API/SBFrame.h
@@ -253,13 +253,6 @@ class LLDB_API SBFrame {
       const lldb::SBVariablesOptions &options, lldb_private::StackFrame &frame,
       SBValueList &value_list, lldb_private::Debugger &dbg);
 
-  /// Populates `value_list` with recognized arguments of `frame` according to
-  /// `options`.
-  static void FetchRecognizedArguments(const SBVariablesOptions &options,
-                                       lldb_private::StackFrame &frame,
-                                       SBTarget target,
-                                       SBValueList &value_list);
-
   lldb::ExecutionContextRefSP m_opaque_sp;
 };
 
diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index ed09fd2f2f90d..29f72b4d68acb 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -762,26 +762,23 @@ SBFrame::WasInterrupted SBFrame::FetchVariablesUnlessInterrupted(
   return WasInterrupted::No;
 }
 
-void SBFrame::FetchRecognizedArguments(const SBVariablesOptions &options,
-                                       StackFrame &frame, SBTarget target,
-                                       SBValueList &value_list) {
+/// Populates `value_list` with recognized arguments of `frame` according to
+/// `options`.
+static llvm::SmallVector<ValueObjectSP>
+FetchRecognizedArguments(const SBVariablesOptions &options, StackFrame &frame,
+                         SBTarget target) {
   if (!options.GetIncludeRecognizedArguments(target))
-    return;
+    return {};
   RecognizedStackFrameSP recognized_frame = frame.GetRecognizedFrame();
   if (!recognized_frame)
-    return;
+    return {};
 
   ValueObjectListSP recognized_arg_list =
       recognized_frame->GetRecognizedArguments();
   if (!recognized_arg_list)
-    return;
+    return {};
 
-  const lldb::DynamicValueType use_dynamic = options.GetUseDynamic();
-  for (auto &rec_value_sp : recognized_arg_list->GetObjects()) {
-    SBValue value_sb;
-    value_sb.SetSP(rec_value_sp, use_dynamic);
-    value_list.Append(value_sb);
-  }
+  return llvm::to_vector(recognized_arg_list->GetObjects());
 }
 
 SBValueList SBFrame::GetVariables(const lldb::SBVariablesOptions &options) {
@@ -804,8 +801,14 @@ SBValueList SBFrame::GetVariables(const lldb::SBVariablesOptions &options) {
                                       exe_ctx->GetTargetPtr()->GetDebugger()))
     return value_list;
 
-  FetchRecognizedArguments(options, *frame, SBTarget(exe_ctx->GetTargetSP()),
-                           value_list);
+  const lldb::DynamicValueType use_dynamic = options.GetUseDynamic();
+  llvm::SmallVector<ValueObjectSP> args = FetchRecognizedArguments(
+      options, *frame, SBTarget(exe_ctx->GetTargetSP()));
+  for (ValueObjectSP arg : args) {
+    SBValue value_sb;
+    value_sb.SetSP(arg, use_dynamic);
+    value_list.Append(value_sb);
+  }
   return value_list;
 }
 

>From 85554976d322e7e26357f96648c58420fa9d4e55 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Thu, 14 Aug 2025 11:27:02 -0700
Subject: [PATCH 3/3] fixup! Remove all changes from SBFrame.h

---
 lldb/include/lldb/API/SBFrame.h | 12 -----------
 lldb/source/API/SBFrame.cpp     | 38 ++++++++++++++++++++++-----------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/lldb/include/lldb/API/SBFrame.h b/lldb/include/lldb/API/SBFrame.h
index 69636822b2c57..5283cdfe53faa 100644
--- a/lldb/include/lldb/API/SBFrame.h
+++ b/lldb/include/lldb/API/SBFrame.h
@@ -13,8 +13,6 @@
 #include "lldb/API/SBValueList.h"
 
 namespace lldb_private {
-class StackFrame;
-class Debugger;
 namespace python {
 class SWIGBridge;
 }
@@ -243,16 +241,6 @@ class LLDB_API SBFrame {
   /// not currently stopped.
   static SBValue CreateProcessIsRunningExprEvalError();
 
-  enum WasInterrupted { Yes, No };
-
-  /// Populates `value_list` with the variables from `frame` according to
-  /// `options`. This method checks whether the Debugger received an interrupt
-  /// before processing every variable, returning `WasInterrupted::yes` in that
-  /// case.
-  static WasInterrupted FetchVariablesUnlessInterrupted(
-      const lldb::SBVariablesOptions &options, lldb_private::StackFrame &frame,
-      SBValueList &value_list, lldb_private::Debugger &dbg);
-
   lldb::ExecutionContextRefSP m_opaque_sp;
 };
 
diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index 29f72b4d68acb..31947a054aaaf 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -705,9 +705,16 @@ static bool IsInRequestedScope(bool statics, bool arguments, bool locals,
   return false;
 }
 
-SBFrame::WasInterrupted SBFrame::FetchVariablesUnlessInterrupted(
+enum WasInterrupted { Yes, No };
+
+/// Populates `value_list` with the variables from `frame` according to
+/// `options`. This method checks whether the Debugger received an interrupt
+/// before processing every variable, returning `WasInterrupted::yes` in that
+/// case.
+static std::pair<WasInterrupted, Status> FetchVariablesUnlessInterrupted(
     const lldb::SBVariablesOptions &options, StackFrame &frame,
-    SBValueList &value_list, Debugger &dbg) {
+    SBValueList &value_list, Debugger &dbg,
+    std::function<SBValue(ValueObjectSP, bool)> to_sbvalue) {
   const bool statics = options.GetIncludeStatics();
   const bool arguments = options.GetIncludeArguments();
   const bool locals = options.GetIncludeLocals();
@@ -721,11 +728,8 @@ SBFrame::WasInterrupted SBFrame::FetchVariablesUnlessInterrupted(
 
   std::set<VariableSP> variable_set;
 
-  if (var_error.Fail())
-    value_list.SetError(std::move(var_error));
-
   if (!variable_list)
-    return WasInterrupted::No;
+    return {WasInterrupted::No, std::move(var_error)};
   const size_t num_variables = variable_list->GetSize();
   size_t num_produced = 0;
   for (const VariableSP &variable_sp : *variable_list) {
@@ -738,7 +742,7 @@ SBFrame::WasInterrupted SBFrame::FetchVariablesUnlessInterrupted(
             "Interrupted getting frame variables with {0} of {1} "
             "produced.",
             num_produced, num_variables))
-      return WasInterrupted::Yes;
+      return {WasInterrupted::Yes, std::move(var_error)};
 
     // Only add variables once so we don't end up with duplicates
     if (variable_set.insert(variable_sp).second == false)
@@ -753,13 +757,11 @@ SBFrame::WasInterrupted SBFrame::FetchVariablesUnlessInterrupted(
         valobj_sp->IsRuntimeSupportValue())
       continue;
 
-    SBValue value_sb;
-    value_sb.SetSP(valobj_sp, use_dynamic);
-    value_list.Append(value_sb);
+    value_list.Append(to_sbvalue(valobj_sp, use_dynamic));
   }
   num_produced++;
 
-  return WasInterrupted::No;
+  return {WasInterrupted::No, std::move(var_error)};
 }
 
 /// Populates `value_list` with recognized arguments of `frame` according to
@@ -795,10 +797,20 @@ SBValueList SBFrame::GetVariables(const lldb::SBVariablesOptions &options) {
   if (!frame)
     return SBValueList();
 
+  auto valobj_to_sbvalue = [](ValueObjectSP valobj, bool use_dynamic) {
+    SBValue value_sb;
+    value_sb.SetSP(valobj, use_dynamic);
+    return value_sb;
+  };
   SBValueList value_list;
-  if (WasInterrupted::Yes ==
+  std::pair<WasInterrupted, Status> fetch_result =
       FetchVariablesUnlessInterrupted(options, *frame, value_list,
-                                      exe_ctx->GetTargetPtr()->GetDebugger()))
+                                      exe_ctx->GetTargetPtr()->GetDebugger(),
+                                      valobj_to_sbvalue);
+  if (fetch_result.second.Fail())
+    value_list.SetError(std::move(fetch_result.second));
+
+  if (fetch_result.first == WasInterrupted::Yes)
     return value_list;
 
   const lldb::DynamicValueType use_dynamic = options.GetUseDynamic();



More information about the lldb-commits mailing list