linux-kernel - Re: Buiild error in i915/xe (was: [PATCH next 4/7] minmax.h: Use BUILD_BUG_ON

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: <CAKbZUD3smtH8YA5jZgg42UP1tHVfSQvVLTfgNuAoKfFsVEXRxg@mail.gmail.com>
Date: Sat, 18 Jan 2025 23:24:02 +0000
From: Pedro Falcato <pedro.falcato@...il.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, Guenter Roeck <linux@...ck-us.net>, 
	David Laight <David.Laight@...lab.com>, Arnd Bergmann <arnd@...nel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Jens Axboe <axboe@...nel.dk>, 
	Matthew Wilcox <willy@...radead.org>, Christoph Hellwig <hch@...radead.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, 
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
	Dan Carpenter <dan.carpenter@...aro.org>, "Jason A . Donenfeld" <Jason@...c4.com>, 
	Mateusz Guzik <mjguzik@...il.com>, "linux-mm@...ck.org" <linux-mm@...ck.org>, 
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, intel-xe@...ts.freedesktop.org, 
	intel-gfx@...ts.freedesktop.org, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Jani Nikula <jani.nikula@...ux.intel.com>, 
	Rodrigo Vivi <rodrigo.vivi@...el.com>
Subject: Re: Buiild error in i915/xe (was: [PATCH next 4/7] minmax.h: Use
 BUILD_BUG_ON_MSG() for the lo < hi test in clamp())

On Sat, Jan 18, 2025 at 10:11 PM David Laight
<david.laight.linux@...il.com> wrote:
>
> On Sat, 18 Jan 2025 13:21:39 -0800
> Linus Torvalds <torvalds@...ux-foundation.org> wrote:
>
> > On Sat, 18 Jan 2025 at 09:49, Guenter Roeck <linux@...ck-us.net> wrote:
> > >
> > > No idea why the compiler would know that the values are invalid.
> >
> > It's not that the compiler knows tat they are invalid, but I bet what
> > happens is in scale() (and possibly other places that do similar
> > checks), which does this:
> >
> >         WARN_ON(source_min > source_max);
> >         ...
> >         source_val = clamp(source_val, source_min, source_max);
> >
> > and the compiler notices that the ordering comparison in the first
> > WARN_ON() is the same as the one in clamp(), so it basically converts
> > the logic to
> >
> >         if (source_min > source_max) {
> >                 WARN(..);
> >                 /* Do the clamp() knowing that source_min > source_max */
> >                 source_val = clamp(source_val, source_min, source_max);
> >         } else {
> >                 /* Do the clamp knowing that source_min <= source_max */
> >                 source_val = clamp(source_val, source_min, source_max);
> >         }
> >
> > (obviously I dropped the other WARN_ON in the conversion, it wasn't
> > relevant for this case).
> >
> > And now that first clamp() case is done with source_min > source_max,
> > and it triggers that build error because that's invalid.
> >
> > So the condition is not statically true in the *source* code, but in
> > the "I have moved code around to combine tests" case it now *is*
> > statically true as far as the compiler is concerned.
>
> Well spotted :-)
>
> One option would be to move the WARN_ON() below the clamp() and
> add an OPTIMISER_HIDE_VAR(source_max) between them.
>
> Or do something more sensible than the WARN().

Given the awful problems we've found with these macros (clamp, min,
max, etc), maybe the best option is to not play these awful games in
general?

These macros seem footgunned to hell just as a way to try to
compensate for C's lack of language features.

-- 
Pedro