Re: [PATCH] c++: Don't try to initialize zero width bitfields in zero initialization [PR109868]

[Jason Merrill <jason@redhat.com>]

On 5/16/23 15:34, Jakub Jelinek wrote:
> Hi!
> 
> My GCC 12 change to avoid removing zero-sized bitfields as they are
> important for ABI and are needed for layout compatibility traits
> apparently causes zero sized bitfields to be initialized in the IL,
> which at least in 13+ results in ICEs in the ranger which is upset
> about zero precision types.
> 
> I think we could even avoid initializing other unnamed bitfields, but
> unfortunately !CONSTRUCTOR_NO_CLEARING doesn't mean in the middle-end
> clearing of padding bits and until we have some new flag that represents
> the request to clear padding bits, I think it is better to keep zeroing
> non-zero sized unnamed bitfields.
> 
> In addition to skipping those fields, I have changed the logic how
> UNION_TYPEs are handled, the current code was a little bit weird in that
> e.g. if first non-static data member had error_mark_node type, we'd happily
> zero initialize the second non-static data member, etc.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/13,
> perhaps even 12?

OK back to 12, I think.

> 2023-05-16  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/109868
> 	* init.cc (build_zero_init_1): Don't initialize zero-width bitfields.
> 	For unions only initialize the first FIELD_DECL.
> 
> 	* g++.dg/init/pr109868.C: New test.
> 
> --- gcc/cp/init.cc.jj	2023-05-01 23:07:05.147417750 +0200
> +++ gcc/cp/init.cc	2023-05-16 10:01:14.512489727 +0200
> @@ -189,15 +189,21 @@ build_zero_init_1 (tree type, tree nelts
>       init = build_zero_cst (type);
>     else if (RECORD_OR_UNION_CODE_P (TREE_CODE (type)))
>       {
> -      tree field;
> +      tree field, next;
>         vec<constructor_elt, va_gc> *v = NULL;
>   
>         /* Iterate over the fields, building initializations.  */
> -      for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> +      for (field = TYPE_FIELDS (type); field; field = next)
>   	{
> +	  next = DECL_CHAIN (field);
> +
>   	  if (TREE_CODE (field) != FIELD_DECL)
>   	    continue;
>   
> +	  /* For unions, only the first field is initialized.  */
> +	  if (TREE_CODE (type) == UNION_TYPE)
> +	    next = NULL_TREE;
> +
>   	  if (TREE_TYPE (field) == error_mark_node)
>   	    continue;
>   
> @@ -212,6 +218,11 @@ build_zero_init_1 (tree type, tree nelts
>   		continue;
>   	    }
>   
> +	  /* Don't add zero width bitfields.  */
> +	  if (DECL_C_BIT_FIELD (field)
> +	      && integer_zerop (DECL_SIZE (field)))
> +	    continue;
> +
>   	  /* Note that for class types there will be FIELD_DECLs
>   	     corresponding to base classes as well.  Thus, iterating
>   	     over TYPE_FIELDs will result in correct initialization of
> @@ -230,10 +241,6 @@ build_zero_init_1 (tree type, tree nelts
>   	      if (value)
>   		CONSTRUCTOR_APPEND_ELT(v, field, value);
>   	    }
> -
> -	  /* For unions, only the first field is initialized.  */
> -	  if (TREE_CODE (type) == UNION_TYPE)
> -	    break;
>   	}
>   
>         /* Build a constructor to contain the initializations.  */
> --- gcc/testsuite/g++.dg/init/pr109868.C.jj	2023-05-16 09:43:54.706278293 +0200
> +++ gcc/testsuite/g++.dg/init/pr109868.C	2023-05-16 09:44:16.581966894 +0200
> @@ -0,0 +1,13 @@
> +// PR c++/109868
> +// { dg-do compile }
> +// { dg-options "-O2" }
> +
> +struct A { virtual void foo (); };
> +struct B { long b; int : 0; };
> +struct C : A { B c; };
> +
> +void
> +bar (C *p)
> +{
> +  *p = C ();
> +}
> 
> 	Jakub
>