On Mon, 22 Jan 2024 at 16:46, Kees Cook <keesc...@chromium.org> wrote:
>
> Refactor open-coded unsigned wrap-around addition test to use
> check_add_overflow(),
NAK.
First off, none of this has anything to do with -fno-strict-overflow.
We do that, because without it gcc ends up doing various odd and
surprising things, the same way it does with strict-aliasing.
IOW, you should think of -fno-strict-overflow as a hardening thing.
Any optimization that depends on "this can overflow, so I can do
anything I want" is just a dangerous optimization for the kernel.
It matches -fno-strict-aliasing and -fno-delete-null-pointer-checks,
in other words.
And I do not understand why you mention it in the first place, since
this code USES UNSIGNED INTEGER ARITHMETIC, and thus has absolutely
nothing to do with that no-strict-overflow flag.
So the commit message is actively misleading and broken. Unsigned
arithmetic has very well-defined behavior, and the code uses that with
a very traditional and valid test.
The comment about "redundant open-coded addition" is also PURE
GARBAGE, since the compiler will trivially do the CSE - and on the
source code level your modified code is actively bigger and uglier.
So your patch improves neither code generation or source code.
And if there's some unsigned wrap-around checker that doesn't
understand this traditional way of doing overflow checking, that piece
of crap needs fixing.
I don't want to see mindless conversion patches that work around some
broken tooling.
I want to see them even less when pretty much EVERY SINGLE WORD in the
commit message seems to be actively misleading and irrelevant garbage.
Stop making the world a worse place.
Linus
