Re: Postpone expanding va_arg until pass_stdarg

[Tom de Vries <Tom_deVries at mentor dot com>]

On 10-02-15 11:10, Richard Biener wrote:
> > The single failing testcase (both with and without -m32) is
> > >g++.dg/torture/pr45843.C:
> > >...
> > >./gcc/testsuite/g++/g++.sum:FAIL: g++.dg/torture/pr45843.C   -O2 -flto
> > >-fno-use-linker-plugin -flto-partition=none  (internal compiler error)
> > >...
> > >
> > >The failure looks like this (it happens during the gimplify_assign after
> > >calling gimplify_va_arg_internal):
> > >...
> > >src/gcc/testsuite/g++.dg/torture/pr45843.C: In function âfoo(int, ...)â:
> > >src/gcc/testsuite/g++.dg/torture/pr45843.C:11:1: internal compiler error:
> > >Segmentation fault
> > >0x10a5b04 crash_signal
> > >         src/gcc/toplev.c:383
> > >0x6a8985 tree_check(tree_node*, char const*, int, char const*, tree_code)
> > >         src/gcc/tree.h:2845
> > >0x7c2f6a is_really_empty_class(tree_node*)
> > >         src/gcc/cp/class.c:7923
> > >0x923855 cp_gimplify_expr(tree_node**, gimple_statement_base**,
> > >gimple_statement_base**)
> > >         src/gcc/cp/cp-gimplify.c:625
> > >0xd34641 gimplify_expr(tree_node**, gimple_statement_base**,
> > >gimple_statement_base**, bool (*)(tree_node*), int)
> > >         src/gcc/gimplify.c:7843
> > >0xd2a04d gimplify_stmt(tree_node**, gimple_statement_base**)
> > >         src/gcc/gimplify.c:5551
> > >0xd173e3 gimplify_and_add(tree_node*, gimple_statement_base**)
> > >         src/gcc/gimplify.c:419
> > >0xd39c94 gimplify_assign(tree_node*, tree_node*, gimple_statement_base**)
> > >         src/gcc/gimplify.c:9452
> > >0x130ad18 execute
> > >         src/gcc/tree-stdarg.c:779
> > >...
> > >
> > >The testcase contains this struct:
> > >...
> > >struct S { struct T { } a[14]; char b; };
> > >...
> > >
> > >and uses that struct S as type in va_arg:
> > >...
> > >   arg = va_arg (ap, struct S);
> > >...
> > >
> > >The segfault happens because we're calling is_really_empty_class for struct
> > >S, and TYPE_BINFO is NULL_TREE, which causes BINFO_BASE_ITERATE to segfault.
> > >I'm not sure yet what this issue is or how this is supposed to be fixed.
> That's probably free_lang_data being more aggressive after Honza
> fiddled with BINFOs?  That is - the gimplifications called from tree-stdarg.c
> (and others from the middle-end) should never call back into the frontend
> via langhooks...

Hmm, that 'should never' sounds like a missing gcc_assert.

This patch is a way to achieve that gimplification doesn't call the actual 
gimplify_expr langhook, and it fixes the failure. But I'm guessing that's not 
the proper way to fix this.

Thanks,
- Tom

diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c
index 443f6d3..d6cd52d 100644
--- a/gcc/tree-stdarg.c
+++ b/gcc/tree-stdarg.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "input.h"
 #include "function.h"
 #include "langhooks.h"
+#include "langhooks-def.h"
 #include "gimple-pretty-print.h"
 #include "target.h"
 #include "bitmap.h"
@@ -734,6 +735,11 @@ pass_stdarg::execute (function *fun)
   const char *funcname = NULL;
   tree cfun_va_list;
   unsigned int retflags = 0;
+  int (*save_gimplify_expr) (tree *, gimple_seq *, gimple_seq *);
+
+  /* Ensure we don't call language hooks from gimplification.  */
+  save_gimplify_expr = lang_hooks.gimplify_expr;
+  lang_hooks.gimplify_expr = lhd_gimplify_expr;

   /* Expand va_arg.  */
   /* TODO: teach pass_stdarg how process the va_arg builtin, and reverse the
@@ -796,6 +802,9 @@ pass_stdarg::execute (function *fun)
        }
     }

+  /* Restore language hook.  */
+  lang_hooks.gimplify_expr = save_gimplify_expr;
+
   if (retflags)
     {
       free_dominance_info (CDI_DOMINATORS);