[regressions in GCC/libjava testsuite] Re: Patch to make bfd compile with -Wc++-compat

[Matthias Klose-6]

On 09.09.2009 23:40, Nick Clifton wrote:
> Hi Martin,
>
>> Reran make check, make check-gas and make check-ld on x86_64-linux and
>> arm-eabi
>> and found no regression.
>
> Approved and applied (mainline and branch).

I identified this patch or the one applied to the binutils subdirectory to cause
regressions in the GCC/libjava testsuite on the 4.3/4.4 branches and trunk, at
least on {x86,x86_64,powerpc}-linux:

FAIL: StackTrace2 execution - source compiled test
FAIL: StackTrace2 -findirect-dispatch execution - source compiled test
FAIL: StackTrace2 -O3 execution - source compiled test
FAIL: StackTrace2 -O3 -findirect-dispatch execution - source compiled test
FAIL: Throw_3 execution - source compiled test
FAIL: Throw_3 -findirect-dispatch execution - source compiled test
FAIL: Throw_3 -O3 execution - source compiled test
FAIL: Throw_3 -O3 -findirect-dispatch execution - source compiled test
FAIL: stacktrace execution - source compiled test
FAIL: stacktrace -findirect-dispatch execution - source compiled test
FAIL: stacktrace -O3 execution - source compiled test
FAIL: stacktrace -O3 -findirect-dispatch execution - source compiled test

The tests fail as:
set_ld_library_path_env_vars:
ld_library_path=.:/scratch/packages/gcc/4.3/u/java/gcj-4.3-4.3.4/build/i486-linux-gnu/./libjava/.libs:/scratch/packages/gcc/4.3/u/java/gcj-4.3-4.3.4/build/gcc
invoke:
/scratch/packages/gcc/4.3/u/java/gcj-4.3-4.3.4/build/i486-linux-gnu/libjava/testsuite/StackTrace2.exe

Setting LD_LIBRARY_PATH to
.:/scratch/packages/gcc/4.3/u/java/gcj-4.3-4.3.4/build/i486-linux-gnu/./libjava/.libs:/scratch/packages/gcc/4.3/u/java/gcj-4.3-4.3.4/build/gcc:.:/scratch/packages/gcc/4.3/u/java/gcj-4.3-4.3.4/build/i486-linux-gnu/./libjava/.libs:/scratch/packages/gcc/4.3/u/java/gcj-4.3-4.3.4/build/gcc
Exception in thread "main" java.lang.NullPointerException
*** Got java.lang.NullPointerException while trying to print stack trace.

No difference, if binutils is built with 4.3.4 or 4.4.1.

   Matthias

[Matthias Klose-6]

On 01.10.2009 01:01, Matthias Klose wrote:

> On 09.09.2009 23:40, Nick Clifton wrote:
>> Hi Martin,
>>
>>> Reran make check, make check-gas and make check-ld on x86_64-linux and
>>> arm-eabi
>>> and found no regression.
>>
>> Approved and applied (mainline and branch).
>
> I identified this patch or the one applied to the binutils subdirectory

It's the one applied to the binutils subdirectory.

[Alan Modra]

On Thu, Oct 01, 2009 at 01:23:33AM +0200, Matthias Klose wrote:
> It's the one applied to the binutils subdirectory.

It is probably the addr2line.c change.  Try this:

Index: binutils/addr2line.c
===================================================================
RCS file: /cvs/src/src/binutils/addr2line.c,v
retrieving revision 1.34
diff -u -p -r1.34 addr2line.c
--- binutils/addr2line.c 10 Sep 2009 13:40:44 -0000 1.34
+++ binutils/addr2line.c 30 Sep 2009 23:47:53 -0000
@@ -107,9 +107,9 @@ slurp_symtab (bfd *abfd)
   if ((bfd_get_file_flags (abfd) & HAS_SYMS) == 0)
     return;
 
-  symcount = bfd_read_minisymbols (abfd, FALSE, &minisyms, &size);
+  symcount = bfd_read_minisymbols (abfd, FALSE, minisyms, &size);
   if (symcount == 0)
