[PATCH] Improve stack reuse for addressable args/retval of inlined functions (PR sanitizer/81715)

Richard Biener rguenther@suse.de
Thu Sep 21 11:38:00 GMT 2017


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  <jakub@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)



More information about the Gcc-patches mailing list