Patch: fix gdb_gcore.sh failure in ash/dash

Patch: fix gdb_gcore.sh failure in ash/dash

[Qunying Pan]

Hi,

Running gdb_gcore.sh in ash/dash fails with message "Hangup detected on 
fd 0". The following patch fixes the problem. Acceptable?

Thanks,

Qunying


2005-09-09 Qunying Pan <qpan@mvista.com>

    *gdb_gcore.sh: Invoke gdb in batch mode to fix gdb_gcore.sh failure 
in ash/dash.


--- src/gdb/gdb_gcore.sh.orig    2005-09-09 16:56:04.000000000 -0700
+++ src/gdb/gdb_gcore.sh    2005-09-09 16:58:12.000000000 -0700
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-#   Copyright 2003  Free Software Foundation, Inc.
+#   Copyright 2003, 2004, 2005 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
@@ -59,7 +59,7 @@ do
     # Avoid need for temporary files by using funky "here
     # document" feature of sh.
 
-    /usr/bin/gdb > /dev/null << EOF
+    /usr/bin/gdb -batch -x /dev/stdin  > /dev/null << EOF
     attach $pid
     gcore $name.$pid
     detach

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Michael Snyder]

Qunying Pan wrote:
> Hi,
> 
> Running gdb_gcore.sh in ash/dash fails with message "Hangup detected on 
> fd 0". The following patch fixes the problem. Acceptable?
> 
> Thanks,
> 
> Qunying

That's creative.  I assume it tests out OK with bash?

> 2005-09-09 Qunying Pan <qpan@mvista.com>
> 
>    *gdb_gcore.sh: Invoke gdb in batch mode to fix gdb_gcore.sh failure 
> in ash/dash.
> 
> 
> --- src/gdb/gdb_gcore.sh.orig    2005-09-09 16:56:04.000000000 -0700
> +++ src/gdb/gdb_gcore.sh    2005-09-09 16:58:12.000000000 -0700
> @@ -1,6 +1,6 @@
> #!/bin/sh
> 
> -#   Copyright 2003  Free Software Foundation, Inc.
> +#   Copyright 2003, 2004, 2005 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
> @@ -59,7 +59,7 @@ do
>     # Avoid need for temporary files by using funky "here
>     # document" feature of sh.
> 
> -    /usr/bin/gdb > /dev/null << EOF
> +    /usr/bin/gdb -batch -x /dev/stdin  > /dev/null << EOF
>     attach $pid
>     gcore $name.$pid
>     detach
> 
> 

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Daniel Jacobowitz]

On Sat, Sep 10, 2005 at 06:12:12PM -0700, Michael Snyder wrote:
> Qunying Pan wrote:
> >Hi,
> >
> >Running gdb_gcore.sh in ash/dash fails with message "Hangup detected on 
> >fd 0". The following patch fixes the problem. Acceptable?
> >
> >Thanks,
> >
> >Qunying
> 
> That's creative.  I assume it tests out OK with bash?

Anybody know how portable /dev/stdin is?  I've got no idea, but I'm a
bit worried that it won't be portable enough.  It may be simpler to
remove the clever bits and use a temporary file.

I don't suppose you know which fixes the ash problem - the -batch or
the -x?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Mark Kettenis]

> Date: Fri, 09 Sep 2005 19:11:13 -0700
> From: Qunying Pan <qpan@mvista.com>
> 
> Hi,
> 
> Running gdb_gcore.sh in ash/dash fails with message "Hangup detected on 
> fd 0". The following patch fixes the problem. Acceptable?

This seems like a bug in ash/dash (whatever that is).  Why isn't your
/bin/sh not a proper bourne shell?

I'm not inclined to apply this patch just because people link /bin/sh
to some random piece of crap.  So I'd like to see somewhat more
motivation.

Mark

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Qunying Pan]

 >> That's creative.  I assume it tests out OK with bash?
Yes, it is OK with bash.

 > Anybody know how portable /dev/stdin is?  I've got no idea, but I'm a
 > bit worried that it won't be portable enough.

