Message-ID: <e71fffb7ff0e4bf29692d006c0fe77c2@AcuMS.aculab.com>
Date: Sun, 8 Dec 2024 16:48:58 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Martin Uecker' <muecker@...g.de>, Linus Torvalds
	<torvalds@...ux-foundation.org>
CC: Vincent Mailhol <mailhol.vincent@...adoo.fr>, Luc Van Oostenryck
	<luc.vanoostenryck@...il.com>, Nathan Chancellor <nathan@...nel.org>, "Nick
 Desaulniers" <ndesaulniers@...gle.com>, Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>, Yury Norov <yury.norov@...il.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>, Kees Cook <kees@...nel.org>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>, Jani Nikula
	<jani.nikula@...ux.intel.com>, Joonas Lahtinen
	<joonas.lahtinen@...ux.intel.com>, Rodrigo Vivi <rodrigo.vivi@...el.com>,
	Tvrtko Ursulin <tursulin@...ulin.net>, David Airlie <airlied@...il.com>,
	Simona Vetter <simona@...ll.ch>, Suzuki K Poulose <suzuki.poulose@....com>,
	Mike Leach <mike.leach@...aro.org>, James Clark <james.clark@...aro.org>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Rikard Falkeborn
	<rikard.falkeborn@...il.com>, "linux-sparse@...r.kernel.org"
	<linux-sparse@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "llvm@...ts.linux.dev"
	<llvm@...ts.linux.dev>, "linux-hardening@...r.kernel.org"
	<linux-hardening@...r.kernel.org>, "intel-gfx@...ts.freedesktop.org"
	<intel-gfx@...ts.freedesktop.org>, "dri-devel@...ts.freedesktop.org"
	<dri-devel@...ts.freedesktop.org>, "coresight@...ts.linaro.org"
	<coresight@...ts.linaro.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH 02/10] compiler.h: add is_const() as a replacement of
 __is_constexpr()

From: Martin Uecker
> Sent: 08 December 2024 12:38
> 
> Am Sonntag, dem 08.12.2024 um 11:26 +0000 schrieb David Laight:
> > From: Martin Uecker
> > > Sent: 07 December 2024 23:52
> > ...
> > > While the compiler can not automatically prove every use
> > > of VLA bounded, it can reliably diagnose the cases where it
> > > can *not* see that it is bounded. Consider this example:
> > >
> > > void oob(int n, char p[n]);
> > > void f(unsigned int n)
> > > {
> > >     char buf[MIN(n, 100)]; // bounded
> > >     oob(n + 10, buf); // warning
> > > }
> > ...
> >
> > The kernel stack has to have enough space for the [100]
> > so the full amount might as well always be allocated.
> > The chance of 'trading off' stack usage with another function
> > in the same call stack that is guaranteed to use less than
> > its maximum is about zero.
> 
> In numerical computing this is a big motivation because
> you can reduce stack usage in recursive divide-and-conquer
> algorithms.  For the kernel, I agree this is not a
> compelling use case, and the better motivation would be
> precise bounds checking and clearer semantics for buffer
> management.

Except that changing the size of the on-stack array makes
absolutely no difference.
Ideally the kernel stack would be a single 4k page, but too
much code uses on-stack buffers so it has been increased and
might be 16k (or more!).
Remember this is physical memory allocated to every user thread.
On Linux it is not swappable.

...
> > This happened for 'constant' sizes from min(16, sizeof (struct))
> > because min() needs to be a statement function to avoid re-evaluating
> > its arguments.
> 
> Can you clarify this?  If the VLA size is constant, even when
> it is not an integer constant expression according to ISO C,
> the compiler should not produce worse code.  For example,

I just tried to reproduce the failing case - and failed.
It was similar to __builtin_constant_p() initially returning 'don't know'
so the 'variable sized' array code got added, then much later
after further optimisation passes the expression became constant.
So you ended up with a 'fixed size' VLA.

Compile with -Wno-vla (and -Werror) and the compile failed.

...
> So a lot of this macro business seems to be necessary
> to avoid creating warnings for ISO VLAs when instead you really
> care about the created code not having a dynamic allocation on
> the stack.

A lot of the 'macro business' for min/max is avoiding unexpected
conversion of negative values to very large unsigned ones.
And no, -Wsign-compare is spectacularly useless.

..
> The issue here is that we miss a language feature in C to
> introduce local variables that help avoid multiple expansion
> of macro arguments.  GCC's statement expressions and __auto_type
> are a solution

or historically 'typeof(x) _x = x'

> #define foo(x) ({ __auto_type __x = (x); ... })
> 
> but this runs into the current limitations that ({ }) can not be used
> at file-scope and can not return constant expressions.
> 
> 
> For other reasons I was thinking about adding names to _Generic,
> as in
> 
> _Generic(x, int i: (i + 1));
> 
> because one design issues with _Generic is that it typechecks
> also the untaken associations and there the 'x' then has the wrong
> type.  Having an 'i' with the right type which is set to the value
> of 'x' when the branch is taken would fix this issue.

That looks even more syntactically obscure than _Generic itself.
Why does it need to do more than very simple syntax analysis of
the unwanted branches - or they could automatically be analysed
with the named variable have the specified type?

> But this feature might also allow writing macros that avoid
> double expansion without requiring statement expressions (which
> are more difficult to fix):
> 
> #define foo(x) _Generic(x, int i: (i + i));

How can that work for things like min() that have multiple arguments?
Not going to work if you need __auto_type either.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Powered by blists - more mailing lists