On Sat, Nov 14, 2020 at 1:08 AM 'Ian Rogers' via Clang Built Linux
<clang-built-li...@googlegroups.com> 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.gt2...@hirez.programming.kicks-ass.net/
>
> 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
