Re: Postpone expanding va_arg until pass_stdarg

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Feb 10, 2015 at 2:20 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> 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.

More like

Index: gcc/tree.c
===================================================================
--- gcc/tree.c  (revision 220578)
+++ gcc/tree.c  (working copy)
@@ -5815,6 +5815,7 @@ free_lang_data (void)
      still be used indirectly via the get_alias_set langhook.  */
   lang_hooks.dwarf_name = lhd_dwarf_name;
   lang_hooks.decl_printable_name = gimple_decl_printable_name;
+  lang_hooks.gimplify_expr = lhd_gimplify_expr;
   /* We do not want the default decl_assembler_name implementation,
      rather if we have fixed everything we want a wrapper around it
      asserting that all non-local symbols already got their assembler

> 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);
>