[Bug debug/47939] New: Missing DW_TAG_typedef for qualified types

[Bug debug/47939] New: Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

           Summary: Missing DW_TAG_typedef for qualified types
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Keywords: wrong-debug
          Severity: normal
          Priority: P3
         Component: debug
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: rguenth@gcc.gnu.org


typedef const struct _George { int dummy; } George_t;
George_t george;

lacks a DW_TAG_typedef for George_t.  Compare that to

typedef struct _George { int dummy; } George_t;
George_t george;

where a DW_TAG_typedef for George_t appears.

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.03.01 14:11:28
                 CC|                            |jsm28 at gcc dot gnu.org
          Component|debug                       |c
     Ever Confirmed|0                           |1

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-01 14:11:28 UTC ---
Happens because when finishing the TYPE_DECL

      if (is_naming_typedef_decl (decl))
        /* We want that all subsequent calls to lookup_type_die with
           TYPE in argument yield the DW_TAG_typedef we have just
           created.  */
        equate_type_number_to_die (type, type_die);

returns false.  Which is because is_cxx () is returning false.

The VAR_DECL itself is not using the type-def id:

 <var_decl 0x7ffff5b49000 george
    type <record_type 0x7ffff5b2a498 _George readonly type_0 SI
        size <integer_cst 0x7ffff7ed36e0 constant 32>
        unit size <integer_cst 0x7ffff7ed33e8 constant 4>
        align 32 symtab -172776352 alias set -1 canonical type 0x7ffff5b2a498
        fields <field_decl 0x7ffff5b36130 dummy type <integer_type
0x7ffff7ee6498 int>
            SI file t.c line 1 col 36 size <integer_cst 0x7ffff7ed36e0 32> unit
size <integer_cst 0x7ffff7ed33e8 4>
            align 32 offset_align 128
            offset <integer_cst 0x7ffff7ed3410 constant 0>
            bit offset <integer_cst 0x7ffff7ed3b68 constant 0> context
<record_type 0x7ffff5b2a3f0 _George>> context <translation_unit_decl
0x7ffff7efca10 D.1588>>
    readonly asm_written public static common SI file t.c line 2 col 10 size
<integer_cst 0x7ffff7ed36e0 32> unit size <integer_cst 0x7ffff7ed33e8 4>
    align 32 context <translation_unit_decl 0x7ffff7efca10 D.1588>
    (mem/s/u/c:SI (symbol_ref:DI ("george") <var_decl 0x7ffff5b49000 george>)
[0 george+0 S4 A32])>

already grokdeclarator creates this by re-applying qualifiers via

  if (!flag_gen_aux_info && (TYPE_QUALS (element_type)))
    type = TYPE_MAIN_VARIANT (type);
  type_quals = ((constp ? TYPE_QUAL_CONST : 0)
                | (restrictp ? TYPE_QUAL_RESTRICT : 0)
                | (volatilep ? TYPE_QUAL_VOLATILE : 0)
                | ENCODE_QUAL_ADDR_SPACE (address_space));

and

6010    type = c_build_qualified_type (type, type_quals);

thereby stripping all uses of typedef ids.

Thus, it doesn't seem to distinguish between qualifiers applied at the
declaration and qualifiers that are part of the typedef.