It is important to consider the portable. But I am wondering if /dev/null
is having the same portable issue. If /dev/null is okay, then /dev/stdin 
should
be okay too? But I am not very sure about that. Need an expert for an 
advice.

 > It may be simpler to remove the clever bits and use a temporary file.

/dev/stdin here cannot be replaced with a temporary file.
     gdb -batch -x tmpfile >/dev/null<<EOF
     ...
     EOF

gdb reads the input stream from the end-of-file, not from the start 
position.

If /dev/stdin is replaced with tmpfile, the script should be divided into
two parts:
     cat >tmpfile <<EOF
      ...
     EOF
     gdb -batch -x tmpfile >/dev/null

Looks a little bit complicated.

 > I don't suppose you know which fixes the ash problem - the -batch or
the -x?

It only works with the combination of both -batch and -x.

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Daniel Jacobowitz]

On Sun, Sep 11, 2005 at 11:18:56PM +0200, Mark Kettenis wrote:
> > Date: Fri, 09 Sep 2005 19:11:13 -0700
> > From: Qunying Pan <qpan@mvista.com>
> > 
> > Hi,
> > 
> > Running gdb_gcore.sh in ash/dash fails with message "Hangup detected on 
> > fd 0". The following patch fixes the problem. Acceptable?
> 
> This seems like a bug in ash/dash (whatever that is).  Why isn't your
> /bin/sh not a proper bourne shell?
> 
> I'm not inclined to apply this patch just because people link /bin/sh
> to some random piece of crap.  So I'd like to see somewhat more
> motivation.

It's perfectly legitimate behavior, as far as I can tell - it depends
how long the shell keeps the pipe open and whether it uses a tempfile.
IIRC bash will prefer to use a tempfile and ash will pipe?  Not sure. 
GDB detects the SIGHUP before it writes out the coredump.

Try this, in either ash or bash:
  printf 'printf "hi\\n"\nquit\n' | gdb

Neither shell's going to give you a printf, just a hangup message.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Daniel Jacobowitz]

On Fri, Sep 09, 2005 at 07:11:13PM -0700, Qunying Pan wrote:
> Hi,
> 
> Running gdb_gcore.sh in ash/dash fails with message "Hangup detected on 
> fd 0". The following patch fixes the problem. Acceptable?

Let's just use a tempfile.  This patch also makes one other change in
behavior: it uses gdb from $PATH instead of hardcoding /usr/bin, since
I needed that to test it and it seems more reasonable.

Anyone have comments on this patch?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-09-17  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb_gcore.sh: Use a temporary file.  Use gdb from $PATH.

Index: gdb_gcore.sh
===================================================================
RCS file: /big/fsf/rsync/src/src/gdb/gdb_gcore.sh,v
retrieving revision 1.1
diff -u -p -r1.1 gdb_gcore.sh
--- gdb_gcore.sh	17 Apr 2003 20:33:09 -0000	1.1
+++ gdb_gcore.sh	17 Sep 2005 21:56:18 -0000
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-#   Copyright 2003  Free Software Foundation, Inc.
+#   Copyright 2003, 2005  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
@@ -16,9 +16,6 @@
 # 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
-
 #
 # gcore.sh
 # Script to generate a core file of a running program.
@@ -48,6 +45,14 @@ then
     shift; shift
 fi
 
+tmpfile=$$.scr
+if test -e $tmpfile; then
+  echo "Could not create temporary file $tmpfile"
+  exit 1
+fi
+touch $tmpfile
+trap "rm -f $tmpfile" EXIT
+
 # Initialise return code.
 rc=0
 
@@ -55,17 +60,15 @@ rc=0
 for pid in $*
 do
 	# Write gdb script for pid $pid.  
-
-	# Avoid need for temporary files by using funky "here
-	# document" feature of sh.
-
-	/usr/bin/gdb > /dev/null << EOF
-	attach $pid
-	gcore $name.$pid
-	detach
-	quit
+	cat >>$tmpfile <<EOF
+attach $pid
+gcore $name.$pid
+detach
+quit
 EOF
 
+	gdb -x $tmpfile -batch
+
 	if [ -r $name.$pid ] ; then 
 	    rc=0
 	else
