When running OS Plugins from dSYM's, make sure start state is correct

lldb/include/lldb/Host/HostThread.h

@@ -9,6 +9,7 @@
 #ifndef LLDB_HOST_HOSTTHREAD_H
 #define LLDB_HOST_HOSTTHREAD_H
 
+#include "lldb/Host/HostNativeThreadBase.h"
 #include "lldb/Host/HostNativeThreadForward.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-types.h"
@@ -43,6 +44,12 @@ class HostThread {
 
   bool EqualsThread(lldb::thread_t thread) const;
 
+  bool HasThread() const {
+    if (!m_native_thread)
+      return false;
+    return m_native_thread->GetSystemHandle() != LLDB_INVALID_HOST_THREAD;
+  }
+
 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/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,17 @@ void Process::CompleteAttach() {
     }
   }
 
-  if (!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 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();
   }
+
   // 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 +5857,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