[PATCH] do not tailcall __sanitizer_cov_trace_pc [PR90746]

[PATCH] do not tailcall __sanitizer_cov_trace_pc [PR90746]

[Alexander Monakov]

When instrumentation is requested via -fsanitize-coverage=trace-pc, GCC
emits calls to __sanitizer_cov_trace_pc callback into each basic block.
This callback is supposed to be implemented by the user, and should be
able to identify the containing basic block by inspecting its return
address. Tailcalling the callback prevents that, so disallow it.

gcc/ChangeLog:

	PR sanitizer/90746
	* calls.cc (can_implement_as_sibling_call_p): Reject calls
	to __sanitizer_cov_trace_pc.

gcc/testsuite/ChangeLog:

	PR sanitizer/90746
	* gcc.dg/sancov/basic0.c: Verify absence of tailcall.
---
 gcc/calls.cc                         | 10 ++++++++++
 gcc/testsuite/gcc.dg/sancov/basic0.c |  4 +++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/gcc/calls.cc b/gcc/calls.cc
index 4d7f6c3d2..c6ed2f189 100644
--- a/gcc/calls.cc
+++ b/gcc/calls.cc
@@ -2541,6 +2541,16 @@ can_implement_as_sibling_call_p (tree exp,
       return false;
     }
 
+  /* __sanitizer_cov_trace_pc is supposed to inspect its return address
+     to identify the caller, and therefore should not be tailcalled.  */
+  if (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
+      && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_SANITIZER_COV_TRACE_PC)
+    {
+      /* No need for maybe_complain_about_tail_call here: the call
+         is synthesized by the compiler.  */
+      return false;
+    }
+
   /* If the called function is nested in the current one, it might access
      some of the caller's arguments, but could clobber them beforehand if
      the argument areas are shared.  */
diff --git a/gcc/testsuite/gcc.dg/sancov/basic0.c b/gcc/testsuite/gcc.dg/sancov/basic0.c
index af69b2d12..dfdaea848 100644
--- a/gcc/testsuite/gcc.dg/sancov/basic0.c
+++ b/gcc/testsuite/gcc.dg/sancov/basic0.c
@@ -1,9 +1,11 @@
 /* Basic test on number of inserted callbacks.  */
 /* { dg-do compile } */
-/* { dg-options "-fsanitize-coverage=trace-pc -fdump-tree-optimized" } */
+/* { dg-options "-fsanitize-coverage=trace-pc -fdump-tree-optimized -fdump-rtl-expand" } */
 
 void foo(void)
 {
 }
 
 /* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_pc \\(\\)" 1 "optimized" } } */
+/* The built-in should not be tail-called: */
+/* { dg-final { scan-rtl-dump-not "call_insn/j" "expand" } } */
-- 
2.39.2

Re: [PATCH] do not tailcall __sanitizer_cov_trace_pc [PR90746]

[Richard Biener]

On Tue, May 2, 2023 at 4:45 PM Alexander Monakov via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> When instrumentation is requested via -fsanitize-coverage=trace-pc, GCC
> emits calls to __sanitizer_cov_trace_pc callback into each basic block.
> This callback is supposed to be implemented by the user, and should be
> able to identify the containing basic block by inspecting its return
> address. Tailcalling the callback prevents that, so disallow it.

LGTM

> gcc/ChangeLog:
>
>         PR sanitizer/90746
>         * calls.cc (can_implement_as_sibling_call_p): Reject calls
>         to __sanitizer_cov_trace_pc.
>
> gcc/testsuite/ChangeLog:
>
>         PR sanitizer/90746
>         * gcc.dg/sancov/basic0.c: Verify absence of tailcall.
> ---
>  gcc/calls.cc                         | 10 ++++++++++
>  gcc/testsuite/gcc.dg/sancov/basic0.c |  4 +++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/calls.cc b/gcc/calls.cc
> index 4d7f6c3d2..c6ed2f189 100644
> --- a/gcc/calls.cc
> +++ b/gcc/calls.cc
> @@ -2541,6 +2541,16 @@ can_implement_as_sibling_call_p (tree exp,
>        return false;
>      }
>
> +  /* __sanitizer_cov_trace_pc is supposed to inspect its return address
> +     to identify the caller, and therefore should not be tailcalled.  */
> +  if (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
> +      && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_SANITIZER_COV_TRACE_PC)
> +    {
> +      /* No need for maybe_complain_about_tail_call here: the call
> +         is synthesized by the compiler.  */
> +      return false;
> +    }
> +
>    /* If the called function is nested in the current one, it might access
>       some of the caller's arguments, but could clobber them beforehand if
>       the argument areas are shared.  */
> diff --git a/gcc/testsuite/gcc.dg/sancov/basic0.c b/gcc/testsuite/gcc.dg/sancov/basic0.c
> index af69b2d12..dfdaea848 100644
> --- a/gcc/testsuite/gcc.dg/sancov/basic0.c
> +++ b/gcc/testsuite/gcc.dg/sancov/basic0.c
> @@ -1,9 +1,11 @@
>  /* Basic test on number of inserted callbacks.  */
>  /* { dg-do compile } */
> -/* { dg-options "-fsanitize-coverage=trace-pc -fdump-tree-optimized" } */
> +/* { dg-options "-fsanitize-coverage=trace-pc -fdump-tree-optimized -fdump-rtl-expand" } */
>
>  void foo(void)
>  {
>  }
>
>  /* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_pc \\(\\)" 1 "optimized" } } */
> +/* The built-in should not be tail-called: */
> +/* { dg-final { scan-rtl-dump-not "call_insn/j" "expand" } } */
> --
> 2.39.2
>