kastiglione created this revision.
kastiglione added reviewers: aprantl, jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Follow up to "Suppress persistent result when running po" (D144044
<https://reviews.llvm.org/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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D150619
Files:
lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectExpression.h
Index: lldb/source/Commands/CommandObjectExpression.h
===================================================================
--- lldb/source/Commands/CommandObjectExpression.h
+++ lldb/source/Commands/CommandObjectExpression.h
@@ -41,6 +41,9 @@
const Target &target,
const OptionGroupValueObjectDisplay &display_opts);
+ bool ShouldSuppressResult(
+ const OptionGroupValueObjectDisplay &display_opts) const;
+
bool top_level;
bool unwind_on_error;
bool ignore_breakpoints;
Index: lldb/source/Commands/CommandObjectExpression.cpp
===================================================================
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ 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 @@
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 @@
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",
@@ -454,14 +460,27 @@
}
}
+ 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 = eval_options.GetLanguage();
+ if (language == lldb::eLanguageTypeUnknown)
+ language = frame->GuessLanguage();
+ if (auto *persistent_state =
+ target.GetPersistentExpressionStateForLanguage(language))
+ persistent_state->RemovePersistentVariable(result_var_sp);
+ }
result.SetStatus(eReturnStatusSuccessFinishResult);
}
} else {
Index: lldb/source/Commands/CommandObjectDWIMPrint.cpp
===================================================================
--- lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ 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"
@@ -77,6 +78,7 @@
// 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();
@@ -89,7 +91,7 @@
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);
// Support for verbose mode.
char const *analogous_command = nullptr;
@@ -110,7 +112,7 @@
valobj_sp = frame->GetValueForVariableExpressionPath(
expr, use_dynamic, options, var_sp, error);
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;
}
@@ -158,6 +160,18 @@
analogous_options);
valobj_sp->Dump(result.GetOutputStream(), dump_options);
+
+ if (suppress_result)
+ if (auto result_var_sp =
+ target.GetPersistentVariable(valobj_sp->GetName())) {
+ auto language = eval_options.GetLanguage();
+ if (language == lldb::eLanguageTypeUnknown)
+ language = frame->GuessLanguage();
+ if (auto *persistent_state =
+ target.GetPersistentExpressionStateForLanguage(language))
+ persistent_state->RemovePersistentVariable(result_var_sp);
+ }
+
result.SetStatus(eReturnStatusSuccessFinishResult);
return true;
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
