Skip to content

Commit 67e22cb

Browse files
adrian-prantladrian-prantl
committed
[lldb] Refactor UserExpression::Evaluate to only have one error channel. (llvm#117186)
Prior to this patch, the function returned an exit status, sometimes a ValueObject with an error and a Status object. This patch removes the Status object and ensures the error is consistently returned as the error of the ValueObject. (cherry picked from commit 6a8a4d5)
1 parent 68d11b7 commit 67e22cb

File tree

12 files changed

+63
-79
lines changed

12 files changed

+63
-79
lines changed

‎lldb/include/lldb/Expression/UserExpression.h‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -239,11 +239,9 @@ class UserExpression : public Expression {
239239
/// definitions to be included when the expression is parsed.
240240
///
241241
/// \param[in,out] result_valobj_sp
242-
/// If execution is successful, the result valobj is placed here.
243-
///
244-
/// \param[out] error
245-
/// Filled in with an error in case the expression evaluation
246-
/// fails to parse, run, or evaluated.
242+
/// If execution is successful, the result valobj is placed
243+
/// here. Otherwise its Error will contain an ExpressionError
244+
/// with details about the failure mode.
247245
///
248246
/// \param[out] fixed_expression
249247
/// If non-nullptr, the fixed expression is copied into the provided
@@ -265,7 +263,7 @@ class UserExpression : public Expression {
265263
static lldb::ExpressionResults
266264
Evaluate(ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
267265
llvm::StringRef expr_cstr, llvm::StringRef expr_prefix,
268-
lldb::ValueObjectSP &result_valobj_sp, Status &error,
266+
lldb::ValueObjectSP &result_valobj_sp,
269267
std::string *fixed_expression = nullptr,
270268
ValueObject *ctx_obj = nullptr);
271269

‎lldb/source/Expression/REPL.cpp‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -359,12 +359,9 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) {
359359

360360
const char *expr_prefix = nullptr;
361361
lldb::ValueObjectSP result_valobj_sp;
362+
lldb::ExpressionResults execution_results = UserExpression::Evaluate(
363+
exe_ctx, expr_options, code.c_str(), expr_prefix, result_valobj_sp);
362364
Status error;
363-
lldb::ExpressionResults execution_results =
364-
UserExpression::Evaluate(exe_ctx, expr_options, code.c_str(),
365-
expr_prefix, result_valobj_sp, error,
366-
nullptr); // fixed expression
367-
368365
if (llvm::Error err = OnExpressionEvaluated(exe_ctx, code, expr_options,
369366
execution_results,
370367
result_valobj_sp, error)) {

‎lldb/source/Expression/UserExpression.cpp‎

Lines changed: 29 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,13 @@ lldb::ExpressionResults
144144
UserExpression::Evaluate(ExecutionContext &exe_ctx,
145145
const EvaluateExpressionOptions &options,
146146
llvm::StringRef expr, llvm::StringRef prefix,
147-
lldb::ValueObjectSP &result_valobj_sp, Status &error,
147+
lldb::ValueObjectSP &result_valobj_sp,
148148
std::string *fixed_expression, ValueObject *ctx_obj) {
149149
Log *log(GetLog(LLDBLog::Expressions | LLDBLog::Step));
150+
auto set_error = [&](Status error) {
151+
result_valobj_sp = ValueObjectConstResult::Create(
152+
exe_ctx.GetBestExecutionContextScope(), std::move(error));
153+
};
150154

151155
if (ctx_obj) {
152156
static unsigned const ctx_type_mask = lldb::TypeFlags::eTypeIsClass |
@@ -155,8 +159,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
155159
if (!(ctx_obj->GetTypeInfo() & ctx_type_mask)) {
156160
LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
157161
"an invalid type, can't run expressions.");
158-
error =
159-
Status::FromErrorString("a context object of an invalid type passed");
162+
set_error(Status("a context object of an invalid type passed"));
160163
return lldb::eExpressionSetupError;
161164
}
162165
}
@@ -168,8 +171,8 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
168171
LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
169172
"a reference type that can't be dereferenced, can't run "
170173
"expressions.");
171-
error = Status::FromErrorString(
172-
"passed context object of an reference type cannot be deferenced");
174+
set_error(Status(
175+
"passed context object of an reference type cannot be deferenced"));
173176
return lldb::eExpressionSetupError;
174177
}
175178

@@ -181,37 +184,34 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
181184
const ResultType desired_type = options.DoesCoerceToId()
182185
? UserExpression::eResultTypeId
183186
: UserExpression::eResultTypeAny;
184-
lldb::ExpressionResults execution_results = lldb::eExpressionSetupError;
185-
186187
Target *target = exe_ctx.GetTargetPtr();
187188
if (!target) {
188189
LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a NULL target, can't "
189190
"run expressions.");
190-
error = Status::FromErrorString("expression passed a null target");
191+
set_error(Status("expression passed a null target"));
191192
return lldb::eExpressionSetupError;
192193
}
193194

194195
Process *process = exe_ctx.GetProcessPtr();
195196

196-
if (process == nullptr && execution_policy == eExecutionPolicyAlways) {
197+
if (!process && execution_policy == eExecutionPolicyAlways) {
197198
LLDB_LOG(log, "== [UserExpression::Evaluate] No process, but the policy is "
198199
"eExecutionPolicyAlways");
199200

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

203-
return execution_results;
203+
return lldb::eExpressionSetupError;
204204
}
205205

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

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

254+
Status error;
254255
lldb::UserExpressionSP user_expression_sp(
255-
target->GetUserExpressionForLanguage(expr, full_prefix, language,
256-
desired_type, options, ctx_obj,
257-
error));
256+
target->GetUserExpressionForLanguage(
257+
expr, full_prefix, language, desired_type, options, ctx_obj, error));
258258
if (error.Fail() || !user_expression_sp) {
259259
LLDB_LOG(log, "== [UserExpression::Evaluate] Getting expression: {0} ==",
260260
error.AsCString());
261+
set_error(std::move(error));
261262
return lldb::eExpressionSetupError;
262263
}
263264

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

270271
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationParse)) {
271-
Status error = Status::FromErrorString(
272-
"expression interrupted by callback before parse");
273-
result_valobj_sp = ValueObjectConstResult::Create(
274-
exe_ctx.GetBestExecutionContextScope(), std::move(error));
272+
set_error(Status("expression interrupted by callback before parse"));
275273
return lldb::eExpressionInterrupted;
276274
}
277275

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

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

