This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[Regression] Segfault on native-extended-gdbserver + fork (was: Re: [PATCH v2 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter)
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Simon Marchi <simon dot marchi at polymtl dot ca>
- Cc: gdb-patches at sourceware dot org, Simon Marchi <simon dot marchi at ericsson dot com>
- Date: Sun, 28 Jan 2018 01:32:12 -0500
- Subject: [Regression] Segfault on native-extended-gdbserver + fork (was: Re: [PATCH v2 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter)
- Authentication-results: sourceware.org; auth=none
- References: <20180119161628.21611-1-simon.marchi@polymtl.ca> <20180119161628.21611-3-simon.marchi@polymtl.ca>
On Friday, January 19 2018, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@ericsson.com>
>
> No changes in v2.
>
> This patch makes these two functions actually use the inferior parameter
> added by the previous patch, instead of reading inferior_ptid. I chose
> these two, because they are the one actually used when I detach on my
> GNU/Linux system, so they were easy to test.
>
> I took the opportunity to pass the inferior being detached to
> inf_ptrace_detach_success, so it could use it too. From there, it made
> sense to add an overload of detach_inferior that takes the inferior
> directly rather than the pid, to avoid having to pass inf->pid only for
> the callee to look up the inferior structure by pid.
Hey Simon,
While working on something else, I noticed a regression introduced by
this patch. Consider the following example program:
#include <unistd.h>
int
main (int argc, char *argv[])
{
fork ();
return 0;
}
When running it under gdbserver:
# ./gdb/gdbserver/gdbserver --multi --once :2345
And debugging it under GDB:
# ./gdb/gdb -q -batch -ex 'set remote exec-file ./a.out' -ex 'tar extended-remote :2345' -ex r ./a.out
Starting program:
...
[Detaching after fork from child process 16102.]
Segmentation fault (core dumped)
The problem happens on inferior.c:detach_inferior:
void
detach_inferior (inferior *inf)
{
/* Save the pid, since exit_inferior_1 will reset it. */
int pid = inf->pid;
^^^^^^^^^
exit_inferior_1 (inf, 0);
if (print_inferior_events)
printf_unfiltered (_("[Inferior %d detached]\n"), pid);
}
When this code is called from remote.c:remote_follow_fork, the PID is
valid but there is not 'inferior' associated with it, which means that
'inf == NULL'.
I've been thinking about the proper fix to this, and arrived at the
patch attached (without a ChangeLog entry; will add that if the patch
seems OK for you). Since we will still want to print inferior events
even if 'inf == NULL', I've duplicated the print on the "detach_inferior
(int pid)" version. Other than that, the patch is basically restoring
the old behaviour of just skipping the detach procedure if there's no
inferior object.
I'm running a regression test on BuildBot to make sure no regressions
are introduced. I was going to write a testcase to exercise this
scenario, but we already have one, gdb.base/foll-vfork.exp. The
failures were marked as ERROR's by dejagnu, which may explain why they
were missed...? Not sure. Oh, and this regression is not present in
the 8.1 branch.
WDYT?
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 38b7369275..94432a92b1 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -272,7 +272,15 @@ detach_inferior (inferior *inf)
void
detach_inferior (int pid)
{
- detach_inferior (find_inferior_pid (pid));
+ inferior *inf = find_inferior_pid (pid);
+
+ if (inf != NULL)
+ detach_inferior (inf);
+ else
+ {
+ if (print_inferior_events)
+ printf_unfiltered (_("[Inferior %d detached]\n"), pid);
+ }
}
void