On Thu, 12 Jun 2025, Jakub Jelinek wrote:
> Hi!
>
> The following testcase is rejected by GCC 15 but accepted (with
> s/gnu/clang/) by clang.
> The problem is that we want to execute a sequence of instructions to
> unpoison all automatic variables in the function and mark the var block
> allocated for use-after-return sanitization poisoned after the call,
> so we were just disabling tail calls if there are any instructions
> returned from asan_emit_stack_protection.
> It is fine and necessary for normal tail calls, but for musttail
> tail calls we actually document that accessing the automatic vars of
> the caller is UB as if they end their lifetime right before the tail
> call, so we also want address sanitizer user-after-return to diagnose
> that.
>
> The following patch will only disable normal tail calls when that sequence
> is present, for musttail it will arrange to emit a copy of that sequence
> before the tail call sequence. That sequence only tweaks the shadow memory
> and nothing in the code emitted by call expansion should touch the shadow
> memory, so it is ok to emit it already before argument setup.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
> later 15.2?
OK.
Thanks,
Richard.
> 2025-06-12 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/120608
> * cfgexpand.cc: Include rtl-iter.h.
> (expand_gimple_tailcall): Add ASAN_EPILOG_SEQ argument, if non-NULL
> and expand_gimple_stmt emitted a tail call, emit a copy of that
> insn sequence before the call sequence.
> (expand_gimple_basic_block): Remove DISABLE_TAIL_CALLS argument, add
> ASAN_EPILOG_SEQ argument. Disable tail call flag only on non-musttail
> calls if that flag is set, pass it to expand_gimple_tailcall.
> (pass_expand::execute): Pass VAR_RET_SEQ directly as last
> expand_gimple_basic_block argument rather than its comparison with
> NULL.
>
> * g++.dg/asan/pr120608.C: New test.
>
> --- gcc/cfgexpand.cc.jj 2025-06-12 10:05:14.961227842 +0200
> +++ gcc/cfgexpand.cc 2025-06-12 11:38:06.875469134 +0200
> @@ -75,6 +75,7 @@ along with GCC; see the file COPYING3.
> #include "builtins.h"
> #include "opts.h"
> #include "gimple-range.h"
> +#include "rtl-iter.h"
>
> /* Some systems use __main in a way incompatible with its use in gcc, in
> these
> cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN
> to
> @@ -4434,9 +4435,10 @@ expand_gimple_stmt (gimple *stmt)
> tailcall) and the normal result happens via a sqrt instruction. */
>
> static basic_block
> -expand_gimple_tailcall (basic_block bb, gcall *stmt, bool *can_fallthru)
> +expand_gimple_tailcall (basic_block bb, gcall *stmt, bool *can_fallthru,
> + rtx_insn *asan_epilog_seq)
> {
> - rtx_insn *last2, *last;
> + rtx_insn *last2, *last, *first = get_last_insn ();
> edge e;
> edge_iterator ei;
> profile_probability probability;
> @@ -4453,6 +4455,58 @@ expand_gimple_tailcall (basic_block bb,
> return NULL;
>
> found:
> +
> + if (asan_epilog_seq)
> + {
> + /* We need to emit a copy of the asan_epilog_seq before
> + the insns emitted by expand_gimple_stmt above. The sequence
> + can contain labels, which need to be remapped. */
> + hash_map<rtx, rtx> label_map;
> + start_sequence ();
> + emit_note (NOTE_INSN_DELETED);
> + for (rtx_insn *insn = asan_epilog_seq; insn; insn = NEXT_INSN (insn))
> + switch (GET_CODE (insn))
> + {
> + case INSN:
> + case CALL_INSN:
> + case JUMP_INSN:
> + emit_copy_of_insn_after (insn, get_last_insn ());
> + break;
> + case CODE_LABEL:
> + label_map.put ((rtx) insn, (rtx) emit_label (gen_label_rtx ()));
> + break;
> + case BARRIER:
> + emit_barrier ();
> + break;
> + default:
> + gcc_unreachable ();
> + }
> + for (rtx_insn *insn = get_insns (); insn; insn = NEXT_INSN (insn))
> + if (JUMP_P (insn))
> + {
> + subrtx_ptr_iterator::array_type array;
> + FOR_EACH_SUBRTX_PTR (iter, array, &PATTERN (insn), ALL)
> + {
> + rtx *loc = *iter;
> + if (LABEL_REF_P (*loc))
> + {
> + rtx *lab = label_map.get ((rtx) label_ref_label (*loc));
> + gcc_assert (lab);
> + set_label_ref_label (*loc, as_a <rtx_insn *> (*lab));
> + }
> + }
> + if (JUMP_LABEL (insn))
> + {
> + rtx *lab = label_map.get (JUMP_LABEL (insn));
> + gcc_assert (lab);
> + JUMP_LABEL (insn) = *lab;
> + }
> + }
> + asan_epilog_seq = NEXT_INSN (get_insns ());
> + end_sequence ();
> + emit_insn_before (asan_epilog_seq, NEXT_INSN (first));
> + }
> +
> /* ??? Wouldn't it be better to just reset any pending stack adjust?
> Any instructions emitted here are about to be deleted. */
> do_pending_stack_adjust ();
> @@ -6118,7 +6172,7 @@ reorder_operands (basic_block bb)
> /* Expand basic block BB from GIMPLE trees to RTL. */
>
> static basic_block
> -expand_gimple_basic_block (basic_block bb, bool disable_tail_calls)
> +expand_gimple_basic_block (basic_block bb, rtx_insn *asan_epilog_seq)
> {
> gimple_stmt_iterator gsi;
> gimple *stmt = NULL;
> @@ -6423,14 +6477,16 @@ expand_gimple_basic_block (basic_block b
> {
> gcall *call_stmt = dyn_cast <gcall *> (stmt);
> if (call_stmt
> + && asan_epilog_seq
> && gimple_call_tail_p (call_stmt)
> - && disable_tail_calls)
> + && !gimple_call_must_tail_p (call_stmt))
> gimple_call_set_tail (call_stmt, false);
>
> if (call_stmt && gimple_call_tail_p (call_stmt))
> {
> bool can_fallthru;
> - new_bb = expand_gimple_tailcall (bb, call_stmt, &can_fallthru);
> + new_bb = expand_gimple_tailcall (bb, call_stmt, &can_fallthru,
> + asan_epilog_seq);
> if (new_bb)
> {
> if (can_fallthru)
> @@ -7207,7 +7263,7 @@ pass_expand::execute (function *fun)
> head_end_for_bb.create (last_basic_block_for_fn (fun));
> FOR_BB_BETWEEN (bb, init_block->next_bb, EXIT_BLOCK_PTR_FOR_FN (fun),
> next_bb)
> - bb = expand_gimple_basic_block (bb, var_ret_seq != NULL_RTX);
> + bb = expand_gimple_basic_block (bb, var_ret_seq);
> disable_ranger (fun);
> FOR_BB_BETWEEN (bb, init_block->next_bb, EXIT_BLOCK_PTR_FOR_FN (fun),
> next_bb)
> --- gcc/testsuite/g++.dg/asan/pr120608.C.jj 2025-06-12 12:39:35.063565552
> +0200
> +++ gcc/testsuite/g++.dg/asan/pr120608.C 2025-06-12 12:44:15.431004623
> +0200
> @@ -0,0 +1,17 @@
> +// PR middle-end/120608
> +// { dg-do compile { target musttail } }
> +// { dg-options "-fsanitize=address" }
> +// { dg-skip-if "" { *-*-* } { "*" } { "-O0" } }
> +
> +struct A;
> +struct B { unsigned long b; };
> +typedef const char *(*F) (void *u, const char *v, void *w, const struct A *x,
> + unsigned long y, struct B z);
> +struct A { F a; };
> +
> +const char *
> +foo (void *u, const char *v, void *w, const struct A *x, unsigned long y,
> + struct B z)
> +{
> + [[gnu::musttail]] return x->a (u, v, w, x, y, z);
> +}
>
> 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)
