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