Skip to content

Commit 208fa0a

Browse files
adrian-prantladrian-prantl
committed
[lldb] Refactor UserExpression::Evaluate to only have one error channel.
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.
1 parent 29afbd5 commit 208fa0a

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
@@ -240,11 +240,9 @@ class UserExpression : public Expression {
240240
/// definitions to be included when the expression is parsed.
241241
///
242242
/// \param[in,out] result_valobj_sp
243-
/// If execution is successful, the result valobj is placed here.
244-
///
245-
/// \param[out] error
246-
/// Filled in with an error in case the expression evaluation
247-
/// fails to parse, run, or evaluated.
243+
/// If execution is successful, the result valobj is placed
244+
/// here. Otherwise its Error will contain an ExpressionError
245+
/// with details about the failure mode.
248246
///
249247
/// \param[out] fixed_expression
250248
/// If non-nullptr, the fixed expression is copied into the provided
@@ -266,7 +264,7 @@ class UserExpression : public Expression {
266264
static lldb::ExpressionResults
267265
Evaluate(ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
268266
llvm::StringRef expr_cstr, llvm::StringRef expr_prefix,
269-
lldb::ValueObjectSP &result_valobj_sp, Status &error,
267+
lldb::ValueObjectSP &result_valobj_sp,
270268
std::string *fixed_expression = nullptr,
271269
ValueObject *ctx_obj = nullptr);
272270

‎lldb/source/Expression/REPL.cpp‎

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

340340
const char *expr_prefix = nullptr;
341341
lldb::ValueObjectSP result_valobj_sp;
342+
lldb::ExpressionResults execution_results = UserExpression::Evaluate(
343+
exe_ctx, expr_options, code.c_str(), expr_prefix, result_valobj_sp);
342344
Status error;
343-
lldb::ExpressionResults execution_results =
344-
UserExpression::Evaluate(exe_ctx, expr_options, code.c_str(),
345-
expr_prefix, result_valobj_sp, error,
346-
nullptr); // fixed expression
347-
348345
if (llvm::Error err = OnExpressionEvaluated(exe_ctx, code, expr_options,
349346
execution_results,
350347
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
// If there is a fixed expression, try to parse it:
292291
if (!parse_success) {
@@ -358,15 +357,13 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
358357
lldb::eExpressionSetupError,
359358
"expression needed to run but couldn't"));
360359
} else if (execution_policy == eExecutionPolicyTopLevel) {
361-
error = Status(UserExpression::kNoResult, lldb::eErrorTypeGeneric);
360+
set_error(Status(UserExpression::kNoResult, lldb::eErrorTypeGeneric));
362361
return lldb::eExpressionCompleted;
363362
} else {
364363
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationExecution)) {
365-
error = Status::FromError(llvm::make_error<ExpressionError>(
364+
set_error(Status::FromError(llvm::make_error<ExpressionError>(
366365
lldb::eExpressionInterrupted,
367-
"expression interrupted by callback before execution"));
368-
result_valobj_sp = ValueObjectConstResult::Create(
369-
exe_ctx.GetBestExecutionContextScope(), std::move(error));
366+
"expression interrupted by callback before execution")));
370367
return lldb::eExpressionInterrupted;
371368
}
372369

@@ -410,17 +407,14 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
410407
}
411408

412409
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationComplete)) {
413-
error = Status::FromError(llvm::make_error<ExpressionError>(
410+
set_error(Status::FromError(llvm::make_error<ExpressionError>(
414411
lldb::eExpressionInterrupted,
415-
"expression interrupted by callback after complete"));
412+
"expression interrupted by callback after complete")));
416413
return lldb::eExpressionInterrupted;
417414
}
418415

419-
if (result_valobj_sp.get() == nullptr) {
420-
result_valobj_sp = ValueObjectConstResult::Create(
421-
exe_ctx.GetBestExecutionContextScope(), std::move(error));
422-
}
423-
416+
if (error.Fail())
417+
set_error(std::move(error));
424418
return execution_results;
425419
}
426420

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

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

324324
ValueObjectSP main_value;
325325
ExecutionContext exe_ctx;
326-
Status eval_error;
327326
frame_sp->CalculateExecutionContext(exe_ctx);
328327
ExpressionResults result = UserExpression::Evaluate(
329328
exe_ctx, options, thread_sanitizer_retrieve_report_data_command, "",
330-
main_value, eval_error);
329+
main_value);
331330
if (result != eExpressionCompleted) {
332331
StreamString ss;
333332
ss << "cannot evaluate ThreadSanitizer expression:\n";
334-
ss << eval_error.AsCString();
333+
if (main_value)
334+
ss << main_value->GetError().AsCString();
335335
Debugger::ReportWarning(ss.GetString().str(),
336336
process_sp->GetTarget().GetDebugger().GetID());
337337
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
@@ -932,10 +932,9 @@ class StopInfoWatchpoint : public StopInfo {
932932
expr_options.SetUnwindOnError(true);
933933
expr_options.SetIgnoreBreakpoints(true);
934934
ValueObjectSP result_value_sp;
935-
Status error;
936935
result_code = UserExpression::Evaluate(
937936
exe_ctx, expr_options, wp_sp->GetConditionText(),
938-
llvm::StringRef(), result_value_sp, error);
937+
llvm::StringRef(), result_value_sp);
939938

940939
if (result_code == eExpressionCompleted) {
941940
if (result_value_sp) {
@@ -959,7 +958,10 @@ class StopInfoWatchpoint : public StopInfo {
959958
}
960959
}
961960
} else {
962-
const char *err_str = error.AsCString("<unknown error>");
961+
const char *err_str = "<unknown error>";
962+
if (result_value_sp)
963+
err_str = result_value_sp->GetError().AsCString();
964+
963965
LLDB_LOGF(log, "Error evaluating condition: \"%s\"\n", err_str);
964966

965967
StreamString strm;

0 commit comments

Comments
 (0)