From: Martin Uecker <muecker@gwdg.de>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Laight <David.Laight@aculab.com>,
Vincent Mailhol <mailhol.vincent@wanadoo.fr>,
Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
Nathan Chancellor <nathan@kernel.org>,
"Nick Desaulniers" <ndesaulniers@google.com>,
Bill Wendling <morbo@google.com>,
Justin Stitt <justinstitt@google.com>,
Yury Norov <yury.norov@gmail.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Kees Cook <kees@kernel.org>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
Jani Nikula <jani.nikula@linux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>,
Tvrtko Ursulin <tursulin@ursulin.net>,
David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Mike Leach <mike.leach@linaro.org>,
James Clark <james.clark@linaro.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Rikard Falkeborn <rikard.falkeborn@gmail.com>,
"linux-sparse@vger.kernel.org" <linux-sparse@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"llvm@lists.linux.dev" <llvm@lists.linux.dev>,
"linux-hardening@vger.kernel.org"
<linux-hardening@vger.kernel.org>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"coresight@lists.linaro.org" <coresight@lists.linaro.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 02/10] compiler.h: add is_const() as a replacement of __is_constexpr()
Date: Sun, 8 Dec 2024 00:52:07 +0100 [thread overview]
Message-ID: <9d9567dbdaf39688bbd0d240e29dec826a5931ee.camel@gwdg.de> (raw)
In-Reply-To: <CAHk-=wjsfYYKBYuW8_6yKjdwHih0MMa2GwUJh_LHcuUNFR7-QA@mail.gmail.com>
Am Samstag, dem 07.12.2024 um 12:28 -0800 schrieb Linus Torvalds:
> On Sat, 7 Dec 2024 at 11:19, Martin Uecker <muecker@gwdg.de> wrote:
> >
> > But that all seem solvable issues on the compiler side.
>
[... Itanium, value range analysis, no assertions in kernel...]
> Now, would we want to have proper value *static* range analysis in the
> kernel for other reasons? Oh yes. It would often be very nice to have
> the ability to state "this value is trusted and is in this range", and
> have it percolate all the way down, both for optimization purposes but
> also for various sanity check purposes.
__bos should give you some access to it, but it seems not as
good as it could be (relative to what the optimizer knows)
and then the information is also not available in the front-end.
>
> But it's simply not sanely available in the generic case.
I was not talking about future still-to-be-invented compiler
technology, but about things that work today.
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
}
void h(unsigned int n)
{
char buf[100]; // static with worst case size
oob(n + 10, buf); // no warning
}
void i(unsigned int n)
{
char buf[n]; // warning!
oob(n + 10, buf);
}
void test_f() { f(50); }
void test_h() { h(50); }
void test_i() { i(50); }
https://godbolt.org/z/b15ajTPda
For both 'f' and 'h' stack is bounded and generally smaller in 'f'.
Also any worst-case stack size analysis that applies to 'h' also
applies to 'f' (and could potentially be improved).
In 'f' one gets a warning because 'oob' will try to do an OOB
access. This is only possibly because the compiler sees the true
size of the array. (One can also get precise information
about the size with __bdos.)
In 'h' the error can not be detected. The information is lost.
One can get a warning only when the static worst-case size is
exceeded, but otherwise the error remains hidden.
In 'i' one gets the warning about the OOB access and with
-Wvla-larget-than=100 one gets a warning that the VLA size may
be unbounded. These case should then not be allowed of course.
Note that this works today in GCC and seems to have better
information than __bos available (not sure why though).
>
> > a) this is not guaranteed in a specific situation (-Wvla-larher-than)
>
> We'd either get horrendous numbers of false positives that we then
> have to manually add special code for, or
>
> > b) transform the array automatically to fixed size array
> > of size X *or* something smaller when it can show this.
>
> we'd just do this by hand *once* and for all, and say "VLA's didn't work out".
>
> So yeah. We did (b) by hand.
>
> We used to have VLA's in the kernel. It was a disaster. We got rid of
> them, because the (big) pain wasn't worth the (few) places it was
> actually useful.
Can you point me to some horror stories?
> So we have been VLA-free for the last five years, and it's been good.
> Simplify.
These macro discussions I get CC-ed on sometimes leave a different
impression ;-)
Martin
next prev parent reply other threads:[~2024-12-07 23:52 UTC|newest]
Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-02 17:33 [PATCH 00/10] compiler.h: refactor __is_constexpr() into is_const{,_true,_false}() Vincent Mailhol via B4 Relay
2024-12-02 17:33 ` [PATCH 01/10] compiler.h: add statically_false() Vincent Mailhol via B4 Relay
2024-12-04 18:30 ` David Laight
2024-12-05 15:25 ` Vincent Mailhol
2024-12-06 3:39 ` David Laight
2024-12-06 4:42 ` Vincent Mailhol
2024-12-02 17:33 ` [PATCH 02/10] compiler.h: add is_const() as a replacement of __is_constexpr() Vincent Mailhol via B4 Relay
2024-12-04 18:39 ` David Laight
2024-12-04 21:20 ` Yury Norov
2024-12-05 15:31 ` Vincent Mailhol
2024-12-06 2:25 ` David Laight
2024-12-06 6:14 ` Linus Torvalds
2024-12-06 7:19 ` Vincent Mailhol
2024-12-06 8:49 ` Vincent Mailhol
2024-12-06 8:29 ` Martin Uecker
2024-12-06 18:30 ` Vincent Mailhol
2024-12-06 18:52 ` Linus Torvalds
2024-12-06 19:02 ` Linus Torvalds
2024-12-06 19:06 ` David Laight
2024-12-06 19:15 ` Linus Torvalds
2024-12-06 19:38 ` Willy Tarreau
2024-12-06 19:43 ` David Laight
2024-12-06 19:38 ` David Laight
2024-12-06 20:23 ` David Laight
2024-12-07 7:42 ` Vincent Mailhol
2024-12-07 11:19 ` David Laight
2024-12-07 12:24 ` Vincent Mailhol
2024-12-07 18:19 ` Linus Torvalds
2024-12-07 19:51 ` Martin Uecker
2024-12-07 20:31 ` Linus Torvalds
2024-12-07 20:54 ` David Laight
2024-12-07 21:00 ` David Laight
2024-12-07 21:06 ` Martin Uecker
2024-12-07 21:45 ` David Laight
2024-12-09 9:59 ` Rasmus Villemoes
2024-12-06 6:40 ` Martin Uecker
2024-12-06 7:26 ` Vincent Mailhol
2024-12-07 8:39 ` Martin Uecker
2024-12-07 10:33 ` David Laight
2024-12-07 13:07 ` Martin Uecker
2024-12-07 18:26 ` Linus Torvalds
2024-12-07 19:19 ` Martin Uecker
2024-12-07 20:28 ` Linus Torvalds
2024-12-07 23:52 ` Martin Uecker [this message]
2024-12-08 1:58 ` Linus Torvalds
2024-12-08 9:18 ` Martin Uecker
2024-12-08 11:26 ` David Laight
2024-12-08 12:38 ` Martin Uecker
2024-12-08 16:48 ` David Laight
2024-12-08 18:10 ` Martin Uecker
2024-12-08 19:05 ` Linus Torvalds
2024-12-07 12:45 ` Vincent Mailhol
2024-12-07 13:18 ` Martin Uecker
2024-12-07 13:50 ` Vincent Mailhol
2024-12-07 14:59 ` Martin Uecker
2024-12-07 15:10 ` Martin Uecker
2024-12-07 15:23 ` Vincent Mailhol
2024-12-07 18:07 ` David Laight
2024-12-06 9:34 ` David Laight
2024-12-02 17:33 ` [PATCH 03/10] compiler.h: add is_const_true() and is_const_false() Vincent Mailhol via B4 Relay
2024-12-04 18:48 ` David Laight
2024-12-05 15:48 ` Vincent Mailhol
2024-12-02 17:33 ` [PATCH 04/10] linux/bits.h: simplify GENMASK_INPUT_CHECK() by using is_const_true() Vincent Mailhol via B4 Relay
2024-12-04 18:52 ` David Laight
2024-12-05 15:49 ` Vincent Mailhol
2024-12-02 17:33 ` [PATCH 05/10] minmax: simplify __clamp_once() by using is_const_false() Vincent Mailhol via B4 Relay
2024-12-04 18:54 ` David Laight
2024-12-05 15:52 ` Vincent Mailhol
2024-12-09 12:32 ` Vincent Mailhol
2024-12-02 17:33 ` [PATCH 06/10] fortify: replace __is_constexpr() by is_const() in strlen() Vincent Mailhol via B4 Relay
2024-12-04 18:58 ` David Laight
2024-12-05 15:53 ` Vincent Mailhol
2024-12-02 17:33 ` [PATCH 07/10] overflow: replace __is_constexpr() by is_const() Vincent Mailhol via B4 Relay
2024-12-02 17:33 ` [PATCH 08/10] drm/i915/reg: replace __is_const_expr() by is_const_true() or is_const() Vincent Mailhol via B4 Relay
2024-12-04 19:00 ` David Laight
2024-12-05 15:56 ` Vincent Mailhol
2024-12-02 17:33 ` [PATCH 09/10] coresight: etm4x: replace __is_const_expr() by is_const() Vincent Mailhol via B4 Relay
2024-12-02 17:33 ` [PATCH 10/10] compiler.h: remove __is_constexpr() Vincent Mailhol via B4 Relay
2024-12-04 23:58 ` [PATCH 00/10] compiler.h: refactor __is_constexpr() into is_const{,_true,_false}() Kees Cook
2024-12-05 15:21 ` Vincent Mailhol
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9d9567dbdaf39688bbd0d240e29dec826a5931ee.camel@gwdg.de \
--to=muecker@gwdg.de \
--cc=David.Laight@aculab.com \
--cc=airlied@gmail.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=coresight@lists.linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gustavoars@kernel.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=james.clark@linaro.org \
--cc=jani.nikula@linux.intel.com \
--cc=joonas.lahtinen@linux.intel.com \
--cc=justinstitt@google.com \
--cc=kees@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sparse@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=llvm@lists.linux.dev \
--cc=luc.vanoostenryck@gmail.com \
--cc=mailhol.vincent@wanadoo.fr \
--cc=mike.leach@linaro.org \
--cc=morbo@google.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=rikard.falkeborn@gmail.com \
--cc=rodrigo.vivi@intel.com \
--cc=simona@ffwll.ch \
--cc=suzuki.poulose@arm.com \
--cc=torvalds@linux-foundation.org \
--cc=tursulin@ursulin.net \
--cc=yury.norov@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®