On Thu, 21 Sep 2017, Jakub Jelinek wrote:
> Hi!
>
> As can be seen on this function (trying to show what some Linux kernel
> driver does a lot), we don't reuse stack slots for addressable args/retvals
> of inlined functions. For variables inside lexical scopes, including inline
> functions, we emit clobber stmts during gimplification, but there are none
> for the addressable args. I think it is a waste of time to emit the
> clobbers during gimplification, they won't be really useful unless the
> function is inlined. And for the retval variables it is even not possible
> to emit them before the return value is assigned.
>
> So, this patch adds those clobbers during inlining instead. CCing Martin
> in case he wants to do something about those in
> -fsanitize-address-use-after-scope too.
>
> I run into a bug that id->retvar wasn't being cleared from one
> expand_call_inline invocation to another one, so a retval from one inlining
> would survive until another one, where it could be clobbered even when it
> should be valid. Fixed by the last hunk in tree-inline.c.
>
> Also, I run into UB in pr8781.C testcase, where we'd construct a reference
> to an inline function's parameter and return an object with that reference
> and then dereference it after the function returned, so with my patch
> the invalid testcase is no longer optimized.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Thanks,
Richard.
> 2017-09-21 Jakub Jelinek <ja...@redhat.com>
>
> PR sanitizer/81715
> * tree-inline.c (expand_call_inline): Emit clobber stmts for
> VAR_DECLs to which addressable non-volatile parameters are mapped
> and for id->retvar after the return value assignment. Clear
> id->retval and id->retbnd after inlining.
>
> * g++.dg/tree-ssa/pr8781.C (noop): Change argument type from
> const predicate to const predicate & to avoid UB.
> * g++.dg/opt/pr81715.C: New test.
>
> --- gcc/tree-inline.c.jj 2017-08-31 23:47:20.448971185 +0200
> +++ gcc/tree-inline.c 2017-09-21 10:42:49.491114240 +0200
> @@ -4796,6 +4796,22 @@ expand_call_inline (basic_block bb, gimp
>
> reset_debug_bindings (id, stmt_gsi);
>
> + if (flag_stack_reuse != SR_NONE)
> + for (tree p = DECL_ARGUMENTS (id->src_fn); p; p = DECL_CHAIN (p))
> + if (!TREE_THIS_VOLATILE (p))
> + {
> + tree *varp = id->decl_map->get (p);
> + if (varp && VAR_P (*varp) && !is_gimple_reg (*varp))
> + {
> + tree clobber = build_constructor (TREE_TYPE (*varp), NULL);
> + gimple *clobber_stmt;
> + TREE_THIS_VOLATILE (clobber) = 1;
> + clobber_stmt = gimple_build_assign (*varp, clobber);
> + gimple_set_location (clobber_stmt, gimple_location (stmt));
> + gsi_insert_before (&stmt_gsi, clobber_stmt, GSI_SAME_STMT);
> + }
> + }
> +
> /* Reset the escaped solution. */
> if (cfun->gimple_df)
> pt_solution_reset (&cfun->gimple_df->escaped);
> @@ -4846,6 +4862,23 @@ expand_call_inline (basic_block bb, gimp
> stmt = gimple_build_assign (gimple_call_lhs (stmt), use_retvar);
> gsi_replace (&stmt_gsi, stmt, false);
> maybe_clean_or_replace_eh_stmt (old_stmt, stmt);
> + /* Append a clobber for id->retvar if easily possible. */
> + if (flag_stack_reuse != SR_NONE
> + && id->retvar
> + && VAR_P (id->retvar)
> + && id->retvar != return_slot
> + && id->retvar != modify_dest
> + && !TREE_THIS_VOLATILE (id->retvar)
> + && !is_gimple_reg (id->retvar)
> + && !stmt_ends_bb_p (stmt))
> + {
> + tree clobber = build_constructor (TREE_TYPE (id->retvar), NULL);
> + gimple *clobber_stmt;
> + TREE_THIS_VOLATILE (clobber) = 1;
> + clobber_stmt = gimple_build_assign (id->retvar, clobber);
> + gimple_set_location (clobber_stmt, gimple_location (old_stmt));
> + gsi_insert_after (&stmt_gsi, clobber_stmt, GSI_SAME_STMT);
> + }
>
> /* Copy bounds if we copy structure with bounds. */
> if (chkp_function_instrumented_p (id->dst_fn)
> @@ -4884,8 +4917,25 @@ expand_call_inline (basic_block bb, gimp
> SSA_NAME_DEF_STMT (name) = gimple_build_nop ();
> }
> }
> + /* Replace with a clobber for id->retvar. */
> + else if (flag_stack_reuse != SR_NONE
> + && id->retvar
> + && VAR_P (id->retvar)
> + && id->retvar != return_slot
> + && id->retvar != modify_dest
> + && !TREE_THIS_VOLATILE (id->retvar)
> + && !is_gimple_reg (id->retvar))
> + {
> + tree clobber = build_constructor (TREE_TYPE (id->retvar), NULL);
> + gimple *clobber_stmt;
> + TREE_THIS_VOLATILE (clobber) = 1;
> + clobber_stmt = gimple_build_assign (id->retvar, clobber);
> + gimple_set_location (clobber_stmt, gimple_location (stmt));
> + gsi_replace (&stmt_gsi, clobber_stmt, false);
> + maybe_clean_or_replace_eh_stmt (stmt, clobber_stmt);
> + }
> else
> - gsi_remove (&stmt_gsi, true);
> + gsi_remove (&stmt_gsi, true);
> }
>
> /* Put returned bounds into the correct place if required. */
> @@ -4934,6 +4984,8 @@ expand_call_inline (basic_block bb, gimp
> cg_edge->callee->remove ();
>
> id->block = NULL_TREE;
> + id->retvar = NULL_TREE;
> + id->retbnd = NULL_TREE;
> successfully_inlined = true;
>
> egress:
> --- gcc/testsuite/g++.dg/tree-ssa/pr8781.C.jj 2015-05-29 11:36:05.000000000
> +0200
> +++ gcc/testsuite/g++.dg/tree-ssa/pr8781.C 2017-09-21 10:48:47.000000000
> +0200
> @@ -13,7 +13,7 @@ public:
> };
>
> template<typename predicate>
> -inline noop_t<predicate> noop(const predicate pred) {
> +inline noop_t<predicate> noop(const predicate &pred) {
> return noop_t<predicate>(pred);
> }
>
> --- gcc/testsuite/g++.dg/opt/pr81715.C.jj 2017-09-20 17:50:04.331557128
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr81715.C 2017-09-20 17:50:04.331557128
> +0200
> @@ -0,0 +1,36 @@
> +// PR sanitizer/81715
> +// { dg-do compile }
> +// Verify the variables for inlined foo parameters are reused
> +// { dg-options "-O2 -Wframe-larger-than=16384" }
> +
> +struct S { int a, b, c, d, e; char f[1024]; };
> +void baz (int *, int *, int *, struct S *, int *, int *);
> +
> +static inline struct S
> +foo (int a, int b, int c, struct S d, int e, int f)
> +{
> + struct S s;
> + baz (&a, &b, &c, &d, &e, &f);
> + s = d;
> + return s;
> +}
> +
> +struct S g[64];
> +
> +void
> +bar (int a, int b, int c, struct S d, int e, int f)
> +{
> +#define A(N) \
> + g[N+0] = foo (a, b, c, d, e, f); \
> + g[N+1] = foo (a, b, c, d, e, f); \
> + g[N+2] = foo (a, b, c, d, e, f); \
> + g[N+3] = foo (a, b, c, d, e, f); \
> + g[N+4] = foo (a, b, c, d, e, f); \
> + g[N+5] = foo (a, b, c, d, e, f); \
> + g[N+6] = foo (a, b, c, d, e, f); \
> + g[N+7] = foo (a, b, c, d, e, f); \
> + foo (a, b, c, d, e, f); \
> + foo (a, b, c, d, e, f)
> + A(0); A(8); A(16); A(24);
> + A(32); A(40); A(48); A(56);
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
