Skip to content

[lldb] Refactor UserExpression::Evaluate to only have one error channel. #117186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 21, 2024
Merged

[lldb] Refactor UserExpression::Evaluate to only have one error channel. #117186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions lldb/include/lldb/Expression/UserExpression.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,9 @@ class UserExpression : public Expression {
/// definitions to be included when the expression is parsed.
///
/// \param[in,out] result_valobj_sp
/// If execution is successful, the result valobj is placed here.
///
/// \param[out] error
/// Filled in with an error in case the expression evaluation
/// fails to parse, run, or evaluated.
/// If execution is successful, the result valobj is placed
/// here. Otherwise its Error will contain an ExpressionError
/// with details about the failure mode.
///
/// \param[out] fixed_expression
/// If non-nullptr, the fixed expression is copied into the provided
Expand All @@ -266,7 +264,7 @@ class UserExpression : public Expression {
static lldb::ExpressionResults
Evaluate(ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
llvm::StringRef expr_cstr, llvm::StringRef expr_prefix,
lldb::ValueObjectSP &result_valobj_sp, Status &error,
lldb::ValueObjectSP &result_valobj_sp,
std::string *fixed_expression = nullptr,
ValueObject *ctx_obj = nullptr);

Expand Down
7 changes: 2 additions & 5 deletions lldb/source/Expression/REPL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,12 +339,9 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) {

const char *expr_prefix = nullptr;
lldb::ValueObjectSP result_valobj_sp;
lldb::ExpressionResults execution_results = UserExpression::Evaluate(
exe_ctx, expr_options, code.c_str(), expr_prefix, result_valobj_sp);
Status error;
lldb::ExpressionResults execution_results =
UserExpression::Evaluate(exe_ctx, expr_options, code.c_str(),
expr_prefix, result_valobj_sp, error,
nullptr); // fixed expression

if (llvm::Error err = OnExpressionEvaluated(exe_ctx, code, expr_options,
execution_results,
result_valobj_sp, error)) {
Expand Down
64 changes: 29 additions & 35 deletions lldb/source/Expression/UserExpression.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,13 @@ lldb::ExpressionResults
UserExpression::Evaluate(ExecutionContext &exe_ctx,
const EvaluateExpressionOptions &options,
llvm::StringRef expr, llvm::StringRef prefix,
lldb::ValueObjectSP &result_valobj_sp, Status &error,
lldb::ValueObjectSP &result_valobj_sp,
std::string *fixed_expression, ValueObject *ctx_obj) {
Log *log(GetLog(LLDBLog::Expressions | LLDBLog::Step));
auto set_error = [&](Status error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the interest of getting rid of Status, should this take an llvm::Error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ValueObject is the prime remaining user of long-lived Status objects, so it would be a purely cosmetic change.

result_valobj_sp = ValueObjectConstResult::Create(
exe_ctx.GetBestExecutionContextScope(), std::move(error));
};

if (ctx_obj) {
static unsigned const ctx_type_mask = lldb::TypeFlags::eTypeIsClass |
Expand All @@ -155,8 +159,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
if (!(ctx_obj->GetTypeInfo() & ctx_type_mask)) {
LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
"an invalid type, can't run expressions.");
error =
Status::FromErrorString("a context object of an invalid type passed");
set_error(Status("a context object of an invalid type passed"));
return lldb::eExpressionSetupError;
}
}
Expand All @@ -168,8 +171,8 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
"a reference type that can't be dereferenced, can't run "
"expressions.");
error = Status::FromErrorString(
"passed context object of an reference type cannot be deferenced");
set_error(Status(
"passed context object of an reference type cannot be deferenced"));
return lldb::eExpressionSetupError;
}

Expand All @@ -181,37 +184,34 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
const ResultType desired_type = options.DoesCoerceToId()
? UserExpression::eResultTypeId
: UserExpression::eResultTypeAny;
lldb::ExpressionResults execution_results = lldb::eExpressionSetupError;