-    symcount = bfd_read_minisymbols (abfd, TRUE /* dynamic */, &minisyms, &size);
+    symcount = bfd_read_minisymbols (abfd, TRUE /* dynamic */, minisyms, &size);
 
   if (symcount < 0)
     bfd_fatal (bfd_get_filename (abfd));

--
Alan Modra
Australia Development Lab, IBM

[Alan Modra]

On Thu, Oct 01, 2009 at 09:18:35AM +0930, Alan Modra wrote:
> On Thu, Oct 01, 2009 at 01:23:33AM +0200, Matthias Klose wrote:
> > It's the one applied to the binutils subdirectory.
>
> It is probably the addr2line.c change.  Try this:

That's not the right patch, but does identify the breakage..
bfd_read_minisymbols ought to take a void * param instead of void **,
I think.

--
Alan Modra
Australia Development Lab, IBM

[Martin Thuresson]

On Wed, Sep 30, 2009 at 5:04 PM, Alan Modra <amodra@...> wrote:
> On Thu, Oct 01, 2009 at 09:18:35AM +0930, Alan Modra wrote:
>> On Thu, Oct 01, 2009 at 01:23:33AM +0200, Matthias Klose wrote:
>> > It's the one applied to the binutils subdirectory.
>>
>> It is probably the addr2line.c change.  Try this:
>
> That's not the right patch, but does identify the breakage..
> bfd_read_minisymbols ought to take a void * param instead of void **,
> I think.

The issue in questions seems to be my patch from here:
http://sourceware.org/ml/binutils/2009-09/msg00276.html

I cant recall why my change stopped the -Wc++-error. Ill take
a look and will submit a fix.

Martin


Excerpt:

