PATCH: c++/9297

[Mark Mitchell <mark at codesourcery dot com>]

PR 9297 is an x86 code-generation regression.  (Although the same bug
probably shows up on other architectures in some cases.)  

The problem is that we are initializing a stack-passed function
argument, and, while doing that, go and call memcpy, thereby
clobbering the argument.

Here's the test case:

  struct C {
    C(char const * s) {
      for (int i=0; i<sizeof(p); ++i) p[i]=s[i];
    }

    char p[64];
  };

  inline C make_c(C y) { return y; }

  C data("ABCDEF");

  int main(int argc, char * argv[])
  {
    C d2( make_c(data) );
  }

For the non-C++ people, here's what that last line does, in C-ish:

  C d2;

  d2 = make_c ((t = data, t));

where t is some invented temporary whose scope is just the expression
above.  The structure assignments are just bitwise copy, as in C.

What happens is that while evaluating the call to make_c, we decide to
put the temporary t at %esp + 4.  Then, we call expand_expr with 
%esp + 4 as the target, to initialize the argument.  That eventually
decides to do a block move into %esp + 4 from t, and that decides to use 
memcpy, which loses.

Here is the generated code with -O0:

	leal	-88(%ebp), %edi ; edi = &d2
	leal	4(%esp), %ecx   ; save *(esp + 4)
	movl	$data, %edx
	movl	$64, %eax
	movl	8(%esp), %esi   ; save *(esp + 8)
	movl	%eax, 8(%esp)
	movl	4(%esp), %ebx
	movl	%edx, 4(%esp)
	movl	%ecx, (%esp)
	call	memcpy		; memcpy (esp + 4, &data, 64)
	movl	%esi, 8(%esp)   ; restore *(esp + 8)
	movl	%ebx, 4(%esp)   ; restore *(esp + 4)
	movl	%edi, (%esp)
	call	_Z6make_c1C	; make_c (&d2, ???)

The problem is that the restores of the stack area clobber the result
of the memcpy, which was the whole point.  Doh!

I fixed this with the attached patch, which I'll check in if nobody
objects.  It looks like there's code in calls.c that thinks it
understands this case already, but it doesn't seem to work.  

Thoughts?

--
Mark Mitchell                   mark at codesourcery dot com
CodeSourcery, LLC               http://www.codesourcery.com