Target *target = exe_ctx.GetTargetPtr();
if (!target) {
LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a NULL target, can't "
"run expressions.");
error = Status::FromErrorString("expression passed a null target");
set_error(Status("expression passed a null target"));
return lldb::eExpressionSetupError;
}

Process *process = exe_ctx.GetProcessPtr();

if (process == nullptr && execution_policy == eExecutionPolicyAlways) {
if (!process && execution_policy == eExecutionPolicyAlways) {
LLDB_LOG(log, "== [UserExpression::Evaluate] No process, but the policy is "
"eExecutionPolicyAlways");

error = Status::FromErrorString(
"expression needed to run but couldn't: no process");
set_error(Status("expression needed to run but couldn't: no process"));

return execution_results;
return lldb::eExpressionSetupError;
}

// Since we might need to allocate memory, we need to be stopped to run
// an expression.
if (process != nullptr && process->GetState() != lldb::eStateStopped) {
error = Status::FromErrorStringWithFormatv(
if (process && process->GetState() != lldb::eStateStopped) {
set_error(Status::FromErrorStringWithFormatv(
"unable to evaluate expression while the process is {0}: the process "
"must be stopped because the expression might require allocating "
"memory.",
StateAsCString(process->GetState()));
return execution_results;
StateAsCString(process->GetState())));
return lldb::eExpressionSetupError;
}

// Explicitly force the IR interpreter to evaluate the expression when the
Expand Down Expand Up @@ -251,13 +251,14 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
language = frame->GetLanguage();
}

Status error;
lldb::UserExpressionSP user_expression_sp(
target->GetUserExpressionForLanguage(expr, full_prefix, language,
desired_type, options, ctx_obj,
error));
target->GetUserExpressionForLanguage(
expr, full_prefix, language, desired_type, options, ctx_obj, error));
if (error.Fail() || !user_expression_sp) {
LLDB_LOG(log, "== [UserExpression::Evaluate] Getting expression: {0} ==",
error.AsCString());
set_error(std::move(error));
return lldb::eExpressionSetupError;
}

Expand All @@ -268,10 +269,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
const bool generate_debug_info = options.GetGenerateDebugInfo();

if (options.InvokeCancelCallback(lldb::eExpressionEvaluationParse)) {
Status error = Status::FromErrorString(
"expression interrupted by callback before parse");
result_valobj_sp = ValueObjectConstResult::Create(
exe_ctx.GetBestExecutionContextScope(), std::move(error));
set_error(Status("expression interrupted by callback before parse"));
return lldb::eExpressionInterrupted;
}

Expand All @@ -287,6 +285,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
fixed_expression = &tmp_fixed_expression;

*fixed_expression = user_expression_sp->GetFixedText().str();
lldb::ExpressionResults execution_results = lldb::eExpressionSetupError;

// If there is a fixed expression, try to parse it:
if (!parse_success) {
Expand Down Expand Up @@ -358,15 +357,13 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
lldb::eExpressionSetupError,
"expression needed to run but couldn't"));
} else if (execution_policy == eExecutionPolicyTopLevel) {
error = Status(UserExpression::kNoResult, lldb::eErrorTypeGeneric);
set_error(Status(UserExpression::kNoResult, lldb::eErrorTypeGeneric));
return lldb::eExpressionCompleted;
} else {
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationExecution)) {
error = Status::FromError(llvm::make_error<ExpressionError>(
set_error(Status::FromError(llvm::make_error<ExpressionError>(
lldb::eExpressionInterrupted,
"expression interrupted by callback before execution"));
result_valobj_sp = ValueObjectConstResult::Create(
exe_ctx.GetBestExecutionContextScope(), std::move(error));
"expression interrupted by callback before execution")));
return lldb::eExpressionInterrupted;
}

Expand Down Expand Up @@ -410,17 +407,14 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
}

if (options.InvokeCancelCallback(lldb::eExpressionEvaluationComplete)) {
error = Status::FromError(llvm::make_error<ExpressionError>(
set_error(Status::FromError(llvm::make_error<ExpressionError>(
lldb::eExpressionInterrupted,
"expression interrupted by callback after complete"));
"expression interrupted by callback after complete")));
return lldb::eExpressionInterrupted;
}

