src/gdb ChangeLog linux-nat.c

[palves at sourceware dot org]

CVSROOT:	/cvs/src
Module name:	src
Changes by:	palves@sourceware.org	2013-08-19 13:44:41

Modified files:
	gdb            : ChangeLog linux-nat.c 

Log message:
	linux-nat.c: no need to block child signals so aggressively.
	
	In http://sourceware.org/ml/gdb-patches/2013-08/msg00174.html , the
	issue of child signal handling around ptrace option support discovery
	being different between GDB and GDBserver came up.
	
	I recalled adding these block_child_signals calls, and the "We don't
	want those ptrace calls to be interrupted" comment, but not exactly
	why.  So I looked into it.  My first guess is that I got confused.
	The patch that added this
	<http://sourceware.org/ml/gdb-patches/2009-04/msg00125.html> rewrote
	the linux native async support completely, and the old async support
	code had the SIGCHLD handler itself do waitpid, so in places that we'd
	want a blocking waitpid, we'd have to have the signal handler blocked.
	That was probably the mindset I had at the time.  Anyway, whatever the
	case, looks like I was wrong on the need for this blocking.
	Given GDBserver doesn't block like this, I investigated why this is
	currently needed on GDB but not on GDBserver.
	
	I removed the block_child_signals (and restore) calls, and hacked
	linux-nat.c to call linux_test_for_tracefork in a loop, like:
	
	@@ -534,7 +534,10 @@ static int
	linux_supports_tracefork (int pid)
	{
	if (linux_supports_tracefork_flag == -1)
	-    linux_test_for_tracefork (pid);
	+    {
	+      while (1)
	+       linux_test_for_tracefork (pid);
	+    }
	return linux_supports_tracefork_flag;
	}
	
	Running the resulting GDB, I then saw bad things happening.
	Specifically, I'd end up with a bunch of zombies, and eventually, the
	machine would refuse to spawn new processes, claming insufficient
	resources.
	
	The issue is that linux_test_for_tracefork test forks, and has the
	child fork again.  If we don't block SIGCHLD on entry to the function,
	the children will inherit SIGCHLD's action/disposition (meaning,
	SIGCHLD will be unblocked in the child).  When the first child forks
	again a second child, and that child exits, the first child gets a
	SIGCHLD.  Now, when we try to wrap up for the whole options test, we
	kill the first child, and collect the waitstatus.  Here, when SIGCHLD
	isn't blocked, GDB will first see the child reporting a stop with
	SIGCHLD.  gdbserver's ptrace options test does a PTRACE_KILL loop at
	the end, which catches the SIGCHLD, and retries the kill.  The GDB
	version did not do that.  So the GDB version would proceed, leaving
	the child zombie (until GDB exists), as nothing collected its final
	waitstatus.
	
	So this patch makes the GDB version of linux_test_for_tracefork do the
	exact same as the GDBserver version, removes all this unnecessary
	blocking throughout, and adds a couple comments at places that do need
	it -- namely: places where we'll use sleep with sigsuspend; and
	linux_async_pipe, as that destroys the pipe the signal handler
	touches.
	
	Tested on x86_64 Fedora 17, sync and async.
	
	gdb/
	2013-08-19  Pedro Alves  <palves@redhat.com>
	
	* linux-nat.c (linux_test_for_tracefork)
	(linux_test_for_tracesysgood, linux_child_follow_fork)
	(lin_lwp_attach_lwp, linux_nat_resume): Don't block child signals.
	(linux_nat_wait_1): Extend comment.
	(linux_async_pipe): Add comment.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.15908&r2=1.15909
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/linux-nat.c.diff?cvsroot=src&r1=1.268&r2=1.269