OpenE2K
/
binutils-gdb
12
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Simplify child_terminal_inferior 

The comment about Lynx in child_terminal_init reads a bit odd, since
it's not exactly clear what "This" in "This is for Lynx" is referring
to.  Looking back in history makes it clearer.  When the comment was
originally added, in commit 91ecc8efa9, back in 1994, the code
looked like this:

~~~
#ifdef PROCESS_GROUP_TYPE
#ifdef PIDGET
      /* This is for Lynx, and should be cleaned up by having Lynx be
         a separate debugging target with a version of
         target_terminal_init_inferior which passes in the process
         group to a generic routine which does all the work (and the
         non-threaded child_terminal_init_inferior can just pass in
         inferior_pid to the same routine).  */
      inferior_process_group = PIDGET (inferior_pid);
#else
      inferior_process_group = inferior_pid;
#endif
#endif
~~~

So this looked like it was about when GDB was growing support for
multi-threading, and inferior_pid was still a single int for most
ports.

Eventually we got ptid_t, so the comment isn't really useful today.
Particularly more so since we no longer support Lynx as a GDB host.

The only caller left of child_terminal_init_with_pgrp is gnu-nat.c
(the Hurd), and that target uses fork-child, so when we reach
gnu_terminal_init after spawning a new child, the current inferior
must already have the PID set, and the child must be a process group
leader.

We can't add a 'getpgid(inf->pid) == inf->pid' assertion to
child_terminal_init though (like a previous version of this patch was
doing [1]), because child_terminal_init is also reached after
attaching to a process.  If we did, the new
gdb.base/attach-non-pgrp-leader.exp test would fail, with:

  (gdb) attach 12415
  Attaching to program: build/gdb/testsuite/outputs/gdb.base/attach-non-pgrp-leader/attach-non-pgrp-leader, process 12415
  src/gdb/inflow.c:180: internal-error: void child_terminal_init(target_ops*): Assertion `getpgid (inf->pid) == inf->pid' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) FAIL: gdb.base/attach-non-pgrp-leader.exp: child: attach to child (GDB internal error)

I'm not making GDB save the pgid for attached processes with getpgid
for now, because the saved process group affects other things which
I'm leaving for following patches, like e.g., the "interrupt" command.

[1] - https://sourceware.org/ml/gdb-patches/2017-11/msg00039.html

gdb/ChangeLog:
2017-11-06  Pedro Alves  <palves@redhat.com>

	* gnu-nat.c (gnu_terminal_init): Delete.
	(gnu_target): Don't install gnu_terminal_init.
	* inflow.c (child_terminal_init_with_pgrp): Delete, merged with ...
	(child_terminal_init): ... this function.
This commit is contained in:
Pedro Alves 2017-11-06 15:36:47 +00:00
parent 46f67f80dd
commit 556e5da513
3 changed files with 12 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
gdb/ChangeLog
View File

@ -1,3 +1,10 @@
2017-11-06 Pedro Alves <palves@redhat.com>
* gnu-nat.c (gnu_terminal_init): Delete.
(gnu_target): Don't install gnu_terminal_init.
* inflow.c (child_terminal_init_with_pgrp): Delete, merged with ...
(child_terminal_init): ... this function.
2017-11-06 Pedro Alves <palves@redhat.com>
* common/common.m4 (GDB_AC_COMMON): No longer check termio.h nor

7
gdb/gnu-nat.c
View File

@ -2279,12 +2279,6 @@ gnu_detach (struct target_ops *ops, const char *args, int from_tty)
inf_child_maybe_unpush_target (ops);
}
static void
gnu_terminal_init (struct target_ops *self)
{
gdb_assert (gnu_current_inf);
child_terminal_init_with_pgrp (gnu_current_inf->pid);
}
static void
gnu_stop (struct target_ops *self, ptid_t ptid)
@ -2693,7 +2687,6 @@ gnu_target (void)
t->to_wait = gnu_wait;
t->to_xfer_partial = gnu_xfer_partial;
t->to_find_memory_regions = gnu_find_memory_regions;
t->to_terminal_init = gnu_terminal_init;
t->to_kill = gnu_kill_inferior;
t->to_create_inferior = gnu_create_inferior;
t->to_mourn_inferior = gnu_mourn_inferior;

22
gdb/inflow.c
View File

@ -165,7 +165,7 @@ gdb_has_a_terminal (void)
before we actually run the inferior. */
void
child_terminal_init_with_pgrp (int pgrp)
child_terminal_init (struct target_ops *self)
{
struct inferior *inf = current_inferior ();
struct terminal_info *tinfo = get_inflow_inferior_data (inf);
@ -173,8 +173,10 @@ child_terminal_init_with_pgrp (int pgrp)
#ifdef HAVE_TERMIOS_H
/* Store the process group even without a terminal as it is used not
only to reset the tty foreground process group, but also to
interrupt the inferior. */
tinfo->process_group = pgrp;
interrupt the inferior. A child we spawn should be a process
group leader (PGID==PID) at this point, though that may not be
true if we're attaching to an existing process. */
tinfo->process_group = inf->pid;
#endif
if (gdb_has_a_terminal ())
@ -204,20 +206,6 @@ gdb_save_tty_state (void)
}
}
void
child_terminal_init (struct target_ops *self)
{
#ifdef HAVE_TERMIOS_H
/* This is for Lynx, and should be cleaned up by having Lynx be a
separate debugging target with a version of target_terminal::init
which passes in the process group to a generic routine which does
all the work (and the non-threaded child_terminal_init can just
pass in inferior_ptid to the same routine). */
/* We assume INFERIOR_PID is also the child's process group. */
child_terminal_init_with_pgrp (ptid_get_pid (inferior_ptid));
#endif /* HAVE_TERMIOS_H */
}
/* Put the inferior's terminal settings into effect.
This is preparation for starting or resuming the inferior.