labath created this revision.
labath added reviewers: mgorny, DavidSpickett.
labath requested review of this revision.
Herald added a project: LLDB.
Our SIGTSTP handler was working, but that was mostly accidental.
The reason it worked is because lldb is multithreaded for most of its
lifetime and the OS is reasonably fast at responding to signals. So,
what happened was that the kill(SIGTSTP) which we sent from inside the
handler was delivered to another thread while the handler was still set
to SIG_DFL (which then correctly put the entire process to sleep).
Sometimes it happened that the other thread got the second signal after
the first thread had already restored the handler, in which case the
signal handler would run again, and it would again attempt to send the
SIGTSTP signal back to itself.
Normally it didn't take many iterations for the signal to be delivered
quickly enough. However, if you were unlucky (or were playing around
with pexpect) you could get SIGTSTP while lldb was single-threaded, and
in that case, lldb would go into an endless loop because the second
SIGTSTP could only be handled on the main thread, and only after the
handler for the first signal returned (and re-installed itself). In that
situation the handler would keep re-sending the signal to itself.
This patch fixes the issue by implementing the handler the way it
supposed to be done:
- before sending the second SIGTSTP, we unblock the signal (it gets
automatically blocked upon entering the handler)
- we use raise to send the signal, which makes sure it gets delivered to the
thread which is running the handler
This also means we don't need the SIGCONT handler, as our TSTP handler
resumes right after the entire process is continued, and we can do the
required work there.
I also include a test case for the SIGTSTP flow. It uses pexpect, but it
includes a couple of extra twists. Specifically, I needed to create an
extra process on top of lldb, which will run lldb in a separate process
group and simulate the role of the shell. This is needed because SIGTSTP
is not effective on a session leader (the signal gets delivered, but it
does not cause a stop) -- normally there isn't anyone to notice the
stop.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D120320
Files:
lldb/packages/Python/lldbsuite/test/lldbpexpect.py
lldb/test/API/driver/job_control/TestJobControl.py
lldb/test/API/driver/job_control/shell.py
lldb/tools/driver/Driver.cpp
Index: lldb/tools/driver/Driver.cpp
===================================================================
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -668,23 +668,30 @@
_exit(signo);
}
+#ifndef _WIN32
void sigtstp_handler(int signo) {
if (g_driver != nullptr)
g_driver->GetDebugger().SaveInputTerminalState();
+ // Unblock the signal and remove our handler.
+ sigset_t set;
+ sigemptyset(&set);
+ sigaddset(&set, signo);
+ pthread_sigmask(SIG_UNBLOCK, &set, nullptr);
signal(signo, SIG_DFL);
- kill(getpid(), signo);
+
+ // Now re-raise the signal. We will immediately suspend...
+ raise(signo);
+ // ... and resume after a SIGCONT.
+
+ // Now undo the modifications.
+ pthread_sigmask(SIG_BLOCK, &set, nullptr);
signal(signo, sigtstp_handler);
-}
-void sigcont_handler(int signo) {
if (g_driver != nullptr)
g_driver->GetDebugger().RestoreInputTerminalState();
-
- signal(signo, SIG_DFL);
- kill(getpid(), signo);
- signal(signo, sigcont_handler);
}
+#endif
void reproducer_handler(void *finalize_cmd) {
if (SBReproducer::Generate()) {
@@ -831,7 +838,6 @@
signal(SIGPIPE, SIG_IGN);
signal(SIGWINCH, sigwinch_handler);
signal(SIGTSTP, sigtstp_handler);
- signal(SIGCONT, sigcont_handler);
#endif
int exit_code = 0;
Index: lldb/test/API/driver/job_control/shell.py
===================================================================
--- /dev/null
+++ lldb/test/API/driver/job_control/shell.py
@@ -0,0 +1,26 @@
+"""
+Launch a process (given through argv) similar to how a shell would do it.
+"""
+
+import signal
+import subprocess
+import sys
+import os
+
+def preexec_fn():
+ orig_mask = signal.pthread_sigmask(signal.SIG_BLOCK, [signal.SIGTTOU])
+ os.setpgid(0, 0)
+ fd = os.open("/dev/tty", os.O_RDONLY)
+ os.tcsetpgrp(fd, os.getpgid(0))
+ os.close(fd)
+ signal.pthread_sigmask(signal.SIG_SETMASK, orig_mask)
+
+if __name__ == "__main__":
+ child = subprocess.Popen(sys.argv[1:], preexec_fn=preexec_fn)
+ print("PID=%d" % child.pid)
+
+ _, status = os.waitpid(child.pid, os.WUNTRACED)
+ print("STATUS=%d" % status)
+
+ returncode = child.wait()
+ print("RETURNCODE=%d" % returncode)
Index: lldb/test/API/driver/job_control/TestJobControl.py
===================================================================
--- /dev/null
+++ lldb/test/API/driver/job_control/TestJobControl.py
@@ -0,0 +1,36 @@
+"""
+Test lldb's handling of job control signals (SIGTSTP, SIGCONT).
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+
+class JobControlTest(PExpectTest):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ def test_job_control(self):
+ def post_spawn():
+ self.child.expect("PID=([0-9]+)")
+ self.lldb_pid = int(self.child.match[1])
+
+ run_under = [sys.executable, self.getSourcePath('shell.py')]
+ self.launch(run_under=run_under, post_spawn=post_spawn)
+ child = self.child
+
+ os.kill(self.lldb_pid, signal.SIGTSTP)
+ self.child.expect("STATUS=([0-9]+)")
+ status = int(self.child.match[1])
+
+ self.assertTrue(os.WIFSTOPPED(status))
+ self.assertEquals(os.WSTOPSIG(status), signal.SIGTSTP)
+
+ os.kill(self.lldb_pid, signal.SIGCONT)
+
+ self.child.sendline("quit")
+ self.child.expect("RETURNCODE=0")
Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -23,11 +23,15 @@
def expect_prompt(self):
self.child.expect_exact(self.PROMPT)
- def launch(self, executable=None, extra_args=None, timeout=60, dimensions=None):
+ def launch(self, executable=None, extra_args=None, timeout=60,
+ dimensions=None, run_under=None, post_spawn=None):
logfile = getattr(sys.stdout, 'buffer',
sys.stdout) if self.TraceOn() else None
- args = ['--no-lldbinit', '--no-use-colors']
+ args = []
+ if run_under:
+ args += run_under
+ args += [lldbtest_config.lldbExec, '--no-lldbinit', '--no-use-colors']
for cmd in self.setUpCommands():
args += ['-O', cmd]
if executable is not None:
@@ -41,8 +45,11 @@
import pexpect
self.child = pexpect.spawn(
- lldbtest_config.lldbExec, args=args, logfile=logfile,
+ args[0], args=args[1:], logfile=logfile,
timeout=timeout, dimensions=dimensions, env=env)
+
+ if post_spawn:
+ post_spawn()
self.expect_prompt()
for cmd in self.setUpCommands():
self.child.expect_exact(cmd)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