291290
// Swift Playgrounds disable fixits, but SwiftASTContext may get
292291
// poisoned (see SwiftASTContextForExpressions::ModulesDidLoad())
@@ -365,15 +364,13 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
365364
lldb::eExpressionSetupError,
366365
"expression needed to run but couldn't"));
367366
} else if (execution_policy == eExecutionPolicyTopLevel) {
368-
error = Status(UserExpression::kNoResult, lldb::eErrorTypeGeneric);
367+
set_error(Status(UserExpression::kNoResult, lldb::eErrorTypeGeneric));
369368
return lldb::eExpressionCompleted;
370369
} else {
371370
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationExecution)) {
372-
error = Status::FromError(llvm::make_error<ExpressionError>(
371+
set_error(Status::FromError(llvm::make_error<ExpressionError>(
373372
lldb::eExpressionInterrupted,
374-
"expression interrupted by callback before execution"));
375-
result_valobj_sp = ValueObjectConstResult::Create(
376-
exe_ctx.GetBestExecutionContextScope(), std::move(error));
373+
"expression interrupted by callback before execution")));
377374
return lldb::eExpressionInterrupted;
378375
}
379376

@@ -417,17 +414,14 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
417414
}
418415

419416
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationComplete)) {
420-
error = Status::FromError(llvm::make_error<ExpressionError>(
417+
set_error(Status::FromError(llvm::make_error<ExpressionError>(
421418
lldb::eExpressionInterrupted,
422-
"expression interrupted by callback after complete"));
419+
"expression interrupted by callback after complete")));
423420
return lldb::eExpressionInterrupted;
424421
}
425422

426-
if (result_valobj_sp.get() == nullptr) {
427-
result_valobj_sp = ValueObjectConstResult::Create(
428-
exe_ctx.GetBestExecutionContextScope(), std::move(error));
429-
}
430-
423+
if (error.Fail())
424+
set_error(std::move(error));
431425
return execution_results;
432426
}
433427

