This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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?

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]