public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Sergio Durigan Junior <sergiodj@redhat.com>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: Pedro Alves <palves@redhat.com>,
	 gdb-patches@sourceware.org,
	 Simon Marchi <simon.marchi@ericsson.com>
Subject: Re: [Regression] Segfault on native-extended-gdbserver + fork
Date: Mon, 29 Jan 2018 17:36:00 -0000	[thread overview]
Message-ID: <87372o8t45.fsf@redhat.com> (raw)
In-Reply-To: <87mv0w8tnr.fsf@redhat.com>

On Monday, January 29 2018, I wrote:

> On Monday, January 29 2018, Simon Marchi wrote:
>
>> On 2018-01-29 11:00, Pedro Alves wrote:
>>> On 01/28/2018 04:50 PM, Simon Marchi wrote:
>>>> On 2018-01-28 01:32, Sergio Durigan Junior wrote:
>>>> This means (I just tried it) that it won't show the "[Inferior %d
>>>> detached]\n" message in that case.  So what I would suggest is
>>>> putting
>>>>
>>>>   if (print_inferior_events)
>>>>     printf_unfiltered (_("[Inferior %d detached]\n"), pid);
>>>>
>>>> in its own function, called by both versions of detach_inferior for
>>>> now (bonus, it de-duplicates the printing of the message).  In the
>>>> multi-target branch, remote_target::follow_fork (renamed from
>>>> remote_follow_fork) can call this function in the case where we
>>>> don't have an inferior object.
>>>
>>> But why would we want to print that?  We will have already printed
>>>
>>>   "Detaching after fork from child process PID."
>>>
>>> from the common code.  When native debugging, in this scenario,
>>> we don't call detach_inferior either, right?  Can't see why
>>> we'd want to call it for remote.
>>
>> It's true that it's a bit of a lie to say "[Inferior PID detached]" if
>> there never actually was an inferior for that PID.  Since we never
>> print "[Inferior PID detached]" on native in that case, I am fine with
>> removing the call from remote.c.  Sergio, that would fix the crash you
>> found I think?
>
> I was also unsure about printing the message in this case, because
> there's no real detach happening.  I'm fine with not printing it.  And
> yes, removing the call to "detach_inferior" also fixes the problem.
>
> I'll prepare a patch.

Here's what I have.  WDYT?

I'll address Pedro's comment about changing the "[Inferior PID
detached]" output in another patch.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

From 4a37d08ca6c1aec7f47e2278b0fe78a0038eb9ee Mon Sep 17 00:00:00 2001
From: Sergio Durigan Junior <sergiodj@redhat.com>
Date: Mon, 29 Jan 2018 12:29:21 -0500
Subject: [PATCH] Don't call "detach_inferior" on "remote_follow_fork"

This patch fixes a regression that has been introduced by:

commit bc09b0c14fb713a9aec25e09b78499f3bc2441b5
Date:   Fri Jan 19 11:48:11 2018 -0500

    Make linux_nat_detach/thread_db_detach use the inferior parameter

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, we see a segmentation fault:

  # ./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'.

The proper fix here is to not call "detach_inferior" when doing remote
follow-fork, because we don't have an inferior to detach on the host
side.

This has been regtested using BuildBot and no regressions were found.

gdb/ChangeLog:
2018-01-29  Sergio Durigan Junior  <sergiodj@redhat.com>

	* remote.c (remote_follow_fork): Don't call "detach_inferior".
---
 gdb/remote.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 5ac84df0a0..74d18f7b17 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5206,7 +5206,6 @@ remote_follow_fork (struct target_ops *ops, int follow_child,
 	  child_pid = ptid_get_pid (child_ptid);
 
 	  remote_detach_pid (child_pid);
-	  detach_inferior (child_pid);
 	}
     }
   return 0;
-- 
2.14.3

  reply	other threads:[~2018-01-29 17:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-19 16:16 [PATCH v2 1/3] Remove args from target detach Simon Marchi
2018-01-19 16:16 ` [PATCH v2 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter Simon Marchi
2018-01-28  6:32   ` [Regression] Segfault on native-extended-gdbserver + fork (was: Re: [PATCH v2 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter) Sergio Durigan Junior
2018-01-28 16:50     ` [Regression] Segfault on native-extended-gdbserver + fork Simon Marchi
2018-01-29 16:01       ` Pedro Alves
2018-01-29 16:25         ` Simon Marchi
2018-01-29 16:58           ` Pedro Alves
2018-01-29 17:04             ` Simon Marchi
2018-01-29 17:31               ` Pedro Alves
2018-01-29 17:36                 ` Pedro Alves
2018-01-29 17:24           ` Sergio Durigan Junior
2018-01-29 17:36             ` Sergio Durigan Junior [this message]
2018-01-29 17:47               ` Pedro Alves
2018-01-29 18:06                 ` Sergio Durigan Junior
2018-01-19 16:16 ` [PATCH v2 2/3] Pass inferior down to target_detach and to_detach Simon Marchi
2018-01-19 16:35 ` [PATCH v2 1/3] Remove args from target detach Pedro Alves
2018-01-19 16:57   ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87372o8t45.fsf@redhat.com \
    --to=sergiodj@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    --cc=simon.marchi@ericsson.com \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).