c_build_qualified_type would handle choosing a variant with the same
name just fine - but we feed it the main variant instead of the original
type.

Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c        (revision 170589)
+++ gcc/c-decl.c        (working copy)
@@ -6007,7 +6007,7 @@ grokdeclarator (const struct c_declarato
        /* An uninitialized decl with `extern' is a reference.  */
        int extern_ref = !initialized && storage_class == csc_extern;

-       type = c_build_qualified_type (type, type_quals);
+       type = c_build_qualified_type (declspecs->type, type_quals);

        /* C99 6.2.2p7: It is invalid (compile-time undefined
           behavior) to create an 'extern' declaration for a

fixes this particular testcase.

Joseph?

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-01 14:31:35 UTC ---
The lack of knowledge of George_t is the worst side-effect of this bug.

/* { dg-do run } */
/* { dg-options "-g" } */

typedef struct _Harry { int dummy; } Harry_t;
Harry_t harry; /* { dg-final { gdb-test 12 "((Harry_t *)&harry)->dummy" "0" } }
*/
typedef const struct _George { int dummy; } George_t;
George_t george; /* { dg-final { gdb-test 12 "((George_t *)&george)->dummy" "0"
} } */
typedef const Harry_t Michael_t;
Michael_t michael; /* { dg-final { gdb-test 12 "((Michael_t *)&michael)->dummy"
"0" } } */
int main()
{    
  return 0;
} 

currently George_t and Michael_t using tests are UNSUPPORTED (error in
processing the command).

The patch has weird side-effects on diagnostics, it only shows a possible
hint as to what the reason is we drop the TYPE_NAME.

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-01 14:36:31 UTC ---
Better patch, I'm giving that a quick test.

Index: c-decl.c
===================================================================
--- c-decl.c    (revision 170589)
+++ c-decl.c    (working copy)
@@ -5038,7 +5038,7 @@ grokdeclarator (const struct c_declarato
     error_at (loc, "conflicting named address spaces (%s vs %s)",
              c_addr_space_name (as1), c_addr_space_name (as2));

-  if (!flag_gen_aux_info && (TYPE_QUALS (element_type)))
+  if (!flag_gen_aux_info && type != element_type && (TYPE_QUALS
(element_type)))
     type = TYPE_MAIN_VARIANT (type);
   type_quals = ((constp ? TYPE_QUAL_CONST : 0)
                | (restrictp ? TYPE_QUAL_RESTRICT : 0)

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-01 14:49:43 UTC ---
That said, specifying -aux-info /dev/null also fixes this bug.  Huh.

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-01 15:41:53 UTC ---
The patch bootstrapped and tested ok.  Removing

  if (!flag_gen_aux_info && (TYPE_QUALS (element_type)))
    type = TYPE_MAIN_VARIANT (type);

unconditionally breaks gcc.dg/array-quals-2.c (but nothing else).

Whether that is a bug or such radical fix is wrong remains to be determined.

Changing types based on flag_gen_aux_info definitely looks wrong.

Probably a better general approach would be to make c_build_qualified_type
also take a desired name as argument instead of using TYPE_NAME.

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

--- Comment #6 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-03-01 16:52:37 UTC ---
On Tue, 1 Mar 2011, rguenth at gcc dot gnu.org wrote:

> The patch bootstrapped and tested ok.  Removing
> 
>   if (!flag_gen_aux_info && (TYPE_QUALS (element_type)))
>     type = TYPE_MAIN_VARIANT (type);
> 
> unconditionally breaks gcc.dg/array-quals-2.c (but nothing else).

The point of the logic removing qualifiers here is as described in the 
comment

  /* Now figure out the structure of the declarator proper.
     Descend through it, creating more complex types, until we reach
     the declared identifier (or NULL_TREE, in an absolute declarator).
     At each stage we maintain an unqualified version of the type
     together with any qualifiers that should be applied to it with
     c_build_qualified_type; this way, array types including
     multidimensional array types are first built up in unqualified
     form and then the qualified form is created with
     TYPE_MAIN_VARIANT pointing to the unqualified form.  */

to ensure consistency in TYPE_MAIN_VARIANT for array types; see 
<http://gcc.gnu.org/ml/gcc-patches/2005-01/msg02180.html>.

> Probably a better general approach would be to make c_build_qualified_type
> also take a desired name as argument instead of using TYPE_NAME.

In general it's a desired name for an intermediate type at some level of 
array derivation, not for the type being constructed by 
c_build_qualified_type.  I suppose you could pass both the name and the 
particular type that should have that name, or otherwise indicate the 
qualifiers and level of array nesting where the name should be used if 
possible.

If you have

typedef const int T[1];
T array[2][3];

then "const" is being applied to "int [2][3][1]", and it's the 
intermediate type "const int [1]" that you'd like to get the name T.

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-01 17:01:38 UTC ---
(In reply to comment #6)
> On Tue, 1 Mar 2011, rguenth at gcc dot gnu.org wrote:
> 
> > The patch bootstrapped and tested ok.  Removing
> > 
> >   if (!flag_gen_aux_info && (TYPE_QUALS (element_type)))
> >     type = TYPE_MAIN_VARIANT (type);
> > 
> > unconditionally breaks gcc.dg/array-quals-2.c (but nothing else).
> 
> The point of the logic removing qualifiers here is as described in the 
> comment
> 
>   /* Now figure out the structure of the declarator proper.
>      Descend through it, creating more complex types, until we reach
>      the declared identifier (or NULL_TREE, in an absolute declarator).
>      At each stage we maintain an unqualified version of the type
>      together with any qualifiers that should be applied to it with
>      c_build_qualified_type; this way, array types including
>      multidimensional array types are first built up in unqualified
>      form and then the qualified form is created with
>      TYPE_MAIN_VARIANT pointing to the unqualified form.  */
> 
> to ensure consistency in TYPE_MAIN_VARIANT for array types; see 
> <http://gcc.gnu.org/ml/gcc-patches/2005-01/msg02180.html>.
> 
> > Probably a better general approach would be to make c_build_qualified_type
> > also take a desired name as argument instead of using TYPE_NAME.
> 
> In general it's a desired name for an intermediate type at some level of 
> array derivation, not for the type being constructed by 
> c_build_qualified_type.  I suppose you could pass both the name and the 
> particular type that should have that name, or otherwise indicate the 
> qualifiers and level of array nesting where the name should be used if 
> possible.
> 
> If you have
> 
> typedef const int T[1];
> T array[2][3];
> 
> then "const" is being applied to "int [2][3][1]", and it's the 
> intermediate type "const int [1]" that you'd like to get the name T.

I see.  There is an alternative way to stripping qualifiers - build
an unqualified variant like with

Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c        (revision 170594)
+++ gcc/c-decl.c        (working copy)
@@ -5038,8 +5038,8 @@ grokdeclarator (const struct c_declarato
     error_at (loc, "conflicting named address spaces (%s vs %s)",
              c_addr_space_name (as1), c_addr_space_name (as2));

-  if (!flag_gen_aux_info && (TYPE_QUALS (element_type)))
-    type = TYPE_MAIN_VARIANT (type);
+  if (TYPE_QUALS (element_type))
+    type = c_build_qualified_type (type, 0);
   type_quals = ((constp ? TYPE_QUAL_CONST : 0)
                | (restrictp ? TYPE_QUAL_RESTRICT : 0)
                | (volatilep ? TYPE_QUAL_VOLATILE : 0)

or alternatively using build_qualified_type directly to not strip
qualifiers recursively at this point.  That would preserve the
type-names but also introduce a unqualified variants with that name
if it doesn't exist already.  To avoid the latter in c_build_qualified_type
we could pass TYPE_MAIN_VARIANT to the build_qualified_type calls.

I don't know if I'm the correct person to try fixing this (I'm certainly
new to this part of the code), but at least the flag_gen_aux_info
checking looks suspicious and should be removed.

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-02 13:12:16 UTC ---
I have a patch.

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-21 15:32:37 UTC ---
Author: rguenth
Date: Mon Mar 21 15:32:21 2011
New Revision: 171245

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=171245
Log:
2011-03-21  Richard Guenther  <rguenther@suse.de>

    PR c/47939
    * c-decl.c (grokdeclarator): Drop to the main variant only
    for array types.  Drop flag_gen_aux_info check.

    * gcc.dg/debug/dwarf2/pr47939-1.c: New testcase.
    * gcc.dg/debug/dwarf2/pr47939-2.c: Likewise.
    * gcc.dg/debug/dwarf2/pr47939-3.c: Likewise.
    * gcc.dg/debug/dwarf2/pr47939-4.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-1.c
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-2.c
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-3.c
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-decl.c
    trunk/gcc/testsuite/ChangeLog

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.0

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-22 11:40:01 UTC ---
Fixed for 4.7.

[Bug c/47939] Missing DW_TAG_typedef for qualified types

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47939

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-04-15 20:14:04 UTC ---
Fixed.