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: <37e7ef6d-3a92-42ab-a949-59c18bccc510@app.fastmail.com>
Date:   Fri, 23 Jun 2023 21:21:44 +0200
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Linus Torvalds" <torvalds@...ux-foundation.org>,
        "Dave Airlie" <airlied@...il.com>,
        "Nathan Chancellor" <nathan@...nel.org>,
        "Nick Desaulniers" <ndesaulniers@...gle.com>
Cc:     "Tejun Heo" <tj@...nel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: arm32 build warnings in workqueue.c

On Fri, Jun 23, 2023, at 20:52, Linus Torvalds wrote:
> [ Adding clang people. See this for background:
>
>     
> https://lore.kernel.org/all/CAHk-=wi=eDN4Ub0qSN27ztBAvHSXCyiY2stu3_XbTpYpQX4x7w@mail.gmail.com/
>
>   where that patch not only cleans things up, but seems to make a
> difference to clang ]
>
> On Fri, 23 Jun 2023 at 11:24, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>>
>> So I really think that code needs fixing, and the gcc warning was very valid.
>>
>> Maybe something like the attached. Does this fix the gcc warning?
>> Tejun, comments?
>
> Whee. Inexplicably, that patch also improves code generation with
> clang, with things like this:
>
> -       movabsq $137438953440, %rcx             # imm = 0x1FFFFFFFE0
> -       andq    %rax, %rcx
> -       movabsq $68719476704, %rdx              # imm = 0xFFFFFFFE0
> -       cmpq    %rdx, %rcx
> +       shrq    $5, %rax
> +       cmpl    $2147483647, %eax               # imm = 0x7FFFFFFF
>
> in several places.
>
> Or, even more amusingly, this:
>
> -       movabsq $68719476704, %rax              # imm = 0xFFFFFFFE0
> -       orq     $1, %rax
> +       movabsq $68719476705, %rax              # imm = 0xFFFFFFFE1
>
> where the old code was some truly crazy stuff.
>
> I have *no* idea what drugs clang is on, but clearly clang does some
> really really bad things with large enums, and doesn't simplify things
> correctly.

I sent a fix for the warning earlier this year, and it's been
in linux-next for a while but hasn't made it to your tree yet,
see https://lore.kernel.org/all/20230117164041.1207412-1-arnd@kernel.org/
for the initial submission.

I went with the minimal change there, your more elaborate version
looks fine as well. There were a handful of bugs that came up with
the changed behavior, in other cases they caused the code to actually
misbehave rather than just causing a build warning.

After having looked at all the other instances, I still feel that
the new gcc behavior is the sanest way to handle this, but it was
not communicated well in the gcc-13 release notes, and I would have
preferred to see a warning for source code that had a change in code
generation because of this.

The short explanation of the change is that with the previous
gcc and clang behavior, the type of 'enum foo' would be determined
separately from the type of each individual constant, while the
new behavior in gcc-13 makes them all have the same type.
IIRC with a definition of 

enum {
    A = -1,
    B = UINT_MAX,
} var;

you had 'typeof(A)' as an 'int', 'typeof(B)' as an 'unsigned int'
and 'typeof(var)' as a 'long long', now they are all 'long long'.

     Arnd