‎lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,15 +329,15 @@ StructuredData::ObjectSP InstrumentationRuntimeTSan::RetrieveReportData(
329329

330330
ValueObjectSP main_value;
331331
ExecutionContext exe_ctx;
332-
Status eval_error;
333332
frame_sp->CalculateExecutionContext(exe_ctx);
334333
ExpressionResults result = UserExpression::Evaluate(
335334
exe_ctx, options, thread_sanitizer_retrieve_report_data_command, "",
336-
main_value, eval_error);
335+
main_value);
337336
if (result != eExpressionCompleted) {
338337
StreamString ss;
339338
ss << "cannot evaluate ThreadSanitizer expression:\n";
340-
ss << eval_error.AsCString();
339+
if (main_value)
340+
ss << main_value->GetError().AsCString();
341341
Debugger::ReportWarning(ss.GetString().str(),
342342
process_sp->GetTarget().GetDebugger().GetID());
343343
return StructuredData::ObjectSP();

‎lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,15 +130,15 @@ StructuredData::ObjectSP InstrumentationRuntimeUBSan::RetrieveReportData(
130130

131131
ValueObjectSP main_value;
132132
ExecutionContext exe_ctx;
133-
Status eval_error;
134133
frame_sp->CalculateExecutionContext(exe_ctx);
135134
ExpressionResults result = UserExpression::Evaluate(
136135
exe_ctx, options, ub_sanitizer_retrieve_report_data_command, "",
137-
main_value, eval_error);
136+
main_value);
138137
if (result != eExpressionCompleted) {
139138
StreamString ss;
140139
ss << "cannot evaluate UndefinedBehaviorSanitizer expression:\n";
141-
ss << eval_error.AsCString();
140+
if (main_value)
141+
ss << main_value->GetError().AsCString();
142142
Debugger::ReportWarning(ss.GetString().str(),
143143
process_sp->GetTarget().GetDebugger().GetID());
144144
return StructuredData::ObjectSP();

‎lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,15 @@ ReportRetriever::RetrieveReportData(const ProcessSP process_sp) {
8484

8585
ValueObjectSP return_value_sp;
8686
ExecutionContext exe_ctx;
87-
Status eval_error;
8887
frame_sp->CalculateExecutionContext(exe_ctx);
8988
ExpressionResults result = UserExpression::Evaluate(
9089
exe_ctx, options, address_sanitizer_retrieve_report_data_command, "",
91-
return_value_sp, eval_error);
90+
return_value_sp);
9291
if (result != eExpressionCompleted) {
9392
StreamString ss;
9493
ss << "cannot evaluate AddressSanitizer expression:\n";
95-
ss << eval_error.AsCString();
94+
if (return_value_sp)
95+
ss << return_value_sp->GetError().AsCString();
9696
Debugger::ReportWarning(ss.GetString().str(),
9797
process_sp->GetTarget().GetDebugger().GetID());
9898
return StructuredData::ObjectSP();

‎lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ HistoryThreads MemoryHistoryASan::GetHistoryThreads(lldb::addr_t address) {
162162
ExecutionContext exe_ctx(frame_sp);
163163
ValueObjectSP return_value_sp;
164164
StreamString expr;
165-
Status eval_error;
166165
expr.Printf(memory_history_asan_command_format, address, address);
167166

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

178177
ExpressionResults expr_result = UserExpression::Evaluate(
179-
exe_ctx, options, expr.GetString(), "", return_value_sp, eval_error);
178+
exe_ctx, options, expr.GetString(), "", return_value_sp);
180179
if (expr_result != eExpressionCompleted) {
181180
StreamString ss;
182181
ss << "cannot evaluate AddressSanitizer expression:\n";
183-
ss << eval_error.AsCString();
182+
if (return_value_sp)
183+
ss << return_value_sp->GetError().AsCString();
184184
Debugger::ReportWarning(ss.GetString().str(),
185185
process_sp->GetTarget().GetDebugger().GetID());
186186
return result;

‎lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -536,12 +536,11 @@ Status PlatformPOSIX::EvaluateLibdlExpression(
536536
// don't do the work to trap them.
537537
expr_options.SetTimeout(process->GetUtilityExpressionTimeout());
538538

539-
Status expr_error;
540-
ExpressionResults result =
541-
UserExpression::Evaluate(exe_ctx, expr_options, expr_cstr, expr_prefix,
542-
result_valobj_sp, expr_error);
539+
ExpressionResults result = UserExpression::Evaluate(
540+
exe_ctx, expr_options, expr_cstr, expr_prefix, result_valobj_sp);
543541
if (result != eExpressionCompleted)
544-
return expr_error;
542+
return result_valobj_sp ? result_valobj_sp->GetError().Clone()
543+
: Status("unknown error");
545544

546545
if (result_valobj_sp->GetError().Fail())
547546
return result_valobj_sp->GetError().Clone();

‎lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -798,13 +798,12 @@ extern "C" {
798798
options.SetTrapExceptions(false);
799799
options.SetTimeout(process->GetUtilityExpressionTimeout());
800800

801-
Status error;
802801
ExpressionResults result = UserExpression::Evaluate(
803-
context, options, expression, kLoaderDecls, value, error);
802+
context, options, expression, kLoaderDecls, value);
804803
if (result != eExpressionCompleted)
805-
return error;
804+
return value ? value->GetError().Clone() : Status("unknown error");
806805

807-
if (value->GetError().Fail())
806+
if (value && value->GetError().Fail())
808807
return value->GetError().Clone();
809808

810809
return Status();

‎lldb/source/Target/StopInfo.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -915,10 +915,9 @@ class StopInfoWatchpoint : public StopInfo {
915915
expr_options.SetUnwindOnError(true);
916916
expr_options.SetIgnoreBreakpoints(true);
917917
ValueObjectSP result_value_sp;
918-
Status error;
919918
result_code = UserExpression::Evaluate(
920919
exe_ctx, expr_options, wp_sp->GetConditionText(),
921-
llvm::StringRef(), result_value_sp, error);
920+
llvm::StringRef(), result_value_sp);
922921

923922
if (result_code == eExpressionCompleted) {
924923
if (result_value_sp) {
@@ -942,7 +941,10 @@ class StopInfoWatchpoint : public StopInfo {
942941
}
943942
}
944943
} else {
945-
const char *err_str = error.AsCString("<unknown error>");
944+
const char *err_str = "<unknown error>";
945+
if (result_value_sp)
946+
err_str = result_value_sp->GetError().AsCString();
947+
946948
LLDB_LOGF(log, "Error evaluating condition: \"%s\"\n", err_str);
947949

948950
StreamString strm;

0 commit comments

Comments
 (0)