From b565e7f0c4cb1768f6c43499aed95adb8cc4f04a Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Wed, 9 Nov 2022 10:03:30 -0800 Subject: [PATCH] Don't try to create Expressions when the process is running. We generally prohibit this at a higher level - for instance requiring the process to be stopped for "expr". But when we trigger an expression for internal purposes (e.g. to fetch types from the ObjC runtime) we weren't checking the process state. Now we explicitly check this at the very start of the job so we don't get into bad states. Differential Revision: https://reviews.llvm.org/D137684 --- lldb/source/Expression/FunctionCaller.cpp | 35 ++++++++++++++++--- lldb/source/Expression/UserExpression.cpp | 20 +++++++---- lldb/source/Expression/UtilityFunction.cpp | 7 ++++ .../Clang/ClangUtilityFunction.cpp | 6 ++++ lldb/source/Target/Process.cpp | 5 ++- 5 files changed, 60 insertions(+), 13 deletions(-) diff --git a/lldb/source/Expression/FunctionCaller.cpp b/lldb/source/Expression/FunctionCaller.cpp index 58c9d56b089c0..ffadbf9b32ec5 100644 --- a/lldb/source/Expression/FunctionCaller.cpp +++ b/lldb/source/Expression/FunctionCaller.cpp @@ -66,17 +66,31 @@ bool FunctionCaller::WriteFunctionWrapper( ExecutionContext &exe_ctx, DiagnosticManager &diagnostic_manager) { Process *process = exe_ctx.GetProcessPtr(); - if (!process) + if (!process) { + diagnostic_manager.Printf(eDiagnosticSeverityError, "no process."); return false; - + } + lldb::ProcessSP jit_process_sp(m_jit_process_wp.lock()); - if (process != jit_process_sp.get()) + if (process != jit_process_sp.get()) { + diagnostic_manager.Printf(eDiagnosticSeverityError, + "process does not match the stored process."); return false; - - if (!m_compiled) + } + + if (process->GetState() != lldb::eStateStopped) { + diagnostic_manager.Printf(eDiagnosticSeverityError, + "process is not stopped"); return false; + } + if (!m_compiled) { + diagnostic_manager.Printf(eDiagnosticSeverityError, + "function not compiled"); + return false; + } + if (m_JITted) return true; @@ -213,6 +227,17 @@ bool FunctionCaller::WriteFunctionArguments( bool FunctionCaller::InsertFunction(ExecutionContext &exe_ctx, lldb::addr_t &args_addr_ref, DiagnosticManager &diagnostic_manager) { + // Since we might need to call allocate memory and maybe call code to make + // the caller, we need to be stopped. + Process *process = exe_ctx.GetProcessPtr(); + if (!process) { + diagnostic_manager.PutString(eDiagnosticSeverityError, "no process"); + return false; + } + if (process->GetState() != lldb::eStateStopped) { + diagnostic_manager.PutString(eDiagnosticSeverityError, "process running"); + return false; + } if (CompileFunction(exe_ctx.GetThreadSP(), diagnostic_manager) != 0) return false; if (!WriteFunctionWrapper(exe_ctx, diagnostic_manager)) diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp index 186e414e68793..c1515b0ace817 100644 --- a/lldb/source/Expression/UserExpression.cpp +++ b/lldb/source/Expression/UserExpression.cpp @@ -194,16 +194,22 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx, Process *process = exe_ctx.GetProcessPtr(); - if (process == nullptr || process->GetState() != lldb::eStateStopped) { - if (execution_policy == eExecutionPolicyAlways) { - LLDB_LOG(log, "== [UserExpression::Evaluate] Expression may not run, but " - "is not constant =="); + if (process == nullptr && execution_policy == eExecutionPolicyAlways) { + LLDB_LOG(log, "== [UserExpression::Evaluate] No process, but the policy is " + "eExecutionPolicyAlways"); - error.SetErrorString("expression needed to run but couldn't"); + error.SetErrorString("expression needed to run but couldn't: no process"); - return execution_results; - } + return execution_results; } + // Since we might need to call allocate memory and maybe call code to make + // the caller, we need to be stopped. + if (process != nullptr && process->GetState() != lldb::eStateStopped) { + error.SetErrorString("Can't make a function caller while the process is " + "running"); + return execution_results; + } + // Explicitly force the IR interpreter to evaluate the expression when the // there is no process that supports running the expression for us. Don't diff --git a/lldb/source/Expression/UtilityFunction.cpp b/lldb/source/Expression/UtilityFunction.cpp index 1a4df97227065..5d55d9a5c2c1d 100644 --- a/lldb/source/Expression/UtilityFunction.cpp +++ b/lldb/source/Expression/UtilityFunction.cpp @@ -64,6 +64,13 @@ FunctionCaller *UtilityFunction::MakeFunctionCaller( error.SetErrorString("Can't make a function caller without a process."); return nullptr; } + // Since we might need to call allocate memory and maybe call code to make + // the caller, we need to be stopped. + if (process_sp->GetState() != lldb::eStateStopped) { + error.SetErrorString("Can't make a function caller while the process is " + "running"); + return nullptr; + } Address impl_code_address; impl_code_address.SetOffset(StartAddress()); diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp index f9956e120b603..66493aa3e5921 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp @@ -99,6 +99,12 @@ bool ClangUtilityFunction::Install(DiagnosticManager &diagnostic_manager, return false; } + // Since we might need to call allocate memory and maybe call code to make + // the caller, we need to be stopped. + if (process->GetState() != lldb::eStateStopped) { + diagnostic_manager.PutString(eDiagnosticSeverityError, "process running"); + return false; + } ////////////////////////// // Parse the expression // diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index bbc5cb87f86f6..a0dd4c54a01a3 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -1293,7 +1293,10 @@ uint32_t Process::AssignIndexIDToThread(uint64_t thread_id) { } StateType Process::GetState() { - return m_public_state.GetValue(); + if (CurrentThreadIsPrivateStateThread()) + return m_private_state.GetValue(); + else + return m_public_state.GetValue(); } void Process::SetPublicState(StateType new_state, bool restarted) {