if (result_valobj_sp.get() == nullptr) {
result_valobj_sp = ValueObjectConstResult::Create(
exe_ctx.GetBestExecutionContextScope(), std::move(error));
}

if (error.Fail())
set_error(std::move(error));
return execution_results;
}

Expand Down
6 changes: 3 additions & 3 deletions lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,15 +323,15 @@ StructuredData::ObjectSP InstrumentationRuntimeTSan::RetrieveReportData(

ValueObjectSP main_value;
ExecutionContext exe_ctx;
Status eval_error;
frame_sp->CalculateExecutionContext(exe_ctx);
ExpressionResults result = UserExpression::Evaluate(
exe_ctx, options, thread_sanitizer_retrieve_report_data_command, "",
main_value, eval_error);
main_value);
if (result != eExpressionCompleted) {
StreamString ss;
ss << "cannot evaluate ThreadSanitizer expression:\n";
ss << eval_error.AsCString();
if (main_value)
ss << main_value->GetError().AsCString();
Debugger::ReportWarning(ss.GetString().str(),
process_sp->GetTarget().GetDebugger().GetID());
return StructuredData::ObjectSP();
Expand Down
6 changes: 3 additions & 3 deletions lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,15 @@ StructuredData::ObjectSP InstrumentationRuntimeUBSan::RetrieveReportData(

ValueObjectSP main_value;
ExecutionContext exe_ctx;
Status eval_error;
frame_sp->CalculateExecutionContext(exe_ctx);
ExpressionResults result = UserExpression::Evaluate(
exe_ctx, options, ub_sanitizer_retrieve_report_data_command, "",
main_value, eval_error);
main_value);
if (result != eExpressionCompleted) {
StreamString ss;
ss << "cannot evaluate UndefinedBehaviorSanitizer expression:\n";
ss << eval_error.AsCString();
if (main_value)
ss << main_value->GetError().AsCString();
Debugger::ReportWarning(ss.GetString().str(),
process_sp->GetTarget().GetDebugger().GetID());
return StructuredData::ObjectSP();
Expand Down
6 changes: 3 additions & 3 deletions lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ ReportRetriever::RetrieveReportData(const ProcessSP process_sp) {

ValueObjectSP return_value_sp;
ExecutionContext exe_ctx;
Status eval_error;
frame_sp->CalculateExecutionContext(exe_ctx);
ExpressionResults result = UserExpression::Evaluate(
exe_ctx, options, address_sanitizer_retrieve_report_data_command, "",
return_value_sp, eval_error);
return_value_sp);
if (result != eExpressionCompleted) {
StreamString ss;
ss << "cannot evaluate AddressSanitizer expression:\n";
ss << eval_error.AsCString();
if (return_value_sp)
ss << return_value_sp->GetError().AsCString();
Debugger::ReportWarning(ss.GetString().str(),
process_sp->GetTarget().GetDebugger().GetID());
return StructuredData::ObjectSP();
Expand Down
6 changes: 3 additions & 3 deletions lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ HistoryThreads MemoryHistoryASan::GetHistoryThreads(lldb::addr_t address) {
ExecutionContext exe_ctx(frame_sp);
ValueObjectSP return_value_sp;
StreamString expr;
Status eval_error;
expr.Printf(memory_history_asan_command_format, address, address);

EvaluateExpressionOptions options;
Expand All @@ -176,11 +175,12 @@ HistoryThreads MemoryHistoryASan::GetHistoryThreads(lldb::addr_t address) {
options.SetLanguage(eLanguageTypeObjC_plus_plus);

ExpressionResults expr_result = UserExpression::Evaluate(
exe_ctx, options, expr.GetString(), "", return_value_sp, eval_error);
exe_ctx, options, expr.GetString(), "", return_value_sp);
if (expr_result != eExpressionCompleted) {
StreamString ss;
ss << "cannot evaluate AddressSanitizer expression:\n";
ss << eval_error.AsCString();
if (return_value_sp)
ss << return_value_sp->GetError().AsCString();
Debugger::ReportWarning(ss.GetString().str(),
process_sp->GetTarget().GetDebugger().GetID());
return result;
Expand Down
9 changes: 4 additions & 5 deletions lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,12 +536,11 @@ Status PlatformPOSIX::EvaluateLibdlExpression(
// don't do the work to trap them.
expr_options.SetTimeout(process->GetUtilityExpressionTimeout());

Status expr_error;
ExpressionResults result =
UserExpression::Evaluate(exe_ctx, expr_options, expr_cstr, expr_prefix,
result_valobj_sp, expr_error);
ExpressionResults result = UserExpression::Evaluate(
exe_ctx, expr_options, expr_cstr, expr_prefix, result_valobj_sp);
if (result != eExpressionCompleted)
return expr_error;
return result_valobj_sp ? result_valobj_sp->GetError().Clone()
: Status("unknown error");

if (result_valobj_sp->GetError().Fail())
return result_valobj_sp->GetError().Clone();
Expand Down
7 changes: 3 additions & 4 deletions lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,13 +798,12 @@ extern "C" {
options.SetTrapExceptions(false);
options.SetTimeout(process->GetUtilityExpressionTimeout());

Status error;
ExpressionResults result = UserExpression::Evaluate(
context, options, expression, kLoaderDecls, value, error);
context, options, expression, kLoaderDecls, value);
if (result != eExpressionCompleted)
return error;
return value ? value->GetError().Clone() : Status("unknown error");

if (value->GetError().Fail())
if (value && value->GetError().Fail())
return value->GetError().Clone();

return Status();
Expand Down
8 changes: 5 additions & 3 deletions lldb/source/Target/StopInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -932,10 +932,9 @@ class StopInfoWatchpoint : public StopInfo {
expr_options.SetUnwindOnError(true);
expr_options.SetIgnoreBreakpoints(true);
ValueObjectSP result_value_sp;
Status error;
result_code = UserExpression::Evaluate(
exe_ctx, expr_options, wp_sp->GetConditionText(),
llvm::StringRef(), result_value_sp, error);
llvm::StringRef(), result_value_sp);

if (result_code == eExpressionCompleted) {
if (result_value_sp) {
Expand All @@ -959,7 +958,10 @@ class StopInfoWatchpoint : public StopInfo {
}
}
} else {
const char *err_str = error.AsCString("<unknown error>");
const char *err_str = "<unknown error>";
if (result_value_sp)
err_str = result_value_sp->GetError().AsCString();

LLDB_LOGF(log, "Error evaluating condition: \"%s\"\n", err_str);

StreamString strm;
Expand Down
11 changes: 3 additions & 8 deletions lldb/source/Target/Target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2842,14 +2842,9 @@ ExpressionResults Target::EvaluateExpression(
execution_results = eExpressionCompleted;
} else {
llvm::StringRef prefix = GetExpressionPrefixContents();
Status error;
execution_results = UserExpression::Evaluate(exe_ctx, options, expr, prefix,
result_valobj_sp, error,
fixed_expression, ctx_obj);
// Pass up the error by wrapping it inside an error result.
if (error.Fail() && !result_valobj_sp)
result_valobj_sp = ValueObjectConstResult::Create(
exe_ctx.GetBestExecutionContextScope(), std::move(error));
execution_results =
UserExpression::Evaluate(exe_ctx, options, expr, prefix,
result_valobj_sp, fixed_expression, ctx_obj);
}

if (execution_results == eExpressionCompleted)
Expand Down
2 changes: 1 addition & 1 deletion lldb/test/API/commands/expression/fixits/TestFixIts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def test_with_target(self):
expr = "struct MyTy { int m; }; MyTy x; MyTy *ptr = &x; int m = ptr.m;"
value = frame.EvaluateExpression(expr, top_level_options)
# A successfully parsed top-level expression will yield an
# unknown error . If a parsing error would have happened we
# unknown error. If a parsing error would have happened we
# would get a different error kind, so let's check the error
# kind here.
self.assertEqual(value.GetError().GetCString(), "unknown error")
Expand Down
Loading