This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Regression] Segfault on native-extended-gdbserver + fork


On 2018-01-28 01:32, Sergio Durigan Junior wrote:
Hey Simon,

While working on something else, I noticed a regression introduced by
this patch.

Hi Sergio,

Thanks for reporting and fixing this!

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

Ah yeah, I hadn't thought about that.

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 fine with this, but I was curious about what happens in Pedro's multi-target branch. I remember he said that the detach_inferior(int) version disappears in that branch, though I can't find where he said that. But looking at the branch I can see it's indeed the case:

https://github.com/palves/gdb/blob/palves/multi-target/gdb/inferior.c#L250

So I was wondering what remote_follow_fork calls in that case, since it can't call the detach_inferior(inferior *) version without an inferior. Apparently it calls a new remote_detach_pid function:

https://github.com/palves/gdb/blob/palves/multi-target/gdb/remote.c#L5859

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.

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?

That's fine with me with the printing of the message in its own function, as explained above.

--
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

Thanks,

Simon


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]