@@ -78,4 +81,3 @@ EOF
 done
 
 exit $rc
-

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Mark Kettenis]

> Date: Sat, 17 Sep 2005 17:58:30 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Fri, Sep 09, 2005 at 07:11:13PM -0700, Qunying Pan wrote:
> > Hi,
> > 
> > Running gdb_gcore.sh in ash/dash fails with message "Hangup detected on 
> > fd 0". The following patch fixes the problem. Acceptable?
> 
> Let's just use a tempfile.  This patch also makes one other change in
> behavior: it uses gdb from $PATH instead of hardcoding /usr/bin, since
> I needed that to test it and it seems more reasonable.
> 
> Anyone have comments on this patch?

I think using a temporary file is a good idea, but if I read your
patch correctly, it seems the temporary file is created in the current
directory, which might not be writable.

Another problem is that the name is predictable, which is a potential
security risk.  Assuming mktemp(1) is available is probably not a good
idea, but it'd be nice to use it if it's avaiable.

Mark

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Daniel Jacobowitz]

On Sun, Sep 18, 2005 at 12:17:43AM +0200, Mark Kettenis wrote:
> > Date: Sat, 17 Sep 2005 17:58:30 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > On Fri, Sep 09, 2005 at 07:11:13PM -0700, Qunying Pan wrote:
> > > Hi,
> > > 
> > > Running gdb_gcore.sh in ash/dash fails with message "Hangup detected on 
> > > fd 0". The following patch fixes the problem. Acceptable?
> > 
> > Let's just use a tempfile.  This patch also makes one other change in
> > behavior: it uses gdb from $PATH instead of hardcoding /usr/bin, since
> > I needed that to test it and it seems more reasonable.
> > 
> > Anyone have comments on this patch?
> 
> I think using a temporary file is a good idea, but if I read your
> patch correctly, it seems the temporary file is created in the current
> directory, which might not be writable.

That's where we default to creating the core file, too.  I can easily
enough honor -o and create it in that directory instead.

> Another problem is that the name is predictable, which is a potential
> security risk.  Assuming mktemp(1) is available is probably not a good
> idea, but it'd be nice to use it if it's avaiable.

Seemed excessively complicated when I was first doing this, but might
as well.  This better?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-09-17  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb_gcore.sh: Use a temporary file.  Use gdb from $PATH.

Index: gdb_gcore.sh
===================================================================
RCS file: /big/fsf/rsync/src/src/gdb/gdb_gcore.sh,v
retrieving revision 1.1
diff -u -p -r1.1 gdb_gcore.sh
--- gdb_gcore.sh	17 Apr 2003 20:33:09 -0000	1.1
+++ gdb_gcore.sh	17 Sep 2005 22:26:10 -0000
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-#   Copyright 2003  Free Software Foundation, Inc.
+#   Copyright 2003, 2005  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
@@ -16,9 +16,6 @@
 # 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
-
 #
 # gcore.sh
 # Script to generate a core file of a running program.
@@ -48,6 +45,17 @@ then
     shift; shift
 fi
 
+# Create a temporary file.  Use mktemp if available, but cope if it is not.
+tmpfile=`mktemp ${name}.XXXXXX 2>/dev/null` || {
+  tmpfile=${name}.$$
+  if test -e $tmpfile; then
+    echo "Could not create temporary file $tmpfile"
+    exit 1
+  fi
+  touch $tmpfile
+}
+trap "rm -f $tmpfile" EXIT
+
 # Initialise return code.
 rc=0
 
@@ -55,17 +63,15 @@ rc=0
 for pid in $*
 do
 	# Write gdb script for pid $pid.  
-
-	# Avoid need for temporary files by using funky "here
-	# document" feature of sh.
-
-	/usr/bin/gdb > /dev/null << EOF
-	attach $pid
-	gcore $name.$pid
-	detach
-	quit
+	cat >>$tmpfile <<EOF
+attach $pid
+gcore $name.$pid
+detach
+quit
 EOF
 
