Ada testsuite failures

Ada testsuite failures

[Daniel Jacobowitz]

Joel, I have seen Ada testsuite failures on GNU/Linux for about as long
as we've had tests.  Do you know the status of these tests when run
against FSF releases of gnat, rather than the AdaCore version?  I
suspect it's inadequate debug info.

I've got at the moment:

print e_one_two_three
$2 = (0 => 1, 2, 3)
(gdb) FAIL: gdb.ada/arrayidx.exp: print e_one_two_three, indexes off
print r_two_three
$3 = (2, 3)
(gdb) FAIL: gdb.ada/arrayidx.exp: print r_two_three, indexes off
print u_one_two_three
No definition of "u_one_two_three" in current context.
(gdb) FAIL: gdb.ada/arrayidx.exp: print u_one_two_three, indexes off
print p_one_two_three
$4 = warning: could not find bounds information on packed array
can't unpack array

(gdb) FAIL: gdb.ada/arrayidx.exp: print p_one_two_three, indexes off


print e_one_two_three
$9 = (0 => 1, 1 => 2, 2 => 3)
(gdb) FAIL: gdb.ada/arrayidx.exp: print e_one_two_three
print r_two_three
$10 = (1 => 2, 2 => 3)
(gdb) FAIL: gdb.ada/arrayidx.exp: print r_two_three
print u_one_two_three
No definition of "u_one_two_three" in current context.
(gdb) FAIL: gdb.ada/arrayidx.exp: print u_one_two_three
print p_one_two_three
$11 = warning: could not find bounds information on packed array
can't unpack array

(gdb) FAIL: gdb.ada/arrayidx.exp: print p_one_two_three


ptype empty
type = function return int
(gdb) FAIL: gdb.ada/null_record.exp: ptype on null record


(gdb) print var
$1 = warning: could not find bounds information on packed array
can't unpack array

(gdb) FAIL: gdb.ada/packed_array.exp: print var


-- 
Daniel Jacobowitz
CodeSourcery

Re: Ada testsuite failures

[Joel Brobecker]

> Joel, I have seen Ada testsuite failures on GNU/Linux for about as long
> as we've had tests.  Do you know the status of these tests when run
> against FSF releases of gnat, rather than the AdaCore version?

I don't work at all with the FSF release of GCC, only with the AdaCore one.

> I suspect it's inadequate debug info.

I wouldn't be surprised. Recents versions of GCC are much better at
not emitting debugging info for unused entity. This has caused us a lot
of trouble when moving to GCC 4.1 for instance, because GNAT generates
of lot of entities that appear unused from the compiler point of view,
but are necessary for the debugger to work properly. We're getting back
to the same project of getting rid of all these encoding in favor of
standard DWARF. This is actually a very active project that we're
working on right now, so hopefully the situation won't be as bad soon.

I can ask that the local patches we maintain in our tree be submitted
for inclusion if you think they'd be accepted. Our opinion was that they
would probably be rejected as a step backward.

> print e_one_two_three
> $2 = (0 => 1, 2, 3)

In this case, for instance, you are probably missing the debugging
info for a type named: p__etable___XA. If you have a look at exp_dbug.ads
in the compiler sources, this is what we call a parallel type which
contains one field. The name of the field is the name of array index type.  

Not very elegant, but necessary in many cases when the platform does not
support stabs.

> print p_one_two_three
> $4 = warning: could not find bounds information on packed array
> can't unpack array
> 
> (gdb) FAIL: gdb.ada/arrayidx.exp: print p_one_two_three, indexes off

