Re: linux: build failure: error: "__has_attribute" is not defined

From: Nathan Chancellor <nathan@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Kees Cook <keescook@chromium.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: linux: build failure: error: "__has_attribute" is not defined
Date: Tue, 14 Sep 2021 20:45:39 -0700	[thread overview]
Message-ID: <91B93D4D-BEC8-4ACD-B71F-27C54D8B1B78@kernel.org> (raw)
In-Reply-To: <CAHk-=wiiJ47YP7Q4AJC=YSfJdY-HK-8Bh7W=+hrZRqdM2UrAFg@mail.gmail.com>

> On Sep 14, 2021, at 7:59 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> Now, the _second_ bug was then that when Nathan fixed the tooling
> header file in commit d0ee23f9d78b ("tools: compiler-gcc.h: Guard
> error attribute use with __has_attribute"), he did it the wrong way.
> 
> The gcc docs are fairly clear about how to test for __has_attibute correctly:
> 
>    https://gcc.gnu.org/onlinedocs/cpp/_005f_005fhas_005fattribute.html
> 
> and a host build environment should probably have used
> 
>  #if defined __has_attribute
>  #  if __has_attribute (error)
>  ...
> 
> and not used any version checks at all.

Ugh, yes, I had assumed that the host compiler would always equal the
compiler used for the kernel because I am so used to clang so I figured
the 'defined(__has_attribute)' was redundant but that obviously is not
true, especially when cross compiling.

> Of course, I'm not convinced it should do that __compiletime_error()
> at all, and again, I think it would be better to remove the complexity
> rather than anything else.
> 
> Anybody want to tackle those issues in
> 
>   tools/include/linux/overflow.h
>   tools/include/linux/compiler-gcc.h
> 
> and try to simplify the code?

I made the tools/include/linux/compiler-gcc.h situation worse so I am
more than happy to try to resolve it.

__compiletime_error() is ultimately used within BUILD_BUG_ON(), which
does appear to be used a couple places within the tools/ directory so I
am not sure it should be eliminated; doing so would cause BUILD_BUG_ON()
failures to be a lot more cryptic (although it is entirely possible
those are not at all common so who cares). I could be entirely wrong
though if I am misreading the code, there are a few different
BUILD_BUG_ON() defines...

If it cannot be eliminated, we could either add back the
GCC_VERSION >= 40300 check (because there does not appear to be a
minimum host version of GCC) along with my fix or we could just do the
'defined(__has_attribute)' and leave GCC 4.3 to 5.1 behind. I do not
have a strong opinion.

Cheers,
Nathan