linux-kernel - Re: [PATCH] bug: Fix no-return-statement warning with !CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a87bed5b-edb7-4ba2-bdd1-88fcd1da7b69@csgroup.eu>
Date: Mon, 15 Apr 2024 17:07:15 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Arnd Bergmann <arnd@...db.de>, Michael Ellerman <mpe@...erman.id.au>,
	Adrian Hunter <adrian.hunter@...el.com>
CC: Peter Zijlstra <peterz@...radead.org>, Dave Hansen
	<dave.hansen@...ux.intel.com>, John Stultz <jstultz@...gle.com>, "H. Peter
 Anvin" <hpa@...or.com>, Alexander Gordeev <agordeev@...ux.ibm.com>, Vincenzo
 Frascino <vincenzo.frascino@....com>, "linux-s390@...r.kernel.org"
	<linux-s390@...r.kernel.org>, Naresh Kamboju <naresh.kamboju@...aro.org>,
	"x86@...nel.org" <x86@...nel.org>, Aneesh Kumar K.V
	<aneesh.kumar@...nel.org>, Ingo Molnar <mingo@...hat.com>, "Naveen N. Rao"
	<naveen.n.rao@...ux.ibm.com>, Christian Borntraeger
	<borntraeger@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>, Heiko
 Carstens <hca@...ux.ibm.com>, Nicholas Piggin <npiggin@...il.com>, Borislav
 Petkov <bp@...en8.de>, Andy Lutomirski <luto@...nel.org>, Bjorn Helgaas
	<bhelgaas@...gle.com>, Thomas Gleixner <tglx@...utronix.de>, Anna-Maria
 Gleixner <anna-maria@...utronix.de>, Stephen Boyd <sboyd@...nel.org>, Randy
 Dunlap <rdunlap@...radead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Sven Schnelle <svens@...ux.ibm.com>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH] bug: Fix no-return-statement warning with !CONFIG_BUG



Le 15/04/2024 à 17:35, Arnd Bergmann a écrit :
> On Mon, Apr 15, 2024, at 04:19, Michael Ellerman wrote:
>> "Arnd Bergmann" <arnd@...db.de> writes:
>>> On Thu, Apr 11, 2024, at 11:27, Adrian Hunter wrote:
>>>> On 11/04/24 11:22, Christophe Leroy wrote:
>>>>
>>>> That is fragile because it depends on defined(__OPTIMIZE__),
>>>> so it should still be:
>>>
>>> If there is a function that is defined but that must never be
>>> called, I think we are doing something wrong.
>>
>> It's a pretty inevitable result of using IS_ENABLED(), which the docs
>> encourage people to use.
> 
> Using IS_ENABLED() is usually a good idea, as it helps avoid
> adding extra #ifdef checks and just drops static functions as
> dead code, or lets you call extern functions that are conditionally
> defined in a different file.
> 
> The thing is that here it does not do either of those and
> adds more complexity than it avoids.
> 
>> In this case it could easily be turned into a build error by just making
>> it an extern rather than a static inline.
>>
>> But I think Christophe's solution is actually better, because it's more
>> explicit, ie. this function should not be called and if it is that's a
>> build time error.
> 
> I haven't seen a good solution here. Ideally we'd just define
> the functions unconditionally and have IS_ENABLED() take care
> of letting the compiler drop them silently, but that doesn't
> build because of missing struct members.
> 
> I won't object to either an 'extern' declaration or the
> 'BUILD_BUG_ON()' if you and others prefer that, both are better
> than BUG() here. I still think my suggestion would be a little
> simpler.

The advantage of the BUILD_BUG() against the extern is that the error 
gets detected at buildtime. With the extern it gets detected only at 
link-time.

But agree with you, the missing struct members defeats the advantages of 
IS_ENABLED().

At the end, how many instances of struct timekeeper do we have in the 
system ? With a quick look I see only two instances: tkcore.timekeeper 
and shadow_timekeeper. If I'm correct, wouldn't it just be simpler to 
have the three debug struct members defined at all time ?

Christophe