Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751605AbdILPyt (ORCPT ); Tue, 12 Sep 2017 11:54:49 -0400 Received: from mail-it0-f42.google.com ([209.85.214.42]:38776 "EHLO mail-it0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751441AbdILPyn (ORCPT ); Tue, 12 Sep 2017 11:54:43 -0400 X-Google-Smtp-Source: AOwi7QAsYTsGmBcgPSo8CddG730iMwz95s1r5BdeCez4XLfGjm3o3RjaSjWIUBNpTHZWuiwRcMBOFYd35pbTcrGO/SU= MIME-Version: 1.0 In-Reply-To: References: <20170912151851.GA24313@flask> From: Dmitry Vyukov Date: Tue, 12 Sep 2017 17:54:21 +0200 Message-ID: Subject: Re: "KVM: x86: generalize guest_cpuid_has_ helpers" breaks clang To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Cc: David Hildenbrand , Paolo Bonzini , LKML , KVM list , llvmlinux@lists.linuxfoundation.org, Alexander Potapenko , andreyknvl , Michael Davidson , Greg Hackmann , Nick Desaulniers Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v8CFssoH006974 Content-Length: 3586 Lines: 85 On Tue, Sep 12, 2017 at 5:51 PM, Dmitry Vyukov wrote: > On Tue, Sep 12, 2017 at 5:18 PM, Radim Krčmář 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. 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 :))