+	gdb -x $tmpfile -batch
+
 	if [ -r $name.$pid ] ; then 
 	    rc=0
 	else
@@ -78,4 +84,3 @@ EOF
 done
 
 exit $rc
-

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Daniel Jacobowitz]

On Sat, Sep 17, 2005 at 06:26:49PM -0400, Daniel Jacobowitz wrote:
> 2005-09-17  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	* gdb_gcore.sh: Use a temporary file.  Use gdb from $PATH.

Did you have any comments on the revised patch, Mark?  Otherwise I'll
check this in.

> Index: gdb_gcore.sh
> ===================================================================
> RCS file: /big/fsf/rsync/src/src/gdb/gdb_gcore.sh,v
> retrieving revision 1.1
> diff -u -p -r1.1 gdb_gcore.sh
> --- gdb_gcore.sh	17 Apr 2003 20:33:09 -0000	1.1
> +++ gdb_gcore.sh	17 Sep 2005 22:26:10 -0000
> @@ -1,6 +1,6 @@
>  #!/bin/sh
>  
> -#   Copyright 2003  Free Software Foundation, Inc.
> +#   Copyright 2003, 2005  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
> @@ -16,9 +16,6 @@
>  # 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
> -
>  #
>  # gcore.sh
>  # Script to generate a core file of a running program.
> @@ -48,6 +45,17 @@ then
>      shift; shift
>  fi
>  
> +# Create a temporary file.  Use mktemp if available, but cope if it is not.
> +tmpfile=`mktemp ${name}.XXXXXX 2>/dev/null` || {
> +  tmpfile=${name}.$$
> +  if test -e $tmpfile; then
> +    echo "Could not create temporary file $tmpfile"
> +    exit 1
> +  fi
> +  touch $tmpfile
> +}
> +trap "rm -f $tmpfile" EXIT
> +
>  # Initialise return code.
>  rc=0
>  
> @@ -55,17 +63,15 @@ rc=0
>  for pid in $*
>  do
>  	# Write gdb script for pid $pid.  
> -
> -	# Avoid need for temporary files by using funky "here
> -	# document" feature of sh.
> -
> -	/usr/bin/gdb > /dev/null << EOF
> -	attach $pid
> -	gcore $name.$pid
> -	detach
> -	quit
> +	cat >>$tmpfile <<EOF
> +attach $pid
> +gcore $name.$pid
> +detach
> +quit
>  EOF
>  
> +	gdb -x $tmpfile -batch
> +
>  	if [ -r $name.$pid ] ; then 
>  	    rc=0
>  	else
> @@ -78,4 +84,3 @@ EOF
>  done
>  
>  exit $rc
> -
> 

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Mark Kettenis]

> On Sat, Sep 17, 2005 at 06:26:49PM -0400, Daniel Jacobowitz wrote:
>> 2005-09-17  Daniel Jacobowitz  <dan@codesourcery.com>
>>
>> 	* gdb_gcore.sh: Use a temporary file.  Use gdb from $PATH.
>
> Did you have any comments on the revised patch, Mark?  Otherwise I'll
> check this in.

Sorry, been to busy making OpenBSD work on my new iMac G5 ;-)

Yes this is exactly what I had in mind.  Please go ahead and commit.

Mark

Re: Patch: fix gdb_gcore.sh failure in ash/dash

[Daniel Jacobowitz]

On Mon, Sep 26, 2005 at 09:17:16AM +0200, Mark Kettenis wrote:
> > On Sat, Sep 17, 2005 at 06:26:49PM -0400, Daniel Jacobowitz wrote:
> >> 2005-09-17  Daniel Jacobowitz  <dan@codesourcery.com>
> >>
> >> 	* gdb_gcore.sh: Use a temporary file.  Use gdb from $PATH.
> >
> > Did you have any comments on the revised patch, Mark?  Otherwise I'll
> > check this in.
> 
> Sorry, been to busy making OpenBSD work on my new iMac G5 ;-)
> 
> Yes this is exactly what I had in mind.  Please go ahead and commit.

Checked in, then.

-- 
Daniel Jacobowitz
CodeSourcery, LLC