diff options
author | jeffreytan81 <jeffreytan@meta.com> | 2024-03-06 10:50:32 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-03-06 10:50:32 -0800 |
commit | 8bdddcf0bb5a40e6ce6cbf7fc6b7ce576e2b032d (patch) | |
tree | 99a7095eb3d11047bfa161d275191f211251505b /lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | |
parent | 954f891af20d193f6a5f41d7ede6a9313a49cfc3 (diff) | |
download | llvm-8bdddcf0bb5a40e6ce6cbf7fc6b7ce576e2b032d.zip llvm-8bdddcf0bb5a40e6ce6cbf7fc6b7ce576e2b032d.tar.gz llvm-8bdddcf0bb5a40e6ce6cbf7fc6b7ce576e2b032d.tar.bz2 |
Fix lldb crash while handling concurrent vfork() (#81564)
We got user reporting lldb crash while the debuggee is calling vfork()
concurrently from multiple threads.
The crash happens because the current implementation can only handle
single vfork, vforkdone protocol transaction.
This diff fixes the crash by lldb-server storing forked debuggee's <pid,
tid> pair in jstopinfo which will be decoded by lldb client to create
StopInfoVFork for follow parent/child policy. Each StopInfoVFork will
later have a corresponding vforkdone packet. So the patch also changes
the `m_vfork_in_progress` to be reference counting based.
Two new test cases are added which crash/assert without the changes in
this patch.
---------
Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
Diffstat (limited to 'lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp')
-rw-r--r-- | lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 28 |
1 files changed, 17 insertions, 11 deletions
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 51ceb12..5b9a9d7 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -263,10 +263,9 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP target_sp, m_continue_C_tids(), m_continue_s_tids(), m_continue_S_tids(), m_max_memory_size(0), m_remote_stub_max_memory_size(0), m_addr_to_mmap_size(), m_thread_create_bp_sp(), - m_waiting_for_attach(false), - m_command_sp(), m_breakpoint_pc_offset(0), + m_waiting_for_attach(false), m_command_sp(), m_breakpoint_pc_offset(0), m_initial_tid(LLDB_INVALID_THREAD_ID), m_allow_flash_writes(false), - m_erased_flash_ranges(), m_vfork_in_progress(false) { + m_erased_flash_ranges(), m_vfork_in_progress_count(0) { m_async_broadcaster.SetEventName(eBroadcastBitAsyncThreadShouldExit, "async thread should exit"); m_async_broadcaster.SetEventName(eBroadcastBitAsyncContinue, @@ -5293,8 +5292,10 @@ public: (ProcessGDBRemote *)m_interpreter.GetExecutionContext() .GetProcessPtr(); if (process) { - StreamSP output_stream_sp( - m_interpreter.GetDebugger().GetAsyncOutputStream()); + StreamSP output_stream_sp = result.GetImmediateOutputStream(); + if (!output_stream_sp) + output_stream_sp = + StreamSP(m_interpreter.GetDebugger().GetAsyncOutputStream()); result.SetImmediateOutputStream(output_stream_sp); const uint32_t num_packets = @@ -5634,8 +5635,11 @@ void ProcessGDBRemote::DidFork(lldb::pid_t child_pid, lldb::tid_t child_tid) { void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid, lldb::tid_t child_tid) { Log *log = GetLog(GDBRLog::Process); - assert(!m_vfork_in_progress); - m_vfork_in_progress = true; + LLDB_LOG( + log, + "ProcessGDBRemote::DidFork() called for child_pid: {0}, child_tid {1}", + child_pid, child_tid); + ++m_vfork_in_progress_count; // Disable all software breakpoints for the duration of vfork. if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) @@ -5689,8 +5693,8 @@ void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid, lldb::tid_t child_tid) { } void ProcessGDBRemote::DidVForkDone() { - assert(m_vfork_in_progress); - m_vfork_in_progress = false; + assert(m_vfork_in_progress_count > 0); + --m_vfork_in_progress_count; // Reenable all software breakpoints that were enabled before vfork. if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) @@ -5700,7 +5704,9 @@ void ProcessGDBRemote::DidVForkDone() { void ProcessGDBRemote::DidExec() { // If we are following children, vfork is finished by exec (rather than // vforkdone that is submitted for parent). - if (GetFollowForkMode() == eFollowChild) - m_vfork_in_progress = false; + if (GetFollowForkMode() == eFollowChild) { + if (m_vfork_in_progress_count > 0) + --m_vfork_in_progress_count; + } Process::DidExec(); } |