linux-kernel - Re: [PATCH v2 4/6] compiler-gcc.h: add asm

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: <CAKwvOdk-AQVJnD6-=Z0eUQ6KPvDp2eS2zUV=-oL2K2JBCYaOeQ@mail.gmail.com>
Date:   Fri, 6 Sep 2019 16:42:58 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Segher Boessenkool <segher@...nel.crashing.org>
Cc:     Jakub Jelinek <jakub@...hat.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "gcc-patches@....gnu.org" <gcc-patches@....gnu.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>
Subject: Re: [PATCH v2 4/6] compiler-gcc.h: add asm_inline definition

On Fri, Sep 6, 2019 at 3:56 PM Segher Boessenkool
<segher@...nel.crashing.org> wrote:
>
> On Fri, Sep 06, 2019 at 03:35:02PM -0700, Nick Desaulniers wrote:
> > On Fri, Sep 6, 2019 at 3:03 PM Segher Boessenkool
> > <segher@...nel.crashing.org> wrote:
> > > And if instead you tested whether the actual feature you need works as
> > > you need it to, it would even work fine if there was a bug we fixed that
> > > breaks things for the kernel.  Without needing a new compiler.
> >
> > That assumes a feature is broken out of the gate and is putting the
> > cart before the horse.  If a feature is available, it should work.
>
> GCC currently has 91696 bug reports.

Fair.

>
> > > Or as another example, if we added support for some other flags. (x86
> > > has only a few flags; many other archs have many more, and in some cases
> > > newer hardware actually has more flags than older).
> >
> > I think compiler flags are orthogonal to GNU C extensions we're discussing here.
>
> No, I am talking exactly about what you brought up.  The flags output
> for inline assembler, using the =@ syntax.  If I had implemented that
> for Power when it first came up, I would by now have had to add support
> for another flag (the CA32 flag).  Oh, and I would not have implemented
> support for OV or SO at all originally, but perhaps they are useful, so
> let's add that as well.  And there is OV32 now as well.

Oh, I misunderstood.  I see your point.  Define the symbol as a number
for what level of output flags you support (ie. the __cplusplus
macro).

>
> > > With the "macro" scheme we would need to add new macros in all these
> > > cases.  And since those are target-specific macros, that quickly expands
> > > beyond reasonable bounds.
> >
> > I don't think so.  Can you show me an example codebase that proves me wrong?
>
> No, of course not, because we aren't silly enough to implement something
> like that.

I still don't think feature detection is worse than version detection
(until you find your feature broken in a way that forces the use of
version detection).

Just to prove my point about version checks being brittle, it looks
like Rasmus' version check isn't even right.  GCC supported `asm
inline` back in the 8.3 release, not 9.1 as in this patch:
https://godbolt.org/z/1woitS .  So users of gcc-8.2 and gcc-8.3
wouldn't be able to take advantage of `asm inline` even though their
compiler supported it.

Or was it "broken" until 9.1?  Lord knows, as `asm inline` wasn't in
any release notes or bug reports I can find:
8: https://gcc.gnu.org/gcc-8/changes.html
9: https://gcc.gnu.org/gcc-9/changes.html
Bug tracker query:
https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=SUSPENDED&bug_status=WAITING&bug_status=REOPENED&cf_known_to_fail_type=allwords&cf_known_to_work_type=allwords&query_format=advanced&short_desc=asm%20inline&short_desc_type=anywordssubstr

Ah, here it is:
https://github.com/gcc-mirror/gcc/commit/6de46ad5326fc5e6b730a2feb8c62d09c1561f92
Which looks like the qualifier was added to this page:
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html

I like many of the GNU C extensions, and I want to help support them
in Clang so that they can be used in more places, but the current
process (and questions I have when I implement some of them) leaves me
with the sense that there's probably room for improvement on some of
these things before they go out the door.

Segher, next time there's discussion about new C extensions for the
kernel, can you please include me in the discussions?
-- 
Thanks,
~Nick Desaulniers