]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
expand: Allow musttail tail calls with -fsanitize=address [PR120608]
authorJakub Jelinek <jakub@redhat.com>
Mon, 23 Jun 2025 13:58:55 +0000 (15:58 +0200)
committerJakub Jelinek <jakub@gcc.gnu.org>
Mon, 23 Jun 2025 13:58:55 +0000 (15:58 +0200)
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.

2025-06-23  Jakub Jelinek  <jakub@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
gcc/testsuite/g++.dg/asan/pr120608.C [new file with mode: 0644]

index e1cdb718e127cc50bdcbe8a158fd7b2115eb3877..33649d43f71cbd9d3e6a763fb086c9d15c5009ed 100644 (file)
@@ -75,6 +75,7 @@ along with GCC; see the file COPYING3.  If not see
 #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
@@ -4458,9 +4459,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;
@@ -4477,6 +4479,58 @@ expand_gimple_tailcall (basic_block bb, gcall *stmt, bool *can_fallthru)
   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 ();
@@ -6142,7 +6196,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;
@@ -6447,14 +6501,16 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls)
        {
          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)
@@ -7227,7 +7283,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)
diff --git a/gcc/testsuite/g++.dg/asan/pr120608.C b/gcc/testsuite/g++.dg/asan/pr120608.C
new file mode 100644 (file)
index 0000000..d06508d
--- /dev/null
@@ -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);
+}