linux-kernel - Re: [PATCH] workqueue: fix enum type for gcc-13

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: <a1d48420-862e-47e6-9435-462e23bb6be3@app.fastmail.com>
Date:   Wed, 18 Jan 2023 21:32:45 +0100
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Tejun Heo" <tj@...nel.org>
Cc:     "Arnd Bergmann" <arnd@...nel.org>,
        "Lai Jiangshan" <jiangshanlai@...il.com>,
        "Richard Clark" <richard.xnu.clark@...il.com>,
        Jonathan Neuschäfer <j.neuschaefer@....net>,
        "Andrey Grodzovsky" <andrey.grodzovsky@....com>,
        "Tetsuo Handa" <penguin-kernel@...ove.sakura.ne.jp>,
        "linux-kernel@...r.kernel.org," <linux-kernel@...r.kernel.org>,
        "David Laight" <David.Laight@...lab.com>
Subject: Re: [PATCH] workqueue: fix enum type for gcc-13

On Wed, Jan 18, 2023, at 17:38, Tejun Heo wrote:
>> From: Arnd Bergmann
>> > Sent: 17 January 2023 16:41
>> > 
>> > In gcc-13, the WORK_STRUCT_WQ_DATA_MASK constant is a signed 64-bit
>> > type on 32-bit architectures because the enum definition has both
>> > negative numbers and numbers above LONG_MAX in it:
>> > 
>> ...
>> >  	/* convenience constants */
>> >  	WORK_STRUCT_FLAG_MASK	= (1UL << WORK_STRUCT_FLAG_BITS) - 1,
>> > -	WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
>> > +	WORK_STRUCT_WQ_DATA_MASK = (unsigned long)~WORK_STRUCT_FLAG_MASK,
>> >  	WORK_STRUCT_NO_POOL	= (unsigned long)WORK_OFFQ_POOL_NONE << WORK_OFFQ_POOL_SHIFT,
>
> I have a hard time understanding why gcc would change its behavior so that
> there's no way to compile the same code in a consistent manner across two
> adjacent compiler versions. The new behavior is fine but it makes no sense
> to introduce it like this. If at all possible, marking gcc13 broken sounds
> about right to me.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107405 has some more
information on the change. In short, the old behavior was a gcc
extension that was somewhat surprising and not well documented,
while the new behavior is consistent with C23 and C++ as well
as easier to understand: Any constant that is defined as part of
an enum now has the same type as the enum itself, even if it fits
within a shorter type. In a definition like

enum e {
   A = -1,
   B = -1u,
};

the enum type has to be compatible with 'long long' because
anything shorter would not fit both -1 and -1u (UINT_MAX).
A and B were both signed types to match the signedness of the
enum type, but A was actually a 32-bit integer since that is
sufficient, while B was also a 64-bit type since it exceeds
INT_MAX. Now they are both the same type.

I don't think there is a chance they will revert to the old behavior,
though we could try asking for an command line flag to warn about
cases where this changes code generation.

     Arnd