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

Reply via email to