[Lldb-commits] [lldb] db81455 - [lldb] Delay removal of persistent results
Dave Lee via lldb-commits
lldb-commits at lists.llvm.org
Thu May 18 09:44:17 PDT 2023
Author: Dave Lee
Date: 2023-05-18T09:44:11-07:00
New Revision: db814552132d614e410118e22b22c89d35ae6062
URL: https://github.com/llvm/llvm-project/commit/db814552132d614e410118e22b22c89d35ae6062
DIFF: https://github.com/llvm/llvm-project/commit/db814552132d614e410118e22b22c89d35ae6062.diff
LOG: [lldb] Delay removal of persistent results
Follow up to "Suppress persistent result when running po" (D144044).
This change delays removal of the persistent result until after `Dump` has been called.
In doing so, the persistent result is available for the purpose of getting its object
description.
In the original change, the persistent result removal happens indirectly, by setting
`EvaluateExpressionOptions::SetSuppressPersistentResult`. In practice this has worked,
however this exposed a latent bug in swift-lldb. The subtlety, and the bug, depend on
when the persisteted result variable is removed.
When the result is removed via `SetSuppressPersistentResult`, it happens within the call
to `Target::EvaluateExpression`. That is, by the time the call returns, the persistent
result is already removed.
The issue occurs shortly thereafter, when `ValueObject::Dump` is called, it cannot make
use of the persistent result variable (instead it uses the `ValueObjectConstResult`). In
swift-lldb, this causes an additional expression evaluation to happen. It first tries an
expression that reference `$R0` etc, but that always fails because `$R0` is removed. The
fallback to this failure does work most of the time, but there's at least one bug
involving imported Clang types.
Differential Revision: https://reviews.llvm.org/D150619
Added:
Modified:
lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectExpression.h
Removed:
################################################################################
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 0b5dde9ccaa67..8fc702a1a220e 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -10,6 +10,7 @@
#include "lldb/Core/ValueObject.h"
#include "lldb/DataFormatters/DumpValueObjectOptions.h"
+#include "lldb/Expression/ExpressionVariable.h"
#include "lldb/Interpreter/CommandInterpreter.h"
#include "lldb/Interpreter/CommandObject.h"
#include "lldb/Interpreter/CommandReturnObject.h"
@@ -76,6 +77,7 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
// If the user has not specified, default to disabling persistent results.
if (m_expr_options.suppress_persistent_result == eLazyBoolCalculate)
m_expr_options.suppress_persistent_result = eLazyBoolYes;
+ bool suppress_result = m_expr_options.ShouldSuppressResult(m_varobj_options);
auto verbosity = GetDebugger().GetDWIMPrintVerbosity();
@@ -83,18 +85,23 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
// Fallback to the dummy target, which can allow for expression evaluation.
Target &target = target_ptr ? *target_ptr : GetDummyTarget();
- const EvaluateExpressionOptions eval_options =
+ EvaluateExpressionOptions eval_options =
m_expr_options.GetEvaluateExpressionOptions(target, m_varobj_options);
+ // This command manually removes the result variable, make sure expression
+ // evaluation doesn't do it first.
+ eval_options.SetSuppressPersistentResult(false);
DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
m_expr_options.m_verbosity, m_format_options.GetFormat());
- dump_options.SetHideRootName(eval_options.GetSuppressPersistentResult());
+ dump_options.SetHideRootName(suppress_result);
+
+ StackFrame *frame = m_exe_ctx.GetFramePtr();
// First, try `expr` as the name of a frame variable.
- if (StackFrame *frame = m_exe_ctx.GetFramePtr()) {
+ if (frame) {
auto valobj_sp = frame->FindVariable(ConstString(expr));
if (valobj_sp && valobj_sp->GetError().Success()) {
- if (!eval_options.GetSuppressPersistentResult()) {
+ if (!suppress_result) {
if (auto persisted_valobj = valobj_sp->Persist())
valobj_sp = persisted_valobj;
}
@@ -129,6 +136,16 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
}
valobj_sp->Dump(result.GetOutputStream(), dump_options);
+
+ if (suppress_result)
+ if (auto result_var_sp =
+ target.GetPersistentVariable(valobj_sp->GetName())) {
+ auto language = valobj_sp->GetPreferredDisplayLanguage();
+ if (auto *persistent_state =
+ target.GetPersistentExpressionStateForLanguage(language))
+ persistent_state->RemovePersistentVariable(result_var_sp);
+ }
+
result.SetStatus(eReturnStatusSuccessFinishResult);
return true;
} else {
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 5d68f662a771f..e7e6e3820b991 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -10,6 +10,7 @@
#include "CommandObjectExpression.h"
#include "lldb/Core/Debugger.h"
+#include "lldb/Expression/ExpressionVariable.h"
#include "lldb/Expression/REPL.h"
#include "lldb/Expression/UserExpression.h"
#include "lldb/Host/OptionParser.h"
@@ -21,6 +22,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/StackFrame.h"
#include "lldb/Target/Target.h"
+#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-private-enumerations.h"
using namespace lldb;
@@ -200,13 +202,6 @@ CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions(
const Target &target, const OptionGroupValueObjectDisplay &display_opts) {
EvaluateExpressionOptions options;
options.SetCoerceToId(display_opts.use_objc);
- // Explicitly disabling persistent results takes precedence over the
- // m_verbosity/use_objc logic.
- if (suppress_persistent_result != eLazyBoolCalculate)
- options.SetSuppressPersistentResult(suppress_persistent_result ==
- eLazyBoolYes);
- else if (m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact)
- options.SetSuppressPersistentResult(display_opts.use_objc);
options.SetUnwindOnError(unwind_on_error);
options.SetIgnoreBreakpoints(ignore_breakpoints);
options.SetKeepInMemory(true);
@@ -242,6 +237,17 @@ CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions(
return options;
}
+bool CommandObjectExpression::CommandOptions::ShouldSuppressResult(
+ const OptionGroupValueObjectDisplay &display_opts) const {
+ // Explicitly disabling persistent results takes precedence over the
+ // m_verbosity/use_objc logic.
+ if (suppress_persistent_result != eLazyBoolCalculate)
+ return suppress_persistent_result == eLazyBoolYes;
+
+ return display_opts.use_objc &&
+ m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact;
+}
+
CommandObjectExpression::CommandObjectExpression(
CommandInterpreter &interpreter)
: CommandObjectRaw(interpreter, "expression",
@@ -424,8 +430,12 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
return false;
}
- const EvaluateExpressionOptions eval_options =
+ EvaluateExpressionOptions eval_options =
m_command_options.GetEvaluateExpressionOptions(target, m_varobj_options);
+ // This command manually removes the result variable, make sure expression
+ // evaluation doesn't do it first.
+ eval_options.SetSuppressPersistentResult(false);
+
ExpressionResults success = target.EvaluateExpression(
expr, frame, result_valobj_sp, eval_options, &m_fixed_expression);
@@ -454,14 +464,25 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
}
}
+ bool suppress_result =
+ m_command_options.ShouldSuppressResult(m_varobj_options);
+
DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(
m_command_options.m_verbosity, format));
- options.SetHideRootName(eval_options.GetSuppressPersistentResult());
+ options.SetHideRootName(suppress_result);
options.SetVariableFormatDisplayLanguage(
result_valobj_sp->GetPreferredDisplayLanguage());
result_valobj_sp->Dump(output_stream, options);
+ if (suppress_result)
+ if (auto result_var_sp =
+ target.GetPersistentVariable(result_valobj_sp->GetName())) {
+ auto language = result_valobj_sp->GetPreferredDisplayLanguage();
+ if (auto *persistent_state =
+ target.GetPersistentExpressionStateForLanguage(language))
+ persistent_state->RemovePersistentVariable(result_var_sp);
+ }
result.SetStatus(eReturnStatusSuccessFinishResult);
}
} else {
diff --git a/lldb/source/Commands/CommandObjectExpression.h b/lldb/source/Commands/CommandObjectExpression.h
index d6a4bb19fd650..b2b8fc73a1ee8 100644
--- a/lldb/source/Commands/CommandObjectExpression.h
+++ b/lldb/source/Commands/CommandObjectExpression.h
@@ -41,6 +41,9 @@ class CommandObjectExpression : public CommandObjectRaw,
const Target &target,
const OptionGroupValueObjectDisplay &display_opts);
+ bool ShouldSuppressResult(
+ const OptionGroupValueObjectDisplay &display_opts) const;
+
bool top_level;
bool unwind_on_error;
bool ignore_breakpoints;
More information about the lldb-commits
mailing list