Merge pull request #6477 from jimingham/fix-run-locker-again

Fix the run locker setting for async launches that don't stop at the

Author: jimingham

lldb/source/API/SBFrame.cpp

@@ -20,6 +20,7 @@
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Core/ValueObjectRegister.h"
 #include "lldb/Core/ValueObjectVariable.h"
+#include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/Host.h"
@@ -997,6 +998,12 @@ SBValue SBFrame::EvaluateExpression(const char *expr) {
     else
       options.SetLanguage(frame->GetLanguage());
     return EvaluateExpression(expr, options);
+  } else {
+    Status error;
+    error.SetErrorString("can't evaluate expressions when the "
+                           "process is running.");
+    ValueObjectSP error_val_sp = ValueObjectConstResult::Create(nullptr, error);
+    result.SetSP(error_val_sp, false);
   }
   return result;
 }
@@ -1060,7 +1067,6 @@ lldb::SBValue SBFrame::EvaluateExpression(const char *expr,
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
-
   StackFrame *frame = nullptr;
   Target *target = exe_ctx.GetTargetPtr();
   Process *process = exe_ctx.GetProcessPtr();
@@ -1084,13 +1090,30 @@ lldb::SBValue SBFrame::EvaluateExpression(const char *expr,
         target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
         expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
       }
+    } else {
+      Status error;
+      error.SetErrorString("can't evaluate expressions when the "
+                           "process is running.");
+      expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
+      expr_result.SetSP(expr_value_sp, false);
     }
+  } else {
+      Status error;
+      error.SetErrorString("sbframe object is not valid.");
+      expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
+      expr_result.SetSP(expr_value_sp, false);
   }
 
-  LLDB_LOGF(expr_log,
-            "** [SBFrame::EvaluateExpression] Expression result is "
-            "%s, summary %s **",
-            expr_result.GetValue(), expr_result.GetSummary());
+  if (expr_result.GetError().Success())
+    LLDB_LOGF(expr_log,
+              "** [SBFrame::EvaluateExpression] Expression result is "
+              "%s, summary %s **",
+              expr_result.GetValue(), expr_result.GetSummary());
+  else
+    LLDB_LOGF(expr_log,
+              "** [SBFrame::EvaluateExpression] Expression evaluation failed: "
+              "%s **",
+              expr_result.GetError().GetCString());
 
   return expr_result;
 }

lldb/source/API/SBTarget.cpp

@@ -2278,12 +2278,25 @@ lldb::SBValue SBTarget::EvaluateExpression(const char *expr,
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     ExecutionContext exe_ctx(m_opaque_sp.get());
 
-
     frame = exe_ctx.GetFramePtr();
     Target *target = exe_ctx.GetTargetPtr();
+    Process *process = exe_ctx.GetProcessPtr();
 
     if (target) {
-      target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
+      // If we have a process, make sure to lock the runlock:
+      if (process) {
+        Process::StopLocker stop_locker;
+        if (stop_locker.TryLock(&process->GetRunLock())) {
+          target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
+        } else {
+          Status error;
+          error.SetErrorString("can't evaluate expressions when the "
+                               "process is running.");
+          expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
+        }
+      } else {
+        target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
+      }
 
       expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
     }

lldb/source/Target/Target.cpp

@@ -3627,7 +3627,7 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) {
       // SyncResume hijacker.
       m_process_sp->ResumeSynchronous(stream);
     else
-      error = m_process_sp->PrivateResume();
+      error = m_process_sp->Resume();
     if (!error.Success()) {
       Status error2;
       error2.SetErrorStringWithFormat(

lldb/test/API/python_api/run_locker/Makefile

@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules

lldb/test/API/python_api/run_locker/TestRunLocker.py

@@ -0,0 +1,89 @@
+"""
+Test that the run locker really does work to keep
+us from running SB API that should only be run
+while stopped.  This test is mostly concerned with
+what happens between launch and first stop.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestRunLocker(TestBase):
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_run_locker(self):
+        """Test that the run locker is set correctly when we launch"""
+        self.build()
+        self.runlocker_test(False)
+
+    def test_run_locker_stop_at_entry(self):
+        """Test that the run locker is set correctly when we launch"""
+        self.build()
+        self.runlocker_test(False)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        self.main_source_file = lldb.SBFileSpec("main.c")
+
+    def runlocker_test(self, stop_at_entry):
+        """The code to stop at entry handles events slightly differently, so 
+           we test both versions of process launch."""
+
+        target = lldbutil.run_to_breakpoint_make_target(self)
+        
+        launch_info = target.GetLaunchInfo()
+        if stop_at_entry:
+            flags = launch_info.GetFlags()
+            launch_info.SetFlags(flags | lldb.eLaunchFlagStopAtEntry)
+
+        error = lldb.SBError()
+        # We are trying to do things when the process is running, so
+        # we have to run the debugger asynchronously.
+        self.dbg.SetAsync(True)
+        
+        listener = lldb.SBListener("test-run-lock-listener")
+        launch_info.SetListener(listener)
+        process = target.Launch(launch_info, error)
+        self.assertSuccess(error, "Launched the process")
+        
+        event = lldb.SBEvent()
+        
+        event_result = listener.WaitForEvent(10, event)
+        self.assertTrue(event_result, "timed out waiting for launch")
+        state_type = lldb.SBProcess.GetStateFromEvent(event)
+        # We don't always see a launching...
+        if state_type == lldb.eStateLaunching:
+            event_result = listener.WaitForEvent(10, event)
+            self.assertTrue(event_result, "Timed out waiting for running after launching")
+            state_type = lldb.SBProcess.GetStateFromEvent(event)
+            
+        self.assertState(state_type, lldb.eStateRunning, "Didn't get a running event")
+        
+        # We aren't checking the entry state, but just making sure
+        # the running state is set properly if we continue in this state.
+        
+        if stop_at_entry:
+            event_result = listener.WaitForEvent(10, event)
+            self.assertTrue(event_result, "Timed out waiting for stop at entry stop")
+            state_type = lldb.SBProcess.GetStateFromEvent(event)
+            self.assertState(state_type, eStateStopped, "Stop at entry stopped")
+            process.Continue()
+
+        # Okay, now the process is running, make sure we can't do things
+        # you aren't supposed to do while running, and that we get some
+        # actual error:
+        val = target.EvaluateExpression("SomethingToCall()")
+        error = val.GetError()
+        self.assertTrue(error.Fail(), "Failed to run expression")
+        print(f"Got Error: {error.GetCString()}")
+        self.assertIn("can't evaluate expressions when the process is running", error.GetCString(), "Stopped by stop locker")
+
+        # This should also fail if we try to use the script interpreter directly:
+        interp = self.dbg.GetCommandInterpreter()
+        result = lldb.SBCommandReturnObject()
+        ret = interp.HandleCommand("script var = lldb.frame.EvaluateExpression('SomethingToCall()'); var.GetError().GetCString()", result)
+        self.assertIn("can't evaluate expressions when the process is running", result.GetOutput())

lldb/test/API/python_api/run_locker/main.c

@@ -0,0 +1,15 @@
+#include <unistd.h>
+
+int
+SomethingToCall() {
+  return 100;
+}
+
+int
+main()
+{
+  while (1) {
+    sleep(1);
+  }
+  return SomethingToCall();
+}