Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1730974

Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang

Show all headers | View raw

On Tue, Sep 12, 2017 at 5:18 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> 2017-09-12 16:42+0200, Dmitry Vyukov:
>> Hi Radim,
>>
>> I've just noticed that your commit "KVM: x86: generalize
>> guest_cpuid_has_ helpers" breaks clang build on this assert:
>>
>> static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned x86_feature)
>> {
>>     unsigned x86_leaf = x86_feature / 32;
>>
>>     BUILD_BUG_ON(!__builtin_constant_p(x86_leaf));
>>
>>
>> In clang __builtin_constant_p is never true for function arguments,
>> it's true only for compile-time constants (what you can use as stack
>> array size, or C++ template argument). What would work is an
>> additional macro along the lines of:
>
> GCC optimizes it thanks to __always_inline, so the x86_feature is
> constant in each instance of this function ... the goal is to have
> compile-time input checking.
>
>> #define x86_feature_cpuid(x) (BUILD_BUG_ON(!__builtin_constant_p(x),
>> __x86_feature_cpuid(x))
>>
>> But again assuming that caller pass the constant directly.
>
> The __builtin_constant_p() check is just a canary, the important ones
> are
>
>   BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
>   BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
>
> and these would be very awkward if moved out of the function.
>
>> Could you please fix it?
>
> Sure, I can just make them BUG_ON (or WARN_ON with error handling), but
> I tried with clang version 4.0.1 and got no errors -- are you using an
> older version?  (or a command other than `make HOSTCC=clang CC=clang`)


Interesting.

I use clang version 6.0.0 (trunk 313027).

I am on 8fac2f96ab86b0e14ec4e42851e21e9b518bdc55 on Linus tree. Here
is my config (which is basically defconfig + kvm enabled):
https://gist.githubusercontent.com/dvyukov/4360060ab49374b1e983312f587f1b4e/raw/2e4def6f4318bde81ab316546400513b02673bc9/gistfile1.txt
Build with: make CC=/build/bin/clang
and get:

  MODPOST vmlinux.o
arch/x86/kvm/x86.o: In function `kvm_set_apic_base':
x86.c:(.text+0x4738): undefined reference to `__compiletime_assert_62'
arch/x86/kvm/x86.o: In function `kvm_set_cr4':
x86.c:(.text+0x54ae): undefined reference to `__compiletime_assert_62'
x86.c:(.text+0x54dc): undefined reference to `__compiletime_assert_62'
x86.c:(.text+0x5509): undefined reference to `__compiletime_assert_62'
x86.c:(.text+0x5536): undefined reference to `__compiletime_assert_62'
arch/x86/kvm/x86.o:x86.c:(.text+0x5563): more undefined references to
`__compiletime_assert_62' follow
make: *** [vmlinux] Error 1


I've commented out the first BUILD_BUG_ON, and these did not cause build errors:

  BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
  BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);

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.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

"KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Dmitry Vyukov <dvyukov@google.com> - 2017-09-12 16:50 +0200
  Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Radim Krčmář <rkrcmar@redhat.com> - 2017-09-12 17:20 +0200
    Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Dmitry Vyukov <dvyukov@google.com> - 2017-09-12 18:00 +0200
      Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Paolo Bonzini <pbonzini@redhat.com> - 2017-09-12 18:10 +0200
        Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Dmitry Vyukov <dvyukov@google.com> - 2017-09-12 18:20 +0200
          Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Paolo Bonzini <pbonzini@redhat.com> - 2017-09-12 18:40 +0200
            Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Dmitry Vyukov <dvyukov@google.com> - 2017-09-12 19:40 +0200
      Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Radim Krčmář <rkrcmar@redhat.com> - 2017-09-13 13:00 +0200
    Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang Dmitry Vyukov <dvyukov@google.com> - 2017-09-12 18:00 +0200

csiph-web