From 07565115371ea5cdf9e6d75ea3777540d1d31bda Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Tue, 15 Apr 2025 14:09:55 +0200 Subject: [PATCH] tailc: Fix up musttail calls vs. -fsanitize=thread [PR119801] 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). 2025-04-15 Jakub Jelinek 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 | 2 +- gcc/testsuite/c-c++-common/tsan/pr119801.c | 24 +++++ gcc/tree-tailcall.cc | 103 +++++++++++++++++++-- 3 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/tsan/pr119801.c diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def index 4b7c9dc41e4..c5a9c2dea40 100644 --- a/gcc/sanitizer.def +++ b/gcc/sanitizer.def @@ -247,7 +247,7 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_INIT, "__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", diff --git a/gcc/testsuite/c-c++-common/tsan/pr119801.c b/gcc/testsuite/c-c++-common/tsan/pr119801.c new file mode 100644 index 00000000000..d3a6bb4549e --- /dev/null +++ b/gcc/testsuite/c-c++-common/tsan/pr119801.c @@ -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); +} diff --git a/gcc/tree-tailcall.cc b/gcc/tree-tailcall.cc index d5c4c7bf267..f593363dae4 100644 --- a/gcc/tree-tailcall.cc +++ b/gcc/tree-tailcall.cc @@ -51,6 +51,8 @@ along with GCC; see the file COPYING3. If not see #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 char *err, bool diag_musttail) 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 cnt) 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 cnt) 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 cnt) static live_vars_map *live_vars; static vec 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 tailcall **ret, bool only_musttail, 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 tailcall **ret, bool only_musttail, || 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 tailcall **ret, bool only_musttail, /* 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 tailcall **ret, bool only_musttail, 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 tailcall **ret, bool only_musttail, || 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 tailcall **ret, bool only_musttail, 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 tailcall **ret, bool only_musttail, 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, bool opt_tailcalls, 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_tailcalls, bool only_musttail, /* Only traverse the normal exits, i.e. those that end with return statement. */ if (safe_is_a (*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_tailcalls, bool only_musttail, && gimple_call_must_tail_p (as_a (c)) && gimple_call_noreturn_p (as_a (c))) find_tail_calls (bb, &tailcalls, only_musttail, opt_tailcalls, - diag_musttail); + diag_musttail, retry_tsan_func_exit); } if (live_vars)