[patch] fix PR middle-end/35616, sibcall miscompilation

[patch] fix PR middle-end/35616, sibcall miscompilation

[Michael Matz]

Hi,

as seen in the bugreport expand_call fails to ensure that the call address 
doesn't overlap with the arguments for sibcalls (it checks all 
inter-argument overlap and disables sibcalls then, but if the arguments 
don't overlap, but one of them overlaps the call address it's clobbered as 
in the testcase).

Regstrapping in progress on i686 and x86_64.  Okay for trunk if this 
passes?  What with 4.3?


Ciao,
Michael.
	PR middle-end/35616
	* calls.c (expand_call): Check overlap of arguments with call
	address for sibcalls.

	* gcc.dg/pr35616.c: New test.

Index: gcc/calls.c
===================================================================
--- gcc/calls.c	(revision 133304)
+++ gcc/calls.c	(working copy)
@@ -2326,7 +2326,7 @@ expand_call (tree exp, rtx target, int i
       int save_pending_stack_adjust = 0;
       int save_stack_pointer_delta = 0;
       rtx insns;
-      rtx before_call, next_arg_reg;
+      rtx before_call, next_arg_reg, after_args;
 
       if (pass == 0)
 	{
@@ -2756,6 +2756,7 @@ expand_call (tree exp, rtx target, int i
 	    use_reg (&call_fusage, struct_value);
 	}
 
+      after_args = get_last_insn ();
       funexp = prepare_call_address (funexp, static_chain_value,
 				     &call_fusage, reg_parm_seen, pass == 0);
 
@@ -2790,6 +2791,13 @@ expand_call (tree exp, rtx target, int i
 		   next_arg_reg, valreg, old_inhibit_defer_pop, call_fusage,
 		   flags, & args_so_far);
 
+      /* If the call setup or the call itself overlaps with anything
+	 of the argument setup we probably clobbered our call address.
+	 In that case we can't do sibcalls.  */
+      if (pass == 0
+	  && check_sibcall_argument_overlap (after_args, 0, 0))
+	sibcall_failure = 1;
+
       /* If a non-BLKmode value is returned at the most significant end
 	 of a register, shift the register right by the appropriate amount
 	 and update VALREG accordingly.  BLKmode values are handled by the
Index: gcc/testsuite/gcc.dg/pr35616.c
===================================================================
--- gcc/testsuite/gcc.dg/pr35616.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr35616.c	(revision 0)
@@ -0,0 +1,43 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+typedef void (*listener_fun)(
+        int a,
+        int b,
+        int c);
+
+struct data_t
+{
+  int a;
+
+  listener_fun listener;
+
+  int b;
+  int c;
+  int d;
+};
+
+extern void abort(void);
+void function_calling_listener (struct data_t data);
+
+void function_calling_listener (struct data_t data)
+{
+  data.listener(data.a, data.c, data.d);
+}
+
+void my_listener(int a, int b, int c)
+{
+  if (a != 42 || b != 44 || c != 45)
+    abort ();
+}
+
+int main()
+{
+  struct data_t d;
+  d.a = 42;
+  d.b = 43;
+  d.c = 44;
+  d.d = 45;
+  d.listener = my_listener;
+  function_calling_listener (d);
+  return 0;
+}

Re: [patch] fix PR middle-end/35616, sibcall miscompilation

[Richard Guenther]

On Tue, Mar 18, 2008 at 5:03 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
>  as seen in the bugreport expand_call fails to ensure that the call address
>  doesn't overlap with the arguments for sibcalls (it checks all
>  inter-argument overlap and disables sibcalls then, but if the arguments
>  don't overlap, but one of them overlaps the call address it's clobbered as
>  in the testcase).
>
>  Regstrapping in progress on i686 and x86_64.  Okay for trunk if this
>  passes?  What with 4.3?

Ok for trunk and all open branches.

Thanks,
Richard.

>
>  Ciao,
>  Michael.
>         PR middle-end/35616
>         * calls.c (expand_call): Check overlap of arguments with call
>         address for sibcalls.
>
>         * gcc.dg/pr35616.c: New test.
>
>  Index: gcc/calls.c
>  ===================================================================
>  --- gcc/calls.c (revision 133304)
>  +++ gcc/calls.c (working copy)
>  @@ -2326,7 +2326,7 @@ expand_call (tree exp, rtx target, int i
>        int save_pending_stack_adjust = 0;
>        int save_stack_pointer_delta = 0;
>        rtx insns;
>  -      rtx before_call, next_arg_reg;
>  +      rtx before_call, next_arg_reg, after_args;
>
>        if (pass == 0)
>         {
>  @@ -2756,6 +2756,7 @@ expand_call (tree exp, rtx target, int i
>             use_reg (&call_fusage, struct_value);
>         }
>
>  +      after_args = get_last_insn ();
>        funexp = prepare_call_address (funexp, static_chain_value,
>                                      &call_fusage, reg_parm_seen, pass == 0);
>
>  @@ -2790,6 +2791,13 @@ expand_call (tree exp, rtx target, int i
>                    next_arg_reg, valreg, old_inhibit_defer_pop, call_fusage,
>                    flags, & args_so_far);
>
>  +      /* If the call setup or the call itself overlaps with anything
>  +        of the argument setup we probably clobbered our call address.
>  +        In that case we can't do sibcalls.  */
>  +      if (pass == 0
>  +         && check_sibcall_argument_overlap (after_args, 0, 0))
>  +       sibcall_failure = 1;
>  +
>        /* If a non-BLKmode value is returned at the most significant end
>          of a register, shift the register right by the appropriate amount
>          and update VALREG accordingly.  BLKmode values are handled by the
>  Index: gcc/testsuite/gcc.dg/pr35616.c
>  ===================================================================
>  --- gcc/testsuite/gcc.dg/pr35616.c      (revision 0)
>  +++ gcc/testsuite/gcc.dg/pr35616.c      (revision 0)
>  @@ -0,0 +1,43 @@
>  +/* { dg-do run } */
>  +/* { dg-options "-O2" } */
>  +typedef void (*listener_fun)(
>  +        int a,
>  +        int b,
>  +        int c);
>  +
>  +struct data_t
>  +{
>  +  int a;
>  +
>  +  listener_fun listener;
>  +
>  +  int b;
>  +  int c;
>  +  int d;
>  +};
>  +
>  +extern void abort(void);
>  +void function_calling_listener (struct data_t data);
>  +
>  +void function_calling_listener (struct data_t data)
>  +{
>  +  data.listener(data.a, data.c, data.d);
>  +}
>  +
>  +void my_listener(int a, int b, int c)
>  +{
>  +  if (a != 42 || b != 44 || c != 45)
>  +    abort ();
>  +}
>  +
>  +int main()
>  +{
>  +  struct data_t d;
>  +  d.a = 42;
>  +  d.b = 43;
>  +  d.c = 44;
>  +  d.d = 45;
>  +  d.listener = my_listener;
>  +  function_calling_listener (d);
>  +  return 0;
>  +}
>