Re: [PATCH] perf test: Fix dwarf unwind for optimized builds.

[Miguel Ojeda]

On Sat, Nov 14, 2020 at 1:08 AM 'Ian Rogers' via Clang Built Linux
<clang-built-linux@xxxxxxxxxxxxxxxx> wrote:
>
> To ensure the stack frames are on the stack tail calls optimizations
> need to be inhibited. If your compiler supports an attribute use it,
> otherwise use an asm volatile barrier.
>
> The barrier fix was suggested here:
> https://lore.kernel.org/lkml/20201028081123.GT2628@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
>
> Fixes: 9ae1e990f1ab ("perf tools: Remove broken __no_tail_call
>        attribute")
> ---
>  tools/perf/tests/dwarf-unwind.c | 39 +++++++++++++++++++++++++++------
>  1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
> index 83638097c3bc..c8ce86bceea8 100644
> --- a/tools/perf/tests/dwarf-unwind.c
> +++ b/tools/perf/tests/dwarf-unwind.c
> @@ -24,6 +24,23 @@
>  /* For bsearch. We try to unwind functions in shared object. */
>  #include <stdlib.h>
>
> +/*
> + * The test will assert frames are on the stack but tail call optimizations lose
> + * the frame of the caller. Clang can disable this optimization on a called
> + * function but GCC currently (11/2020) lacks this attribute. The barrier is
> + * used to inhibit tail calls in these cases.
> + */

It would be nice to put the GCC version rather than the date.

> +#ifdef __has_attribute
> +#if __has_attribute(disable_tail_calls)
> +#define NO_TAIL_CALL_ATTRIBUTE __attribute__((disable_tail_calls))
> +#define NO_TAIL_CALL_BARRIER
> +#endif
> +#endif
> +#ifndef NO_TAIL_CALL_ATTRIBUTE
> +#define NO_TAIL_CALL_ATTRIBUTE
> +#define NO_TAIL_CALL_BARRIER __asm__ __volatile__("" : : : "memory");
> +#endif

I would try avoid this nest of conditions and instead do it like in
`compiler_attributes.h`, i.e. make use of `__has_attribute`
unconditional by making sure it works for all versions/compilers, and
then just:

    #if __has_attribute(disable_tail_calls)
    # define NO_TAIL_CALL_ATTRIBUTE __attribute__((disable_tail_calls))
    # define NO_TAIL_CALL_BARRIER
    #else
    # define NO_TAIL_CALL_ATTRIBUTE
    # define NO_TAIL_CALL_BARRIER __asm__ __volatile__("" : : : "memory");
    #endif

In fact, I think it would be best to simply have a mimic of
`compiler_attributes.h` suitable for `tools/`.

Cheers,
Miguel