Skip to content

Commit 596eb40

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 1466711 commit 596eb40

File tree

12 files changed

+60
-75
lines changed

12 files changed

+60
-75
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: 26 additions & 31 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

@@ -187,30 +190,29 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
187190
if (!target) {
188191
LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a NULL target, can't "
189192
"run expressions.");
190-
error = Status::FromErrorString("expression passed a null target");
193+
set_error(Status("expression passed a null target"));
191194
return lldb::eExpressionSetupError;
192195
}
193196

194197
Process *process = exe_ctx.GetProcessPtr();
195198

196-
if (process == nullptr && execution_policy == eExecutionPolicyAlways) {
199+
if (process && execution_policy == eExecutionPolicyAlways) {
197200
LLDB_LOG(log, "== [UserExpression::Evaluate] No process, but the policy is "
198201
"eExecutionPolicyAlways");
199202

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

203205
return execution_results;
204206
}
205207

206208
// Since we might need to allocate memory, we need to be stopped to run
207209
// an expression.
208-
if (process != nullptr && process->GetState() != lldb::eStateStopped) {
209-
error = Status::FromErrorStringWithFormatv(
210+
if (process && process->GetState() != lldb::eStateStopped) {
211+
set_error(Status::FromErrorStringWithFormatv(
210212
"unable to evaluate expression while the process is {0}: the process "
211213
"must be stopped because the expression might require allocating "
212214
"memory.",
213-
StateAsCString(process->GetState()));
215+
StateAsCString(process->GetState())));
214216
return execution_results;
215217
}
216218

@@ -251,13 +253,14 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
251253
language = frame->GetLanguage();
252254
}
253255

256+
Status error;
254257
lldb::UserExpressionSP user_expression_sp(
255-
target->GetUserExpressionForLanguage(expr, full_prefix, language,
256-
desired_type, options, ctx_obj,
257-
error));
258+
target->GetUserExpressionForLanguage(
259+
expr, full_prefix, language, desired_type, options, ctx_obj, error));
258260
if (error.Fail() || !user_expression_sp) {
259261
LLDB_LOG(log, "== [UserExpression::Evaluate] Getting expression: {0} ==",
260262
error.AsCString());
263+
set_error(std::move(error));
261264
return lldb::eExpressionSetupError;
262265
}
263266

@@ -268,10 +271,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
268271
const bool generate_debug_info = options.GetGenerateDebugInfo();
269272

270273
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));
274+
set_error(Status("expression interrupted by callback before parse"));
275275
return lldb::eExpressionInterrupted;
276276
}
277277

@@ -358,15 +358,13 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
358358
lldb::eExpressionSetupError,
359359
"expression needed to run but couldn't"));
360360
} else if (execution_policy == eExecutionPolicyTopLevel) {
361-
error = Status(UserExpression::kNoResult, lldb::eErrorTypeGeneric);
361+
set_error(Status(UserExpression::kNoResult, lldb::eErrorTypeGeneric));
362362
return lldb::eExpressionCompleted;
363363
} else {
364364
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationExecution)) {
365-
error = Status::FromError(llvm::make_error<ExpressionError>(
365+
set_error(Status::FromError(llvm::make_error<ExpressionError>(
366366
lldb::eExpressionInterrupted,
367-
"expression interrupted by callback before execution"));
368-
result_valobj_sp = ValueObjectConstResult::Create(
369-
exe_ctx.GetBestExecutionContextScope(), std::move(error));
367+
"expression interrupted by callback before execution")));
370368
return lldb::eExpressionInterrupted;
371369
}
372370

@@ -410,17 +408,14 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
410408
}
411409

412410
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationComplete)) {
413-
error = Status::FromError(llvm::make_error<ExpressionError>(
411+
set_error(Status::FromError(llvm::make_error<ExpressionError>(
414412
lldb::eExpressionInterrupted,
415-
"expression interrupted by callback after complete"));
413+
"expression interrupted by callback after complete")));
416414
return lldb::eExpressionInterrupted;
417415
}
418416

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

‎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)