linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: "Uecker, Martin" <Martin.Uecker@med.uni-goettingen.de>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 03/13] usb: remove the usage of the list iterator after the loop
Date: Fri, 25 Feb 2022 14:02:46 -0800	[thread overview]
Message-ID: <CAHk-=whFMxks63sfMQ-0_YO1GsTmoLfsO4ciMtoiCHNgaG_+GA@mail.gmail.com> (raw)
In-Reply-To: <bd43bd47c8eaa4c22c1a1549cee66f7ef960b1fc.camel@med.uni-goettingen.de>

On Fri, Feb 25, 2022 at 1:36 PM Uecker, Martin
<Martin.Uecker@med.uni-goettingen.de> wrote:
>
> Implementation-defined only means that it needs to be
> documented (and clang does not even do this), so
> I am not sure what difference this would make.

No, implementation-defined means something *much* more than "undefined".

Yes, it means that the behavior has to be documented.

But that's not actually the big deal from a user standpoint (although
it might be a big annoyance from a compiler writer's standpoint -
compiler writers would probably much prefer "let me do the obvious
sane thing" over "let me do the obvious sane thing and then have to
write documentation about how obviously sane it is").

From a user standpoint, the big thing is that "implementation-defined"
means that the behavior has to have *SOME* well-defined behavior.
Documentation is irrelevant. But RELIABILITY is relevant.

See?

That's a big big deal. The documentation is almost incidental - the
important part is that the code acts the same on the same
architecture, regardless of compiler options, and regardless of the
phase of the moon. When you upgrade your compiler to a new version,
the end result doesn't suddenly change.

In contrast, "undefined" means that the same C code can do two totally
different things with different optimization options, or based on just
any random thing - like some random register allocation issue.

So "undefined" behavior means that changes to code that isn't even
near the code in question can change what the code generation for that
code is. And the compiler may not even report it.

That is a complete disaster. It's a disaster from a security
standpoint, it's a disaster from a maintenance standpoint, it's just
*bad*.

And the C standards committee has traditionally used to think it was a
good thing, because they listened to compiler writers that claimed
that it allowed them to do wild optimizations and improve code
generation.

Example: the completely broken type-based aliasing rules that changed
semantics of C for the worse.

Reality: it doesn't actually improve code generation all that much.
Instead it just results in a lot of problems, and any sane C code base
that cares about security and stability will just turn that off.

Same goes for integer overflow etc.

The only really valid use-case for "undefined" is when you're doing
things like accessing past your own allocations. The compiler can't do
much about things like that.

But the C standards body has actually actively screwed the pooch over
the years, and turned perfectly traditional code into "undefined" code
for no good reason. "-1 << 1" is just one particularly simplistic
example.

> > Hey, some more googling on my part seems to say that somebody saw the
> > light, and it's likely getting fixed in newer C standard version.
>
> I don't think it is changed. But C23 will require
> integers to be repreeted using two's complement,
> so there is a better chance to fix things
> like this in the future.

If integers are guaranteed to be two's complement, then the only
possible explanation for "left shift is undefined" goes away. So
presumably the thing goes hand-in-hand with just making (the de-facto)
negative shifting well-defined.

(Btw, shifting *by* a negative number - or by amounts bigger than the
width of the type - is different. There are at least real reasons to
complain about that, although I think "implementation defined" would
still be hugely preferred over "undefined")

> The right people to complain to are the
> compiler vendors, because they decide what
> UB does in their implementation.

No. The bug was in the spec to begin with. The people to complain
about are the incompetents that literally changed the C standard to be
less well-specified.

As far as I know, no actual compiler has ever made integer left-shift
done anything bad. You'd literally have to do extra work for it, so
there's no reason to do so.

But because the standards committee messed up, the compiler writers
*did* do the extra work to warn about the fact (for the trivial
"negative constant" case - never mind that technically it's undefined
for non-constants: that would have been too much work).

End result: the warning is stupid. The warning only exists for
artificially stupid reasons.

