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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANiq72m=OK2bF2Nc-ht=ibNa2m6RcBCjFuhrv9kyoxE6yaMqVA@mail.gmail.com>
Date:   Sat, 14 Nov 2020 09:45:47 +0100
From:   Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To:     Ian Rogers <irogers@...gle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        Ard Biesheuvel <ardb@...nel.org>,
        Miguel Ojeda <ojeda@...nel.org>,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH] perf test: Fix dwarf unwind for optimized builds.

On Sat, Nov 14, 2020 at 1:08 AM 'Ian Rogers' via Clang Built Linux
<clang-built-linux@...glegroups.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.GT2628@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