Re: [PATCH] Ensure x86_64 TYPE_EMPTY_P args have distinct addresses (PR c++/92384)

[Richard Biener <rguenther at suse dot de>]

On Thu, 7 Nov 2019, Jakub Jelinek wrote:

> Hi!
> 
> TYPE_EMPTY_P arguments (which right now only x86_64 uses) have
> data->entry_parm == data->stack_parm being a stack slot with zero size in
> the stack parameter passing area.
> The problem with that is that in C++ they should have distinct addresses
> from other objects, which is not the case right now, as their address is
> equal to whatever argument is after it on the stack.
> 
> The following patch in the third hunk, if the TYPE_EMPTY_P argument has address
> taken forces stack_parm to be NULL, so that a new slot will be created for
> it, and the other two hunks just make sure we don't actually copy anything,
> as the TYPE_EMPTY_P types contain solely padding and so it is ok if they
> are just allocated on the stack, but are not copied.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2019-11-07  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/92384
> 	* function.c (assign_parm_setup_block, assign_parm_setup_stack): Don't
> 	copy TYPE_EMPTY_P arguments from data->entry_parm to data->stack_parm
> 	slot.
> 	(assign_parms): For TREE_ADDRESSABLE parms with TYPE_EMPTY_P type
> 	force creation of a unique data.stack_parm slot.
> 
> 	* g++.dg/torture/pr92384.C: New test.
> 
> --- gcc/function.c.jj	2019-10-01 18:16:13.205128183 +0200
> +++ gcc/function.c	2019-11-06 09:30:47.097353832 +0100
> @@ -3087,7 +3087,7 @@ assign_parm_setup_block (struct assign_p
>  	move_block_from_reg (REGNO (entry_parm), mem,
>  			     size_stored / UNITS_PER_WORD);
>      }
> -  else if (data->stack_parm == 0)
> +  else if (data->stack_parm == 0 && !TYPE_EMPTY_P (data->arg.type))
>      {
>        push_to_sequence2 (all->first_conversion_insn, all->last_conversion_insn);
>        emit_block_move (stack_parm, data->entry_parm, GEN_INT (size),
> @@ -3488,7 +3488,9 @@ assign_parm_setup_stack (struct assign_p
>        dest = validize_mem (copy_rtx (data->stack_parm));
>        src = validize_mem (copy_rtx (data->entry_parm));
>  
> -      if (MEM_P (src))
> +      if (TYPE_EMPTY_P (data->arg.type))
> +	/* Empty types don't really need to be copied.  */;
> +      else if (MEM_P (src))
>  	{
>  	  /* Use a block move to handle potentially misaligned entry_parm.  */
>  	  if (!to_conversion)
> @@ -3643,6 +3645,16 @@ assign_parms (tree fndecl)
>  	{
>  	  assign_parm_find_stack_rtl (parm, &data);
>  	  assign_parm_adjust_entry_rtl (&data);
> +	  /* For arguments that occupy no space in the parameter
> +	     passing area, have non-zero size and have address taken,
> +	     force creation of a stack slot so that they have distinct
> +	     address from other parameters.  */
> +	  if (TYPE_EMPTY_P (data.arg.type)
> +	      && TREE_ADDRESSABLE (parm)
> +	      && data.entry_parm == data.stack_parm
> +	      && MEM_P (data.entry_parm)
> +	      && int_size_in_bytes (data.arg.type))
> +	    data.stack_parm = NULL_RTX;
>  	}
>        /* Record permanently how this parm was passed.  */
>        if (data.arg.pass_by_reference)
> --- gcc/testsuite/g++.dg/torture/pr92384.C.jj	2019-11-06 10:01:39.794413342 +0100
> +++ gcc/testsuite/g++.dg/torture/pr92384.C	2019-11-06 10:02:44.281441632 +0100
> @@ -0,0 +1,38 @@
> +// PR c++/92384
> +// { dg-do run }
> +
> +struct S {};
> +struct T : public S { S a, b, c, d, e, f, g, h, i, j, k, l, m; };
> +struct U { long long a, b, c; };
> +
> +U
> +foo (S, S, S, T, T, T, U g)
> +{
> +  return g;
> +}
> +
> +__attribute__((noipa)) bool
> +bar (S a, S b, S c, T d, T e, T f, U g, void **h)
> +{
> +  h[0] = (void *) &a;
> +  h[1] = (void *) &b;
> +  h[2] = (void *) &c;
> +  h[3] = (void *) &d;
> +  h[4] = (void *) &e;
> +  h[5] = (void *) &f;
> +  h[6] = (void *) &g;
> +  asm volatile ("" : : "r" (h) : "memory");
> +  return (h[0] != h[1] && h[1] != h[2] && h[2] != h[3]
> +	  && h[3] != h[4] && h[4] != h[5] && h[5] != h[6]);
> +}
> +
> +int
> +main ()
> +{
> +  S a;
> +  T b;
> +  U c = { 1, 2, 3 };
> +  void *d[7];
> +  if (!bar (a, a, a, b, b, b, c, d))
> +    __builtin_abort ();
> +}
> 
> 	Jakub
> 
> 

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