Index: calls.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/calls.c,v
retrieving revision 1.257
diff -c -5 -p -r1.257 calls.c
*** calls.c	19 Feb 2003 18:03:07 -0000	1.257
--- calls.c	21 Feb 2003 20:04:35 -0000
*************** store_one_arg (arg, argblock, flags, var
*** 4386,4401 ****
  
  	 If this argument is initialized by a function which takes the
  	 address of the argument (a C++ constructor or a C function
  	 returning a BLKmode structure), then stack_usage_map is
  	 insufficient and expand_call must push the stack around the
! 	 function call.  Such arguments have pass_on_stack == 1.
  
  	 Note that it is always safe to set stack_arg_under_construction,
  	 but this generates suboptimal code if set when not needed.  */
  
!       if (arg->pass_on_stack)
  	stack_arg_under_construction++;
  
        arg->value = expand_expr (pval,
  				(partial
  				 || TYPE_MODE (TREE_TYPE (pval)) != arg->mode)
--- 4386,4401 ----
  
  	 If this argument is initialized by a function which takes the
  	 address of the argument (a C++ constructor or a C function
  	 returning a BLKmode structure), then stack_usage_map is
  	 insufficient and expand_call must push the stack around the
! 	 function call.  Such arguments have arg->stack set.
  
  	 Note that it is always safe to set stack_arg_under_construction,
  	 but this generates suboptimal code if set when not needed.  */
  
!       if (arg->stack)
  	stack_arg_under_construction++;
  
        arg->value = expand_expr (pval,
  				(partial
  				 || TYPE_MODE (TREE_TYPE (pval)) != arg->mode)
*************** store_one_arg (arg, argblock, flags, var
*** 4407,4417 ****
  
        if (arg->mode != TYPE_MODE (TREE_TYPE (pval)))
  	arg->value = convert_modes (arg->mode, TYPE_MODE (TREE_TYPE (pval)),
  				    arg->value, arg->unsignedp);
  
!       if (arg->pass_on_stack)
  	stack_arg_under_construction--;
      }
  
    /* Don't allow anything left on stack from computation
       of argument to alloca.  */
--- 4407,4417 ----
  
        if (arg->mode != TYPE_MODE (TREE_TYPE (pval)))
  	arg->value = convert_modes (arg->mode, TYPE_MODE (TREE_TYPE (pval)),
  				    arg->value, arg->unsignedp);
  
!       if (arg->stack)
  	stack_arg_under_construction--;
      }
  
    /* Don't allow anything left on stack from computation
       of argument to alloca.  */
Index: expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.506
diff -c -5 -p -r1.506 expr.c
*** expr.c	19 Feb 2003 14:38:45 -0000	1.506
--- expr.c	21 Feb 2003 20:04:38 -0000
*************** emit_block_move (x, y, size, method)
*** 1706,1717 ****
    unsigned int align;
  
    switch (method)
      {
      case BLOCK_OP_NORMAL:
!       may_use_call = true;
!       break;
  
      case BLOCK_OP_CALL_PARM:
        may_use_call = block_move_libcall_safe_for_call_parm ();
  
        /* Make inhibit_defer_pop nonzero around the library call
--- 1706,1721 ----
    unsigned int align;
  
    switch (method)
      {
      case BLOCK_OP_NORMAL:
!       if (!stack_arg_under_construction)
! 	{
! 	  may_use_call = true;
! 	  break;
! 	}
!       /* Fall through.  */
  
      case BLOCK_OP_CALL_PARM:
        may_use_call = block_move_libcall_safe_for_call_parm ();
  
        /* Make inhibit_defer_pop nonzero around the library call
Index: expr.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.h,v
retrieving revision 1.128
diff -c -5 -p -r1.128 expr.h
*** expr.h	21 Jan 2003 07:55:53 -0000	1.128
--- expr.h	21 Feb 2003 20:04:38 -0000
*************** extern rtx convert_modes PARAMS ((enum m
*** 375,386 ****
--- 375,391 ----
  
  /* Emit code to move a block Y to a block X.  */
  
  enum block_op_methods
  {
+   /* It is OK to use a libcall to implement the move.  */
    BLOCK_OP_NORMAL,
+   /* The block move is occurring during the evaluation of the argument
+      to some other function.  It is OK to use a libcall only if the
+      libcall will not clobber the argument presently being evaluated.  */
    BLOCK_OP_CALL_PARM,
+   /* It is not OK to use a libcall to implement the move.  */
    BLOCK_OP_NO_LIBCALL
  };
  
  extern rtx emit_block_move PARAMS ((rtx, rtx, rtx, enum block_op_methods));
  
Index: function.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/function.h,v
retrieving revision 1.94
diff -c -5 -p -r1.94 function.h
*** function.h	15 Feb 2003 21:22:55 -0000	1.94
--- function.h	21 Feb 2003 20:04:38 -0000
*************** struct function GTY(())
*** 512,521 ****
--- 512,528 ----
  extern GTY(()) struct function *cfun;
  
  /* Nonzero if we've already converted virtual regs to hard regs.  */
  extern int virtuals_instantiated;
  
+ /* stack_arg_under_construction is nonzero when an argument may be
+    initialized with a constructor call (including a C function that
+    returns a BLKmode struct) and expand_call must take special action
+    to make sure the object being constructed does not overlap the
+    argument list for the constructor call.  */
+ extern int stack_arg_under_construction;
+ 
  /* For backward compatibility... eventually these should all go away.  */
  #define current_function_name (cfun->name)
  #define current_function_pops_args (cfun->pops_args)
  #define current_function_returns_struct (cfun->returns_struct)
  #define current_function_returns_pcc_struct (cfun->returns_pcc_struct)