From 1f454c7f1637f5d89f2a332d89d614680167d157 Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Tue, 29 Oct 2019 15:13:02 +0100 Subject: [PATCH] [lldb][NFC] Make LLVMUserExpression::DoExecute return early The giant if-else isn't conforming to LLVM code style. (cherry picked from commit 3011c7eb31c58526066841e84e7f0a6b9b733b57) Thank you compnerd for doing the resolution! --- lldb/source/Expression/LLVMUserExpression.cpp | 319 ++++++++---------- 1 file changed, 150 insertions(+), 169 deletions(-) diff --git a/lldb/source/Expression/LLVMUserExpression.cpp b/lldb/source/Expression/LLVMUserExpression.cpp index 9c0174bfc0f5b1..aac7968bbd6f40 100644 --- a/lldb/source/Expression/LLVMUserExpression.cpp +++ b/lldb/source/Expression/LLVMUserExpression.cpp @@ -70,208 +70,189 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager, Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EXPRESSIONS | LIBLLDB_LOG_STEP)); - if (m_jit_start_addr != LLDB_INVALID_ADDRESS || m_can_interpret) { - lldb::addr_t struct_address = LLDB_INVALID_ADDRESS; + if (m_jit_start_addr == LLDB_INVALID_ADDRESS && !m_can_interpret) { + diagnostic_manager.PutString( + eDiagnosticSeverityError, + "Expression can't be run, because there is no JIT compiled function"); + return lldb::eExpressionSetupError; + } - if (!PrepareToExecuteJITExpression(diagnostic_manager, exe_ctx, - struct_address)) { - diagnostic_manager.Printf( - eDiagnosticSeverityError, - "errored out in %s, couldn't PrepareToExecuteJITExpression", - __FUNCTION__); - return lldb::eExpressionSetupError; - } + lldb::addr_t struct_address = LLDB_INVALID_ADDRESS; - lldb::addr_t function_stack_bottom = LLDB_INVALID_ADDRESS; - lldb::addr_t function_stack_top = LLDB_INVALID_ADDRESS; + if (!PrepareToExecuteJITExpression(diagnostic_manager, exe_ctx, + struct_address)) { + diagnostic_manager.Printf( + eDiagnosticSeverityError, + "errored out in %s, couldn't PrepareToExecuteJITExpression", + __FUNCTION__); + return lldb::eExpressionSetupError; + } - lldb::ValueObjectSP error_backstop_result_sp; + lldb::addr_t function_stack_bottom = LLDB_INVALID_ADDRESS; + lldb::addr_t function_stack_top = LLDB_INVALID_ADDRESS; - if (m_can_interpret) { - llvm::Module *module = m_execution_unit_sp->GetModule(); - llvm::Function *function = m_execution_unit_sp->GetFunction(); + if (m_can_interpret) { + llvm::Module *module = m_execution_unit_sp->GetModule(); + llvm::Function *function = m_execution_unit_sp->GetFunction(); - if (!module || !function) { - diagnostic_manager.PutString( - eDiagnosticSeverityError, - "supposed to interpret, but nothing is there"); - return lldb::eExpressionSetupError; - } + if (!module || !function) { + diagnostic_manager.PutString( + eDiagnosticSeverityError, + "supposed to interpret, but nothing is there"); + return lldb::eExpressionSetupError; + } - Status interpreter_error; + Status interpreter_error; - std::vector args; + std::vector args; - if (!AddArguments(exe_ctx, args, struct_address, diagnostic_manager)) { - diagnostic_manager.Printf(eDiagnosticSeverityError, - "errored out in %s, couldn't AddArguments", - __FUNCTION__); - return lldb::eExpressionSetupError; - } - - function_stack_bottom = m_stack_frame_bottom; - function_stack_top = m_stack_frame_top; + if (!AddArguments(exe_ctx, args, struct_address, diagnostic_manager)) { + diagnostic_manager.Printf(eDiagnosticSeverityError, + "errored out in %s, couldn't AddArguments", + __FUNCTION__); + return lldb::eExpressionSetupError; + } - IRInterpreter::Interpret(*module, *function, args, *m_execution_unit_sp, - interpreter_error, function_stack_bottom, - function_stack_top, exe_ctx); + function_stack_bottom = m_stack_frame_bottom; + function_stack_top = m_stack_frame_top; - if (!interpreter_error.Success()) { - diagnostic_manager.Printf(eDiagnosticSeverityError, - "supposed to interpret, but failed: %s", - interpreter_error.AsCString()); - return lldb::eExpressionDiscarded; - } - } else { - if (!exe_ctx.HasThreadScope()) { - diagnostic_manager.Printf(eDiagnosticSeverityError, - "%s called with no thread selected", - __FUNCTION__); - return lldb::eExpressionSetupError; - } + IRInterpreter::Interpret(*module, *function, args, *m_execution_unit_sp, + interpreter_error, function_stack_bottom, + function_stack_top, exe_ctx); - Address wrapper_address(m_jit_start_addr); + if (!interpreter_error.Success()) { + diagnostic_manager.Printf(eDiagnosticSeverityError, + "supposed to interpret, but failed: %s", + interpreter_error.AsCString()); + return lldb::eExpressionDiscarded; + } + } else { + if (!exe_ctx.HasThreadScope()) { + diagnostic_manager.Printf(eDiagnosticSeverityError, + "%s called with no thread selected", + __FUNCTION__); + return lldb::eExpressionSetupError; + } - std::vector args; + Address wrapper_address(m_jit_start_addr); - if (!AddArguments(exe_ctx, args, struct_address, diagnostic_manager)) { - diagnostic_manager.Printf(eDiagnosticSeverityError, - "errored out in %s, couldn't AddArguments", - __FUNCTION__); - return lldb::eExpressionSetupError; - } + std::vector args; - lldb::ThreadPlanSP call_plan_sp(new ThreadPlanCallUserExpression( - exe_ctx.GetThreadRef(), wrapper_address, args, options, - shared_ptr_to_me)); + if (!AddArguments(exe_ctx, args, struct_address, diagnostic_manager)) { + diagnostic_manager.Printf(eDiagnosticSeverityError, + "errored out in %s, couldn't AddArguments", + __FUNCTION__); + return lldb::eExpressionSetupError; + } - StreamString ss; - if (!call_plan_sp || !call_plan_sp->ValidatePlan(&ss)) { - diagnostic_manager.PutString(eDiagnosticSeverityError, ss.GetString()); - return lldb::eExpressionSetupError; - } + lldb::ThreadPlanSP call_plan_sp(new ThreadPlanCallUserExpression( + exe_ctx.GetThreadRef(), wrapper_address, args, options, + shared_ptr_to_me)); - ThreadPlanCallUserExpression *user_expression_plan = - static_cast(call_plan_sp.get()); + StreamString ss; + if (!call_plan_sp || !call_plan_sp->ValidatePlan(&ss)) { + diagnostic_manager.PutString(eDiagnosticSeverityError, ss.GetString()); + return lldb::eExpressionSetupError; + } - lldb::addr_t function_stack_pointer = - user_expression_plan->GetFunctionStackPointer(); + ThreadPlanCallUserExpression *user_expression_plan = + static_cast(call_plan_sp.get()); - function_stack_bottom = function_stack_pointer - HostInfo::GetPageSize(); - function_stack_top = function_stack_pointer; + lldb::addr_t function_stack_pointer = + user_expression_plan->GetFunctionStackPointer(); - LLDB_LOGF( - log, - "-- [UserExpression::Execute] Execution of expression begins --"); + function_stack_bottom = function_stack_pointer - HostInfo::GetPageSize(); + function_stack_top = function_stack_pointer; - if (exe_ctx.GetProcessPtr()) - exe_ctx.GetProcessPtr()->SetRunningUserExpression(true); + LLDB_LOGF(log, + "-- [UserExpression::Execute] Execution of expression begins --"); - lldb::ExpressionResults execution_result = - exe_ctx.GetProcessRef().RunThreadPlan(exe_ctx, call_plan_sp, options, - diagnostic_manager); + if (exe_ctx.GetProcessPtr()) + exe_ctx.GetProcessPtr()->SetRunningUserExpression(true); - if (exe_ctx.GetProcessPtr()) - exe_ctx.GetProcessPtr()->SetRunningUserExpression(false); + lldb::ExpressionResults execution_result = + exe_ctx.GetProcessRef().RunThreadPlan(exe_ctx, call_plan_sp, options, + diagnostic_manager); - LLDB_LOGF(log, "-- [UserExpression::Execute] Execution of expression " - "completed --"); + if (exe_ctx.GetProcessPtr()) + exe_ctx.GetProcessPtr()->SetRunningUserExpression(false); - if (execution_result == lldb::eExpressionInterrupted || - execution_result == lldb::eExpressionHitBreakpoint) { - const char *error_desc = nullptr; + LLDB_LOGF(log, "-- [UserExpression::Execute] Execution of expression " + "completed --"); - if (call_plan_sp) { - lldb::StopInfoSP real_stop_info_sp = call_plan_sp->GetRealStopInfo(); - if (real_stop_info_sp) - error_desc = real_stop_info_sp->GetDescription(); - } - if (error_desc) - diagnostic_manager.Printf(eDiagnosticSeverityError, - "Execution was interrupted, reason: %s.", - error_desc); - else - diagnostic_manager.PutString(eDiagnosticSeverityError, - "Execution was interrupted."); - - if ((execution_result == lldb::eExpressionInterrupted && - options.DoesUnwindOnError()) || - (execution_result == lldb::eExpressionHitBreakpoint && - options.DoesIgnoreBreakpoints())) - diagnostic_manager.AppendMessageToDiagnostic( - "The process has been returned to the state before expression " - "evaluation."); - else { - if (execution_result == lldb::eExpressionHitBreakpoint) - user_expression_plan->TransferExpressionOwnership(); - diagnostic_manager.AppendMessageToDiagnostic( - "The process has been left at the point where it was " - "interrupted, " - "use \"thread return -x\" to return to the state before " - "expression evaluation."); - } + if (execution_result == lldb::eExpressionInterrupted || + execution_result == lldb::eExpressionHitBreakpoint) { + const char *error_desc = nullptr; - return execution_result; - } else if (execution_result == lldb::eExpressionStoppedForDebug) { - diagnostic_manager.PutString( - eDiagnosticSeverityRemark, - "Execution was halted at the first instruction of the expression " - "function because \"debug\" was requested.\n" - "Use \"thread return -x\" to return to the state before expression " + if (call_plan_sp) { + lldb::StopInfoSP real_stop_info_sp = call_plan_sp->GetRealStopInfo(); + if (real_stop_info_sp) + error_desc = real_stop_info_sp->GetDescription(); + } + if (error_desc) + diagnostic_manager.Printf(eDiagnosticSeverityError, + "Execution was interrupted, reason: %s.", + error_desc); + else + diagnostic_manager.PutString(eDiagnosticSeverityError, + "Execution was interrupted."); + + if ((execution_result == lldb::eExpressionInterrupted && + options.DoesUnwindOnError()) || + (execution_result == lldb::eExpressionHitBreakpoint && + options.DoesIgnoreBreakpoints())) + diagnostic_manager.AppendMessageToDiagnostic( + "The process has been returned to the state before expression " "evaluation."); - return execution_result; - } else if (execution_result == lldb::eExpressionCompleted) { - if (user_expression_plan->HitErrorBackstop()) { - // This should only happen in Playground & REPL. The code threw an - // uncaught error, so we already rolled up - // the stack past our execution point. We're not going to be able to - // get any or our expression variables - // since they've already gone out of scope. But at least we can - // gather the error result... - if (user_expression_plan->GetReturnValueObject() && - user_expression_plan->GetReturnValueObject() - ->GetError() - .Success()) { - error_backstop_result_sp = - user_expression_plan->GetReturnValueObject(); + else { + if (execution_result == lldb::eExpressionHitBreakpoint) + user_expression_plan->TransferExpressionOwnership(); + diagnostic_manager.AppendMessageToDiagnostic( + "The process has been left at the point where it was " + "interrupted, " + "use \"thread return -x\" to return to the state before " + "expression evaluation."); + } + + return execution_result; + } else if (execution_result == lldb::eExpressionStoppedForDebug) { + diagnostic_manager.PutString( + eDiagnosticSeverityRemark, + "Execution was halted at the first instruction of the expression " + "function because \"debug\" was requested.\n" + "Use \"thread return -x\" to return to the state before expression " + "evaluation."); + return execution_result; + } else if (execution_result == lldb::eExpressionCompleted) { + if (user_expression_plan->HitErrorBackstop()) { + // This should only happen in Playground & REPL. The code threw an + // uncaught error, so we already rolled up the stack past our + // execution point. We're not going to be able to get any of our + // expression variables since they've already gone out of scope. But + // at least we can gather the error result... + + if (auto return_object = user_expression_plan->GetReturnValueObject()) { + if (return_object->GetError().Success()) { + Target *target = exe_ctx.GetTargetPtr(); + if (auto *state = target->GetPersistentExpressionStateForLanguage(Language())) + result = state->CreatePersistentVariable(return_object); + return lldb::eExpressionCompleted; } } - } else { - diagnostic_manager.Printf( - eDiagnosticSeverityError, - "Couldn't execute function; result was %s", - Process::ExecutionResultAsCString(execution_result)); - return execution_result; } + } else if (execution_result != lldb::eExpressionCompleted) { + diagnostic_manager.Printf( + eDiagnosticSeverityError, "Couldn't execute function; result was %s", + Process::ExecutionResultAsCString(execution_result)); + return execution_result; } + } - if (error_backstop_result_sp) { - // This should only happen in Playground & REPL. The code threw an - // uncaught error, so we already rolled up - // the stack past our execution point. We're not going to be able to get - // any or our expression variables - // since they've already gone out of scope. But at least we can gather - // the error result... - Target *target = exe_ctx.GetTargetPtr(); - PersistentExpressionState *expression_state = - target->GetPersistentExpressionStateForLanguage(Language()); - if (expression_state) - result = expression_state->CreatePersistentVariable( - error_backstop_result_sp); - - return lldb::eExpressionCompleted; - } else if (FinalizeJITExecution(diagnostic_manager, exe_ctx, result, - function_stack_bottom, - function_stack_top)) { - return lldb::eExpressionCompleted; - } else { - return lldb::eExpressionResultUnavailable; - } + if (FinalizeJITExecution(diagnostic_manager, exe_ctx, result, + function_stack_bottom, function_stack_top)) { + return lldb::eExpressionCompleted; } else { - diagnostic_manager.PutString( - eDiagnosticSeverityError, - "Expression can't be run, because there is no JIT compiled function"); - return lldb::eExpressionSetupError; + return lldb::eExpressionResultUnavailable; } }