On Thu, Mar 20, 2025 at 06:25:26PM +0100, Jakub Jelinek wrote:
> On Thu, Mar 20, 2025 at 10:01:02AM -0700, Andi Kleen wrote:
> > So it could be as simple as that patch? It solves your test case at least
> > for x86.
>
> Not sure I like this, but if others (e.g. Richi, Joseph, Jason) are ok with
> it I can live with it. But we'd need a good documentation, ideally some
> some new warning about it (perhaps enabled in -Wextra) and testcase
> coverage.
> Looking around, clang warns for
> int foo (int *);
> int bar (int *x)
> {
> *x = 42;
> int a = 0;
> [[clang::musttail]] return foo (&a);
> }
> by default (without -Wall/-Wextra) with
> warning: address of stack memory associated with local variable 'a' passed to
> musttail function [-Wreturn-stack-address]
> Dunno if that warning is about other stuff or just that.
> If just that, perhaps we'd need multiple levels for it, diagnose passing
> those to musttail arguments by default and have in -Wextra stronger variant
> that would diagnose even the
> int baz (int *x)
> {
> *x = 42;
> int a = 0;
> foo (&a);
> [[clang::musttail]] return foo (nullptr);
> }
> case, i.e. escaped variables that are still in scope.
> That variant of the warning perhaps should suggest to users to end the
> lifetime of those variables before the musttail call.
It looks like it's reasonably easy to provide a single warning
covering both the escape and the passing case. Doing it more
fine grained would need much more code by redoing parts
of ref_may_be_used_by_stmt_p etc.
Perhaps that is good enough?
This version also fixes a memory leak in my earlier variant.
diff --git a/gcc/tree-tailcall.cc b/gcc/tree-tailcall.cc
index f97df31eb3cf..066037ed09c6 100644
--- a/gcc/tree-tailcall.cc
+++ b/gcc/tree-tailcall.cc
@@ -714,29 +714,36 @@ find_tail_calls (basic_block bb, struct tailcall **ret,
bool only_musttail,
{
if (TREE_CODE (var) != PARM_DECL
&& auto_var_in_fn_p (var, cfun->decl)
- && may_be_aliased (var)
- && (ref_maybe_used_by_stmt_p (call, var, false)
- || call_may_clobber_ref_p (call, var, false)))
+ && may_be_aliased (var))
{
- if (!VAR_P (var))
+ if (VAR_P (var)
+ && !bitmap_bit_p (local_live_vars,
+ *live_vars->get (DECL_UID (var))))
+ continue;
+ if (ref_maybe_used_by_stmt_p (call, var, false))
{
- if (local_live_vars)
- BITMAP_FREE (local_live_vars);
- maybe_error_musttail (call,
- _("call invocation refers to locals"));
- return;
+ if (gimple_call_must_tail_p (call))
+ {
+ warning_at (call->location, 0,
+ "Address of local variable %qE may be used by
musttail call",
+ var);
+ continue;
+ }
}
- else
+ else if (call_may_clobber_ref_p (call, var, false))
{
- unsigned int *v = live_vars->get (DECL_UID (var));
- if (bitmap_bit_p (local_live_vars, *v))
+ if (gimple_call_must_tail_p (call))
{
- BITMAP_FREE (local_live_vars);
- maybe_error_musttail (call,
- _("call invocation refers to locals"));
- return;
+ warning_at (call->location, 0,
+ "musttail call may clobber escaped variable %qE",
var);
+ continue;
}
}
+ else
+ continue;
+ if (local_live_vars)
+ BITMAP_FREE (local_live_vars);
+ return;
}
}
