linux-kernel - Re: arm32 build warnings in workqueue.c

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: <ZJZFiupBPBrl9mL_@slm.duckdns.org>
Date:   Fri, 23 Jun 2023 15:23:22 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Dave Airlie <airlied@...il.com>, Arnd Bergmann <arnd@...db.de>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: arm32 build warnings in workqueue.c

Hello, Linus.

On Fri, Jun 23, 2023 at 03:15:51PM -0700, Linus Torvalds wrote:
> On Fri, 23 Jun 2023 at 12:48, Tejun Heo <tj@...nel.org> wrote:
> > The behavior change in gcc-13 is fine on its own but it's frustrating
> > because there's no way to obtain previous behavior
> 
> No, the previous gcc behavior was just buggy and wrong.
> 
> I'm actually surprised at what gcc did. It means that each enum, has a
> different type. That's just completely wrong, and means that there is
> no such thing as a proper enum type.

I agree that the previous behavior is out there. It's just that it's been
like that forever and that's been the only way to make constants externally
visible.

> And the types are entirely random. They are *not* the types of the
> initializers. Try this:
> 
>     enum {
>         A = (char) 1,
>         B = (unsigned long) 2,
>         C = (long long) 3,
>         D = (unsigned long) 0x123456789,
>     };
> 
>     int tA(void) { return sizeof(A); }
>     int tB(void) { return sizeof(B); }
>     int tC(void) { return sizeof(C); }
>     int tD(void) { return sizeof(D); }
>     int T(void)  { return sizeof(enum bad); }
>     int crazy(void)  { return sizeof(typeof(A)); }
> 
> and look at the insanity when you compile it with "gcc -S".

Oh yeah, the sizes make no senes.

> So no. There is NO WAY we want to ever "obtain previous behavior".
> That is just garbage. Any code that depends on that behavior is just
> actively wrong.

Nothing is willingly depending on the crazy type behavior. However, enums
are the only thing we've got if we want to make the constant values visible
externally, so we have to make do with them somehow. It has always been iffy
but also mostly bearable. Recent round of problems stem from the fact that
gcc-13 changed the behavior and it sometimes gets nasty to satisfy both
interpretations with the same code. Hypothetically, if we were to declare
that we no longer supported old craziness, cleaning things up would be
straight-forward.

That day will come in the future but, in the meantime, we have to deal with
enums whose sizes are going to be different depending on the complier
version. It's pretty unpleasant but likely manageable in practice. The only
way I can think of to keep the before and after behaviors the same is
defining every enum in its own enum block. ie. Explicitly force each to be
its own type. With a macro helper, code would probably look okay and we'd be
explicitly telling the compiler to pick the type that can contain the value
while maintaining signedness. That would give us consistent behaviors across
the gcc-12/13 boundary. It sure doesn't feel great tho.

Unless we declare that we don't care about external visibility and are done
with enums, none of the available options aren't great, which isn't too
surprising given the sad state enums have been in.

Thanks.

-- 
tejun