Re: Extend -fstack-protector-strong to cover calls with return slot

[Jakub Jelinek <jakub@redhat.com>]

On Fri, Jan 03, 2014 at 02:15:51PM +0100, Florian Weimer wrote:
> This patch fixes a loophole in the -fstack-protector-strong
> protection.  If a function call uses the return slot optimization,
> the caller needs stack protector instrumentation because the return
> slot is addressable.
> 
> Bootstrapped and regression-tested on x86_64-redhat-linux-gnu, with
> C/C++/Java enabled.  Okay for trunk?
> 
> -- 
> Florian Weimer / Red Hat Product Security Team

> 2014-01-03  Florian Weimer  <fweimer@redhat.com>
> 
> 	* cfgexpand.c (call_with_return_slot_opt_p): New function.
> 	(expand_used_vars): Emit stack protector instrumentation in strong
> 	mode if call_with_return_slot_opt_p.
> 
> gcc/testsuite/
> 
> 2014-01-03  Florian Weimer  <fweimer@redhat.com>
> 
> 	* gcc.dg/fstack-protector-strong.c: Add coverage for named return
> 	values.
> 	* g++.dg/fstack-protector-strong.C: Likewise.
> 
> Index: gcc/cfgexpand.c
> ===================================================================
> --- gcc/cfgexpand.c	(revision 206311)
> +++ gcc/cfgexpand.c	(working copy)
> @@ -1599,6 +1599,22 @@
>    return 0;
>  }
>  
> +/* Check if the basic block has a call which uses a return slot.  */
> +
> +static bool
> +call_with_return_slot_opt_p (basic_block bb)
> +{
> +  for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
> +       !gsi_end_p (gsi); gsi_next (&gsi))
> +    {
> +      gimple stmt = gsi_stmt (gsi);
> +      if (gimple_code (stmt) == GIMPLE_CALL

That would be is_gimple_call (stmt) instead.

Also, I'd think the function is misnamed, given that it checks if there
are any calls with return_slot_opt_p in a bb.  I think it would be
better to move FOR_ALL_BB_FN (bb, cfun) loop also into the
function and call it any_call_...

Lastly, I wonder if gimple_call_return_slot_opt_p is really what you are
after, why does NRV matter here?  Isn't what you are looking for instead
whether the called function returns value through invisible reference,
because then you'll always have some (aggregate) addressable object
in the caller's frame and supposedly you are after making sure that the
callee doesn't overflow the return object.

So, looking at tree-nrv.c, that check would be roughly:
          if (is_gimple_call (stmt)
              && gimple_call_lhs (stmt)
              && aggregate_value_p (TREE_TYPE (gimple_call_lhs (stmt)),
                                    gimple_call_fndecl (stmt)))

	Jakub