[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5eb6554f5f600f09d5072bc524757912a36c5057.camel@tugraz.at>
Date: Sat, 22 Feb 2025 11:25:26 +0100
From: Martin Uecker <uecker@...raz.at>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, Steven Rostedt
<rostedt@...dmis.org>, Greg KH <gregkh@...uxfoundation.org>, Boqun Feng
<boqun.feng@...il.com>, "H. Peter Anvin" <hpa@...or.com>, Miguel Ojeda
<miguel.ojeda.sandonis@...il.com>, Christoph Hellwig <hch@...radead.org>,
rust-for-linux <rust-for-linux@...r.kernel.org>, David Airlie
<airlied@...il.com>, linux-kernel@...r.kernel.org, ksummit@...ts.linux.dev
Subject: Re: Rust kernel policy
Am Samstag, dem 22.02.2025 um 12:45 +0300 schrieb Dan Carpenter:
> On Fri, Feb 21, 2025 at 07:31:11PM +0100, Martin Uecker wrote:
> > > This is non-negotiable. Anybody who thinks that a compiler is valid
> > > warning about
> > >
> > > if (x < 0 || x >= 10) {
> > >
> > > just because 'x' may in some cases be an unsigned entity is not worth
> > > even discussing with.
> >
> > Do you think the warning is useless in macros, or in general?
>
> This is a fair question. In smatch I often will turn off a static
> checker warnings if they're inside a macro. For example, macros will
> have NULL checks which aren't required in every place where they're
> used so it doesn't make sense to warn about inconsistent NULL checking
> if the NULL check is done in a macro.
>
> In this unsigned less than zero example, we can easily see that it works
> to clamp 0-9 and the compiler could silence the warning based on that.
> I mentioned that Justin filtered out idiomatic integer overflows like
> if (a < a + b) { and we could do the same here. That would silence most
> of the false positives. It's a culture debate not a technical problem.
>
> Silencing the checks inside macros would silence quite a few of the
> remaining false positives. In Smatch, I've silenced a few false
> positives that way for specific macros but I haven't felt the need to
> go all the way and turning the check off inside all macros.
>
> There are also a handful of defines which can be zero depending on the
> circumstances like DPMCP_MIN_VER_MINOR:
>
> if (dpmcp_dev->obj_desc.ver_minor < DPMCP_MIN_VER_MINOR)
> return -ENOTSUPP;
>
> Or another example is in set_iter_tags()
>
> /* This never happens if RADIX_TREE_TAG_LONGS == 1 */
> if (tag_long < RADIX_TREE_TAG_LONGS - 1) {
>
> The other thing is that in Smatch, I don't try to silence every false
> positives. Or any false positives. :P So long as I can handle the work
> load of reviewing new warnings it's fine. I look at a warning once and
> then I'm done.
Thanks, this is useful. I was asking because it would be relatively
easy to tweak the warnings in GCC too. GCC has similar heuristics for
other warnings to turn them off in macros and one can certainly also
make it smarter. (Again, the two problems here seem lack of communication
and lack of resources. One needs to understand what needs to be done
and someone has to do it. But even a limited amount of time/money could
make a difference.)
Martin
>
Powered by blists - more mailing lists