When running OS Plugins from dSYM's, make sure start state is correct (llvm#146441)

This is an odd corner case of the use of scripts loaded from dSYM's - a
macOS only feature, which can load OS Plugins that re-present the thread
state of the program we attach to. If we find out about and load the
dSYM scripts when we discover a target in the course of attaching to it,
we can end up running the OS plugin before we've started up the private
state thread. However, the os_plugin in that case will be running before
we broadcast the stop event to the public event listener. So it should
formally use the private state and not the public state for the Python
code environment.

This patch says that if we have not yet started up the private state
thread, then any thread that is servicing events is doing so on behalf
of the private state machinery, and should see the private state, not
the public state.

Most of the patch is getting a test that will actually reproduce the
error. Only the test `test_python_os_plugin_remote` actually reproduced
the error. In `test_python_os_plugin` we actually do start up the
private state thread before handling the event. `test_python_os_plugin`
is there for completeness sake.

Author: jimingham

lldb/include/lldb/Host/HostThread.h

@@ -43,6 +43,8 @@ class HostThread {
 
   bool EqualsThread(lldb::thread_t thread) const;
 
+  bool HasThread() const;
+
 private:
   std::shared_ptr<HostNativeThreadBase> m_native_thread;
 };

lldb/include/lldb/Target/Process.h

@@ -2547,6 +2547,8 @@ void PruneThreadPlans();
 
   bool CurrentThreadIsPrivateStateThread();
 
