Author: jimingham Date: 2025-07-11T10:02:07-07:00 New Revision: 9adc8ddad02d062e0b395d8079f051e8ae4552b4
URL: https://github.com/llvm/llvm-project/commit/9adc8ddad02d062e0b395d8079f051e8ae4552b4 DIFF: https://github.com/llvm/llvm-project/commit/9adc8ddad02d062e0b395d8079f051e8ae4552b4.diff LOG: When running OS Plugins from dSYM's, make sure start state is correct (#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. Added: lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/Makefile lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/TestOSIndSYM.py lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/main.c lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/operating_system.py Modified: lldb/include/lldb/Host/HostThread.h lldb/include/lldb/Target/Process.h lldb/source/Host/common/HostThread.cpp lldb/source/Target/Process.cpp lldb/source/Target/StackFrameList.cpp lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/HostThread.h b/lldb/include/lldb/Host/HostThread.h index d3477e115e2d8..c969492f5b20a 100644 --- a/lldb/include/lldb/Host/HostThread.h +++ b/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; }; diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index a8892e9c43225..637b0774ec7db 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/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."); diff --git a/lldb/source/Host/common/HostThread.cpp b/lldb/source/Host/common/HostThread.cpp index eec029be1c091..8822be016b0a1 100644 --- a/lldb/source/Host/common/HostThread.cpp +++ b/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; +} diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index bba1230c79920..2aa02fd58335e 100644 --- a/lldb/source/Target/Process.cpp +++ b/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(); diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index 9c6208e9e0a65..16cd2548c2784 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/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); diff --git a/lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py b/lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py index f4404d78492f9..de9900cae4b75 100644 --- a/lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py +++ b/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 = [ { diff --git a/lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/Makefile b/lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/Makefile new file mode 100644 index 0000000000000..93618844a7a4d --- /dev/null +++ b/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 diff --git a/lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/TestOSIndSYM.py b/lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/TestOSIndSYM.py new file mode 100644 index 0000000000000..f0d192be661bb --- /dev/null +++ b/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") diff --git a/lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/main.c b/lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/main.c new file mode 100644 index 0000000000000..8e03f395e6110 --- /dev/null +++ b/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; +} diff --git a/lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/operating_system.py b/lldb/test/API/functionalities/plugins/python_os_plugin/os_plugin_in_dsym/operating_system.py new file mode 100644 index 0000000000000..0f9cec670b73f --- /dev/null +++ b/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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
