On Tue, 15 Apr 2025, Jakub Jelinek wrote:
> Hi!
>
> Calls with musttail attribute don't really work with -fsanitize=thread in
> GCC. The problem is that TSan instrumentation adds
> __tsan_func_entry (__builtin_return_address (0));
> calls at the start of each instrumented function and
> __tsan_func_exit ();
> call at the end of those and the latter stands in a way of normal tail calls
> as well as musttail tail calls.
>
> Looking at what LLVM does, for normal calls -fsanitize=thread also prevents
> tail calls like in GCC (well, the __tsan_func_exit () call itself can be
> tail called in GCC (and from what I see not in clang)).
> But for [[clang::musttail]] calls it arranges to move the
> __tsan_func_exit () before the musttail call instead of after it.
>
> The following patch handles it similarly. If we for -fsanitize=thread
> instrumented function detect __builtin_tsan_func_exit () call, we process
> it normally (so that the call can be tail called in function returning void)
> but set a flag that the builtin has been seen (only for cfun->has_musttail
> in the diag_musttail phase). And then let tree_optimize_tail_calls_1
> call find_tail_calls again in a new mode where the __tsan_func_exit ()
> call is ignored and so we are able to find calls before it, but only
> accept that if the call before it is actually a musttail. For C++ it needs
> to verify that EH cleanup if any also has the __tsan_func_exit () call
> and if all goes well, the musttail call is registered for tailcalling with
> a flag that it has __tsan_func_exit () after it and when optimizing that
> we emit __tsan_func_exit (); call before the musttail tail call (or musttail
> tail recursion).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2025-04-15 Jakub Jelinek <ja...@redhat.com>
>
> PR sanitizer/119801
> * sanitizer.def (BUILT_IN_TSAN_FUNC_EXIT): Use BT_FN_VOID rather
> than BT_FN_VOID_PTR.
> * tree-tailcall.cc: Include attribs.h and asan.h.
> (struct tailcall): Add has_tsan_func_exit member.
> (empty_eh_cleanup): Add eh_has_tsan_func_exit argument, set what
> it points to to 1 if there is exactly one __tsan_func_exit call
> and ignore that call otherwise. Adjust recursive call.
> (find_tail_calls): Add RETRY_TSAN_FUNC_EXIT argument, pass it
> to recursive calls. When seeing __tsan_func_exit call with
> RETRY_TSAN_FUNC_EXIT 0, set it to -1. If RETRY_TSAN_FUNC_EXIT
> is 1, initially ignore __tsan_func_exit calls. Adjust
> empty_eh_cleanup caller. When looking through stmts after the call,
> ignore exactly one __tsan_func_exit call but remember it in
> t->has_tsan_func_exit. Diagnose if EH cleanups didn't have
> __tsan_func_exit and normal path did or vice versa.
> (optimize_tail_call): Emit __tsan_func_exit before the tail call
> or tail recursion.
> (tree_optimize_tail_calls_1): Adjust find_tail_calls callers. If
> find_tail_calls changes retry_tsan_func_exit to -1, set it to 1
> and call it again with otherwise the same arguments.
>
> * c-c++-common/tsan/pr119801.c: New test.
>
> --- gcc/sanitizer.def.jj 2025-04-14 19:30:31.804837079 +0200
> +++ gcc/sanitizer.def 2025-04-15 09:48:23.752349037 +0200
> @@ -247,7 +247,7 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_INIT
> DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_FUNC_ENTRY, "__tsan_func_entry",
> BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST)
> DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_FUNC_EXIT, "__tsan_func_exit",
> - BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST)
> + BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
> DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VPTR_UPDATE, "__tsan_vptr_update",
> BT_FN_VOID_PTR_PTR, ATTR_NOTHROW_LEAF_LIST)
> DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_READ1, "__tsan_read1",
> --- gcc/tree-tailcall.cc.jj 2025-04-14 19:30:31.976834786 +0200
> +++ gcc/tree-tailcall.cc 2025-04-15 10:12:48.879501238 +0200
> @@ -51,6 +51,8 @@ along with GCC; see the file COPYING3.
> #include "symbol-summary.h"
> #include "ipa-cp.h"
> #include "ipa-prop.h"
> +#include "attribs.h"
> +#include "asan.h"
>
> /* The file implements the tail recursion elimination. It is also used to
> analyze the tail calls in general, passing the results to the rtl level
> @@ -122,6 +124,9 @@ struct tailcall
> /* True if it is a call to the current function. */
> bool tail_recursion;
>
> + /* True if there is __tsan_func_exit call after the call. */
> + bool has_tsan_func_exit;
> +
> /* The return value of the caller is mult * f + add, where f is the return
> value of the call. */
> tree mult, add;
> @@ -504,7 +509,7 @@ maybe_error_musttail (gcall *call, const
> Search at most CNT basic blocks (so that we don't need to do trivial
> loop discovery). */
> static bool
> -empty_eh_cleanup (basic_block bb, int cnt)
> +empty_eh_cleanup (basic_block bb, int *eh_has_tsan_func_exit, int cnt)
> {
> if (EDGE_COUNT (bb->succs) > 1)
> return false;
> @@ -515,6 +520,14 @@ empty_eh_cleanup (basic_block bb, int cn
> gimple *g = gsi_stmt (gsi);
> if (is_gimple_debug (g) || gimple_clobber_p (g))
> continue;
> + if (eh_has_tsan_func_exit
> + && !*eh_has_tsan_func_exit
> + && sanitize_flags_p (SANITIZE_THREAD)
> + && gimple_call_builtin_p (g, BUILT_IN_TSAN_FUNC_EXIT))
> + {
> + *eh_has_tsan_func_exit = 1;
> + continue;
> + }
> if (is_gimple_resx (g) && stmt_can_throw_external (cfun, g))
> return true;
> return false;
> @@ -523,7 +536,7 @@ empty_eh_cleanup (basic_block bb, int cn
> return false;
> if (cnt == 1)
> return false;
> - return empty_eh_cleanup (single_succ (bb), cnt - 1);
> + return empty_eh_cleanup (single_succ (bb), eh_has_tsan_func_exit, cnt - 1);
> }
>
> /* Argument for compute_live_vars/live_vars_at_stmt and what
> compute_live_vars
> @@ -531,14 +544,22 @@ empty_eh_cleanup (basic_block bb, int cn
> static live_vars_map *live_vars;
> static vec<bitmap_head> live_vars_vec;
>
> -/* Finds tailcalls falling into basic block BB. The list of found tailcalls
> is
> +/* Finds tailcalls falling into basic block BB. The list of found tailcalls
> is
> added to the start of RET. When ONLY_MUSTTAIL is set only handle
> musttail.
> Update OPT_TAILCALLS as output parameter. If DIAG_MUSTTAIL, diagnose
> - failures for musttail calls. */
> + failures for musttail calls. RETRY_TSAN_FUNC_EXIT is initially 0 and
> + in that case the last call is attempted to be tail called, including
> + __tsan_func_exit with -fsanitize=thread. It is set to -1 if we
> + detect __tsan_func_exit call and in that case tree_optimize_tail_calls_1
> + will retry with it set to 1 (regardless of whether turning the
> + __tsan_func_exit was successfully detected as tail call or not) and that
> + will allow turning musttail calls before that call into tail calls as well
> + by adding __tsan_func_exit call before the call. */
>
> static void
> find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail,
> - bool &opt_tailcalls, bool diag_musttail)
> + bool &opt_tailcalls, bool diag_musttail,
> + int &retry_tsan_func_exit)
> {
> tree ass_var = NULL_TREE, ret_var, func, param;
> gimple *stmt;
> @@ -552,6 +573,8 @@ find_tail_calls (basic_block bb, struct
> size_t idx;
> tree var;
> bool only_tailr = false;
> + bool has_tsan_func_exit = false;
> + int eh_has_tsan_func_exit = -1;
>
> if (!single_succ_p (bb)
> && (EDGE_COUNT (bb->succs) || !cfun->has_musttail || !diag_musttail))
> @@ -585,6 +608,17 @@ find_tail_calls (basic_block bb, struct
> || is_gimple_debug (stmt))
> continue;
>
> + if (cfun->has_musttail
> + && sanitize_flags_p (SANITIZE_THREAD)
> + && gimple_call_builtin_p (stmt, BUILT_IN_TSAN_FUNC_EXIT)
> + && diag_musttail)
> + {
> + if (retry_tsan_func_exit == 0)
> + retry_tsan_func_exit = -1;
> + else if (retry_tsan_func_exit == 1)
> + continue;
> + }
> +
> if (!last_stmt)
> last_stmt = stmt;
> /* Check for a call. */
> @@ -635,7 +669,7 @@ find_tail_calls (basic_block bb, struct
> /* Recurse to the predecessors. */
> FOR_EACH_EDGE (e, ei, bb->preds)
> find_tail_calls (e->src, ret, only_musttail, opt_tailcalls,
> - diag_musttail);
> + diag_musttail, retry_tsan_func_exit);
>
> return;
> }
> @@ -715,8 +749,12 @@ find_tail_calls (basic_block bb, struct
> return;
> }
>
> + if (diag_musttail && gimple_call_must_tail_p (call))
> + eh_has_tsan_func_exit = 0;
> if (!gimple_call_must_tail_p (call)
> - || !empty_eh_cleanup (e->dest, 20)
> + || !empty_eh_cleanup (e->dest,
> + eh_has_tsan_func_exit
> + ? NULL : &eh_has_tsan_func_exit, 20)
> || EDGE_COUNT (bb->succs) > 2)
> {
> maybe_error_musttail (call, _("call may throw exception caught "
> @@ -947,6 +985,17 @@ find_tail_calls (basic_block bb, struct
> || is_gimple_debug (stmt))
> continue;
>
> + if (cfun->has_musttail
> + && sanitize_flags_p (SANITIZE_THREAD)
> + && retry_tsan_func_exit == 1
> + && gimple_call_builtin_p (stmt, BUILT_IN_TSAN_FUNC_EXIT)
> + && !has_tsan_func_exit
> + && gimple_call_must_tail_p (call))
> + {
> + has_tsan_func_exit = true;
> + continue;
> + }
> +
> if (gimple_code (stmt) != GIMPLE_ASSIGN)
> {
> maybe_error_musttail (call, _("unhandled code after call"),
> @@ -1110,6 +1159,19 @@ find_tail_calls (basic_block bb, struct
> return;
> }
>
> + if (eh_has_tsan_func_exit != -1
> + && eh_has_tsan_func_exit != has_tsan_func_exit)
> + {
> + if (eh_has_tsan_func_exit)
> + maybe_error_musttail (call, _("call may throw exception caught "
> + "locally or perform cleanups"),
> + diag_musttail);
> + else
> + maybe_error_musttail (call, _("exception cleanups omit "
> + "__tsan_func_exit call"), diag_musttail);
> + return;
> + }
> +
> /* Move queued defs. */
> if (tail_recursion)
> {
> @@ -1138,6 +1200,7 @@ find_tail_calls (basic_block bb, struct
> nw->call_gsi = gsi;
>
> nw->tail_recursion = tail_recursion;
> + nw->has_tsan_func_exit = has_tsan_func_exit;
>
> nw->mult = m;
> nw->add = a;
> @@ -1472,6 +1535,14 @@ static bool
> optimize_tail_call (struct tailcall *t, bool opt_tailcalls,
> class loop *&new_loop)
> {
> + if (t->has_tsan_func_exit && (t->tail_recursion || opt_tailcalls))
> + {
> + tree builtin_decl = builtin_decl_implicit (BUILT_IN_TSAN_FUNC_EXIT);
> + gimple *g = gimple_build_call (builtin_decl, 0);
> + gimple_set_location (g, cfun->function_end_locus);
> + gsi_insert_before (&t->call_gsi, g, GSI_SAME_STMT);
> + }
> +
> if (t->tail_recursion)
> {
> eliminate_tail_call (t, new_loop);
> @@ -1490,6 +1561,7 @@ optimize_tail_call (struct tailcall *t,
> print_gimple_stmt (dump_file, stmt, 0, dump_flags);
> fprintf (dump_file, " in bb %i\n", (gsi_bb (t->call_gsi))->index);
> }
> + return t->has_tsan_func_exit;
> }
>
> return false;
> @@ -1539,12 +1611,23 @@ tree_optimize_tail_calls_1 (bool opt_tai
> /* Only traverse the normal exits, i.e. those that end with return
> statement. */
> if (safe_is_a <greturn *> (*gsi_last_bb (e->src)))
> - find_tail_calls (e->src, &tailcalls, only_musttail, opt_tailcalls,
> - diag_musttail);
> + {
> + int retry_tsan_func_exit = 0;
> + find_tail_calls (e->src, &tailcalls, only_musttail, opt_tailcalls,
> + diag_musttail, retry_tsan_func_exit);
> + if (retry_tsan_func_exit == -1)
> + {
> + retry_tsan_func_exit = 1;
> + find_tail_calls (e->src, &tailcalls, only_musttail,
> + opt_tailcalls, diag_musttail,
> + retry_tsan_func_exit);
> + }
> + }
> }
> if (cfun->has_musttail && diag_musttail)
> {
> basic_block bb;
> + int retry_tsan_func_exit = 0;
> FOR_EACH_BB_FN (bb, cfun)
> if (EDGE_COUNT (bb->succs) == 0
> || (single_succ_p (bb)
> @@ -1554,7 +1637,7 @@ tree_optimize_tail_calls_1 (bool opt_tai
> && gimple_call_must_tail_p (as_a <gcall *> (c))
> && gimple_call_noreturn_p (as_a <gcall *> (c)))
> find_tail_calls (bb, &tailcalls, only_musttail, opt_tailcalls,
> - diag_musttail);
> + diag_musttail, retry_tsan_func_exit);
> }
>
> if (live_vars)
> --- gcc/testsuite/c-c++-common/tsan/pr119801.c.jj 2025-04-15
> 09:48:23.759348942 +0200
> +++ gcc/testsuite/c-c++-common/tsan/pr119801.c 2025-04-15
> 09:48:23.759348942 +0200
> @@ -0,0 +1,24 @@
> +/* PR sanitizer/119801 */
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=thread" } */
> +
> +[[gnu::noipa]] int
> +bar (int *p)
> +{
> + return ++*p;
> +}
> +
> +int
> +foo (int *p)
> +{
> + ++*p;
> + [[gnu::musttail]] return bar (p);
> +}
> +
> +[[gnu::noinline]] int
> +baz (int x)
> +{
> + if (x < 10)
> + return x;
> + [[gnu::musttail]] return baz (x - 2);
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
