gas --statistics segfault

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

gas --statistics segfault

Dasn-2

Hi, guys.

In my box, 'as --statistics' sometimes segfault.
-------------------------------------------------------------
$ uname -msr
OpenBSD 3.8 i386
$ echo "main(){}" > p.c
$ cat -n test.sh
 1  #!/bin/sh
 2  count=100;failed=0;i=0
 3  cc -S p.c -o p.s
 4  while [ $i -lt $count ]
 5  do
 6   ./as-new --statistics p.s > /dev/null 2>&1
 7   if [ $? -ne 0 ]; then
 8   let failed=failed+1
 9   fi
10   let i=i+1
11  done
12  echo "$failed/$count failed."
13  rm -rf as-new.core a.out
$ sh test.sh
100/100 failed.
-------------------------------------------------------------

The following patch simply comments out 'output_file_close' function may
solve this problem.

Index: as.c
===================================================================
RCS file: /cvs/src/src/gas/as.c,v
retrieving revision 1.67
diff -u -p -r1.67 as.c
--- as.c 30 Oct 2005 18:08:52 -0000 1.67
+++ as.c 5 Feb 2006 12:41:27 -0000
@@ -1170,7 +1170,7 @@ main (int argc, char ** argv)
 #endif
 
 #ifndef OBJ_VMS /* Does its own file handling.  */
-  output_file_close (out_file_name);
+  /* output_file_close (out_file_name); */
 #endif
 
   if (flag_fatal_warnings && had_warnings () > 0 && had_errors () == 0)
Reply | Threaded
Open this post in threaded view
|

Re: gas --statistics segfault

H.J. Lu-27
On Sun, Feb 05, 2006 at 09:02:19PM +0800, Dasn Clainst wrote:

>
> Hi, guys.
>
> In my box, 'as --statistics' sometimes segfault.
> -------------------------------------------------------------
> $ uname -msr
> OpenBSD 3.8 i386
> $ echo "main(){}" > p.c
> $ cat -n test.sh
>  1  #!/bin/sh
>  2  count=100;failed=0;i=0
>  3  cc -S p.c -o p.s
>  4  while [ $i -lt $count ]
>  5  do
>  6   ./as-new --statistics p.s > /dev/null 2>&1
>  7   if [ $? -ne 0 ]; then
>  8   let failed=failed+1
>  9   fi
> 10   let i=i+1
> 11  done
> 12  echo "$failed/$count failed."
> 13  rm -rf as-new.core a.out
> $ sh test.sh
> 100/100 failed.

You were making it harder for people to reproduce it. Please open a bug
report at

http://www.sourceware.org/bugzilla/

with a testcase.


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: gas --statistics segfault

Dasn-2
On Sun, Feb 05, 2006 at 08:09:35AM -0800, H. J. Lu wrote:

> On Sun, Feb 05, 2006 at 09:02:19PM +0800, Dasn Clainst wrote:
> >
> > Hi, guys.
> >
> > In my box, 'as --statistics' sometimes segfault.
> > -------------------------------------------------------------
> > $ uname -msr
> > OpenBSD 3.8 i386
> > $ echo "main(){}" > p.c
> > $ cat -n test.sh
> >  1  #!/bin/sh
> >  2  count=100;failed=0;i=0
> >  3  cc -S p.c -o p.s
> >  4  while [ $i -lt $count ]
> >  5  do
> >  6   ./as-new --statistics p.s > /dev/null 2>&1
> >  7   if [ $? -ne 0 ]; then
> >  8   let failed=failed+1
> >  9   fi
> > 10   let i=i+1
> > 11  done
> > 12  echo "$failed/$count failed."
> > 13  rm -rf as-new.core a.out
> > $ sh test.sh
> > 100/100 failed.
>
> You were making it harder for people to reproduce it. Please open a bug
> report at
>
> http://www.sourceware.org/bugzilla/
>
> with a testcase.
>
>
> H.J.

Usually, --statistics will cause segfault, so no need testcase. The
test.sh is used to find how often segfaults will produce (usually 100%,
sometimes 50%). IMHO, the 'bfd_close' free all frch_seg in frchain_root
before 'subsegs_print_statistics' access them.
 
Reply | Threaded
Open this post in threaded view
|

Re: gas --statistics segfault

H.J. Lu-27
On Mon, Feb 06, 2006 at 08:31:06AM +0800, Dasn Clainst wrote:

