Return-Path: Received: from mail-lj1-f195.google.com ([209.85.208.195]:39089 "EHLO mail-lj1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727429AbeKBFTy (ORCPT ); Fri, 2 Nov 2018 01:19:54 -0400 MIME-Version: 1.0 References: <20181022105944.GA1411@gmail.com> In-Reply-To: From: Miguel Ojeda Date: Thu, 1 Nov 2018 21:15:12 +0100 Message-ID: Subject: Re: [GIT PULL] Compiler Attributes for v4.20-rc1 To: Linus Torvalds Cc: Andrey Ryabinin , Dan , Andreas Dilger , Masahiro Yamada , Michal Marek , Steven Rostedt , Mauro Carvalho Chehab , Olof Johansson , Konstantin Ryabitsev , David Miller , Kees Cook , Thomas Gleixner , Ingo Molnar , Paul Lawrence , Sandipan Das , Andrey Konovalov , David Woodhouse , Will Deacon , Philippe Ombredanne , Paul Burton , David Rientjes , Willy Tarreau , Martin Sebor , Christopher Li , Jonathan Corbet , "Ted Ts'o" , Geert Uytterhoeven , Rasmus Villemoes , Joe Perches , Arnd Bergmann , Dominique Martinet , Stefan Agner , Luc Van Oostenryck , Nick Desaulniers , Andrew Morton , Greg KH , Linux Doc Mailing List , Ext4 Developers List , linux-sparse@vger.kernel.org, linux-kbuild@vger.kernel.org, gor@linux.ibm.com, schwidefsky@de.ibm.com Content-Type: text/plain; charset="UTF-8" Sender: linux-ext4-owner@vger.kernel.org List-ID: On Thu, Nov 1, 2018 at 6:06 PM Linus Torvalds wrote: > > I'm not sure how much that matters (maybe the original check for 4.9.2 > was just a random pick by Andrey? Added to cc), but together with the > movement to that looks like it also > wouldn't want the CONFIG_KASAN tests, I wonder what the right merge > resolution would be. Good catch. I don't recall any special logic when I did that change, so most likely I simply did like for the rest of the attributes and took a look at when it was first supported (and documentation in gcc's docs) in order to implement __has_attribute by hand. But indeed, it *may* be that there is an (undocumented) problem between 4.8 <= gcc < 4.9.2 with it. If so, we should document it down and fix it. Andrey? > Yes, I see the resolution in linux-next, and I think that one is odd > and dubious, and now it *mixes* that old test of gcc-4.9.2 with the > different test in compiler-attributes. I missed that conflict completely, my bad (I did not miss all of them, at least; one required fixing). Hm.... at a quick look, why is it only on compiler-gcc.h? It should either have a corresponding #define elsewhere or just be put directly in another common header, no? (Adding Vasily & Martin to CC.) > But I'm also unsure whether you meant for the "__has_attribute()" test > to be usable outside the linux/compiler_attributes.h file, in which > case I could just do > > #if defined(CONFIG_KASAN) && __has_attribute(__no_sanitize_address__) > > instead. I think that (using __has_attribute() outside) may be a good idea: I wanted to keep compiler_attributes.h as simple as possible by avoiding #ifdefs inside that header (except for __has_attribute itself), as an attempt to avoid going back to the mess of #ifdefs we had previously. Basically, keeping the attributes in compiler_attributes.h that do not depend on complex logic. So using __has_attribute *outside* the header actually goes well with that principle, because it helps keeping stuff out of it if they depend on other config options; without having to rely on GCC_VERSION either. [By the way, in case it clarifies: note that "optional" in that file actually is a bit of a misnomer. I meant to say "optional" as in "not supported by all compilers, so conditionally defined" ("optionally" defined); rather than "optional" in the sense of "code still works without the attribute". It caught Rasmus in one of his patches sent a few days ago on top of this tree, so I want to change it or explain it to avoid confusion.] Cheers, Miguel