[Lldb-commits] [lldb] b565e7f - Don't try to create Expressions when the process is running.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-11-09T10:07:09-08:00
New Revision: b565e7f0c4cb1768f6c43499aed95adb8cc4f04a

URL: https://github.com/llvm/llvm-project/commit/b565e7f0c4cb1768f6c43499aed95adb8cc4f04a
DIFF: https://github.com/llvm/llvm-project/commit/b565e7f0c4cb1768f6c43499aed95adb8cc4f04a.diff

LOG: 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

Added: 
    

Modified: 
    lldb/source/Expression/FunctionCaller.cpp
    lldb/source/Expression/UserExpression.cpp
    lldb/source/Expression/UtilityFunction.cpp
    lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
    lldb/source/Target/Process.cpp

Removed: 
    


################################################################################
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) {