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