This case is probably the same sort of issue. I looked at the sources
and it looks like GDB did not find the parallel type correspoinding
to this array. The parallel type is computed by stripping the ___XP
suffix in the type name (in this case, it looks like GDB failed to
find type p__ptable.

> ptype empty
> type = function return int
> (gdb) FAIL: gdb.ada/null_record.exp: ptype on null record

Strangely, I just noticed that I have the same failure on my side.
I will investigate.

> $1 = warning: could not find bounds information on packed array
> can't unpack array
> 
> (gdb) FAIL: gdb.ada/packed_array.exp: print var

This is probably the same as the packed array case above.

-- 
Joel

Re: Ada testsuite failures

[Joel Brobecker]

Daniel,

> ptype empty
> type = function return int
> (gdb) FAIL: gdb.ada/null_record.exp: ptype on null record

My appologies when I said I had the same failure. I do have a failure,
but the symptoms are different. What does the debugger print in your
case when you do:

        (gdb) ptype bar.empty

Can you perhaps cd to testsuite/gdb.ada/null_record, do the following:

        % gnatmake -f -g null_record -cargs -dA -save-temps

Then tar up the null_record directory and send it to me. I should be
able to tell you whether it's a debugging information issue or something
else.

Thanks you,
-- 
Joel

Re: Ada testsuite failures

[Daniel Jacobowitz]

On Tue, Jan 02, 2007 at 03:39:03PM +0400, Joel Brobecker wrote:
> Daniel,
> 
> > ptype empty
> > type = function return int
> > (gdb) FAIL: gdb.ada/null_record.exp: ptype on null record
> 
> My appologies when I said I had the same failure. I do have a failure,
> but the symptoms are different. What does the debugger print in your
> case when you do:
> 
>         (gdb) ptype bar.empty

(gdb) ptype empty
type = function return int

(gdb) p bar.empty
No definition of "bar.empty" in current context.

(gdb) p empty
$1 = {<text variable, no debug info>} 0x2aaaaad3d590 <gnat__sockets__empty>

Isn't that interesting - that's something completely different.  There
are zero references to the string "empty" in the debug info for
null_record, so it is definitely a debugging information problem.

Looking at the test I'm not surprised.  Empty is unused; adding
something that references it is probably sufficient.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Ada testsuite failures

[Daniel Jacobowitz]

On Tue, Jan 02, 2007 at 11:38:27AM +0400, Joel Brobecker wrote:
> I wouldn't be surprised. Recents versions of GCC are much better at
> not emitting debugging info for unused entity. This has caused us a lot
> of trouble when moving to GCC 4.1 for instance, because GNAT generates
> of lot of entities that appear unused from the compiler point of view,
> but are necessary for the debugger to work properly. We're getting back
> to the same project of getting rid of all these encoding in favor of
> standard DWARF. This is actually a very active project that we're
> working on right now, so hopefully the situation won't be as bad soon.

The unused type purging does sound like a likely culprit for these,
yeah.  Just adding -fno-eliminate-unused-debug-types doesn't fix it,
but it's probably related.

> I can ask that the local patches we maintain in our tree be submitted
> for inclusion if you think they'd be accepted. Our opinion was that they
> would probably be rejected as a step backward.

Well, I'd like to see them, anyway.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Ada testsuite failures

[Joel Brobecker]

> (gdb) p empty
> $1 = {<text variable, no debug info>} 0x2aaaaad3d590 <gnat__sockets__empty>
> 
> Isn't that interesting - that's something completely different.  There
> are zero references to the string "empty" in the debug info for
> null_record, so it is definitely a debugging information problem.

Yes and no. The test isn't representative of "real code". It will
be more productive to enhance the testcase a bit, I was very naive
when I wrote this code - it must have been the reduced testcase
I have been using to track down the issue, the code we have in our
testsuite is a bit better that way.

> Looking at the test I'm not surprised.  Empty is unused; adding
> something that references it is probably sufficient.

Yes, I agree. Could you try the following patch?

2006-01-02  Joel Brobecker  <brobecker@adacore.com>

	* gdb.ada/null_record/bar.ads (Do_Nothing): Add extra parameter.
	* gdb.ada/null_record/bar.adb (Do_Nothing): Likewise.
	* gdb.ada/null_record/null_record.adb: Fake use of type Empty
	to prevent this type from being optimized out.

That should help,
-- 
Joel

Re: Ada testsuite failures

[Joel Brobecker]

> > I can ask that the local patches we maintain in our tree be submitted
> > for inclusion if you think they'd be accepted. Our opinion was that they
> > would probably be rejected as a step backward.
> 
> Well, I'd like to see them, anyway.

I'll send a message to the coworker that works on them. He'll be back
in a couple of days.

-- 
Joel

Re: Ada testsuite failures

[Joel Brobecker]

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

[with the patch, sorry]
> (gdb) p empty
> $1 = {<text variable, no debug info>} 0x2aaaaad3d590 <gnat__sockets__empty>
> 
> Isn't that interesting - that's something completely different.  There
> are zero references to the string "empty" in the debug info for
> null_record, so it is definitely a debugging information problem.

Yes and no. The test isn't representative of "real code". It will
be more productive to enhance the testcase a bit, I was very naive
when I wrote this code - it must have been the reduced testcase
I have been using to track down the issue, the code we have in our
testsuite is a bit better that way.

> Looking at the test I'm not surprised.  Empty is unused; adding
> something that references it is probably sufficient.

Yes, I agree. Could you try the following patch?

2006-01-02  Joel Brobecker  <brobecker@adacore.com>

	* gdb.ada/null_record/bar.ads (Do_Nothing): Add extra parameter.
	* gdb.ada/null_record/bar.adb (Do_Nothing): Likewise.
	* gdb.ada/null_record/null_record.adb: Fake use of type Empty
	to prevent this type from being optimized out.

That should help,
-- 
Joel

[-- Attachment #2: nr.diff --]
[-- Type: text/plain, Size: 1691 bytes --]

Index: gdb.ada/null_record/bar.ads
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.ada/null_record/bar.ads,v
retrieving revision 1.1
diff -u -p -r1.1 bar.ads
--- gdb.ada/null_record/bar.ads	3 Feb 2005 03:58:53 -0000	1.1
+++ gdb.ada/null_record/bar.ads	2 Jan 2007 16:26:19 -0000
@@ -14,10 +14,12 @@
 --  along with this program; if not, write to the Free Software
 --  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 
+with System;
+
 package Bar is
 
    type Empty is null record;
 
-   procedure Do_Nothing;
+   procedure Do_Nothing (E : System.Address);
 
 end Bar;
Index: gdb.ada/null_record/bar.adb
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.ada/null_record/bar.adb,v
retrieving revision 1.1
diff -u -p -r1.1 bar.adb
--- gdb.ada/null_record/bar.adb	3 Feb 2005 03:58:53 -0000	1.1
+++ gdb.ada/null_record/bar.adb	2 Jan 2007 16:26:19 -0000
@@ -16,7 +16,7 @@
 
 package body Bar is
 
-   procedure Do_Nothing is
+   procedure Do_Nothing (E : System.Address) is
    begin
       null;
    end Do_Nothing;
Index: gdb.ada/null_record/null_record.adb
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.ada/null_record/null_record.adb,v
retrieving revision 1.1
diff -u -p -r1.1 null_record.adb
--- gdb.ada/null_record/null_record.adb	3 Feb 2005 03:58:53 -0000	1.1
+++ gdb.ada/null_record/null_record.adb	2 Jan 2007 16:26:19 -0000
@@ -17,7 +17,8 @@
 with Bar; use Bar;
 
 procedure Null_Record is
+   E : Empty;
 begin
-   Do_Nothing;
+   Do_Nothing (E'Address);
 end Null_Record;
 

Re: Ada testsuite failures

[Daniel Jacobowitz]

On Tue, Jan 02, 2007 at 08:33:02PM +0400, Joel Brobecker wrote:
> > Looking at the test I'm not surprised.  Empty is unused; adding
> > something that references it is probably sufficient.
> 
> Yes, I agree. Could you try the following patch?
> 
> 2006-01-02  Joel Brobecker  <brobecker@adacore.com>
> 
> 	* gdb.ada/null_record/bar.ads (Do_Nothing): Add extra parameter.
> 	* gdb.ada/null_record/bar.adb (Do_Nothing): Likewise.
> 	* gdb.ada/null_record/null_record.adb: Fake use of type Empty
> 	to prevent this type from being optimized out.
> 
> That should help,

Sort of...

(gdb) ptype E
type = record null; end record
(gdb) ptype empty
type = function return int
(gdb) ptype bar.empty
No definition of "bar.empty" in current context.

The debug info does mention bar__empty now, at least.


-- 
Daniel Jacobowitz
CodeSourcery

Re: Ada testsuite failures

[Joel Brobecker]

> Sort of...
> 
> (gdb) ptype E
> type = record null; end record
> (gdb) ptype empty
> type = function return int
> (gdb) ptype bar.empty
> No definition of "bar.empty" in current context.
> 
> The debug info does mention bar__empty now, at least.

Could you build with -save-temps and send me the contents of your
directory:

        % cd gdb.ada/null_record
        % gnatmake -f -g null_record -cargs -dA -save-temps

I will have a look.

Shall I commit the testcase change in the meantime? It looks like
it's an improvement already.

-- 
Joel

Re: Ada testsuite failures

[Joel Brobecker]

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

> (gdb) ptype bar.empty
> No definition of "bar.empty" in current context.

Thanks for sending me your objfiles. I know what's going on, now.
Once I understood the source of this issue, it was actually fairly
simple to reproduce the problem with a C program. Consider:

    struct empty {};
    struct not_empty { long long a; };
    
    int
    main (void)
    {
      struct empty e = {};
      struct not_empty n = { 1 };
    
      n.a = (long long) &e;
    
      return 0;
    }

The heart of the problem is that struct empty is a struct that does
not have any children. We have some guards in dwarf2read.c for both
partial symbols and full symbols readers that make GDB ignore the
DW_TAG_structure DIEs for type empty:

    add_partial_symbol (...)
    {
        case DW_TAG_structure_type:
          /* Skip aggregate types without children, these are external
             references.  */
          /* NOTE: carlton/2003-10-07: See comment in new_symbol about
             static vs. global.  */
 !!!->    if (pdi->has_children == 0)
            return;
          add_psymbol_to_list (actual_name, strlen (actual_name),

Same for full symbols:

    process_structure_scope (...)
    {
    [...]
      if (die->child != NULL && ! die_is_declaration (die, cu))
        new_symbol (die, die->type, cu);

What saves us with our version of GNAT Pro is that the compiler
also creates a DW_TAG_typedef DIE associated to this struct.
This is this DIE that allows GDB to create a symbol for this type.

With the C example above, where the language allows me to control
whether typedefs are emitted or not, I can reproduce with the compiler
I use the same problem as yours.

The guards cited above have been there since version 1.1 of dwarf2read.c.
I don't know how relevant they are now - presumably today's compilers
would rather use a declaration attribute rather than empty structs.
Strictly speaking, the guard as implemented is wrong. So I propose to
simply remove it.

2006-02-03  Joel Brobecker  <brobecker@adacore.com>

        * dwarf2read.c (add_partial_symbol): Update copyright year.
        Do not skip struct, union and enum types with no children.

2006-02-03  Joel Brobecker  <brobecker@adacore.com>

        * gdb.base/nofield.c: New file.
        * gdb.base/nofield.exp: New testcase.

Tested on x86_64-linux. No regression. Nofield.exp has two FAILs before
the patch, and is all PASS after.

OK to apply?

(It should fix the problem you are seeing, but I am unable to verify
this, because I can't run your program on my machine. It was compiled
with the shared version of the GNAT runtime (libgnat-4.0.so). Next
time I'll make sure to ask to add -bargs -static. Can you verify that
your FAIL is gone with this patch? - Thank you!)

-- 
Joel

[-- Attachment #2: nochild.diff --]
[-- Type: text/plain, Size: 1450 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.207
diff -u -p -r1.207 dwarf2read.c
--- dwarf2read.c	27 Dec 2006 22:38:57 -0000	1.207
+++ dwarf2read.c	3 Jan 2007 13:58:37 -0000
@@ -1,7 +1,7 @@
 /* DWARF 2 debugging format support for GDB.
 
    Copyright (C) 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001,
-                 2002, 2003, 2004, 2005, 2006
+                 2002, 2003, 2004, 2005, 2006, 2007
    Free Software Foundation, Inc.
 
    Adapted by Gary Funck (gary@intrepid.com), Intrepid Technology,
@@ -1979,12 +1979,8 @@ add_partial_symbol (struct partial_die_i
     case DW_TAG_structure_type:
     case DW_TAG_union_type:
     case DW_TAG_enumeration_type:
-      /* Skip aggregate types without children, these are external
-         references.  */
       /* NOTE: carlton/2003-10-07: See comment in new_symbol about
 	 static vs. global.  */
-      if (pdi->has_children == 0)
-	return;
       add_psymbol_to_list (actual_name, strlen (actual_name),
 			   STRUCT_DOMAIN, LOC_TYPEDEF,
 			   (cu->language == language_cplus
@@ -4017,7 +4013,7 @@ process_structure_scope (struct die_info
       child_die = sibling_die (child_die);
     }
 
-  if (die->child != NULL && ! die_is_declaration (die, cu))
+  if (!die_is_declaration (die, cu))
     new_symbol (die, die->type, cu);
 
   processing_current_prefix = previous_prefix;

[-- Attachment #3: nofield.c --]
[-- Type: text/plain, Size: 1169 bytes --]

/* This testcase is part of GDB, the GNU debugger.

   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  */

struct empty {};
union empty_union {};

struct not_empty
{
  long long e;
  long long u;
};

int
main (void)
{
  struct empty e = {};
  union empty_union u;
  struct not_empty n = {1, 2};

  n.e = (long long) &e;
  n.u = (long long) &u;

  return 0;
}


[-- Attachment #4: nofield.exp --]
[-- Type: text/plain, Size: 1594 bytes --]

# 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., 51 Franklin Street, Fifth Floor, Boston,
# MA 02110-1301, USA

if $tracelevel {
    strace $tracelevel
}

set testfile "nofield"
set srcfile ${testfile}.c
set binfile ${objdir}/${subdir}/${testfile}
if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
    untested "Couldn't compile ${srcfile}"
    return -1
}

set eol "\[\r\n\]+"
set sp "\[ \t\]*"

gdb_exit
gdb_start
gdb_reinitialize_dir $srcdir/$subdir
gdb_load ${binfile}

gdb_test "ptype struct not_empty" \
         "type = struct not_empty {$eol${sp}long long int e;$eol${sp}long long int u;$eol}" \
         "ptype struct not_empty"

gdb_test "ptype struct empty" \
         "type = struct empty {$eol$sp<no data fields>$eol}" \
         "ptype struct empty"

gdb_test "ptype union empty_union" \
         "type = union empty_union {$eol$sp<no data fields>$eol}" \
         "ptype union empty_union"

Re: Ada testsuite failures

[Joel Brobecker]

[applogies for not having sent this message to gdb-patches - mistake
on my part].

-- 
Joel

Re: Ada testsuite failures

[Jim Blandy]

Joel Brobecker <brobecker@adacore.com> writes:
> The guards cited above have been there since version 1.1 of dwarf2read.c.
> I don't know how relevant they are now - presumably today's compilers
> would rather use a declaration attribute rather than empty structs.
> Strictly speaking, the guard as implemented is wrong. So I propose to
> simply remove it.
>
> 2006-02-03  Joel Brobecker  <brobecker@adacore.com>
>
>         * dwarf2read.c (add_partial_symbol): Update copyright year.
>         Do not skip struct, union and enum types with no children.
>
> 2006-02-03  Joel Brobecker  <brobecker@adacore.com>
>
>         * gdb.base/nofield.c: New file.
>         * gdb.base/nofield.exp: New testcase.
>
> Tested on x86_64-linux. No regression. Nofield.exp has two FAILs before
> the patch, and is all PASS after.

The change to process_structure_type is almost right, but I'm pretty
sure the change to add_partial_symbol isn't.

The partial symtab scan shouldn't create psymbols for incomplete
struct types, since we don't want to read in this compilation unit
when the user enters a reference to the type.  So the test in
add_partial_symbol should use the criteria in the DWARF spec for
recognizing incomplete structure types; section 5.6.1 "Structure,
Union, and Class Type Entries", says "An incomplete structure, union
or class type is represented by a structure, union or class entry that
does not have a byte size attribute and that has a DW_AT_declaration
attribute."

At the moment, there's nothing in 'struct partial_die_info' that would
tell you if the die has a size, but it looks easy enough to add.

In process_structure_scope, we should drop the child test, as your
patch does, but also check for the presence of DW_AT_byte_size.