[Lldb-commits] [lldb] 83a131b - Fix an over-suspend bug with LaunchInNewTerminalWithAppleScript sessions
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Tue Jan 21 14:55:53 PST 2020
Author: Jason Molenda
Date: 2020-01-21T14:55:46-08:00
New Revision: 83a131b276426a0dc97f43c139a0f3b308f24154
URL: https://github.com/llvm/llvm-project/commit/83a131b276426a0dc97f43c139a0f3b308f24154
DIFF: https://github.com/llvm/llvm-project/commit/83a131b276426a0dc97f43c139a0f3b308f24154.diff
LOG: Fix an over-suspend bug with LaunchInNewTerminalWithAppleScript sessions
When launching an inferior in a new terminal window via AppleScript
and the darwin-debug helper program, we could often end up with the
inferior process having a too-high suspend count, and it would never
resume execution.
lldb tries to wait until darwin-debug has finished its work and has
launched the inferior (WaitForProcessToSIGSTOP) but this wasn't
working correctly - and cannot be made to work.
This patch removes WaitForProcessToSIGSTOP, adds a special tiny
segment to the darwin-debug executable so it can be identified as
that binary (ExecExtraSuspend), and adds code to debugserver to
detect this segment. When debugserver sees this segment, it notes
that the next exec will be done with a launch-suspended flag. When
the next exec happens, debugserver forces an extra task_resume when
we resume the inferior.
An alternative approach would be if lldb could detect when the
inferior has been launched by darwin-debug unambiguously; monitoring
when the unix socket between darwin-debug and lldb was closed would
have been a reasonable way to do this too.
<rdar://problem/29760580>
Differential Revision: https://reviews.llvm.org/D72963
Added:
Modified:
lldb/source/Host/macosx/objcxx/Host.mm
lldb/tools/darwin-debug/CMakeLists.txt
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/MacOSX/MachTask.h
lldb/tools/debugserver/source/MacOSX/MachTask.mm
Removed:
################################################################################
diff --git a/lldb/source/Host/macosx/objcxx/Host.mm b/lldb/source/Host/macosx/objcxx/Host.mm
index 9febb8fb8b29..233734109c41 100644
--- a/lldb/source/Host/macosx/objcxx/Host.mm
+++ b/lldb/source/Host/macosx/objcxx/Host.mm
@@ -153,33 +153,6 @@
return NULL;
}
-static bool WaitForProcessToSIGSTOP(const lldb::pid_t pid,
- const int timeout_in_seconds) {
- const int time_delta_usecs = 100000;
- const int num_retries = timeout_in_seconds / time_delta_usecs;
- for (int i = 0; i < num_retries; i++) {
- struct proc_bsdinfo bsd_info;
- int error = ::proc_pidinfo(pid, PROC_PIDTBSDINFO, (uint64_t)0, &bsd_info,
- PROC_PIDTBSDINFO_SIZE);
-
- switch (error) {
- case EINVAL:
- case ENOTSUP:
- case ESRCH:
- case EPERM:
- return false;
-
- default:
- break;
-
- case 0:
- if (bsd_info.pbi_status == SSTOP)
- return true;
- }
- ::usleep(time_delta_usecs);
- }
- return false;
-}
#if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
const char *applscript_in_new_tty = "tell application \"Terminal\"\n"
@@ -325,11 +298,6 @@ repeat with the_window in (get windows)\n\
lldb_error = accept_thread->Join(&accept_thread_result);
if (lldb_error.Success() && accept_thread_result) {
pid = (intptr_t)accept_thread_result;
-
- // Wait for process to be stopped at the entry point by watching
- // for the process status to be set to SSTOP which indicates it it
- // SIGSTOP'ed at the entry point
- WaitForProcessToSIGSTOP(pid, 5);
}
llvm::sys::fs::remove(unix_socket_name);
diff --git a/lldb/tools/darwin-debug/CMakeLists.txt b/lldb/tools/darwin-debug/CMakeLists.txt
index b902788f05ab..ab3dbf0060ab 100644
--- a/lldb/tools/darwin-debug/CMakeLists.txt
+++ b/lldb/tools/darwin-debug/CMakeLists.txt
@@ -1,3 +1,11 @@
+
+# Create an LC_SEGMENT with the special name ExecExtraSuspend which
+# debugserver can detect - it tells debugserver that it will exec a
+# process and that process will start suspended, so debugserver will
+# need to double-resume it to make it run. A random file is copied
+# into the segment.
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-sectcreate,ExecExtraSuspend,ExecExtraSuspend,${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt")
+
add_lldb_tool(darwin-debug ADD_TO_FRAMEWORK
darwin-debug.cpp
)
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 40facdfb5cf9..e4ec6129f70c 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -734,6 +734,8 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
this_seg.nsects = seg.nsects;
this_seg.flags = seg.flags;
inf.segments.push_back(this_seg);
+ if (this_seg.name == "ExecExtraSuspend")
+ m_task.TaskWillExecProcessesSuspended();
}
if (lc.cmd == LC_SEGMENT_64) {
struct segment_command_64 seg;
@@ -755,6 +757,8 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
this_seg.nsects = seg.nsects;
this_seg.flags = seg.flags;
inf.segments.push_back(this_seg);
+ if (this_seg.name == "ExecExtraSuspend")
+ m_task.TaskWillExecProcessesSuspended();
}
if (lc.cmd == LC_UUID) {
struct uuid_command uuidcmd;
diff --git a/lldb/tools/debugserver/source/MacOSX/MachTask.h b/lldb/tools/debugserver/source/MacOSX/MachTask.h
index c975e15a5551..62f90729393c 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachTask.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachTask.h
@@ -85,6 +85,7 @@ class MachTask {
const MachProcess *Process() const { return m_process; }
nub_size_t PageSize();
+ void TaskWillExecProcessesSuspended() { m_exec_will_be_suspended = true; }
protected:
MachProcess *m_process; // The mach process that owns this MachTask
@@ -97,6 +98,12 @@ class MachTask {
// need it
mach_port_t m_exception_port; // Exception port on which we will receive child
// exceptions
+ bool m_exec_will_be_suspended; // If this task exec's another process, that
+ // process will be launched suspended and we will
+ // need to execute one extra Resume to get it
+ // to progress from dyld_start.
+ bool m_do_double_resume; // next time we task_resume(), do it twice to
+ // fix a too-high suspend count.
typedef std::map<mach_vm_address_t, size_t> allocation_collection;
allocation_collection m_allocations;
diff --git a/lldb/tools/debugserver/source/MacOSX/MachTask.mm b/lldb/tools/debugserver/source/MacOSX/MachTask.mm
index 0d5a63a28f20..5d18c5628c63 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachTask.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachTask.mm
@@ -69,7 +69,8 @@
//----------------------------------------------------------------------
MachTask::MachTask(MachProcess *process)
: m_process(process), m_task(TASK_NULL), m_vm_memory(),
- m_exception_thread(0), m_exception_port(MACH_PORT_NULL) {
+ m_exception_thread(0), m_exception_port(MACH_PORT_NULL),
+ m_exec_will_be_suspended(false), m_do_double_resume(false) {
memset(&m_exc_port_info, 0, sizeof(m_exc_port_info));
}
@@ -103,6 +104,14 @@
err = BasicInfo(task, &task_info);
if (err.Success()) {
+ if (m_do_double_resume && task_info.suspend_count == 2) {
+ err = ::task_resume(task);
+ if (DNBLogCheckLogBit(LOG_TASK) || err.Fail())
+ err.LogThreaded("::task_resume double-resume after exec-start-stopped "
+ "( target_task = 0x%4.4x )", task);
+ }
+ m_do_double_resume = false;
+
// task_resume isn't counted like task_suspend calls are, are, so if the
// task is not suspended, don't try and resume it since it is already
// running
@@ -135,6 +144,8 @@
m_task = TASK_NULL;
m_exception_thread = 0;
m_exception_port = MACH_PORT_NULL;
+ m_exec_will_be_suspended = false;
+ m_do_double_resume = false;
}
//----------------------------------------------------------------------
@@ -651,6 +662,9 @@ static void get_threads_profile_data(DNBProfileDataScanType scanType,
err.LogThreaded("::mach_port_deallocate ( task = 0x%4.4x, name = 0x%4.4x )",
task_self, exception_port);
+ m_exec_will_be_suspended = false;
+ m_do_double_resume = false;
+
return err.Status();
}
@@ -960,4 +974,14 @@ static void get_threads_profile_data(DNBProfileDataScanType scanType,
void MachTask::TaskPortChanged(task_t task)
{
m_task = task;
+
+ // If we've just exec'd to a new process, and it
+ // is started suspended, we'll need to do two
+ // task_resume's to get the inferior process to
+ // continue.
+ if (m_exec_will_be_suspended)
+ m_do_double_resume = true;
+ else
+ m_do_double_resume = false;
+ m_exec_will_be_suspended = false;
}
More information about the lldb-commits
mailing list