* [PATCH][C] Fix PR47939
@ 2011-03-02 13:10 Richard Guenther
2011-03-10 17:41 ` Joseph S. Myers
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guenther @ 2011-03-02 13:10 UTC (permalink / raw)
To: gcc-patches; +Cc: Joseph S. Myers
This patch is one way to fix PR47939, the fact that we fail to use
a typedef id for variables if it is qualified. It is not exactly
clear to me whether the code as it is currently written obviously
misses something.
! if (!flag_gen_aux_info && (TYPE_QUALS (element_type)))
! type = TYPE_MAIN_VARIANT (type);
First of all, using -aux-info will change types and thus debug info.
Second, testing qualifiers of element_type to drop qualifiers on
type suggests this dropping may have been intended only for
array types.
The main problem is that dropping to TYPE_MAIN_VARIANT gets us to
use qualified variants with the same TYPE_NAME as the
TYPE_MAIN_VARIANT (thus, not typedef variants).
Thus the following fix implements what the existing code suggests,
drop to the main variant only for array types. That has the advantage
of not changing the code for the array case (which is sort-of weird
to me).
Without this patch gcc.dg/debug/dwarf2/pr47939-2.c and
gcc.dg/debug/dwarf2/pr47939-4.c fail for me, -1 and -3 are
to complete test coverage of the already working cases
(feel free to suggest other variants).
I can omit the removal of !flag_gen_aux_info if you like
(this two lines date back to rev. 319, the initial import of this
code by RMS).
Bootstrapped on x86_64-unknown-linux-gnu, re-testing in progress
(an equivalent patch bootstrapped and tested fine).
Ok for 4.7?
Thanks,
Richard.
2011-03-02 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.
Index: gcc/c-decl.c
===================================================================
*** gcc/c-decl.c (revision 170594)
--- gcc/c-decl.c (working copy)
*************** grokdeclarator (const struct c_declarato
*** 5038,5044 ****
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);
type_quals = ((constp ? TYPE_QUAL_CONST : 0)
| (restrictp ? TYPE_QUAL_RESTRICT : 0)
--- 5038,5044 ----
error_at (loc, "conflicting named address spaces (%s vs %s)",
c_addr_space_name (as1), c_addr_space_name (as2));
! if (TREE_CODE (type) == ARRAY_TYPE && TYPE_QUALS (element_type))
type = TYPE_MAIN_VARIANT (type);
type_quals = ((constp ? TYPE_QUAL_CONST : 0)
| (restrictp ? TYPE_QUAL_RESTRICT : 0)
Index: gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-1.c
===================================================================
*** gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-1.c (revision 0)
--- gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-1.c (revision 0)
***************
*** 0 ****
--- 1,8 ----
+ /* { dg-do compile } */
+ /* { dg-options "-save-temps -g -dA" } */
+
+ typedef struct _Harry { int dummy; } Harry_t;
+ Harry_t harry;
+
+ /* { dg-final { scan-assembler "DW_TAG_typedef\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_name: \"Harry_t\"" } } */
+ /* { dg-final { cleanup-saved-temps } } */
Index: gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-2.c
===================================================================
*** gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-2.c (revision 0)
--- gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-2.c (revision 0)
***************
*** 0 ****
--- 1,8 ----
+ /* { dg-do compile } */
+ /* { dg-options "-save-temps -g -dA" } */
+
+ typedef const struct _Harry { int dummy; } Harry_t;
+ Harry_t harry;
+
+ /* { dg-final { scan-assembler "DW_TAG_typedef\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_name: \"Harry_t\"" } } */
+ /* { dg-final { cleanup-saved-temps } } */
Index: gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-3.c
===================================================================
*** gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-3.c (revision 0)
--- gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-3.c (revision 0)
***************
*** 0 ****
--- 1,8 ----
+ /* { dg-do compile } */
+ /* { dg-options "-save-temps -g -dA" } */
+
+ typedef struct _Harry { int dummy; } Harry_t;
+ const Harry_t harry[5];
+
+ /* { dg-final { scan-assembler "DW_TAG_typedef\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_name: \"Harry_t\"" } } */
+ /* { dg-final { cleanup-saved-temps } } */
Index: gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-4.c
===================================================================
*** gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-4.c (revision 0)
--- gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-4.c (revision 0)
***************
*** 0 ****
--- 1,8 ----
+ /* { dg-do compile } */
+ /* { dg-options "-save-temps -g -dA" } */
+
+ typedef const struct _Harry { int dummy; } Harry_t;
+ Harry_t harry[10];
+
+ /* { dg-final { scan-assembler "DW_TAG_typedef\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_name: \"Harry_t\"" } } */
+ /* { dg-final { cleanup-saved-temps } } */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][C] Fix PR47939
2011-03-02 13:10 [PATCH][C] Fix PR47939 Richard Guenther
@ 2011-03-10 17:41 ` Joseph S. Myers
2011-03-11 11:25 ` Richard Guenther
0 siblings, 1 reply; 6+ messages in thread
From: Joseph S. Myers @ 2011-03-10 17:41 UTC (permalink / raw)
To: Richard Guenther; +Cc: gcc-patches
On Wed, 2 Mar 2011, Richard Guenther wrote:
> 2011-03-02 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.
I can't convince myself that this is safe - that is, that it will maintain
the invariant that TYPE_MAIN_VARIANT for an array type always points to a
version where the ultimate element type is unqualified. The problem would
be where the type from the declaration specifiers is a qualified type, not
an array type, but becomes an array element type through array declarators
being processed by grokdeclarator, and so arrays are built up directly
with qualified element type without the unqualified variants being built
first to become the main variants. It certainly appears that in a case
such as
typedef const int T;
T a[10];
const int b[10];
you get multiple copies of the const int[10] type, some of which have a
const int[10] variant as their main variant (incorrect) and some of which
have an int[10] variant as their main variant (correct); the canonical
types look rather odd as well. It's reasonable have multiple copies here
- for the type T[10] to remember the type name T it was derived from - but
in such a case they should still all be on the variant chain for a single
int[10] main variant.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][C] Fix PR47939
2011-03-10 17:41 ` Joseph S. Myers
@ 2011-03-11 11:25 ` Richard Guenther
2011-03-11 13:08 ` Joseph S. Myers
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guenther @ 2011-03-11 11:25 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: gcc-patches
On Thu, 10 Mar 2011, Joseph S. Myers wrote:
> On Wed, 2 Mar 2011, Richard Guenther wrote:
>
> > 2011-03-02 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.
>
> I can't convince myself that this is safe - that is, that it will maintain
> the invariant that TYPE_MAIN_VARIANT for an array type always points to a
> version where the ultimate element type is unqualified. The problem would
> be where the type from the declaration specifiers is a qualified type, not
> an array type, but becomes an array element type through array declarators
> being processed by grokdeclarator, and so arrays are built up directly
> with qualified element type without the unqualified variants being built
> first to become the main variants. It certainly appears that in a case
> such as
>
> typedef const int T;
> T a[10];
> const int b[10];
>
> you get multiple copies of the const int[10] type, some of which have a
> const int[10] variant as their main variant (incorrect) and some of which
> have an int[10] variant as their main variant (correct); the canonical
> types look rather odd as well. It's reasonable have multiple copies here
> - for the type T[10] to remember the type name T it was derived from - but
> in such a case they should still all be on the variant chain for a single
> int[10] main variant.
Indeed. I tried to let the array case alone (because it's so
complicated) but failed to do so. Appearantly
if (declarator->kind == cdk_array && TYPE_QUALS (element_type))
type = TYPE_MAIN_VARIANT (type);
leaves it alone (and doesn't emit a DW_TAG_typedef for T for your
testcase). Thus out of the set of testcases I added the array
case now fails with the above (as I'd have expected but were of
course positively surprised as it didn't ...).
I verified the main variants and canonical types are sane with
the above variant for your testcase.
If you think such change isn't safe either I'll pursue a dwarf2out.c
local change, somehow forcing out the typedef DIE even if it is unused.
I don't feel at home in the grokdeclarator dungeon.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][C] Fix PR47939
2011-03-11 11:25 ` Richard Guenther
@ 2011-03-11 13:08 ` Joseph S. Myers
2011-03-18 10:13 ` Richard Guenther
0 siblings, 1 reply; 6+ messages in thread
From: Joseph S. Myers @ 2011-03-11 13:08 UTC (permalink / raw)
To: Richard Guenther; +Cc: gcc-patches
On Fri, 11 Mar 2011, Richard Guenther wrote:
> Indeed. I tried to let the array case alone (because it's so
> complicated) but failed to do so. Appearantly
>
> if (declarator->kind == cdk_array && TYPE_QUALS (element_type))
> type = TYPE_MAIN_VARIANT (type);
>
> leaves it alone (and doesn't emit a DW_TAG_typedef for T for your
> testcase). Thus out of the set of testcases I added the array
> case now fails with the above (as I'd have expected but were of
> course positively surprised as it didn't ...).
>
> I verified the main variants and canonical types are sane with
> the above variant for your testcase.
>
> If you think such change isn't safe either I'll pursue a dwarf2out.c
> local change, somehow forcing out the typedef DIE even if it is unused.
> I don't feel at home in the grokdeclarator dungeon.
What I think is safe in grokdeclarator is using TYPE_MAIN_VARIANT here if
*either* the type given in the declaration specifiers is an array type
(TREE_CODE (type) == ARRAY_TYPE, as in your previous patch) *or* the first
declarator that is not cdk_attrs is cdk_array (as in this version, but
checking through a chain of declarator->declarator to find a possible
cdk_array after a sequence of cdk_attrs).
(Aside from all this it is a longstanding known bug that the debug
information for arrays of qualified types isn't quite right: PR 8354.)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][C] Fix PR47939
2011-03-11 13:08 ` Joseph S. Myers
@ 2011-03-18 10:13 ` Richard Guenther
2011-03-18 18:39 ` Joseph S. Myers
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guenther @ 2011-03-18 10:13 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: gcc-patches
On Fri, 11 Mar 2011, Joseph S. Myers wrote:
> On Fri, 11 Mar 2011, Richard Guenther wrote:
>
> > Indeed. I tried to let the array case alone (because it's so
> > complicated) but failed to do so. Appearantly
> >
> > if (declarator->kind == cdk_array && TYPE_QUALS (element_type))
> > type = TYPE_MAIN_VARIANT (type);
> >
> > leaves it alone (and doesn't emit a DW_TAG_typedef for T for your
> > testcase). Thus out of the set of testcases I added the array
> > case now fails with the above (as I'd have expected but were of
> > course positively surprised as it didn't ...).
> >
> > I verified the main variants and canonical types are sane with
> > the above variant for your testcase.
> >
> > If you think such change isn't safe either I'll pursue a dwarf2out.c
> > local change, somehow forcing out the typedef DIE even if it is unused.
> > I don't feel at home in the grokdeclarator dungeon.
>
> What I think is safe in grokdeclarator is using TYPE_MAIN_VARIANT here if
> *either* the type given in the declaration specifiers is an array type
> (TREE_CODE (type) == ARRAY_TYPE, as in your previous patch) *or* the first
> declarator that is not cdk_attrs is cdk_array (as in this version, but
> checking through a chain of declarator->declarator to find a possible
> cdk_array after a sequence of cdk_attrs).
>
> (Aside from all this it is a longstanding known bug that the debug
> information for arrays of qualified types isn't quite right: PR 8354.)
Ok, the following works for me.
Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk?
Thanks,
Richard.
2011-03-17 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.
Index: gcc/c-decl.c
===================================================================
*** gcc/c-decl.c (revision 171097)
--- gcc/c-decl.c (working copy)
*************** grokdeclarator (const struct c_declarato
*** 4892,4897 ****
--- 4892,4898 ----
const char *errmsg;
tree expr_dummy;
bool expr_const_operands_dummy;
+ enum c_declarator_kind first_non_attr_kind;
if (TREE_CODE (type) == ERROR_MARK)
return error_mark_node;
*************** grokdeclarator (const struct c_declarato
*** 4911,4916 ****
--- 4912,4918 ----
{
const struct c_declarator *decl = declarator;
+ first_non_attr_kind = cdk_attrs;
while (decl)
switch (decl->kind)
{
*************** grokdeclarator (const struct c_declarato
*** 4922,4927 ****
--- 4924,4931 ----
case cdk_pointer:
funcdef_syntax = (decl->kind == cdk_function);
decl = decl->declarator;
+ if (first_non_attr_kind == cdk_attrs)
+ first_non_attr_kind = decl->kind;
break;
case cdk_attrs:
*************** grokdeclarator (const struct c_declarato
*** 4932,4937 ****
--- 4936,4943 ----
loc = decl->id_loc;
if (decl->u.id)
name = decl->u.id;
+ if (first_non_attr_kind == cdk_attrs)
+ first_non_attr_kind = decl->kind;
decl = 0;
break;
*************** grokdeclarator (const struct c_declarato
*** 5038,5044 ****
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);
type_quals = ((constp ? TYPE_QUAL_CONST : 0)
| (restrictp ? TYPE_QUAL_RESTRICT : 0)
--- 5044,5052 ----
error_at (loc, "conflicting named address spaces (%s vs %s)",
c_addr_space_name (as1), c_addr_space_name (as2));
! if ((TREE_CODE (type) == ARRAY_TYPE
! || first_non_attr_kind == cdk_array)
! && TYPE_QUALS (element_type))
type = TYPE_MAIN_VARIANT (type);
type_quals = ((constp ? TYPE_QUAL_CONST : 0)
| (restrictp ? TYPE_QUAL_RESTRICT : 0)
Index: gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-1.c
===================================================================
*** gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-1.c (revision 0)
--- gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-1.c (revision 0)
***************
*** 0 ****
--- 1,8 ----
+ /* { dg-do compile } */
+ /* { dg-options "-save-temps -g -dA" } */
+
+ typedef struct _Harry { int dummy; } Harry_t;
+ Harry_t harry;
+
+ /* { dg-final { scan-assembler "DW_TAG_typedef\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_name: \"Harry_t\"" } } */
+ /* { dg-final { cleanup-saved-temps } } */
Index: gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-2.c
===================================================================
*** gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-2.c (revision 0)
--- gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-2.c (revision 0)
***************
*** 0 ****
--- 1,8 ----
+ /* { dg-do compile } */
+ /* { dg-options "-save-temps -g -dA" } */
+
+ typedef const struct _Harry { int dummy; } Harry_t;
+ Harry_t harry;
+
+ /* { dg-final { scan-assembler "DW_TAG_typedef\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_name: \"Harry_t\"" } } */
+ /* { dg-final { cleanup-saved-temps } } */
Index: gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-3.c
===================================================================
*** gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-3.c (revision 0)
--- gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-3.c (revision 0)
***************
*** 0 ****
--- 1,8 ----
+ /* { dg-do compile } */
+ /* { dg-options "-save-temps -g -dA" } */
+
+ typedef struct _Harry { int dummy; } Harry_t;
+ const Harry_t harry[5];
+
+ /* { dg-final { scan-assembler "DW_TAG_typedef\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_name: \"Harry_t\"" } } */
+ /* { dg-final { cleanup-saved-temps } } */
Index: gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-4.c
===================================================================
*** gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-4.c (revision 0)
--- gcc/testsuite/gcc.dg/debug/dwarf2/pr47939-4.c (revision 0)
***************
*** 0 ****
--- 1,8 ----
+ /* { dg-do compile } */
+ /* { dg-options "-save-temps -g -dA" } */
+
+ typedef const struct _Harry { int dummy; } Harry_t;
+ Harry_t harry[10];
+
+ /* { dg-final { scan-assembler "DW_TAG_typedef\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*DW_AT_name: \"Harry_t\"" } } */
+ /* { dg-final { cleanup-saved-temps } } */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][C] Fix PR47939
2011-03-18 10:13 ` Richard Guenther
@ 2011-03-18 18:39 ` Joseph S. Myers
0 siblings, 0 replies; 6+ messages in thread
From: Joseph S. Myers @ 2011-03-18 18:39 UTC (permalink / raw)
To: Richard Guenther; +Cc: gcc-patches
On Fri, 18 Mar 2011, Richard Guenther wrote:
> > What I think is safe in grokdeclarator is using TYPE_MAIN_VARIANT here if
> > *either* the type given in the declaration specifiers is an array type
> > (TREE_CODE (type) == ARRAY_TYPE, as in your previous patch) *or* the first
> > declarator that is not cdk_attrs is cdk_array (as in this version, but
> > checking through a chain of declarator->declarator to find a possible
> > cdk_array after a sequence of cdk_attrs).
> >
> > (Aside from all this it is a longstanding known bug that the debug
> > information for arrays of qualified types isn't quite right: PR 8354.)
>
> Ok, the following works for me.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk?
This patch version is OK.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-18 18:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-02 13:10 [PATCH][C] Fix PR47939 Richard Guenther
2011-03-10 17:41 ` Joseph S. Myers
2011-03-11 11:25 ` Richard Guenther
2011-03-11 13:08 ` Joseph S. Myers
2011-03-18 10:13 ` Richard Guenther
2011-03-18 18:39 ` Joseph S. Myers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).