+  bool CurrentThreadPosesAsPrivateStateThread();
+
   virtual Status SendEventData(const char *data) {
     return Status::FromErrorString(
         "Sending an event is not supported for this process.");

lldb/source/Host/common/HostThread.cpp

@@ -44,3 +44,9 @@ lldb::thread_result_t HostThread::GetResult() const {
 bool HostThread::EqualsThread(lldb::thread_t thread) const {
   return m_native_thread->EqualsThread(thread);
 }
+
+bool HostThread::HasThread() const {
+  if (!m_native_thread)
+    return false;
+  return m_native_thread->GetSystemHandle() != LLDB_INVALID_HOST_THREAD;
+}

lldb/source/Target/Process.cpp

@@ -1271,7 +1271,7 @@ uint32_t Process::AssignIndexIDToThread(uint64_t thread_id) {
 }
 
 StateType Process::GetState() {
-  if (CurrentThreadIsPrivateStateThread())
+  if (CurrentThreadPosesAsPrivateStateThread())
     return m_private_state.GetValue();
   else
     return m_public_state.GetValue();
@@ -3144,16 +3144,19 @@ void Process::CompleteAttach() {
     }
   }
 
-  if (!m_os_up) {
+  // If we don't have an operating system plugin loaded yet, see if
+  // LoadOperatingSystemPlugin can find one (and stuff it in m_os_up).
+  if (!m_os_up)
     LoadOperatingSystemPlugin(false);
-    if (m_os_up) {
-      // Somebody might have gotten threads before now, but we need to force the
-      // update after we've loaded the OperatingSystem plugin or it won't get a
-      // chance to process the threads.
-      m_thread_list.Clear();
-      UpdateThreadListIfNeeded();
-    }
+
+  if (m_os_up) {
+    // Somebody might have gotten threads before we loaded the OS Plugin above,
+    // so we need to force the update now or the newly loaded plugin won't get
+    // a chance to process the threads.
+    m_thread_list.Clear();
+    UpdateThreadListIfNeeded();
   }
+
   // Figure out which one is the executable, and set that in our target:
   ModuleSP new_executable_module_sp;
   for (ModuleSP module_sp : GetTarget().GetImages().Modules()) {
@@ -5856,6 +5859,13 @@ bool Process::CurrentThreadIsPrivateStateThread()
   return m_private_state_thread.EqualsThread(Host::GetCurrentThread());
 }
 
+bool Process::CurrentThreadPosesAsPrivateStateThread() {
+  // If we haven't started up the private state thread yet, then whatever thread
+  // is fetching this event should be temporarily the private state thread.
+  if (!m_private_state_thread.HasThread())
+    return true;
+  return m_private_state_thread.EqualsThread(Host::GetCurrentThread());
+}
 
 void Process::Flush() {
   m_thread_list.Flush();

lldb/source/Target/StackFrameList.cpp

@@ -723,7 +723,7 @@ void StackFrameList::SelectMostRelevantFrame() {
   // Don't call into the frame recognizers on the private state thread as
   // they can cause code to run in the target, and that can cause deadlocks
   // when fetching stop events for the expression.
-  if (m_thread.GetProcess()->CurrentThreadIsPrivateStateThread())
+  if (m_thread.GetProcess()->CurrentThreadPosesAsPrivateStateThread())
     return;
 
   Log *log = GetLog(LLDBLog::Thread);

lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py

@@ -24,6 +24,10 @@ def create_thread(self, tid, context):
         return None
 
     def get_thread_info(self):
+        if self.process.state != lldb.eStateStopped:
+            print("Error: get_thread_info called with state not stopped")
+            return []
+
         if not self.threads:
             self.threads = [
                 {

lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/Makefile

@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+ENABLE_THREADS := YES
+
+include Makefile.rules

lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/TestOSIndSYM.py

@@ -0,0 +1,153 @@
+"""
+Test that an OS plugin in a dSYM sees the right process state
+when run from a dSYM on attach
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbgdbserverutils import get_debugserver_exe
+
+import os
+import lldb
+import time
+import socket
+import shutil
+
+
+class TestOSPluginIndSYM(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    # The port used by debugserver.
+    PORT = 54638
+
+    # The number of attempts.
+    ATTEMPTS = 10
+
+    # Time given to the binary to launch and to debugserver to attach to it for
+    # every attempt. We'll wait a maximum of 10 times 2 seconds while the
+    # inferior will wait 10 times 10 seconds.
+    TIMEOUT = 2
+
+    def no_debugserver(self):
+        if get_debugserver_exe() is None:
+            return "no debugserver"
+        return None
+
+    def port_not_available(self):
+        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        if s.connect_ex(("127.0.0.1", self.PORT)) == 0:
+            return "{} not available".format(self.PORT)
+        return None
+
+    @skipUnlessDarwin
+    def test_python_os_plugin(self):
+        self.do_test_python_os_plugin(False)
+
+    @skipTestIfFn(no_debugserver)
+    @skipTestIfFn(port_not_available)
+    def test_python_os_plugin_remote(self):
+        self.do_test_python_os_plugin(True)
+
+    def do_test_python_os_plugin(self, remote):
+        """Test that the environment for os plugins in dSYM's is correct"""
+        executable = self.build_dsym("my_binary")
+
+        # Make sure we're set up to load the symbol file's python
+        self.runCmd("settings set target.load-script-from-symbol-file true")
+
+        target = self.dbg.CreateTarget(None)
+
+        error = lldb.SBError()
+
+        # Now run the process, and then attach.  When the attach
+        # succeeds, make sure that we were in the right state when
+        # the OS plugins were run.
+        if not remote:
+            popen = self.spawnSubprocess(executable, [])
+
+            process = target.AttachToProcessWithID(lldb.SBListener(), popen.pid, error)
+            self.assertSuccess(error, "Attach succeeded")
+        else:
+            self.setup_remote_platform(executable)
+            process = target.process
+            self.assertTrue(process.IsValid(), "Got a valid process from debugserver")
+
+        # We should have figured out the target from the result of the attach:
+        self.assertTrue(target.IsValid, "Got a valid target")
+
+        # Make sure that we got the right plugin:
+        self.expect(
+            "settings show target.process.python-os-plugin-path",
+            substrs=["operating_system.py"],
+        )
+
+        for thread in process.threads:
+            stack_depth = thread.num_frames
+            reg_threads = thread.frames[0].reg
+
+        # OKAY, that realized the threads, now see if the creation
+        # state was correct.  The way we use the OS plugin, it doesn't need
+        # to create a thread, and doesn't have to call get_register_info,
+        # so we don't expect those to get called.
+        self.expect(
+            "test_report_command",
+            substrs=[
+                "in_init=1",
+                "in_get_thread_info=1",
+                "in_create_thread=2",
+                "in_get_register_info=2",
+                "in_get_register_data=1",
+            ],
+        )
+
+    def build_dsym(self, name):
+        self.build(debug_info="dsym", dictionary={"EXE": name})
+        executable = self.getBuildArtifact(name)
+        dsym_path = self.getBuildArtifact(name + ".dSYM")
+        python_dir_path = dsym_path
+        python_dir_path = os.path.join(dsym_path, "Contents", "Resources", "Python")
+        if not os.path.exists(python_dir_path):
+            os.mkdir(python_dir_path)
+        python_file_name = name + ".py"
+
+        os_plugin_dir = os.path.join(python_dir_path, "OS_Plugin")
+        if not os.path.exists(os_plugin_dir):
+            os.mkdir(os_plugin_dir)
+
+        plugin_dest_path = os.path.join(os_plugin_dir, "operating_system.py")
+        plugin_origin_path = os.path.join(self.getSourceDir(), "operating_system.py")
+        shutil.copy(plugin_origin_path, plugin_dest_path)
+
+        module_dest_path = os.path.join(python_dir_path, python_file_name)
+        with open(module_dest_path, "w") as f:
+            f.write("def __lldb_init_module(debugger, unused):\n")
+            f.write(
+                f"    debugger.HandleCommand(\"settings set target.process.python-os-plugin-path '{plugin_dest_path}'\")\n"
+            )
+            f.close()
+
+        return executable
+
+    def setup_remote_platform(self, exe):
+        # Get debugserver to start up our process for us, and then we
+        # can use `process connect` to attach to it.
+        debugserver = get_debugserver_exe()
+        debugserver_args = ["localhost:{}".format(self.PORT), exe]
+        self.spawnSubprocess(debugserver, debugserver_args)
+
+        # Select the platform.
+        self.runCmd("platform select remote-gdb-server")
+
+        # Connect to debugserver
+        interpreter = self.dbg.GetCommandInterpreter()
+        connected = False
+        for i in range(self.ATTEMPTS):
+            result = lldb.SBCommandReturnObject()
+            interpreter.HandleCommand(f"gdb-remote localhost:{self.PORT}", result)
+            connected = result.Succeeded()
+            if connected:
+                break
+            time.sleep(self.TIMEOUT)
+
+        self.assertTrue(connected, "could not connect to debugserver")

lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/main.c

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

lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/operating_system.py

@@ -0,0 +1,83 @@
+#!/usr/bin/env python
+
+import lldb
+import struct
+
+# Value is:
+#   0 called - state is not stopped
+#   1 called - state is stopped
+#   2 not called
+
+stop_state = {
+    "in_init": 2,
+    "in_get_thread_info": 2,
+    "in_create_thread": 2,
+    "in_get_register_info": 2,
+    "in_get_register_data": 2,
+}
+
+
+def ReportCommand(debugger, command, exe_ctx, result, unused):
+    global stop_state
+    for state in stop_state:
+        result.AppendMessage(f"{state}={stop_state[state]}\n")
+    result.SetStatus(lldb.eReturnStatusSuccessFinishResult)
+
+
+class OperatingSystemPlugIn:
+    """This class checks that all the"""
+
+    def __init__(self, process):
+        """Initialization needs a valid.SBProcess object.
+        global stop_state
+
+        This plug-in will get created after a live process is valid and has stopped for the
+        first time."""
+        self.process = process
+        stop_state["in_init"] = self.state_is_stopped()
+        interp = process.target.debugger.GetCommandInterpreter()
+        result = lldb.SBCommandReturnObject()
+        cmd_str = (
+            f"command script add test_report_command -o -f {__name__}.ReportCommand"
+        )
+        interp.HandleCommand(cmd_str, result)
+
+    def state_is_stopped(self):
+        if self.process.state == lldb.eStateStopped:
+            return 1
+        else:
+            return 0
+
+    def does_plugin_report_all_threads(self):
+        return True
+
+    def create_thread(self, tid, context):
+        global stop_state
+        stop_state["in_create_thread"] = self.state_is_stopped()
+
+        return None
+
+    def get_thread_info(self):
+        global stop_state
+        stop_state["in_get_thread_info"] = self.state_is_stopped()
+        idx = self.process.threads[0].idx
+        return [
+            {
+                "tid": 0x111111111,
+                "name": "one",
+                "queue": "queue1",
+                "state": "stopped",
+                "stop_reason": "breakpoint",
+                "core": idx,
+            }
+        ]
+
+    def get_register_info(self):
+        global stop_state
+        stop_state["in_get_register_info"] = self.state_is_stopped()
+        return None
+
+    def get_register_data(self, tid):
+        global stop_state
+        stop_state["in_get_register_data"] = self.state_is_stopped()
+        return None