And those reasons are literally "broken C standards committee behavior".

                      Linus

  reply	other threads:[~2022-02-25 22:03 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-17 18:48 [RFC PATCH 00/13] Proposal for speculative safe list iterator Jakob Koschel
2022-02-17 18:48 ` [RFC PATCH 01/13] list: introduce speculative safe list_for_each_entry() Jakob Koschel
2022-02-17 19:29   ` Greg Kroah-Hartman
2022-02-18 16:29     ` Jann Horn
2022-02-18 16:29   ` Jann Horn
2022-02-23 14:32     ` Jakob
2022-02-19 19:44   ` Jann Horn
2022-02-17 18:48 ` [RFC PATCH 02/13] scripts: coccinelle: adapt to find list_for_each_entry nospec issues Jakob Koschel
2022-02-17 18:48 ` [RFC PATCH 03/13] usb: remove the usage of the list iterator after the loop Jakob Koschel
2022-02-17 19:28   ` Linus Torvalds
2022-02-23 14:13     ` Jakob
2022-02-23 14:16       ` Jakob
2022-02-24 10:33         ` Greg Kroah-Hartman
2022-02-24 17:56           ` Linus Torvalds
     [not found]         ` <6d191223d93249a98511177d4af08420@pexch012b.vu.local>
2022-02-24 10:46           ` Cristiano Giuffrida
2022-02-24 11:26             ` Greg Kroah-Hartman
2022-02-23 18:47       ` Linus Torvalds
2022-02-23 19:23         ` Linus Torvalds
2022-02-23 19:43           ` Linus Torvalds
2022-02-23 20:24           ` Arnd Bergmann
2022-02-23 20:43             ` Linus Torvalds
2022-02-23 20:48               ` Arnd Bergmann
2022-02-23 21:53                 ` Linus Torvalds
2022-02-24 16:04                   ` Nathan Chancellor
2022-02-23 20:54               ` Linus Torvalds
2022-02-23 22:21                 ` David Laight
2022-02-25 21:36                 ` Uecker, Martin
2022-02-25 22:02                   ` Linus Torvalds [this message]
2022-02-26  1:21                     ` Martin Uecker
2022-02-27 18:12                       ` Miguel Ojeda
2022-02-28  7:08                         ` Martin Uecker
2022-02-28 13:49                           ` Miguel Ojeda
2022-03-01 20:26                             ` Linus Torvalds
2022-03-02  7:27                               ` Martin Uecker
2022-02-26 12:42           ` Segher Boessenkool
2022-02-26 22:14             ` Arnd Bergmann
2022-02-26 23:03               ` Linus Torvalds
2022-02-27  1:19                 ` Segher Boessenkool
2022-02-27  1:09               ` Segher Boessenkool
2022-02-27  7:10                 ` David Laight
2022-02-27 11:32                   ` Segher Boessenkool
2022-02-27 18:09                     ` Miguel Ojeda
2022-02-27 20:17                       ` Segher Boessenkool
2022-02-27 21:04                         ` Linus Torvalds
2022-02-28  6:15                           ` David Laight
2022-02-27 22:43                         ` Miguel Ojeda
2022-02-27 21:28                 ` Arnd Bergmann
2022-02-27 22:43                   ` Segher Boessenkool
2022-02-17 18:48 ` [RFC PATCH 04/13] vfio/mdev: " Jakob Koschel
2022-02-18 15:12   ` Jason Gunthorpe
2022-02-23 14:18     ` Jakob
2022-02-23 19:06       ` Linus Torvalds
2022-02-23 19:12         ` Jason Gunthorpe
2022-02-23 19:31           ` Linus Torvalds
2022-02-23 20:15             ` Jakob
2022-02-23 20:22               ` Linus Torvalds
2022-02-23 22:08                 ` Jakob
2022-02-23 20:19             ` Rasmus Villemoes
2022-02-23 20:34               ` Linus Torvalds
2022-02-17 18:48 ` [RFC PATCH 05/13] drivers/perf: " Jakob Koschel
2022-02-17 18:48 ` [RFC PATCH 06/13] ARM: mmp: " Jakob Koschel
2022-02-17 18:48 ` [RFC PATCH 07/13] udp_tunnel: " Jakob Koschel
2022-02-23 20:00   ` Christophe JAILLET
2022-02-24  6:20     ` Dan Carpenter
2022-02-17 18:48 ` [RFC PATCH 08/13] net: dsa: future proof usage of " Jakob Koschel
2022-02-17 18:48 ` [RFC PATCH 09/13] drbd: " Jakob Koschel
2022-02-17 18:48 ` [RFC PATCH 10/13] powerpc/spufs: " Jakob Koschel
2022-02-17 18:48 ` [RFC PATCH 11/13] ath6kl: remove use " Jakob Koschel
2022-02-17 18:48 ` [RFC PATCH 12/13] staging: greybus: audio: Remove usage " Jakob Koschel
2022-02-17 18:48 ` [RFC PATCH 13/13] scsi: mpt3sas: comment about invalid usage of the list iterator Jakob Koschel

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='CAHk-=whFMxks63sfMQ-0_YO1GsTmoLfsO4ciMtoiCHNgaG_+GA@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=Martin.Uecker@med.uni-goettingen.de \
    --cc=linux-kernel@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).