Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751659AbdILPvi (ORCPT ); Tue, 12 Sep 2017 11:51:38 -0400 Received: from mail-it0-f51.google.com ([209.85.214.51]:43756 "EHLO mail-it0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751431AbdILPvb (ORCPT ); Tue, 12 Sep 2017 11:51:31 -0400 X-Google-Smtp-Source: AOwi7QBn8f6ImgdagynB5d+w1GMAooNj3saO1tg6h5AgGDs4ft5jZ1l5b5Zk39gAFZ6g+pR8eWOGe0kOPyBj2SBEAKA= MIME-Version: 1.0 In-Reply-To: <20170912151851.GA24313@flask> References: <20170912151851.GA24313@flask> From: Dmitry Vyukov Date: Tue, 12 Sep 2017 17:51:10 +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 v8CFphqF006928 Content-Length: 3134 Lines: 78 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.