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>] [day] [month] [year] [list]

Date:   Mon, 16 Nov 2020 12:03:14 +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>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [PATCH] perf test: Fix dwarf unwind for optimized builds.

On Mon, Nov 16, 2020 at 7:48 AM Ian Rogers <irogers@...gle.com> wrote:
>
> GCC [0-9]+ :-) Perhaps just a reference to the GCC bug rather than a date.

That would be very good.

> In linux/compiler_attributes.h add:
> #define __GCC4_has_attribute_disable_tail_calls 0
> to the #ifndef __has_attribute block. We can't do this locally here as after that #include __has_attribute will be defined.

As far as I know, `tools/` use their own `compiler*` files, which is
why I was suggesting creating the equivalent there.

> In terms of lines of code, there's not much difference. Arguably there is a bit more cognitive load from the #include and that disable_tail_call needs the funny handling that's here but won't obviously be hinted at by placing it in a shared header. I'm a little concerned that someone will come across this in shared code and then go and break this test again with well intentioned cleanup.

Fewer lines, fewer conditions :-) The `#include` is hardly important
given kernel developers already know and use compiler attributes in
many places (they are included in the majority of compilation units).

Actually, we can simplify further. The attribute itself should be
pulled from the `compiler_attributes.h` (a `tools/` one, if needed),
and the barrier should likely be the `barrier()` macro (ditto).

Then, we just need:

    #if __has_attribute(disable_tail_calls)
    # define NO_TAIL_CALL_BARRIER
    #else
    # define NO_TAIL_CALL_BARRIER barrier()
    #endif

because using the attribute directly just works -- the only special
thing here is the conditional barrier.

And this conditional barrier should probably be shared, too, defining
it wherever `barrier()` (or equivalent) is defined for `tools/`. And
the name could be `barrier_for_tail_call()` or something like that.

Of course, we don't need to do all this for this patch, but we should
always attempt to minimize/simplify the diffs later on -- that is why
I suggested using the unconditional `__has_attribute` as if it was
already properly defined (we had to untangle similar stuff when I
added `compiler_attributes.h`).

Cheers,
Miguel