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/8] 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/8] 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();
}
}
>From 2e7bf2386394ff35fa43f0cb78ea029e0e075bc8 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffrey...@fb.com>
Date: Wed, 14 Feb 2024 11:41:56 -0800
Subject: [PATCH 3/8] Address review feedback
---
.../Process/Linux/NativeThreadLinux.cpp | 16 +++----
.../Process/gdb-remote/ProcessGDBRemote.cpp | 13 +++---
.../Process/gdb-remote/ProcessGDBRemote.h | 2 +-
.../concurrent_vfork/TestConcurrentVFork.py | 22 +++++++++-
.../fork/concurrent_vfork/main.cpp | 43 +++++++++++++++----
5 files changed, 69 insertions(+), 27 deletions(-)
diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
index cf8a1e7d34392a..554b06362b54c0 100644
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -124,19 +124,17 @@ bool NativeThreadLinux::GetStopReason(ThreadStopInfo
&stop_info,
if (log)
LogThreadStopInfo(*log, m_stop_info, "m_stop_info in thread:");
stop_info = m_stop_info;
- description = m_stop_description;
- if (log)
- LogThreadStopInfo(*log, stop_info, "returned stop_info:");
-
// Include child process PID/TID for forks.
- // Client expects "<fork_pid> <fork_tid>" format.
+ // Client expects "<fork_pid> <fork_tid>" format for parsing.
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);
+ m_stop_description = std::to_string(stop_info.details.fork.child_pid);
+ m_stop_description += " ";
+ m_stop_description += std::to_string(stop_info.details.fork.child_tid);
}
-
+ description = m_stop_description;
+ if (log)
+ LogThreadStopInfo(*log, stop_info, "returned stop_info:");
return true;
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6fdb062e712c78..081c9935cf3492 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -265,7 +265,7 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP target_sp,
m_addr_to_mmap_size(), m_thread_create_bp_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(0) {
+ m_erased_flash_ranges(), m_vfork_in_progress_count(0) {
m_async_broadcaster.SetEventName(eBroadcastBitAsyncThreadShouldExit,
"async thread should exit");
m_async_broadcaster.SetEventName(eBroadcastBitAsyncContinue,
@@ -5673,8 +5673,9 @@ void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid,
lldb::tid_t child_tid) {
}
void ProcessGDBRemote::DidVForkDone() {
- --m_vfork_in_progress;
- assert(m_vfork_in_progress >= 0);
+ assert(m_vfork_in_progress_count > 0);
+ if (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))
@@ -5685,9 +5686,9 @@ void ProcessGDBRemote::DidExec() {
// If we are following children, vfork is finished by exec (rather than
// vforkdone that is submitted for parent).
if (GetFollowForkMode() == eFollowChild) {
- if (m_vfork_in_progress > 0)
- --m_vfork_in_progress;
- assert(m_vfork_in_progress >= 0);
+ assert(m_vfork_in_progress_count > 0);
+ if (m_vfork_in_progress_count > 0)
+ --m_vfork_in_progress_count;
}
Process::DidExec();
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 29ed770c1275ea..4ed26b0e09ad2a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -302,7 +302,7 @@ class ProcessGDBRemote : public Process,
FlashRangeVector m_erased_flash_ranges;
// Number of vfork in process.
- int m_vfork_in_progress;
+ int m_vfork_in_progress_count;
// Accessors
bool IsRunning(lldb::StateType state) {
diff --git
a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
index fcd26d6f936850..5da93b05ac424f 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
@@ -12,20 +12,38 @@
class TestConcurrentVFork(TestBase):
NO_DEBUG_INFO_TESTCASE = True
+ def get_pid_from_variable(self):
+ target = self.dbg.GetTargetAtIndex(0)
+ return target.FindFirstGlobalVariable("g_pid").GetValueAsUnsigned()
+
@skipIfWindows
def test_vfork_follow_parent(self):
+ """
+ Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-parent.
+ And follow-parent successfully detach all child processes and exit
debugger.
+ """
+
self.build()
lldbutil.run_to_source_breakpoint(
self, "// break here", lldb.SBFileSpec("main.cpp")
)
+ parent_pid = self.get_pid_from_variable()
self.runCmd("settings set target.process.follow-fork-mode parent")
- self.expect("continue", substrs=["exited with status = 0"])
+ self.expect(
+ "continue", substrs=[f"Process {parent_pid} exited with status =
0"]
+ )
@skipIfWindows
def test_vfork_follow_child(self):
+ """
+ Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-child.
+ And follow-child successfully detach parent process and exit child
process with correct exit code.
+ """
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"])
+ # Child process exits with code "index + 10" since index is [0-4]
+ # so the exit code should be 1[0-4]
+ self.expect("continue", patterns=[r'exited with status = 1[0-4]'])
diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
index 1b75852c3164d0..f8e481663acd6a 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
@@ -1,11 +1,15 @@
#include <iostream>
+#include <mutex>
+#include <sys/wait.h>
#include <thread>
#include <unistd.h>
#include <vector>
-int call_vfork() {
- printf("Before vfork\n");
+pid_t g_pid = 0;
+std::mutex g_child_pids_mutex;
+std::vector<pid_t> g_child_pids;
+int call_vfork(int index) {
pid_t child_pid = vfork();
if (child_pid == -1) {
@@ -14,32 +18,53 @@ int call_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
+ g_pid = getpid();
+ printf("Child process: %d\n", g_pid);
+ _exit(index + 10); // Exit the child process
} else {
// This code is executed by the parent process
- printf("Parent process\n");
+ printf("[Parent] Forked process id: %d\n", child_pid);
}
-
- printf("After vfork\n");
return 0;
}
-void worker_thread() { call_vfork(); }
+void wait_all_children_to_exit() {
+ std::lock_guard<std::mutex> Lock(g_child_pids_mutex);
+ for (pid_t child_pid : g_child_pids) {
+ int child_status = 0;
+ pid_t pid = waitpid(child_pid, &child_status, 0);
+ if (child_status != 0) {
+ int exit_code = WEXITSTATUS(child_status);
+ if (exit_code > 15 || exit_code < 10) {
+ printf("Error: child process exits with unexpected code %d\n",
+ exit_code);
+ _exit(1); // This will let our program know that some child processes
+ // didn't exist with an expected exit status.
+ }
+ }
+ if (pid != child_pid)
+ _exit(2); // This will let our program know it didn't succeed
+ }
+}
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));
+ threads.emplace_back(std::thread(call_vfork, i));
}
printf("Created %d threads, joining...\n",
num_threads); // end_of_create_threads
for (auto &thread : threads) {
thread.join();
}
+ wait_all_children_to_exit();
}
int main() {
+ g_pid = getpid();
+ printf("Entering main() pid: %d\n", g_pid);
+
int num_threads = 5; // break here
create_threads(num_threads);
+ printf("Exiting main() pid: %d\n", g_pid);
}
>From 734037200acf6338cb0d07db0bfe235e52821dc3 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffrey...@fb.com>
Date: Wed, 14 Feb 2024 12:15:05 -0800
Subject: [PATCH 4/8] Fix python format
---
.../fork/concurrent_vfork/TestConcurrentVFork.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
index 5da93b05ac424f..8953dbbf67f98e 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
@@ -46,4 +46,4 @@ def test_vfork_follow_child(self):
self.runCmd("settings set target.process.follow-fork-mode child")
# Child process exits with code "index + 10" since index is [0-4]
# so the exit code should be 1[0-4]
- self.expect("continue", patterns=[r'exited with status = 1[0-4]'])
+ self.expect("continue", patterns=[r"exited with status = 1[0-4]"])
>From 6a528dda8388532ec943d7d304ce760c1b660dcd Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffrey...@fb.com>
Date: Tue, 20 Feb 2024 15:15:31 -0800
Subject: [PATCH 5/8] Address review comments
---
.../Plugins/Process/Linux/NativeThreadLinux.cpp | 15 +++++----------
.../Plugins/Process/gdb-remote/ProcessGDBRemote.h | 4 ++--
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
index 554b06362b54c0..26cb26daabf52c 100644
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -120,23 +120,15 @@ 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;
- // Include child process PID/TID for forks.
- // Client expects "<fork_pid> <fork_tid>" format for parsing.
- if (stop_info.reason == eStopReasonFork ||
- stop_info.reason == eStopReasonVFork) {
- m_stop_description = std::to_string(stop_info.details.fork.child_pid);
- m_stop_description += " ";
- m_stop_description += std::to_string(stop_info.details.fork.child_tid);
- }
description = m_stop_description;
if (log)
LogThreadStopInfo(*log, stop_info, "returned stop_info:");
+
return true;
- }
case eStateInvalid:
case eStateConnected:
@@ -464,6 +456,9 @@ void NativeThreadLinux::SetStoppedByFork(bool is_vfork,
lldb::pid_t child_pid) {
m_stop_info.signo = SIGTRAP;
m_stop_info.details.fork.child_pid = child_pid;
m_stop_info.details.fork.child_tid = child_pid;
+ m_stop_description = std::to_string(child_pid);
+ m_stop_description += " ";
+ m_stop_description += std::to_string(child_pid);
}
void NativeThreadLinux::SetStoppedByVForkDone() {
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 4ed26b0e09ad2a..610a1ee0b34d2f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -301,8 +301,8 @@ class ProcessGDBRemote : public Process,
using FlashRange = FlashRangeVector::Entry;
FlashRangeVector m_erased_flash_ranges;
- // Number of vfork in process.
- int m_vfork_in_progress_count;
+ // Number of vfork() operations being handled.
+ uint32_t m_vfork_in_progress_count;
// Accessors
bool IsRunning(lldb::StateType state) {
>From bc36011d81e6f582bfea6b0da65b64d48fd8e4b3 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffrey...@fb.com>
Date: Tue, 5 Mar 2024 09:06:46 -0800
Subject: [PATCH 6/8] Adding exec() and fork() testcases
---
.../Process/gdb-remote/ProcessGDBRemote.cpp | 13 ++-
.../concurrent_vfork/TestConcurrentVFork.py | 100 +++++++++++++++---
.../fork/concurrent_vfork/main.cpp | 45 +++++++-
3 files changed, 129 insertions(+), 29 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 081c9935cf3492..0b7d42162482ae 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -5271,8 +5271,10 @@ class CommandObjectProcessGDBRemoteSpeedTest : public
CommandObjectParsed {
(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 =
@@ -5618,8 +5620,7 @@ void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid,
lldb::tid_t child_tid) {
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;
+ ++m_vfork_in_progress_count;
// Disable all software breakpoints for the duration of vfork.
if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware))
@@ -5674,8 +5675,7 @@ void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid,
lldb::tid_t child_tid) {
void ProcessGDBRemote::DidVForkDone() {
assert(m_vfork_in_progress_count > 0);
- if (m_vfork_in_progress_count > 0)
- --m_vfork_in_progress_count;
+ --m_vfork_in_progress_count;
// Reenable all software breakpoints that were enabled before vfork.
if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware))
@@ -5686,7 +5686,6 @@ void ProcessGDBRemote::DidExec() {
// If we are following children, vfork is finished by exec (rather than
// vforkdone that is submitted for parent).
if (GetFollowForkMode() == eFollowChild) {
- assert(m_vfork_in_progress_count > 0);
if (m_vfork_in_progress_count > 0)
--m_vfork_in_progress_count;
}
diff --git
a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
index 8953dbbf67f98e..c51e3657e02eb5 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
@@ -16,34 +16,100 @@ def get_pid_from_variable(self):
target = self.dbg.GetTargetAtIndex(0)
return target.FindFirstGlobalVariable("g_pid").GetValueAsUnsigned()
- @skipIfWindows
- def test_vfork_follow_parent(self):
- """
- Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-parent.
- And follow-parent successfully detach all child processes and exit
debugger.
- """
-
+ def build_run_to_breakpoint(self, use_fork, call_exec):
self.build()
+
+ args = []
+ if use_fork:
+ args.append("--fork")
+ if call_exec:
+ args.append("--exec")
+ launch_info = lldb.SBLaunchInfo(args)
+ launch_info.SetWorkingDirectory(self.getBuildDir())
+
lldbutil.run_to_source_breakpoint(
self, "// break here", lldb.SBFileSpec("main.cpp")
)
+
+ def follow_parent_helper(self, use_fork, call_exec):
+ self.build_run_to_breakpoint(use_fork, call_exec)
+
parent_pid = self.get_pid_from_variable()
self.runCmd("settings set target.process.follow-fork-mode parent")
+ self.runCmd("settings set target.process.stop-on-exec False",
check=False)
self.expect(
"continue", substrs=[f"Process {parent_pid} exited with status =
0"]
)
- @skipIfWindows
- def test_vfork_follow_child(self):
- """
- Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-child.
- And follow-child successfully detach parent process and exit child
process with correct exit code.
- """
- self.build()
- lldbutil.run_to_source_breakpoint(
- self, "// break here", lldb.SBFileSpec("main.cpp")
- )
+ def follow_child_helper(self, use_fork, call_exec):
+ self.build_run_to_breakpoint(use_fork, call_exec)
+
self.runCmd("settings set target.process.follow-fork-mode child")
+ self.runCmd("settings set target.process.stop-on-exec False",
check=False)
# Child process exits with code "index + 10" since index is [0-4]
# so the exit code should be 1[0-4]
self.expect("continue", patterns=[r"exited with status = 1[0-4]"])
+
+ @skipUnlessPlatform(["linux"])
+ def test_follow_parent_vfork_no_exec(self):
+ """
+ Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-parent.
+ And follow-parent successfully detach all child processes and exit
debugger without calling exec.
+ """
+ self.follow_parent_helper(use_fork=False, call_exec=False)
+
+ @skipUnlessPlatform(["linux"])
+ def test_follow_parent_fork_no_exec(self):
+ """
+ Make sure that debugging concurrent fork() from multiple threads won't
crash lldb during follow-parent.
+ And follow-parent successfully detach all child processes and exit
debugger without calling exec
+ """
+ self.follow_parent_helper(use_fork=True, call_exec=False)
+
+ @skipUnlessPlatform(["linux"])
+ def test_follow_parent_vfork_call_exec(self):
+ """
+ Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-parent.
+ And follow-parent successfully detach all child processes and exit
debugger after calling exec.
+ """
+ self.follow_parent_helper(use_fork=False, call_exec=True)
+
+ @skipUnlessPlatform(["linux"])
+ def test_follow_parent_fork_call_exec(self):
+ """
+ Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-parent.
+ And follow-parent successfully detach all child processes and exit
debugger after calling exec.
+ """
+ self.follow_parent_helper(use_fork=True, call_exec=True)
+
+ @skipUnlessPlatform(["linux"])
+ def test_follow_child_vfork_no_exec(self):
+ """
+ Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-child.
+ And follow-child successfully detach parent process and exit child
process with correct exit code without calling exec.
+ """
+ self.follow_child_helper(use_fork=False, call_exec=False)
+
+ @skipUnlessPlatform(["linux"])
+ def test_follow_child_fork_no_exec(self):
+ """
+ Make sure that debugging concurrent fork() from multiple threads won't
crash lldb during follow-child.
+ And follow-child successfully detach parent process and exit child
process with correct exit code without calling exec.
+ """
+ self.follow_child_helper(use_fork=True, call_exec=False)
+
+ @skipUnlessPlatform(["linux"])
+ def test_follow_child_vfork_call_exec(self):
+ """
+ Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-child.
+ And follow-child successfully detach parent process and exit child
process with correct exit code after calling exec.
+ """
+ self.follow_child_helper(use_fork=False, call_exec=True)
+
+ @skipUnlessPlatform(["linux"])
+ def test_follow_child_fork_call_exec(self):
+ """
+ Make sure that debugging concurrent fork() from multiple threads won't
crash lldb during follow-child.
+ And follow-child successfully detach parent process and exit child
process with correct exit code after calling exec.
+ """
+ self.follow_child_helper(use_fork=True, call_exec=True)
diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
index f8e481663acd6a..b0a4446ba01581 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
@@ -1,3 +1,4 @@
+#include <assert.h>
#include <iostream>
#include <mutex>
#include <sys/wait.h>
@@ -9,8 +10,17 @@ pid_t g_pid = 0;
std::mutex g_child_pids_mutex;
std::vector<pid_t> g_child_pids;
+const char *g_program = nullptr;
+bool g_use_vfork = true; // Use vfork by default.
+bool g_call_exec = false; // Does not call exec by default.
+
int call_vfork(int index) {
- pid_t child_pid = vfork();
+ pid_t child_pid = 0;
+ if (g_use_vfork) {
+ child_pid = vfork();
+ } else {
+ child_pid = fork();
+ }
if (child_pid == -1) {
// Error handling
@@ -20,7 +30,13 @@ int call_vfork(int index) {
// This code is executed by the child process
g_pid = getpid();
printf("Child process: %d\n", g_pid);
- _exit(index + 10); // Exit the child process
+
+ if (g_call_exec) {
+ std::string child_exit_code = std::to_string(index + 10);
+ execl(g_program, g_program, "--child", child_exit_code.c_str(), NULL);
+ } else {
+ _exit(index + 10);
+ }
} else {
// This code is executed by the parent process
printf("[Parent] Forked process id: %d\n", child_pid);
@@ -60,11 +76,30 @@ void create_threads(int num_threads) {
wait_all_children_to_exit();
}
-int main() {
+// Can be called in various ways:
+// 1. [program]: use vfork and not call exec
+// 2. [program] --fork: use fork and not call exec
+// 3. [program] --fork --exec: use fork and call exec
+// 4. [program] --exec: use vfork and call exec
+// 5. [program] --child [exit_code]: child process
+int main(int argc, char *argv[]) {
g_pid = getpid();
- printf("Entering main() pid: %d\n", g_pid);
+ g_program = argv[0];
+
+ for (int i = 1; i < argc; ++i) {
+ if (strcmp(argv[i], "--child") == 0) {
+ assert(i + 1 < argc);
+ int child_exit_code = std::stoi(argv[i + 1]);
+ printf("Child process: %d, exiting with code %d\n", g_pid,
+ child_exit_code);
+ _exit(child_exit_code);
+ } else if (strcmp(argv[i], "--fork") == 0)
+ g_use_vfork = false;
+ else if (strcmp(argv[i], "--exec") == 0)
+ g_call_exec = true;
+ }
int num_threads = 5; // break here
create_threads(num_threads);
- printf("Exiting main() pid: %d\n", g_pid);
+ return 0;
}
>From f06d81a098241775f811476351a7cfc27ab4d4ed Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffrey...@fb.com>
Date: Wed, 6 Mar 2024 09:26:50 -0800
Subject: [PATCH 7/8] Use target from run_to_source_breakpoint() API
---
.../fork/concurrent_vfork/TestConcurrentVFork.py | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git
a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
index c51e3657e02eb5..93340354a4c8c9 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
@@ -12,8 +12,7 @@
class TestConcurrentVFork(TestBase):
NO_DEBUG_INFO_TESTCASE = True
- def get_pid_from_variable(self):
- target = self.dbg.GetTargetAtIndex(0)
+ def get_pid_from_variable(self, target):
return target.FindFirstGlobalVariable("g_pid").GetValueAsUnsigned()
def build_run_to_breakpoint(self, use_fork, call_exec):
@@ -27,14 +26,16 @@ def build_run_to_breakpoint(self, use_fork, call_exec):
launch_info = lldb.SBLaunchInfo(args)
launch_info.SetWorkingDirectory(self.getBuildDir())
- lldbutil.run_to_source_breakpoint(
+ return lldbutil.run_to_source_breakpoint(
self, "// break here", lldb.SBFileSpec("main.cpp")
)
def follow_parent_helper(self, use_fork, call_exec):
- self.build_run_to_breakpoint(use_fork, call_exec)
+ (target, process, thread, bkpt) = self.build_run_to_breakpoint(
+ use_fork, call_exec
+ )
- parent_pid = self.get_pid_from_variable()
+ parent_pid = self.get_pid_from_variable(target)
self.runCmd("settings set target.process.follow-fork-mode parent")
self.runCmd("settings set target.process.stop-on-exec False",
check=False)
self.expect(
>From 476511f969fc8de338e501a88121b506545b78b6 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffrey...@fb.com>
Date: Wed, 6 Mar 2024 10:28:34 -0800
Subject: [PATCH 8/8] Fix nit comment
---
.../fork/concurrent_vfork/TestConcurrentVFork.py | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git
a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
index 93340354a4c8c9..2dcbb728549fb4 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
@@ -2,7 +2,6 @@
Make sure that the concurrent vfork() from multiple threads works correctly.
"""
-
import lldb
import lldbsuite.test.lldbutil as lldbutil
from lldbsuite.test.lldbtest import *
@@ -12,9 +11,6 @@
class TestConcurrentVFork(TestBase):
NO_DEBUG_INFO_TESTCASE = True
- def get_pid_from_variable(self, target):
- return target.FindFirstGlobalVariable("g_pid").GetValueAsUnsigned()
-
def build_run_to_breakpoint(self, use_fork, call_exec):
self.build()
@@ -35,7 +31,7 @@ def follow_parent_helper(self, use_fork, call_exec):
use_fork, call_exec
)
- parent_pid = self.get_pid_from_variable(target)
+ parent_pid =
target.FindFirstGlobalVariable("g_pid").GetValueAsUnsigned()
self.runCmd("settings set target.process.follow-fork-mode parent")
self.runCmd("settings set target.process.stop-on-exec False",
check=False)
self.expect(
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
