linux-kernel - Re: "KVM: x86: generalize guest_cpuid_has

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: <CACT4Y+bRNVv9gcTcK+XCZROawa5pmiDBpb9fAQUhHm1Vej5Tfw@mail.gmail.com>
Date:   Tue, 12 Sep 2017 18:16:27 +0200
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Radim Krčmář <rkrcmar@...hat.com>,
        David Hildenbrand <david@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        KVM list <kvm@...r.kernel.org>,
        llvmlinux@...ts.linuxfoundation.org,
        Alexander Potapenko <glider@...gle.com>,
        andreyknvl <andreyknvl@...gle.com>,
        Michael Davidson <md@...gle.com>,
        Greg Hackmann <ghackmann@...gle.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang

On Tue, Sep 12, 2017 at 6:03 PM, Paolo Bonzini <pbonzini@...hat.com> wrote:
> On 12/09/2017 17:54, Dmitry Vyukov wrote:
>>> I guess clang still eliminates dead branches. Clang optimizer does
>>> know that these are constant, it just does not allow build
>>> success/failure nor runtime behavior depend on optimization level and
>>> compiler version. I.e. with gcc you can get build failure with only
>>> some compiler flags and/or compiler versions. Clang gives stable
>>> result. But the optimizer does use constant propagation, etc during
>>> optimization.
>
> I can reproduce it:
>
> $ cat f.c
> int bad_code();
>
> static inline void __attribute__((always_inline)) f(int x)
> {
>         if (!__builtin_constant_p(x))
>                 bad_code();
> }
>
> int main()
> {
>         f(0);
>         f(1);
>         f(100);
> }
>
> $ clang --version
> clang version 4.0.0 (tags/RELEASE_400/final)
> $ clang f.c -O2 -c -o f.o
> $ nm f.o
>                  U bad_code
> 0000000000000000 T main
>
> $ gcc f.c -O2 -c -o f.o
> $ nm f.o
> 0000000000000000 T main
>
> ... but I don't know, it seems very weird.  The purpose of
> __builtin_constant_p is to be resolved only relatively late in the
> optimization pipeline, and it has been like this for at least 15 years
> in GCC.
>
> The docs say what to expect:
>
>   You may use this built-in function in either a macro or an inline
>   function. However, if you use it in an inlined function and pass an
>   argument of the function as the argument to the built-in, GCC never
>   returns 1 when you call the inline function with a string constant or
>   compound literal (see Compound Literals) and does not return 1 when
>   you pass a constant numeric value to the inline function **unless you
>   specify the -O option**.
>
> (emphasis mine).


Yes, I know. This difference was surprising for me and lots of other
people as well. But this is a fundamental position for clang and is
related to some implementation choices. Namely, C/C++ frontend needs
to know values of compile-time const expressions in order to verify
correctness and generate middle-end representation. But for gcc's
meaning of __builtin_constant_p, its value only becomes known deep
inside of middle-end. Which kinda creates a cycle. In gcc it's all
somehow mixed together (front-end/middle-end) and somehow works. Can't
possibly work for clang with strict separation between front-end and
middle-end.

I proposed to introduce another builtin that returns a value that is
constant from optimizer point of view (e.g. it can eliminate dead code
on branches), but is not constant from language/front-end point of
view (e.g. you can't declare a stack array using the value as size).
It should do in such cases and should be implementable in clang. But
we don't have it yet, and again it's not __builtin_constant_p, because
gcc's __builtin_constant_p returns a compile-time constant. So we need
to deal with this situation somehow.


>> I've installed clang-3.9 (the closest version to yours my distribution
>> gives me) and still got the same error with it. I would expect that
>> 4.0 should give the same result as well... Are you sure you enabled
>> KVM/intel/amd? (yes, I know you are maintaining KVM code :))
>
> Heh, I don't know about Radim but "^Rmake" goes straight to "make
> arch/x86/kvm/" for me. :)
>
> Paolo