https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/81564
>From d65900f5e6169062fc0988b57fb5be2474789025 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 12 Feb 2024 18:08:23 -0800 Subject: [PATCH 1/2] Fix lldb crash while handling concurrent vfork() --- .../Process/Linux/NativeThreadLinux.cpp | 12 ++++- .../Process/gdb-remote/ProcessGDBRemote.cpp | 21 ++++++--- .../Process/gdb-remote/ProcessGDBRemote.h | 3 +- .../fork/concurrent_vfork/Makefile | 4 ++ .../concurrent_vfork/TestConcurrentVFork.py | 31 +++++++++++++ .../fork/concurrent_vfork/main.cpp | 46 +++++++++++++++++++ 6 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 lldb/test/API/functionalities/fork/concurrent_vfork/Makefile create mode 100644 lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py create mode 100644 lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp index b62e9f643fa792..cf8a1e7d34392a 100644 --- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp @@ -120,7 +120,7 @@ bool NativeThreadLinux::GetStopReason(ThreadStopInfo &stop_info, case eStateCrashed: case eStateExited: case eStateSuspended: - case eStateUnloaded: + case eStateUnloaded: { if (log) LogThreadStopInfo(*log, m_stop_info, "m_stop_info in thread:"); stop_info = m_stop_info; @@ -128,7 +128,17 @@ bool NativeThreadLinux::GetStopReason(ThreadStopInfo &stop_info, if (log) LogThreadStopInfo(*log, stop_info, "returned stop_info:"); + // Include child process PID/TID for forks. + // Client expects "<fork_pid> <fork_tid>" format. + if (stop_info.reason == eStopReasonFork || + stop_info.reason == eStopReasonVFork) { + description = std::to_string(stop_info.details.fork.child_pid); + description += " "; + description += std::to_string(stop_info.details.fork.child_tid); + } + return true; + } case eStateInvalid: case eStateConnected: diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 629b191f3117aa..8ab2257e0a79b9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -266,7 +266,7 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP target_sp, 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(0) { m_async_broadcaster.SetEventName(eBroadcastBitAsyncThreadShouldExit, "async thread should exit"); m_async_broadcaster.SetEventName(eBroadcastBitAsyncContinue, @@ -5615,8 +5615,12 @@ 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); + assert(m_vfork_in_progress >= 0); + ++m_vfork_in_progress; // Disable all software breakpoints for the duration of vfork. if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) @@ -5670,8 +5674,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; + --m_vfork_in_progress; + assert(m_vfork_in_progress >= 0); // Reenable all software breakpoints that were enabled before vfork. if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) @@ -5681,7 +5685,10 @@ 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 > 0) + --m_vfork_in_progress; + assert(m_vfork_in_progress >= 0); + } Process::DidExec(); } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index c1ea1cc7905587..29ed770c1275ea 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -301,7 +301,8 @@ class ProcessGDBRemote : public Process, using FlashRange = FlashRangeVector::Entry; FlashRangeVector m_erased_flash_ranges; - bool m_vfork_in_progress; + // Number of vfork in process. + int m_vfork_in_progress; // Accessors bool IsRunning(lldb::StateType state) { diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/Makefile b/lldb/test/API/functionalities/fork/concurrent_vfork/Makefile new file mode 100644 index 00000000000000..c46619c6623481 --- /dev/null +++ b/lldb/test/API/functionalities/fork/concurrent_vfork/Makefile @@ -0,0 +1,4 @@ +CXX_SOURCES := main.cpp +ENABLE_THREADS := YES + +include Makefile.rules diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py new file mode 100644 index 00000000000000..fcd26d6f936850 --- /dev/null +++ b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py @@ -0,0 +1,31 @@ +""" +Make sure that the concurrent vfork() from multiple threads works correctly. +""" + + +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * + + +class TestConcurrentVFork(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + @skipIfWindows + def test_vfork_follow_parent(self): + self.build() + lldbutil.run_to_source_breakpoint( + self, "// break here", lldb.SBFileSpec("main.cpp") + ) + self.runCmd("settings set target.process.follow-fork-mode parent") + self.expect("continue", substrs=["exited with status = 0"]) + + @skipIfWindows + def test_vfork_follow_child(self): + self.build() + lldbutil.run_to_source_breakpoint( + self, "// break here", lldb.SBFileSpec("main.cpp") + ) + self.runCmd("settings set target.process.follow-fork-mode child") + self.expect("continue", substrs=["exited with status = 0"]) diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp new file mode 100644 index 00000000000000..1bb225b1caf604 --- /dev/null +++ b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp @@ -0,0 +1,46 @@ +#include <thread> +#include <unistd.h> +#include <iostream> +#include <vector> + +int call_vfork() { + printf("Before vfork\n"); + + pid_t child_pid = vfork(); + + if (child_pid == -1) { + // Error handling + perror("vfork"); + return 1; + } else if (child_pid == 0) { + // This code is executed by the child process + printf("Child process\n"); + _exit(0); // Exit the child process + } else { + // This code is executed by the parent process + printf("Parent process\n"); + } + + printf("After vfork\n"); + return 0; +} + +void worker_thread() { + call_vfork(); +} + +void create_threads(int num_threads) { + std::vector<std::thread> threads; + for (int i = 0; i < num_threads; ++i) { + threads.emplace_back(std::thread(worker_thread)); + } + printf("Created %d threads, joining...\n", num_threads); // end_of_create_threads + for (auto &thread: threads) { + thread.join(); + } +} + +int main() { + int num_threads = 5; // break here + create_threads(num_threads); +} >From b4c60c368792627b4fac741e620f1c0b63f24b6b Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 12 Feb 2024 18:36:22 -0800 Subject: [PATCH 2/2] Fix format --- .../Process/gdb-remote/ProcessGDBRemote.cpp | 3 +-- .../fork/concurrent_vfork/main.cpp | 27 +++++++++---------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 8ab2257e0a79b9..6fdb062e712c78 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -263,8 +263,7 @@ 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(0) { m_async_broadcaster.SetEventName(eBroadcastBitAsyncThreadShouldExit, diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp index 1bb225b1caf604..1b75852c3164d0 100644 --- a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp +++ b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp @@ -1,6 +1,6 @@ +#include <iostream> #include <thread> #include <unistd.h> -#include <iostream> #include <vector> int call_vfork() { @@ -9,33 +9,32 @@ int call_vfork() { pid_t child_pid = vfork(); if (child_pid == -1) { - // Error handling - perror("vfork"); - return 1; + // Error handling + perror("vfork"); + return 1; } else if (child_pid == 0) { - // This code is executed by the child process - printf("Child process\n"); - _exit(0); // Exit the child process + // This code is executed by the child process + printf("Child process\n"); + _exit(0); // Exit the child process } else { - // This code is executed by the parent process - printf("Parent process\n"); + // This code is executed by the parent process + printf("Parent process\n"); } printf("After vfork\n"); return 0; } -void worker_thread() { - call_vfork(); -} +void worker_thread() { call_vfork(); } void create_threads(int num_threads) { std::vector<std::thread> threads; for (int i = 0; i < num_threads; ++i) { threads.emplace_back(std::thread(worker_thread)); } - printf("Created %d threads, joining...\n", num_threads); // end_of_create_threads - for (auto &thread: threads) { + printf("Created %d threads, joining...\n", + num_threads); // end_of_create_threads + for (auto &thread : threads) { thread.join(); } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits