PR c/43288 (ICE with __attribute__ ((common)))

PR c/43288 (ICE with __attribute__ ((common)))

[Jan Hubicka]

Hi,
we now get ICE on the following:
static int a __attribute__ ((common));
because of sanity check in function_and_variable_visibility.  Declaring the
code invalid in handle_common_attribute is not enough, since the static and
common can be merged together.

It would be probably feasible to test this both at at handle_common_attribute
and declaratio nmerging, this is just one of cases where frontend produce
nonsential visibility flag that we regularize in function_and_variable_visibility.

So instead of doing that I added code there. Problem is that varasm already
does this regularization at two places and turning those into ICEs.  These
regularizations happen too late, and invalid combinations are still getting
through the IPA passes.  However turnging these to ICE also shows that
assemble_alias might enforce this to happen early (i.e. before visibility
pass).  This is problem too: before visibility pass we should not produce
DECL_RTLs.  (visibility pass changes visibilities for i.e. -fwhole-program and
computing DECL_RTL early will result in wrong assembly).

The patch thus moves the DECL_RTL generation after visibility pass 

Bootstrapped/regsted at x86_64-linux and also at alphaev67-dec-osf5 (where patch
was reported to cause bootstrap ICE earlier).

OK for mainline? (with the testcase above)

Honza

	PR c/43288
	* ipa.c (function_and_variable_visibility) Normalize COMMON bits.
	* varasm.c (get_variable_section): Don't do that here...
	(make_decl_rtl): ... and here.
	(do_assemble_alias): Produce decl RTL.
	(assemble_alias): Likewise.
Index: ipa.c
===================================================================
*** ipa.c	(revision 155961)
--- ipa.c	(working copy)
*************** function_and_variable_visibility (bool w
*** 409,420 ****
  			   && !DECL_EXTERNAL (node->decl)
  			   && !node->local.externally_visible);
      }
    for (vnode = varpool_nodes_queue; vnode; vnode = vnode->next_needed)
      {
        if (!vnode->finalized)
          continue;
-       gcc_assert ((!DECL_WEAK (vnode->decl) && !DECL_COMMON (vnode->decl))
-       		  || TREE_PUBLIC (vnode->decl) || DECL_EXTERNAL (vnode->decl));
        if (vnode->needed
  	  && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
  	  && (!whole_program
--- 409,446 ----
  			   && !DECL_EXTERNAL (node->decl)
  			   && !node->local.externally_visible);
      }
+   for (vnode = varpool_nodes; vnode; vnode = vnode->next)
+     {
+       /* weak flag makes no sense on local variables.  */
+       gcc_assert (!DECL_WEAK (vnode->decl)
+       		  || TREE_PUBLIC (vnode->decl) || DECL_EXTERNAL (vnode->decl));
+       /* In several cases declarations can not be common:
+ 
+ 	 - when declaration has initializer
+ 	 - when it is in weak
+ 	 - when it has specific section
+ 	 - when it resides in non-generic address space.
+ 	 - if declaration is local, it will get into .local common section
+ 	   so common flag is not needed.  Frontends still produce these in
+ 	   certain cases, such as for:
+ 
+ 	     static int a __attribute__ ((common))
+ 
+ 	 Canonicalize things here and clear the redundant flag.  */
+       if (DECL_COMMON (vnode->decl)
+ 	  && (!(TREE_PUBLIC (vnode->decl) || DECL_EXTERNAL (vnode->decl))
+ 	      || (DECL_INITIAL (vnode->decl)
+ 		  && DECL_INITIAL (vnode->decl) != error_mark_node)
+ 	      || DECL_WEAK (vnode->decl)
+ 	      || DECL_SECTION_NAME (vnode->decl) != NULL
+ 	      || ! (ADDR_SPACE_GENERIC_P
+ 		    (TYPE_ADDR_SPACE (TREE_TYPE (vnode->decl))))))
+ 	DECL_COMMON (vnode->decl) = 0;
+     }
    for (vnode = varpool_nodes_queue; vnode; vnode = vnode->next_needed)
      {
        if (!vnode->finalized)
          continue;
        if (vnode->needed
  	  && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
  	  && (!whole_program
Index: varasm.c
===================================================================
*** varasm.c	(revision 155961)
--- varasm.c	(working copy)
*************** get_variable_section (tree decl, bool pr
*** 1174,1185 ****
    if (TREE_TYPE (decl) != error_mark_node)
      as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
  
!   /* If the decl has been given an explicit section name, or it resides
!      in a non-generic address space, then it isn't common, and shouldn't
!      be handled as such.  */
!   if (DECL_COMMON (decl) && DECL_SECTION_NAME (decl) == NULL
!       && ADDR_SPACE_GENERIC_P (as))
      {
        if (DECL_THREAD_LOCAL_P (decl))
  	return tls_comm_section;
        /* This cannot be common bss for an emulated TLS object without
--- 1174,1186 ----
    if (TREE_TYPE (decl) != error_mark_node)
      as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
  
!   if (DECL_COMMON (decl))
      {
+       /* If the decl has been given an explicit section name, or it resides
+ 	 in a non-generic address space, then it isn't common, and shouldn't
+ 	 be handled as such.  */
+       gcc_assert (DECL_SECTION_NAME (decl) == NULL
+ 		  && ADDR_SPACE_GENERIC_P (as));
        if (DECL_THREAD_LOCAL_P (decl))
  	return tls_comm_section;
        /* This cannot be common bss for an emulated TLS object without
*************** make_decl_rtl (tree decl)
*** 1434,1448 ****
  
    /* Specifying a section attribute on a variable forces it into a
       non-.bss section, and thus it cannot be common.  */
!   if (TREE_CODE (decl) == VAR_DECL
!       && DECL_SECTION_NAME (decl) != NULL_TREE
!       && DECL_INITIAL (decl) == NULL_TREE
!       && DECL_COMMON (decl))
!     DECL_COMMON (decl) = 0;
  
    /* Variables can't be both common and weak.  */
!   if (TREE_CODE (decl) == VAR_DECL && DECL_WEAK (decl))
!     DECL_COMMON (decl) = 0;
  
    if (use_object_blocks_p () && use_blocks_for_decl_p (decl))
      x = create_block_symbol (name, get_block_for_decl (decl), -1);
--- 1435,1450 ----
  
    /* Specifying a section attribute on a variable forces it into a
       non-.bss section, and thus it cannot be common.  */
!   gcc_assert (!(TREE_CODE (decl) == VAR_DECL
! 	      && DECL_SECTION_NAME (decl) != NULL_TREE
! 	      && DECL_INITIAL (decl) == NULL_TREE
! 	      && DECL_COMMON (decl))
! 	      || !DECL_COMMON (decl));
  
    /* Variables can't be both common and weak.  */
!   gcc_assert (TREE_CODE (decl) != VAR_DECL
! 	      || !DECL_WEAK (decl)
! 	      || !DECL_COMMON (decl));
  
    if (use_object_blocks_p () && use_blocks_for_decl_p (decl))
      x = create_block_symbol (name, get_block_for_decl (decl), -1);
*************** do_assemble_alias (tree decl, tree targe
*** 5446,5451 ****
--- 5448,5457 ----
    if (TREE_ASM_WRITTEN (decl))
      return;
  
+   /* We must force creation of DECL_RTL for debug info generation, even though
+      we don't use it here.  */
+   make_decl_rtl (decl);
+ 
    TREE_ASM_WRITTEN (decl) = 1;
    TREE_ASM_WRITTEN (DECL_ASSEMBLER_NAME (decl)) = 1;
  
*************** assemble_alias (tree decl, tree target)
*** 5663,5672 ****
  # endif
  #endif
      }
- 
-   /* We must force creation of DECL_RTL for debug info generation, even though
-      we don't use it here.  */
-   make_decl_rtl (decl);
    TREE_USED (decl) = 1;
  
    /* A quirk of the initial implementation of aliases required that the user
--- 5669,5674 ----

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Richard Guenther]

On Wed, Mar 10, 2010 at 4:25 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> we now get ICE on the following:
> static int a __attribute__ ((common));
> because of sanity check in function_and_variable_visibility.  Declaring the
> code invalid in handle_common_attribute is not enough, since the static and
> common can be merged together.
>
> It would be probably feasible to test this both at at handle_common_attribute
> and declaratio nmerging, this is just one of cases where frontend produce
> nonsential visibility flag that we regularize in function_and_variable_visibility.
>
> So instead of doing that I added code there. Problem is that varasm already
> does this regularization at two places and turning those into ICEs.  These
> regularizations happen too late, and invalid combinations are still getting
> through the IPA passes.  However turnging these to ICE also shows that
> assemble_alias might enforce this to happen early (i.e. before visibility
> pass).  This is problem too: before visibility pass we should not produce
> DECL_RTLs.  (visibility pass changes visibilities for i.e. -fwhole-program and
> computing DECL_RTL early will result in wrong assembly).
>
> The patch thus moves the DECL_RTL generation after visibility pass
>
> Bootstrapped/regsted at x86_64-linux and also at alphaev67-dec-osf5 (where patch
> was reported to cause bootstrap ICE earlier).
>
> OK for mainline? (with the testcase above)

Ok.

Thanks,
Richard.

> Honza
>
>        PR c/43288
>        * ipa.c (function_and_variable_visibility) Normalize COMMON bits.
>        * varasm.c (get_variable_section): Don't do that here...
>        (make_decl_rtl): ... and here.
>        (do_assemble_alias): Produce decl RTL.
>        (assemble_alias): Likewise.
> Index: ipa.c
> ===================================================================
> *** ipa.c       (revision 155961)
> --- ipa.c       (working copy)
> *************** function_and_variable_visibility (bool w
> *** 409,420 ****
>                           && !DECL_EXTERNAL (node->decl)
>                           && !node->local.externally_visible);
>      }
>    for (vnode = varpool_nodes_queue; vnode; vnode = vnode->next_needed)
>      {
>        if (!vnode->finalized)
>          continue;
> -       gcc_assert ((!DECL_WEAK (vnode->decl) && !DECL_COMMON (vnode->decl))
> -                         || TREE_PUBLIC (vnode->decl) || DECL_EXTERNAL (vnode->decl));
>        if (vnode->needed
>          && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
>          && (!whole_program
> --- 409,446 ----
>                           && !DECL_EXTERNAL (node->decl)
>                           && !node->local.externally_visible);
>      }
> +   for (vnode = varpool_nodes; vnode; vnode = vnode->next)
> +     {
> +       /* weak flag makes no sense on local variables.  */
> +       gcc_assert (!DECL_WEAK (vnode->decl)
> +                         || TREE_PUBLIC (vnode->decl) || DECL_EXTERNAL (vnode->decl));
> +       /* In several cases declarations can not be common:
> +
> +        - when declaration has initializer
> +        - when it is in weak
> +        - when it has specific section
> +        - when it resides in non-generic address space.
> +        - if declaration is local, it will get into .local common section
> +          so common flag is not needed.  Frontends still produce these in
> +          certain cases, such as for:
> +
> +            static int a __attribute__ ((common))
> +
> +        Canonicalize things here and clear the redundant flag.  */
> +       if (DECL_COMMON (vnode->decl)
> +         && (!(TREE_PUBLIC (vnode->decl) || DECL_EXTERNAL (vnode->decl))
> +             || (DECL_INITIAL (vnode->decl)
> +                 && DECL_INITIAL (vnode->decl) != error_mark_node)
> +             || DECL_WEAK (vnode->decl)
> +             || DECL_SECTION_NAME (vnode->decl) != NULL
> +             || ! (ADDR_SPACE_GENERIC_P
> +                   (TYPE_ADDR_SPACE (TREE_TYPE (vnode->decl))))))
> +       DECL_COMMON (vnode->decl) = 0;
> +     }
>    for (vnode = varpool_nodes_queue; vnode; vnode = vnode->next_needed)
>      {
>        if (!vnode->finalized)
>          continue;
>        if (vnode->needed
>          && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
>          && (!whole_program
> Index: varasm.c
> ===================================================================
> *** varasm.c    (revision 155961)
> --- varasm.c    (working copy)
> *************** get_variable_section (tree decl, bool pr
> *** 1174,1185 ****
>    if (TREE_TYPE (decl) != error_mark_node)
>      as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
>
> !   /* If the decl has been given an explicit section name, or it resides
> !      in a non-generic address space, then it isn't common, and shouldn't
> !      be handled as such.  */
> !   if (DECL_COMMON (decl) && DECL_SECTION_NAME (decl) == NULL
> !       && ADDR_SPACE_GENERIC_P (as))
>      {
>        if (DECL_THREAD_LOCAL_P (decl))
>        return tls_comm_section;
>        /* This cannot be common bss for an emulated TLS object without
> --- 1174,1186 ----
>    if (TREE_TYPE (decl) != error_mark_node)
>      as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
>
> !   if (DECL_COMMON (decl))
>      {
> +       /* If the decl has been given an explicit section name, or it resides
> +        in a non-generic address space, then it isn't common, and shouldn't
> +        be handled as such.  */
> +       gcc_assert (DECL_SECTION_NAME (decl) == NULL
> +                 && ADDR_SPACE_GENERIC_P (as));
>        if (DECL_THREAD_LOCAL_P (decl))
>        return tls_comm_section;
>        /* This cannot be common bss for an emulated TLS object without
> *************** make_decl_rtl (tree decl)
> *** 1434,1448 ****
>
>    /* Specifying a section attribute on a variable forces it into a
>       non-.bss section, and thus it cannot be common.  */
> !   if (TREE_CODE (decl) == VAR_DECL
> !       && DECL_SECTION_NAME (decl) != NULL_TREE
> !       && DECL_INITIAL (decl) == NULL_TREE
> !       && DECL_COMMON (decl))
> !     DECL_COMMON (decl) = 0;
>
>    /* Variables can't be both common and weak.  */
> !   if (TREE_CODE (decl) == VAR_DECL && DECL_WEAK (decl))
> !     DECL_COMMON (decl) = 0;
>
>    if (use_object_blocks_p () && use_blocks_for_decl_p (decl))
>      x = create_block_symbol (name, get_block_for_decl (decl), -1);
> --- 1435,1450 ----
>
>    /* Specifying a section attribute on a variable forces it into a
>       non-.bss section, and thus it cannot be common.  */
> !   gcc_assert (!(TREE_CODE (decl) == VAR_DECL
> !             && DECL_SECTION_NAME (decl) != NULL_TREE
> !             && DECL_INITIAL (decl) == NULL_TREE
> !             && DECL_COMMON (decl))
> !             || !DECL_COMMON (decl));
>
>    /* Variables can't be both common and weak.  */
> !   gcc_assert (TREE_CODE (decl) != VAR_DECL
> !             || !DECL_WEAK (decl)
> !             || !DECL_COMMON (decl));
>
>    if (use_object_blocks_p () && use_blocks_for_decl_p (decl))
>      x = create_block_symbol (name, get_block_for_decl (decl), -1);
> *************** do_assemble_alias (tree decl, tree targe
> *** 5446,5451 ****
> --- 5448,5457 ----
>    if (TREE_ASM_WRITTEN (decl))
>      return;
>
> +   /* We must force creation of DECL_RTL for debug info generation, even though
> +      we don't use it here.  */
> +   make_decl_rtl (decl);
> +
>    TREE_ASM_WRITTEN (decl) = 1;
>    TREE_ASM_WRITTEN (DECL_ASSEMBLER_NAME (decl)) = 1;
>
> *************** assemble_alias (tree decl, tree target)
> *** 5663,5672 ****
>  # endif
>  #endif
>      }
> -
> -   /* We must force creation of DECL_RTL for debug info generation, even though
> -      we don't use it here.  */
> -   make_decl_rtl (decl);
>    TREE_USED (decl) = 1;
>
>    /* A quirk of the initial implementation of aliases required that the user
> --- 5669,5674 ----
>

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Steve Ellcey]

>        PR c/43288
>        * ipa.c (function_and_variable_visibility) Normalize COMMON bits.
>        * varasm.c (get_variable_section): Don't do that here...
>        (make_decl_rtl): ... and here.
>        (do_assemble_alias): Produce decl RTL.
>        (assemble_alias): Likewise.

This patch (r157366) is causing my HPPA bootstrap to fail during the
libstdc++ build.

With this cutdown test case:

namespace std {
  class type_info { virtual ~type_info(); };
}
namespace __cxxabiv1 {
  class __fundamental_type_info : public std::type_info {};
}


I get:

a.C:7:1: internal compiler error: in make_decl_rtl, at varasm.c:1447
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.


I think I have a var decl that is weak and common because that is
how HPPA is doing comdats in SOM.   Dave can correct me if I have
that wrong.

Steve Ellcey
sje@cup.hp.com

Re: PR c/43288 (ICE with __attribute__ ((common)))

[John David Anglin]

> I think I have a var decl that is weak and common because that is
> how HPPA is doing comdats in SOM.   Dave can correct me if I have
> that wrong.

See define for MAKE_DECL_ONE_ONLY in som.h.  We don't set both
DECL_COMMON and DECL_WEAK.  Some other code must be setting
DECL_COMMON or DECL_WEAK to cause the error (or maybe MAKE_DECL_ONE_ONLY
is used twice, once before DECL_INITIAL is set).

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Steve Ellcey]

On Thu, 2010-03-11 at 16:54 -0500, John David Anglin wrote:
> > I think I have a var decl that is weak and common because that is
> > how HPPA is doing comdats in SOM.   Dave can correct me if I have
> > that wrong.
> 
> See define for MAKE_DECL_ONE_ONLY in som.h.  We don't set both
> DECL_COMMON and DECL_WEAK.  Some other code must be setting
> DECL_COMMON or DECL_WEAK to cause the error (or maybe MAKE_DECL_ONE_ONLY
> is used twice, once before DECL_INITIAL is set).
> 
> Dave

Yes, MAKE_DECL_ONE_ONLY is called twice because make_decl_one_only is
called twice.  The first call seems to happen before we have an
initializer and the second one after.  We could workaround this by
setting DECL_COMMON to 0 when setting DECL_WEAK to 1 (and visa versa) in
MAKE_DECL_ONE_ONLY but I think that is just a workaround for the real
problem.  The problem seems to only happen when using the
"__fundamental_type_info" class.  If I change the name of this class to
something generic like "foo" then I don't get the bug.

Steve Ellcey
sje@cup.hp.com


$ cat a.C                       
class t { virtual ~t(); };
namespace  __cxxabiv1 {
class __fundamental_type_info : public t {};
}

$ obj_gcc/gcc/cc1plus -quiet a.C
a.C:4:1: internal compiler error: in make_decl_rtl, at varasm.c:1447
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

$ cat b.C                       
class t { virtual ~t(); };
namespace  __cxxabiv1 {
class foo : public t {};
}

$ obj_gcc/gcc/cc1plus -quiet b.C

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Jan Hubicka]

> 
> I think I have a var decl that is weak and common because that is
> how HPPA is doing comdats in SOM.   Dave can correct me if I have
> that wrong.

It seems to me that it is most probably because of C++ FE insisting on
ononsential DECL_COMMON (in comdat_linkage) and the fact that we somehow call
make_decl_rtl too early.

I just built cross compiler for both AIX and HPUX but can not reproduce the
problem.  Was is fixed somehow in meantime?

Honza

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Steve Ellcey]

On Wed, 2010-03-17 at 16:19 +0100, Jan Hubicka wrote:
> > 
> > I think I have a var decl that is weak and common because that is
> > how HPPA is doing comdats in SOM.   Dave can correct me if I have
> > that wrong.
> 
> It seems to me that it is most probably because of C++ FE insisting on
> ononsential DECL_COMMON (in comdat_linkage) and the fact that we somehow call
> make_decl_rtl too early.
> 
> I just built cross compiler for both AIX and HPUX but can not reproduce the
> problem.  Was is fixed somehow in meantime?
> 
> Honza

It failed for me last night (r157506) so I don't think this has been
fixed.  Which HPUX target did you build?  It only fails on a 32 bit PA
target like hppa1.1-hp-hpux* or hppa2.0w-hp-hpux* (I am building
hppa2.0w-hp-hpux11.11).  I don't know if it fails on HPPA linux targets
(I don't build any of those) and I know it doesn't fail on IA64 HP-UX
targets or 64 bit PA targets.

Steve Ellcey
sje@cup.hp.com

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Jan Hubicka]

> On Wed, 2010-03-17 at 16:19 +0100, Jan Hubicka wrote:
> > > 
> > > I think I have a var decl that is weak and common because that is
> > > how HPPA is doing comdats in SOM.   Dave can correct me if I have
> > > that wrong.
> > 
> > It seems to me that it is most probably because of C++ FE insisting on
> > ononsential DECL_COMMON (in comdat_linkage) and the fact that we somehow call
> > make_decl_rtl too early.
> > 
> > I just built cross compiler for both AIX and HPUX but can not reproduce the
> > problem.  Was is fixed somehow in meantime?
> > 
> > Honza
> 
> It failed for me last night (r157506) so I don't think this has been
> fixed.  Which HPUX target did you build?  It only fails on a 32 bit PA
> target like hppa1.1-hp-hpux* or hppa2.0w-hp-hpux* (I am building
> hppa2.0w-hp-hpux11.11).  I don't know if it fails on HPPA linux targets

Hmm, I rebuilt for this tripplet, but still no luck:

jh@gcc14:~/trunk/build-hppa2/gcc$ cat a.C
class t { virtual ~t(); };
namespace  __cxxabiv1 {
class __fundamental_type_info : public t {};
}

jh@gcc14:~/trunk/build-hppa2/gcc$ ./cc1plus a.C -quiet
jh@gcc14:~/trunk/build-hppa2/gcc$

There seems to be no changes today that would fix this.  If the testcase seems
fine, could you try to get a backtrace for me?  I think we might only need to
avoid make_decl_rtl from being called too early.

Honza
> (I don't build any of those) and I know it doesn't fail on IA64 HP-UX
> targets or 64 bit PA targets.
> 
> Steve Ellcey
> sje@cup.hp.com

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Steve Ellcey]

On Wed, 2010-03-17 at 16:35 +0100, Jan Hubicka wrote:

> Hmm, I rebuilt for this tripplet, but still no luck:
> 
> jh@gcc14:~/trunk/build-hppa2/gcc$ cat a.C
> class t { virtual ~t(); };
> namespace  __cxxabiv1 {
> class __fundamental_type_info : public t {};
> }
> 
> jh@gcc14:~/trunk/build-hppa2/gcc$ ./cc1plus a.C -quiet
> jh@gcc14:~/trunk/build-hppa2/gcc$
> 
> There seems to be no changes today that would fix this.  If the testcase seems
> fine, could you try to get a backtrace for me?  I think we might only need to
> avoid make_decl_rtl from being called too early.
> 
> Honza

The test looks fine.  It still fails for me on ToT.  I don't think the
backtrace to the abort helps much, it is coming from
cp_write_global_declarations() which is iterating over
unemitted_tinfo_decls and I think we need to understand more about when
and how the decls are put on this list.

Steve Ellcey
sje@cup.hp.com

#0  fancy_abort ()
at /proj/opensrc/sje/reg/src/trunk/gcc/diagnostic.c:763
#1  0x878858 in make_decl_rtl ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/varasm.c:1445
#2  0x879540 in notice_global_symbol ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/varasm.c:1621
#3  0x8f2120 in varpool_enqueue_needed_node ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/varpool.c:202
#4  0x8f2748 in varpool_finalize_decl ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/varpool.c:307
#5  0x709790 in rest_of_decl_compilation ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/passes.c:195
#6  0x921b0 in make_rtl_for_nonlocal_decl ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/cp/decl.c:5367
#7  0x96450 in cp_finish_decl ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/cp/decl.c:5904
#8  0x23cf90 in emit_tinfo_decl ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/cp/rtti.c:1555
#9  0x1d8a54 in cp_write_global_declarations ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/cp/decl2.c:3581
#10 0x766640 in compile_file ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/toplev.c:1065
#11 0x769e44 in do_compile ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/toplev.c:2409
#12 0x769fc4 in toplev_main ()
    at /proj/opensrc/sje/reg/src/trunk/gcc/toplev.c:2451
#13 0x3f9c5c in main () at /proj/opensrc/sje/reg/src/trunk/gcc/main.c:35

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Jan Hubicka]

> On Wed, 2010-03-17 at 16:35 +0100, Jan Hubicka wrote:
> 
> > Hmm, I rebuilt for this tripplet, but still no luck:
> > 
> > jh@gcc14:~/trunk/build-hppa2/gcc$ cat a.C
> > class t { virtual ~t(); };
> > namespace  __cxxabiv1 {
> > class __fundamental_type_info : public t {};
> > }
> > 
> > jh@gcc14:~/trunk/build-hppa2/gcc$ ./cc1plus a.C -quiet
> > jh@gcc14:~/trunk/build-hppa2/gcc$
> > 
> > There seems to be no changes today that would fix this.  If the testcase seems
> > fine, could you try to get a backtrace for me?  I think we might only need to
> > avoid make_decl_rtl from being called too early.
> > 
> > Honza
> 

OK, so the problem is that I need to edit auto-host.h and enable gas weakref.

> #0  fancy_abort ()
> at /proj/opensrc/sje/reg/src/trunk/gcc/diagnostic.c:763
> #1  0x878858 in make_decl_rtl ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/varasm.c:1445
> #2  0x879540 in notice_global_symbol ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/varasm.c:1621
> #3  0x8f2120 in varpool_enqueue_needed_node ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/varpool.c:202

And this is the problem.   We compute decl rtl early because of noticing global
symbol.  It is irritating things to do.  (there was already one patch for it to
reset the DECL_RTL after visibility).  I have to leave now but tonight I will
check if I can do something about this (i.e. avoid making decl rtl in noticing
global symbol) or if we want to return the COMMON mangling code into
make_decl_rtl. (that would bring situation back to how stuff worked before my patch
and there is no need to update other parts)

It is bit sick though - C++ FE is putting DECL_COMMON flag because it needs to keep it
set internally and then sometimes make_decl_rtl gets called early and clears it under
FE's hand.  I guess it all works just by accident (or lack of failing testcase)

Honza

> #4  0x8f2748 in varpool_finalize_decl ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/varpool.c:307
> #5  0x709790 in rest_of_decl_compilation ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/passes.c:195
> #6  0x921b0 in make_rtl_for_nonlocal_decl ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/cp/decl.c:5367
> #7  0x96450 in cp_finish_decl ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/cp/decl.c:5904
> #8  0x23cf90 in emit_tinfo_decl ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/cp/rtti.c:1555
> #9  0x1d8a54 in cp_write_global_declarations ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/cp/decl2.c:3581
> #10 0x766640 in compile_file ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/toplev.c:1065
> #11 0x769e44 in do_compile ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/toplev.c:2409
> #12 0x769fc4 in toplev_main ()
>     at /proj/opensrc/sje/reg/src/trunk/gcc/toplev.c:2451
> #13 0x3f9c5c in main () at /proj/opensrc/sje/reg/src/trunk/gcc/main.c:35

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Jan Hubicka]

> On Wed, 2010-03-17 at 16:35 +0100, Jan Hubicka wrote:
> 
> > Hmm, I rebuilt for this tripplet, but still no luck:
> > 
> > jh@gcc14:~/trunk/build-hppa2/gcc$ cat a.C
> > class t { virtual ~t(); };
> > namespace  __cxxabiv1 {
> > class __fundamental_type_info : public t {};
> > }
> > 
> > jh@gcc14:~/trunk/build-hppa2/gcc$ ./cc1plus a.C -quiet
> > jh@gcc14:~/trunk/build-hppa2/gcc$
> > 
> > There seems to be no changes today that would fix this.  If the testcase seems
> > fine, could you try to get a backtrace for me?  I think we might only need to
> > avoid make_decl_rtl from being called too early.
> > 
> > Honza
> 

Hi,
looking at the corelation about notice_global_symbol and make_decl_rtl,
I don't think I can easilly avoid the scenario.  In longer term we probably
should make this kind of mangling late in cgraph code but until then it seems
best to simply make make_decl_rtl behave (semi) sanily here as it used to.

Does the attached patch fix the bootstrap problem? (it does fix the testcase)

Honza

Index: varasm.c
===================================================================
--- varasm.c	(revision 157518)
+++ varasm.c	(working copy)
@@ -1435,16 +1435,19 @@ make_decl_rtl (tree decl)
 
   /* Specifying a section attribute on a variable forces it into a
      non-.bss section, and thus it cannot be common.  */
-  gcc_assert (!(TREE_CODE (decl) == VAR_DECL
-	      && DECL_SECTION_NAME (decl) != NULL_TREE
-	      && DECL_INITIAL (decl) == NULL_TREE
-	      && DECL_COMMON (decl))
-	      || !DECL_COMMON (decl));
+  /* FIXME: In general this code should not be necessary because
+     visibility pass is doing the same work.  But notice_global_symbol
+     is called early and it needs to make DECL_RTL to get the name.
+     we take care of recomputing the DECL_RTL after visibility is changed.  */
+  if (TREE_CODE (decl) == VAR_DECL
+      && DECL_SECTION_NAME (decl) != NULL_TREE
+      && DECL_INITIAL (decl) == NULL_TREE
+      && DECL_COMMON (decl))
+    DECL_COMMON (decl) = 0;
 
   /* Variables can't be both common and weak.  */
-  gcc_assert (TREE_CODE (decl) != VAR_DECL
-	      || !DECL_WEAK (decl)
-	      || !DECL_COMMON (decl));
+  if (TREE_CODE (decl) == VAR_DECL && DECL_WEAK (decl))
+    DECL_COMMON (decl) = 0;
 
   if (use_object_blocks_p () && use_blocks_for_decl_p (decl))
     x = create_block_symbol (name, get_block_for_decl (decl), -1);

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Steve Ellcey]

On Thu, 2010-03-18 at 00:27 +0100, Jan Hubicka wrote:

> Hi,
> looking at the corelation about notice_global_symbol and make_decl_rtl,
> I don't think I can easilly avoid the scenario.  In longer term we probably
> should make this kind of mangling late in cgraph code but until then it seems
> best to simply make make_decl_rtl behave (semi) sanily here as it used to.
> 
> Does the attached patch fix the bootstrap problem? (it does fix the testcase)
> 
> Honza

Your patch allowed me to bootstrap on HPPA.

Steve Ellcey
sje@cup.hp.com

Re: PR c/43288 (ICE with __attribute__ ((common)))

[John David Anglin]

> On Thu, 2010-03-18 at 00:27 +0100, Jan Hubicka wrote:
> 
> > Hi,
> > looking at the corelation about notice_global_symbol and make_decl_rtl,
> > I don't think I can easilly avoid the scenario.  In longer term we probably
> > should make this kind of mangling late in cgraph code but until then it seems
> > best to simply make make_decl_rtl behave (semi) sanily here as it used to.
> > 
> > Does the attached patch fix the bootstrap problem? (it does fix the testcase)
> > 
> > Honza
> 
> Your patch allowed me to bootstrap on HPPA.

Same here.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Richard Guenther]

On Thu, Mar 18, 2010 at 12:27 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Wed, 2010-03-17 at 16:35 +0100, Jan Hubicka wrote:
>>
>> > Hmm, I rebuilt for this tripplet, but still no luck:
>> >
>> > jh@gcc14:~/trunk/build-hppa2/gcc$ cat a.C
>> > class t { virtual ~t(); };
>> > namespace  __cxxabiv1 {
>> > class __fundamental_type_info : public t {};
>> > }
>> >
>> > jh@gcc14:~/trunk/build-hppa2/gcc$ ./cc1plus a.C -quiet
>> > jh@gcc14:~/trunk/build-hppa2/gcc$
>> >
>> > There seems to be no changes today that would fix this.  If the testcase seems
>> > fine, could you try to get a backtrace for me?  I think we might only need to
>> > avoid make_decl_rtl from being called too early.
>> >
>> > Honza
>>
>
> Hi,
> looking at the corelation about notice_global_symbol and make_decl_rtl,
> I don't think I can easilly avoid the scenario.  In longer term we probably
> should make this kind of mangling late in cgraph code but until then it seems
> best to simply make make_decl_rtl behave (semi) sanily here as it used to.
>
> Does the attached patch fix the bootstrap problem? (it does fix the testcase)

The patch is ok if it passes bootstrap & regtest, given it restores bootstrap
for the affected folks.

Thanks,
Richard.

> Honza
>
> Index: varasm.c
> ===================================================================
> --- varasm.c    (revision 157518)
> +++ varasm.c    (working copy)
> @@ -1435,16 +1435,19 @@ make_decl_rtl (tree decl)
>
>   /* Specifying a section attribute on a variable forces it into a
>      non-.bss section, and thus it cannot be common.  */
> -  gcc_assert (!(TREE_CODE (decl) == VAR_DECL
> -             && DECL_SECTION_NAME (decl) != NULL_TREE
> -             && DECL_INITIAL (decl) == NULL_TREE
> -             && DECL_COMMON (decl))
> -             || !DECL_COMMON (decl));
> +  /* FIXME: In general this code should not be necessary because
> +     visibility pass is doing the same work.  But notice_global_symbol
> +     is called early and it needs to make DECL_RTL to get the name.
> +     we take care of recomputing the DECL_RTL after visibility is changed.  */
> +  if (TREE_CODE (decl) == VAR_DECL
> +      && DECL_SECTION_NAME (decl) != NULL_TREE
> +      && DECL_INITIAL (decl) == NULL_TREE
> +      && DECL_COMMON (decl))
> +    DECL_COMMON (decl) = 0;
>
>   /* Variables can't be both common and weak.  */
> -  gcc_assert (TREE_CODE (decl) != VAR_DECL
> -             || !DECL_WEAK (decl)
> -             || !DECL_COMMON (decl));
> +  if (TREE_CODE (decl) == VAR_DECL && DECL_WEAK (decl))
> +    DECL_COMMON (decl) = 0;
>
>   if (use_object_blocks_p () && use_blocks_for_decl_p (decl))
>     x = create_block_symbol (name, get_block_for_decl (decl), -1);
>

Re: PR c/43288 (ICE with __attribute__ ((common)))

[Jan Hubicka]

> The patch is ok if it passes bootstrap & regtest, given it restores bootstrap
> for the affected folks.

Thanks, I've bootstrapped&regtested it on x86_64-linux and comitted.

Honza
> 
> Thanks,
> Richard.

Re: PR c/43288 (ICE with __attribute__ ((common)))

[David Edelsohn]

On Fri, Mar 12, 2010 at 9:37 AM, Richard Guenther <rguenther@suse.de> wrote:
> On Fri, 12 Mar 2010, David Edelsohn wrote:
>
>> This patch appears to break building libstdc++ on AIX:
>>
>> /farm/dje/src/src/libstdc++-v3/src/iostream-inst.cc:46:1: internal
>> compiler error: in make_decl_rtl, at varasm.c:1447
>> In file included from
>> /tmp/20100311/powerpc-ibm-aix5.3.0.0/libstdc++-v3/include/locale:43:0,
>>                  from /farm/dje/src/src/libstdc++-v3/src/locale-inst.cc:30:
>> /tmp/20100311/powerpc-ibm-aix5.3.0.0/libstdc++-v3/include/bits/locale_facets_nonio.h:
>> In instantiation of 'const bool std::moneypunct<char, false>::intl':
>> /farm/dje/src/src/libstdc++-v3/src/locale-inst.cc:41:18:
>> instantiated from here
>> /tmp/20100311/powerpc-ibm-aix5.3.0.0/libstdc++-v3/include/bits/locale_facets_nonio.h:946:34:
>> internal compiler error: in make_decl_rtl, at varasm.c:1447
>>
>> /farm/dje/src/src/libstdc++-v3/src/fstream-inst.cc:46:1: internal
>> compiler error: in make_decl_rtl, at varasm.c:1447
>>
>> etc., etc.
>
> Similar failure was reported by Steve for HPPA.  His testcase:
>
> namespace std {
>  class type_info { virtual ~type_info(); };
> }
> namespace __cxxabiv1 {
>  class __fundamental_type_info : public std::type_info {};
> }
>
> can you check if that ICEs on AIX as well?

Yes, that testcase ICEs on AIX:

internal compiler error: in make_decl_rtl, at varasm.c:1447

David