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

[Richard Biener <richard.guenther@gmail.com>]

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
>