[PATCH] varasm: Fix output_constructor where a RANGE_EXPR index needs to skip some elts [PR94303]

[Richard Biener]

On Wed, 25 Mar 2020, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase is miscompiled, because output_constructor doesn't
> output the initializer correctly.  The FE creates {[1...2] = 9} in this
> case, and we emit .long 9; long 9; .zero 8 instead of the expected
> .zero 8; .long 9; .long 9.  If the CONSTRUCTOR is {[1] = 9, [2] = 9},
> output_constructor_regular_field has code to notice that the current
> location (local->total_bytes) is smaller than the location we want to write
> to (1*sizeof(elt)) and will call assemble_zeros to skip those.  But
> RANGE_EXPRs are handled by a different function which didn't do this,
> so for RANGE_EXPRs we emitted them properly only if local->total_bytes
> was always equal to the location where the RANGE_EXPR needs to start.

Ouch.

> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

OK.

Thanks,
Richard.

> 2020-03-25  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/94303
> 	* varasm.c (output_constructor_array_range): If local->index
> 	RANGE_EXPR doesn't start at the current location in the constructor,
> 	skip needed number of bytes using assemble_zeros or assert we don't
> 	go backwards.
> 
> 	PR middle-end/94303
> 	* g++.dg/torture/pr94303.C: New test.
> 
> --- gcc/varasm.c.jj	2020-01-22 10:19:24.000000000 +0100
> +++ gcc/varasm.c	2020-03-24 18:03:08.532690584 +0100
> @@ -5152,6 +5152,26 @@ struct oc_local_state {
>  static void
>  output_constructor_array_range (oc_local_state *local)
>  {
> +  /* Perform the index calculation in modulo arithmetic but
> +     sign-extend the result because Ada has negative DECL_FIELD_OFFSETs
> +     but we are using an unsigned sizetype.  */
> +  unsigned prec = TYPE_PRECISION (sizetype);
> +  offset_int idx = wi::sext (wi::to_offset (TREE_OPERAND (local->index, 0))
> +			     - wi::to_offset (local->min_index), prec);
> +  tree valtype = TREE_TYPE (local->val);
> +  HOST_WIDE_INT fieldpos
> +    = (idx * wi::to_offset (TYPE_SIZE_UNIT (valtype))).to_short_addr ();
> +
> +  /* Advance to offset of this element.  */
> +  if (fieldpos > local->total_bytes)
> +    {
> +      assemble_zeros (fieldpos - local->total_bytes);
> +      local->total_bytes = fieldpos;
> +    }
> +  else
> +    /* Must not go backwards.  */
> +    gcc_assert (fieldpos == local->total_bytes);
> +
>    unsigned HOST_WIDE_INT fieldsize
>      = int_size_in_bytes (TREE_TYPE (local->type));
>  
> --- gcc/testsuite/g++.dg/torture/pr94303.C.jj	2020-03-24 18:14:21.114688840 +0100
> +++ gcc/testsuite/g++.dg/torture/pr94303.C	2020-03-24 18:13:56.434055853 +0100
> @@ -0,0 +1,17 @@
> +// PR middle-end/94303
> +// { dg-do run }
> +
> +struct A {
> +  int d = 9;
> +  A () = default;
> +  A (int x) : d(x) {}
> +  void foo () { if (d < 1) __builtin_abort (); }
> +};
> +
> +A a[3] = { 1 };
> +
> +int
> +main ()
> +{
> +  a[2].foo ();
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)