diff -u -u -p -r1.33 addr2line.c
--- binutils/addr2line.c 2 Sep 2009 07:22:31 -0000 1.33
+++ binutils/addr2line.c 9 Sep 2009 23:01:26 -0000
@@ -102,13 +102,14 @@ slurp_symtab (bfd *abfd)
 {
   long symcount;
   unsigned int size;
+  void *minisyms = &syms;

   if ((bfd_get_file_flags (abfd) & HAS_SYMS) == 0)
     return;

-  symcount = bfd_read_minisymbols (abfd, FALSE, (void *) &syms, &size);
+  symcount = bfd_read_minisymbols (abfd, FALSE, &minisyms, &size);
   if (symcount == 0)
-    symcount = bfd_read_minisymbols (abfd, TRUE /* dynamic */, (void
*) &syms, &size);
+    symcount = bfd_read_minisymbols (abfd, TRUE /* dynamic */,
&minisyms, &size);

   if (symcount < 0)
     bfd_fatal (bfd_get_filename (abfd));

[Dave Korn-6]

Martin Thuresson wrote:

> +  void *minisyms = &syms;

> -  symcount = bfd_read_minisymbols (abfd, FALSE, (void *) &syms, &size);
> +  symcount = bfd_read_minisymbols (abfd, FALSE, &minisyms, &size);


  You have effectively replaced &syms by &&syms in what you pass to
bfd_read_minisymbols.


    cheers,
      DaveK

[Martin Thuresson]

Here is a patch that fixes a bug introduced with my patch. Im sorry
for the first mistake.
I was unable to run make check on my mac laptop, but will do it on my
linux box in 30 min
or so.

Martin

2009-09-07  Martin Thuresson  <martin@...>

        * binutils/addr2line.c (slurp_symtab): Fixed pointer bug.


On Wed, Sep 30, 2009 at 5:20 PM, Martin Thuresson <martin@...> wrote:

> On Wed, Sep 30, 2009 at 5:04 PM, Alan Modra <amodra@...> wrote:
>> On Thu, Oct 01, 2009 at 09:18:35AM +0930, Alan Modra wrote:
>>> On Thu, Oct 01, 2009 at 01:23:33AM +0200, Matthias Klose wrote:
>>> > It's the one applied to the binutils subdirectory.
>>>
>>> It is probably the addr2line.c change.  Try this:
>>
>> That's not the right patch, but does identify the breakage..
>> bfd_read_minisymbols ought to take a void * param instead of void **,
>> I think.
>
> The issue in questions seems to be my patch from here:
> http://sourceware.org/ml/binutils/2009-09/msg00276.html
>
> I cant recall why my change stopped the -Wc++-error. Ill take
> a look and will submit a fix.
>
> Martin
>
>
> Excerpt:
>
> diff -u -u -p -r1.33 addr2line.c
> --- binutils/addr2line.c        2 Sep 2009 07:22:31 -0000       1.33
> +++ binutils/addr2line.c        9 Sep 2009 23:01:26 -0000
> @@ -102,13 +102,14 @@ slurp_symtab (bfd *abfd)
>  {
>   long symcount;
>   unsigned int size;
> +  void *minisyms = &syms;
>
>   if ((bfd_get_file_flags (abfd) & HAS_SYMS) == 0)
>     return;
>
> -  symcount = bfd_read_minisymbols (abfd, FALSE, (void *) &syms, &size);
> +  symcount = bfd_read_minisymbols (abfd, FALSE, &minisyms, &size);
>   if (symcount == 0)
> -    symcount = bfd_read_minisymbols (abfd, TRUE /* dynamic */, (void
> *) &syms, &size);
> +    symcount = bfd_read_minisymbols (abfd, TRUE /* dynamic */,
> &minisyms, &size);
>
>   if (symcount < 0)
>     bfd_fatal (bfd_get_filename (abfd));
>

Index: binutils/addr2line.c
===================================================================
RCS file: /cvs/src/src/binutils/addr2line.c,v
retrieving revision 1.34
diff -u -p -r1.34 addr2line.c
--- binutils/addr2line.c 10 Sep 2009 13:40:44 -0000 1.34
+++ binutils/addr2line.c 1 Oct 2009 00:38:32 -0000
@@ -102,7 +102,7 @@ slurp_symtab (bfd *abfd)
 {
   long symcount;
   unsigned int size;
-  void *minisyms = &syms;
+  void *minisyms = syms;
 
   if ((bfd_get_file_flags (abfd) & HAS_SYMS) == 0)
     return;

[Ian Lance Taylor-3]

Martin Thuresson <martin@...> writes:

> Here is a patch that fixes a bug introduced with my patch. Im sorry
> for the first mistake.
> I was unable to run make check on my mac laptop, but will do it on my
> linux box in 30 min
> or so.

I was talking about this with Alan on IRC.  The bug is actually this
change:

2002-10-07  Gordon Chaffee <chaffee@...>

        * addr2line.c (slurp_symtab): Read in dynamic symbols if no
        ordinary ones are available.

That patch was 100% bogus.  We shouldn't be trying to break the
minisyms interface here.  For some reason I can't find the e-mail in
the binutlis mailing list archive.

Ian

[Matt Rice-2]

On Wed, Sep 30, 2009 at 5:55 PM, Ian Lance Taylor <iant@...> wrote:

> For some reason I can't find the e-mail in
> the binutlis mailing list archive.
>


http://thread.gmane.org/gmane.comp.gnu.utils.bugs/3924

looks like it was sent to the coreutils list

[Martin Thuresson]

On Wed, Sep 30, 2009 at 5:45 PM, Martin Thuresson <martin@...> wrote:
> Here is a patch that fixes a bug introduced with my patch. Im sorry
> for the first mistake.
> I was unable to run make check on my mac laptop, but will do it on my
> linux box in 30 min
> or so.

"make check-binutils" passed on target x86_64-linux.  My change
was just a mechanical change to avoid C++ warning, so I have
nothing against the bigger issue being fixed.

2009-09-30  Martin Thuresson  <martin@...>

        * binutils/addr2line.c (slurp_symtab): Fixed pointer bug.

> On Wed, Sep 30, 2009 at 5:20 PM, Martin Thuresson <martin@...> wrote:
>> On Wed, Sep 30, 2009 at 5:04 PM, Alan Modra <amodra@...> wrote:
>>> On Thu, Oct 01, 2009 at 09:18:35AM +0930, Alan Modra wrote:
>>>> On Thu, Oct 01, 2009 at 01:23:33AM +0200, Matthias Klose wrote:
>>>> > It's the one applied to the binutils subdirectory.
>>>>
>>>> It is probably the addr2line.c change.  Try this:
>>>
>>> That's not the right patch, but does identify the breakage..
>>> bfd_read_minisymbols ought to take a void * param instead of void **,
>>> I think.
>>
>> The issue in questions seems to be my patch from here:
>> http://sourceware.org/ml/binutils/2009-09/msg00276.html
>>
>> I cant recall why my change stopped the -Wc++-error. Ill take
>> a look and will submit a fix.
>>
>> Martin
>>
>>
>> Excerpt:
>>
>> diff -u -u -p -r1.33 addr2line.c
>> --- binutils/addr2line.c        2 Sep 2009 07:22:31 -0000       1.33
>> +++ binutils/addr2line.c        9 Sep 2009 23:01:26 -0000
>> @@ -102,13 +102,14 @@ slurp_symtab (bfd *abfd)
>>  {
>>   long symcount;
>>   unsigned int size;
>> +  void *minisyms = &syms;
>>
>>   if ((bfd_get_file_flags (abfd) & HAS_SYMS) == 0)
>>     return;
>>
>> -  symcount = bfd_read_minisymbols (abfd, FALSE, (void *) &syms, &size);
>> +  symcount = bfd_read_minisymbols (abfd, FALSE, &minisyms, &size);
>>   if (symcount == 0)
>> -    symcount = bfd_read_minisymbols (abfd, TRUE /* dynamic */, (void
>> *) &syms, &size);
>> +    symcount = bfd_read_minisymbols (abfd, TRUE /* dynamic */,
>> &minisyms, &size);
>>
>>   if (symcount < 0)
>>     bfd_fatal (bfd_get_filename (abfd));
>>
>

[Alan Modra]

This is what I'll commit when my testsuite runs complete.

        * addr2line.c (slurp_symtab): Don't use bfd_read_minisymbols.

Index: binutils/addr2line.c
===================================================================
RCS file: /cvs/src/src/binutils/addr2line.c,v
retrieving revision 1.34
diff -u -p -r1.34 addr2line.c
--- binutils/addr2line.c 10 Sep 2009 13:40:44 -0000 1.34
+++ binutils/addr2line.c 1 Oct 2009 05:28:04 -0000
@@ -100,17 +100,27 @@ usage (FILE *stream, int status)
 static void
 slurp_symtab (bfd *abfd)
 {
+  long storage;
   long symcount;
-  unsigned int size;
-  void *minisyms = &syms;
+  bfd_boolean dynamic = FALSE;
 
   if ((bfd_get_file_flags (abfd) & HAS_SYMS) == 0)
     return;
 
-  symcount = bfd_read_minisymbols (abfd, FALSE, &minisyms, &size);
-  if (symcount == 0)
-    symcount = bfd_read_minisymbols (abfd, TRUE /* dynamic */, &minisyms, &size);
+  storage = bfd_get_symtab_upper_bound (abfd);
+  if (storage == 0)
+    {
+      storage = bfd_get_dynamic_symtab_upper_bound (abfd);
+      dynamic = TRUE;
+    }
+  if (storage < 0)
+    bfd_fatal (bfd_get_filename (abfd));
 
+  syms = (asymbol **) xmalloc (storage);
+  if (dynamic)
+    symcount = bfd_canonicalize_dynamic_symtab (abfd, syms);
+  else
+    symcount = bfd_canonicalize_symtab (abfd, syms);
   if (symcount < 0)
     bfd_fatal (bfd_get_filename (abfd));
 }

--
Alan Modra
Australia Development Lab, IBM

[Matthias Klose-6]

On 01.10.2009 07:37, Alan Modra wrote:
> This is what I'll commit when my testsuite runs complete.
>
> * addr2line.c (slurp_symtab): Don't use bfd_read_minisymbols.

thanks, the GCC/libjava tests now pass.

   Matthias