> On Sun, Feb 05, 2006 at 08:09:35AM -0800, H. J. Lu wrote:
> > On Sun, Feb 05, 2006 at 09:02:19PM +0800, Dasn Clainst wrote:
> > >
> > > Hi, guys.
> > >
> > > In my box, 'as --statistics' sometimes segfault.
> > > -------------------------------------------------------------
> > > $ uname -msr
> > > OpenBSD 3.8 i386
> > > $ echo "main(){}" > p.c
> > > $ cat -n test.sh
> > >  1  #!/bin/sh
> > >  2  count=100;failed=0;i=0
> > >  3  cc -S p.c -o p.s
> > >  4  while [ $i -lt $count ]
> > >  5  do
> > >  6   ./as-new --statistics p.s > /dev/null 2>&1
> > >  7   if [ $? -ne 0 ]; then
> > >  8   let failed=failed+1
> > >  9   fi
> > > 10   let i=i+1
> > > 11  done
> > > 12  echo "$failed/$count failed."
> > > 13  rm -rf as-new.core a.out
> > > $ sh test.sh
> > > 100/100 failed.
> >
> > You were making it harder for people to reproduce it. Please open a bug
> > report at
> >
> > http://www.sourceware.org/bugzilla/
> >
> > with a testcase.
> >
> >
> > H.J.
>
> Usually, --statistics will cause segfault, so no need testcase. The
> test.sh is used to find how often segfaults will produce (usually 100%,

I can't even cut/paste your test.sh since you used "cat -n".



H.J.
Reply | Threaded
Open this post in threaded view
|

Re: gas --statistics segfault

Nick Clifton
In reply to this post by Dasn-2
Hi Dasn,

> In my box, 'as --statistics' sometimes segfault.

Which version of as is this ?  For which target(s) has it been compiled ?

> The following patch simply comments out 'output_file_close' function may
> solve this problem.

>  #ifndef OBJ_VMS /* Does its own file handling.  */
> -  output_file_close (out_file_name);
> +  /* output_file_close (out_file_name); */
>  #endif

I do not like the idea of doing this without knowing the reason for the
segfault nor having a way to reproduce the problem.  I tried your test
case on my machine and never received a single segfault.

Cheers
   Nick
Reply | Threaded
Open this post in threaded view
|

Re: gas --statistics segfault

Dasn-2
In reply to this post by Dasn-2
On Sun, Feb 05, 2006 at 09:02:19PM +0800, Dasn Clainst wrote:

> ===================================================================
> RCS file: /cvs/src/src/gas/as.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 as.c
> --- as.c 30 Oct 2005 18:08:52 -0000 1.67
> +++ as.c 5 Feb 2006 12:41:27 -0000
> @@ -1170,7 +1170,7 @@ main (int argc, char ** argv)
>  #endif
>  
>  #ifndef OBJ_VMS /* Does its own file handling.  */
> -  output_file_close (out_file_name);
> +  /* output_file_close (out_file_name); */
>  #endif
>  
>    if (flag_fatal_warnings && had_warnings () > 0 && had_errors () == 0)

Please forget this patch, it's just an opinion of my own.

-----------------------------------------------------
$ cat test.sh
#!/bin/sh
uname -msr
as --version

count=100;failed=0;i=0
echo "main(){}" > p.c
cc -S p.c -o p.s
while [ $i -lt $count ]
do
        as --statistics p.s > /dev/null 2>&1
        if [ $? -ne 0 ]; then
                let failed=failed+1
        fi
        let i=i+1
done
echo "$failed/$count failed."
rm -f as.core a.out p.c
$
$ sh test.sh
OpenBSD 3.8 i386
GNU assembler 2.16.91 20060127
Copyright 2005 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License.  This program has absolutely no warranty.
This assembler was configured for a target of `i386-elf-openbsd3.8'.
100/100 failed.
$
$ cat p.s
        .file "p.c"
        .globl __stack_smash_handler
        .section .rodata
.LC0:
        .string "main"
        .text
        .globl main
        .type main, @function
main:
        pushl %ebp
        movl %esp, %ebp
        subl $24, %esp
        andl $-16, %esp
        movl $0, %eax
        subl %eax, %esp
        movl __guard, %eax
        movl %eax, -24(%ebp)
        movl -24(%ebp), %edx
        cmpl __guard, %edx
        je .L2
        subl $8, %esp
        pushl -24(%ebp)
        pushl $.LC0
        call __stack_smash_handler
        addl $16, %esp
.L2:
        leave
        ret
        .size main, .-main
-----------------------------------------------------

After calling 'output_file_close', some pointers to frch_seg in frchain_root
become invalid. I'm not sure whether it is a problem of gas or not,
cause openbsd enables stack smash protection by default. I'll dig it
soon.
Reply | Threaded
Open this post in threaded view
|

PATCH: gas --statistics segfault

H.J. Lu-27
On Tue, Feb 07, 2006 at 06:43:04AM +0800, Dasn Clainst wrote:

> On Sun, Feb 05, 2006 at 09:02:19PM +0800, Dasn Clainst wrote:
> > ===================================================================
> > RCS file: /cvs/src/src/gas/as.c,v
> > retrieving revision 1.67
> > diff -u -p -r1.67 as.c
> > --- as.c 30 Oct 2005 18:08:52 -0000 1.67
> > +++ as.c 5 Feb 2006 12:41:27 -0000
> > @@ -1170,7 +1170,7 @@ main (int argc, char ** argv)
> >  #endif
> >  
> >  #ifndef OBJ_VMS /* Does its own file handling.  */
> > -  output_file_close (out_file_name);
> > +  /* output_file_close (out_file_name); */
> >  #endif
> >  
> >    if (flag_fatal_warnings && had_warnings () > 0 && had_errors () == 0)
>
> Please forget this patch, it's just an opinion of my own.
>

We have

         if (segment_name (frchp->frch_seg)[0] == '*')

in subsegs_print_statistics. segment_name is defined as

define segment_name(SEG)  bfd_get_section_name (stdoutput, SEG)

SEG is allocated with stdoutput. We are using stdoutput after
stdoutput is closed. We should call output_file_close after
dump_statistics. This patch should fix it.

May I suggest you open a bug report at

http://www.sourceware.org/bugzilla/

next time?

Thanks.


H.J.
----
2006-02-07  H.J. Lu  <[hidden email]>

        * as.c (close_output_file): New.
        (main): Register close_output_file with xatexit before
        dump_statistics. Don't call output_file_close.

--- gas/as.c.exit 2005-11-01 21:20:38.000000000 -0800
+++ gas/as.c 2006-02-07 10:04:39.000000000 -0800
@@ -930,6 +930,14 @@ dump_statistics (void)
 #endif
 }
 
+#ifndef OBJ_VMS
+static void
+close_output_file (void)
+{
+  output_file_close (out_file_name);
+}
+#endif
+
 /* The interface between the macro code and gas expression handling.  */
 
 static int
@@ -1081,6 +1089,11 @@ main (int argc, char ** argv)
   input_scrub_begin ();
   expr_begin ();
 
+#ifndef OBJ_VMS /* Does its own file handling.  */
+  /* It has to be called after dump_statistics ().  */
+  xatexit (close_output_file);
+#endif
+
   if (flag_print_statistics)
     xatexit (dump_statistics);
 
@@ -1169,10 +1182,6 @@ main (int argc, char ** argv)
   listing_print (listing_filename);
 #endif
 
-#ifndef OBJ_VMS /* Does its own file handling.  */
-  output_file_close (out_file_name);
-#endif
-
   if (flag_fatal_warnings && had_warnings () > 0 && had_errors () == 0)
     as_bad (_("%d warnings, treating warnings as errors"), had_warnings ());
 
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: gas --statistics segfault

Alan Modra
On Tue, Feb 07, 2006 at 10:13:55AM -0800, H. J. Lu wrote:
> * as.c (close_output_file): New.
> (main): Register close_output_file with xatexit before
> dump_statistics. Don't call output_file_close.

OK, thanks!

--
Alan Modra
IBM OzLabs - Linux Technology Centre
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: gas --statistics segfault

Alan Modra
On Thu, Feb 09, 2006 at 09:38:13AM +1030, Alan Modra wrote:
> On Tue, Feb 07, 2006 at 10:13:55AM -0800, H. J. Lu wrote:
> > * as.c (close_output_file): New.
> > (main): Register close_output_file with xatexit before
> > dump_statistics. Don't call output_file_close.
>
> OK, thanks!

Small problem.  If we abort inside output_file_close, as can happen when
writing relocs (found running gas testsuite with xscale-coff target),
then close_output_file gets called again recursively until we run out of
memory.  I started fixing this by guarding against recursion in
close_output_file, then decided that the real bug is in _bfd_abort.  We
shouldn't run xatexit registered functions on an abort.

        * bfd.c: (_bfd_default_error_handler): Don't call abort on
        error, instead call _exit.
        (_bfd_abort): Call _exit not xexit.

Index: bfd/bfd.c
===================================================================
RCS file: /cvs/src/src/bfd/bfd.c,v
retrieving revision 1.79
diff -u -p -r1.79 bfd.c
--- bfd/bfd.c 7 Dec 2005 14:43:53 -0000 1.79
+++ bfd/bfd.c 21 Feb 2006 00:01:41 -0000
@@ -216,6 +216,11 @@ CODE_FRAGMENT
 #include "libecoff.h"
 #undef obj_symbols
 #include "elf-bfd.h"
+
+#ifndef EXIT_FAILURE
+#define EXIT_FAILURE 1
+#endif
+
 
 /* provide storage for subsystem, stack and heap data which may have been
    passed in on the command line.  Ld puts this data into a bfd_link_info
@@ -437,7 +442,7 @@ _bfd_default_error_handler (const char *
   /* Reserve enough space for the existing format string.  */
   avail -= strlen (fmt) + 1;
   if (avail > 1000)
-    abort ();
+    _exit (EXIT_FAILURE);
 
   p = fmt;
   while (1)
@@ -775,10 +780,6 @@ bfd_assert (const char *file, int line)
 /* A more or less friendly abort message.  In libbfd.h abort is
    defined to call this function.  */
 
-#ifndef EXIT_FAILURE
-#define EXIT_FAILURE 1
-#endif
-
 void
 _bfd_abort (const char *file, int line, const char *fn)
 {
@@ -791,7 +792,7 @@ _bfd_abort (const char *file, int line,
       (_("BFD %s internal error, aborting at %s line %d\n"),
        BFD_VERSION_STRING, file, line);
   (*_bfd_error_handler) (_("Please report this bug.\n"));
-  xexit (EXIT_FAILURE);
+  _exit (EXIT_FAILURE);
 }
 
 /*

--
Alan Modra
IBM OzLabs - Linux Technology Centre