diff mbox

PR64979: S/390: Fix setup of overflow area pointer in va_start

Message ID 20150209095034.GA32189@maggie
State New
Headers show

Commit Message

Andreas Krebbel Feb. 9, 2015, 9:50 a.m. UTC 
Hi,

the attached patch fixes a critical problem in the va_start expansion
code in the S/390 backend. The problem exists since GCC 4.0.

Ok to commit to 4.9 branch and mainline?

Bye,

-Andreas-


2015-02-09  Andreas Krebbel  <Andreas.Krebbel@de.ibm.com>

	PR target/64979
	* config/s390/s390.c (s390_va_start): Make set up of
	__overflow_arg_area unconditional.

2015-02-09  Andreas Krebbel  <Andreas.Krebbel@de.ibm.com>

	PR target/64979
	* gcc.dg/pr64979.c: New testcase.

Comments

Jakub Jelinek Feb. 9, 2015, 11:29 a.m. UTC | #1 
On Mon, Feb 09, 2015 at 10:50:34AM +0100, Andreas Krebbel wrote:
> Hi,
> 
> the attached patch fixes a critical problem in the va_start expansion
> code in the S/390 backend. The problem exists since GCC 4.0.
> 
> Ok to commit to 4.9 branch and mainline?

No.  This isn't a backend problem, but a bug in the tree-stdarg.c pass,
so should be fixed there, for all targets, rather than just worked around by
pessimizing unnecessarily one target.

	Jakub
Andreas Krebbel Feb. 9, 2015, 11:40 a.m. UTC | #2 
On 02/09/2015 12:29 PM, Jakub Jelinek wrote:
> On Mon, Feb 09, 2015 at 10:50:34AM +0100, Andreas Krebbel wrote:
>> Hi,
>>
>> the attached patch fixes a critical problem in the va_start expansion
>> code in the S/390 backend. The problem exists since GCC 4.0.
>>
>> Ok to commit to 4.9 branch and mainline?
> 
> No.  This isn't a backend problem, but a bug in the tree-stdarg.c pass,
> so should be fixed there, for all targets, rather than just worked around by
> pessimizing unnecessarily one target.

I think for the overflow area pointer we would need a different flag indicating whether a pointer to
the va_list structure escapes or not. To my understanding it is not correct to only use the
va_list_gpr_size/va_list_fpr_size fields for that purpose. These only refer to the va_arg expansions
in the current function.

Other backends rs6000/i386 unconditionally set up the overflow area pointer.

-Andreas-
diff mbox

Patch

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index bc6223e..ed22d35 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -9697,23 +9697,19 @@  s390_va_start (tree valist, rtx nextarg ATTRIBUTE_UNUSED)
     }
 
   /* Find the overflow area.  */
-  if (n_gpr + cfun->va_list_gpr_size > GP_ARG_NUM_REG
-      || n_fpr + cfun->va_list_fpr_size > FP_ARG_NUM_REG)
-    {
-      t = make_tree (TREE_TYPE (ovf), virtual_incoming_args_rtx);
+  t = make_tree (TREE_TYPE (ovf), virtual_incoming_args_rtx);
 
-      off = INTVAL (crtl->args.arg_offset_rtx);
-      off = off < 0 ? 0 : off;
-      if (TARGET_DEBUG_ARG)
-	fprintf (stderr, "va_start: n_gpr = %d, n_fpr = %d off %d\n",
-		 (int)n_gpr, (int)n_fpr, off);
+  off = INTVAL (crtl->args.arg_offset_rtx);
+  off = off < 0 ? 0 : off;
+  if (TARGET_DEBUG_ARG)
+    fprintf (stderr, "va_start: n_gpr = %d, n_fpr = %d off %d\n",
+	     (int)n_gpr, (int)n_fpr, off);
 
-      t = fold_build_pointer_plus_hwi (t, off);
+  t = fold_build_pointer_plus_hwi (t, off);
 
-      t = build2 (MODIFY_EXPR, TREE_TYPE (ovf), ovf, t);
-      TREE_SIDE_EFFECTS (t) = 1;
-      expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
-    }
+  t = build2 (MODIFY_EXPR, TREE_TYPE (ovf), ovf, t);
+  TREE_SIDE_EFFECTS (t) = 1;
+  expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
 
   /* Find the register save area.  */
   if ((cfun->va_list_gpr_size && n_gpr < GP_ARG_NUM_REG)
diff --git a/gcc/testsuite/gcc.dg/pr64979.c b/gcc/testsuite/gcc.dg/pr64979.c
new file mode 100644
index 0000000..9c715c9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr64979.c
@@ -0,0 +1,40 @@ 
+/* PR target/64979 */
+
+/* { dg-do run } */
+/* { dg-options "-O1" } */
+
+#include <stdarg.h>
+
+extern void abort (void);
+
+void __attribute__((noinline))
+bar(int msgno, va_list *args)
+{
+  int i;
+
+  for (i = 0; i < 10; i++)
+    if (i != va_arg(*args, int))
+      abort ();
+}
+
+
+void __attribute__((noinline))
+foo(int msgno, ...)
+{
+  va_list args;
+  int nargs;
+
+  va_start(args, msgno);
+  nargs = va_arg(args, int);
+  bar(msgno, (va_list *)((nargs == 0) ? ((void *)0) : &args));
+}
+
+
+int main(void)
+{
+  foo(100 /* msgno */,
+      1 /* nargs - part of vararg list */,
+      0, 1, 2, 3, 4, 5, 6, 7, 8, 9);
+
+  return 0;
+}