[PATCH] The gcore command hangs without a terminal

[PATCH] The gcore command hangs without a terminal

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

Hi,

The function `attach_command' sets up the terminal by calling
target_terminal_inferior ().  This makes gcore(1) to lock up when being run
without its controlling terminal.

I do not find a reason for this terminal setup as GDB does not share its
input/output with the application in the mode of external attached program.

Provided a testcase for `gdb_gcore.sh' at all.


Regards,
Jan


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229517

[-- Attachment #2: gdb-gcore-detached.patch --]
[-- Type: text/plain, Size: 5873 bytes --]

2007-02-25  Jan Kratochvil <jan.kratochvil@redhat.com>

	* infcmd.c (attach_command): Do not call target_terminal_inferior ().

2007-02-25  Jan Kratochvil <jan.kratochvil@redhat.com>

	* gdb.base/gcorebg.exp, gdb.base/gcorebg.c: New files.
	
	
Index: gdb/infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.149
diff -u -p -r1.149 infcmd.c
--- gdb/infcmd.c	9 Jan 2007 17:58:51 -0000	1.149
+++ gdb/infcmd.c	25 Feb 2007 12:21:19 -0000
@@ -1944,8 +1944,11 @@ attach_command (char *args, int from_tty
 
   post_create_inferior (&current_target, from_tty);
 
-  /* Install inferior's terminal modes.  */
-  target_terminal_inferior ();
+  /* Do not install inferior's terminal modes by target_terminal_inferior ()
+     as we do not need to share the terminal with the inferior for external
+     attached processes.
+     We may be run from `gdb_gcore.sh' locking up on the terminal setup here.
+     */
 
   normal_stop ();
 
Index: gdb/testsuite/gdb.base/gcorebg.c
===================================================================
RCS file: gdb/testsuite/gdb.base/gcorebg.c
diff -N gdb/testsuite/gdb.base/gcorebg.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/gcorebg.c	25 Feb 2007 12:21:20 -0000
@@ -0,0 +1,43 @@
+#include <stdio.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <signal.h>
+
+int main (int argc, char **argv)
+{
+  pid_t pid = 0;
+  pid_t ppid;
+  char buf[256];
+
+  if (argc != 4)
+    {
+      fprintf (stderr, "Syntax: %s {standard|detached} <gcore command> <core output file>\n",
+	       argv[0]);
+      exit (1);
+    }
+
+  pid = fork ();
+
+  switch (pid)
+    {
+      case 0:
+        if (strcmp (argv[1], "detached") == 0)
+	  setpgrp ();
+	ppid = getppid ();
+	sprintf (buf, "sh %s -o %s %d", argv[2], argv[3], (int) ppid);
+	system (buf);
+	kill (ppid, SIGTERM);
+	break;
+
+      case -1:
+	perror ("fork err\n");
+	exit (1);
+	break;
+
+      default:
+	sleep (60);
+    }
+
+  return 0;
+}
Index: gdb/testsuite/gdb.base/gcorebg.exp
===================================================================
RCS file: gdb/testsuite/gdb.base/gcorebg.exp
diff -N gdb/testsuite/gdb.base/gcorebg.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/gcorebg.exp	25 Feb 2007 12:21:20 -0000
@@ -0,0 +1,123 @@
+# Copyright 2007 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@prep.ai.mit.edu
+
+# This file was written by Jan Kratochvil <jan.kratochvil@redhat.com>.
+# This is a test for `gdb_gcore.sh' functionality.
+# It also tests a regression with `gdb_gcore.sh' being run without its
+# accessible terminal.
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+if ![info exists GCORE] {
+    if ![is_remote host] {
+	set GCORE [findfile $base_dir/../../gdb/gdb_gcore.sh "$base_dir/../../gdb/gdb_gcore.sh" [transform gdb_gcore.sh]]
+    } else {
+	set GCORE [transform gdb_gcore.sh];
+    }
+}
+verbose "using GCORE = $GCORE" 2
+
+set testfile "gcorebg"
+set srcfile  ${testfile}.c
+set binfile  ${objdir}/${subdir}/${testfile}
+set corefile ${objdir}/${subdir}/${testfile}.test
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested gcorebg.exp
+     return -1
+}
+
+# Cleanup.
+
+proc core_clean {} {
+    global corefile
+
+    foreach file [glob -nocomplain [join [list $corefile *] ""]] {
+	verbose "Delete file $file" 1
+	remote_file target delete $file
+    }
+}
+core_clean
+remote_file target delete "./gdb"
+
+# Generate the core file.
+
+# Provide `./gdb' for `gdb_gcore.sh' running it as a bare `gdb' command.
+# Setup also `$PATH' appropriately.
+# If GDB was not found let `gdb_gcore.sh' to find the system GDB by `$PATH'.
+if {$GDB != "gdb"} {
+    file link ./gdb $GDB
+}
+global env
+set oldpath $env(PATH)
+set env(PATH) [join [list . $env(PATH)] ":"]
+verbose "PATH = $env(PATH)" 2
+
+# Test file body.
+# $detached == "standard" || $detached == "detached"
+
+proc test_body { detached } {
+    global binfile
+    global GCORE
+    global corefile
+
+    set res [remote_spawn target "$binfile $detached $GCORE $corefile"]
+    if { $res < 0 || $res == "" } {
+	fail "Spawning $detached gcore"
+	return 1
+    }
+    pass "Spawning $detached gcore"
+    remote_expect target 20 {
+	timeout {
+	    fail "Spawned $detached gcore finished"
+	    return 1
+	}
+	eof {
+	    pass "Spawned $detached gcore finished"
+	    remote_wait target 20
+	}
+    }
+
+    if {1 == [llength [glob -nocomplain [join [list $corefile *] ""]]]} {
+	pass "Core file generated by $detached gcore"
+    } else {
+	fail "Core file generated by $detached gcore"
+    }
+    core_clean
+}
+
+# First a general `gdb_gcore.sh' spawn with its controlling terminal available.
+
+test_body standard
+
+# And now `gdb_gcore.sh' spawn without its controlling terminal available.
+# It is spawned through `gcorebg.c' using setpgrp ().
+
+test_body detached
+
+
+# Cleanup.
+
+set env(PATH) $oldpath
+remote_file target delete "./gdb"

Re: [PATCH] The gcore command hangs without a terminal

[Daniel Jacobowitz]

On Sun, Feb 25, 2007 at 01:26:42PM +0100, Jan Kratochvil wrote:
> Hi,
> 
> The function `attach_command' sets up the terminal by calling
> target_terminal_inferior ().  This makes gcore(1) to lock up when being run
> without its controlling terminal.
> 
> I do not find a reason for this terminal setup as GDB does not share its
> input/output with the application in the mode of external attached program.

Unfortunately I think this is the wrong place to fix it.  You're
commenting out a call to target_terminal_inferior, but there are lots
of other calls to this function - including one any time we continue
the program.

target_terminal_inferior is a call eventually to terminal_inferior.
This checks gdb_has_a_terminal ().  How are you calling GDB without a
controlling terminal that still lets gdb_has_a_terminal () return
true?  That's probably the right place to fix it.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] The gcore command hangs without a terminal

[Jan Kratochvil]

Hi Daniel,

the patch was more an optimization, it is not a GDB bug, as I see it now.


On Tue, 27 Mar 2007 18:20:41 +0200, Daniel Jacobowitz wrote:
> On Sun, Feb 25, 2007 at 01:26:42PM +0100, Jan Kratochvil wrote:
> > Hi,
> > 
> > The function `attach_command' sets up the terminal by calling
> > target_terminal_inferior ().  This makes gcore(1) to lock up when being run
> > without its controlling terminal.
> > 
> > I do not find a reason for this terminal setup as GDB does not share its
> > input/output with the application in the mode of external attached program.
> 
> Unfortunately I think this is the wrong place to fix it.  You're
> commenting out a call to target_terminal_inferior, but there are lots
> of other calls to this function - including one any time we continue
> the program.

If we `continue' the inferior it may access its terminal and so we must set it
up for the inferior.  It is right there.  But the `gcore' command IMO does not
need to access the inferior's terminal as it never lets the inferior to run.
Accessing the inferior's terminal should be postponed to the time where the
inferior could use it.


> target_terminal_inferior is a call eventually to terminal_inferior.
> This checks gdb_has_a_terminal ().  How are you calling GDB without a
> controlling terminal that still lets gdb_has_a_terminal () return
> true?  That's probably the right place to fix it.

You are right there is primarily a bug in the customer's application (which
spawns the `gcore' command).  It should either provide no terminal or
a terminal with appropriate access rights.



Thanks,
Jan

Re: [PATCH] The gcore command hangs without a terminal

[Daniel Jacobowitz]

On Tue, Mar 27, 2007 at 06:39:16PM +0200, Jan Kratochvil wrote:
> > Unfortunately I think this is the wrong place to fix it.  You're
> > commenting out a call to target_terminal_inferior, but there are lots
> > of other calls to this function - including one any time we continue
> > the program.
> 
> If we `continue' the inferior it may access its terminal and so we must set it
> up for the inferior.  It is right there.  But the `gcore' command IMO does not
> need to access the inferior's terminal as it never lets the inferior to run.
> Accessing the inferior's terminal should be postponed to the time where the
> inferior could use it.

If we attach to a program, it should have its own controlling
terminal, not the same as GDB's.  So why should we do anything with
the terminal at all?  Perhaps all the terminal operations should be
suppressed if we're attached.

> > target_terminal_inferior is a call eventually to terminal_inferior.
> > This checks gdb_has_a_terminal ().  How are you calling GDB without a
> > controlling terminal that still lets gdb_has_a_terminal () return
> > true?  That's probably the right place to fix it.
> 
> You are right there is primarily a bug in the customer's application (which
> spawns the `gcore' command).  It should either provide no terminal or
> a terminal with appropriate access rights.

An alternative would be to detect whatever had gone wrong in GDB and
avoid fiddling with the terminal if this happens.

-- 
Daniel Jacobowitz
CodeSourcery