Re: [RFC PATCH 03/13] usb: remove the usage of the list iterator after the loop

From: Segher Boessenkool <segher@kernel.crashing.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Jakob <jakobkoschel@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>,
	Mike Rapoport <rppt@kernel.org>,
	"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
	Brian Johannesmeyer <bjohannesmeyer@gmail.com>,
	Cristiano Giuffrida <c.giuffrida@vu.nl>,
	"Bos, H.J." <h.j.bos@vu.nl>
Subject: Re: [RFC PATCH 03/13] usb: remove the usage of the list iterator after the loop
Date: Sat, 26 Feb 2022 19:09:56 -0600	[thread overview]
Message-ID: <20220227010956.GW614@gate.crashing.org> (raw)
In-Reply-To: <CAK8P3a2Dd+ZMzn=gDnTzOW=S3RHQVmm1j3Gy=aKmFEbyD-q=rQ@mail.gmail.com>

On Sat, Feb 26, 2022 at 11:14:15PM +0100, Arnd Bergmann wrote:
> On Sat, Feb 26, 2022 at 1:42 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Wed, Feb 23, 2022 at 11:23:39AM -0800, Linus Torvalds wrote:
> 
> > > That said, we seem to only have two cases of it in the kernel, at
> > > least by a x86-64 allmodconfig build. So we could examine the types
> > > there, or we could just add '-Wno-shift-negative-value".
> >
> > Yes.
> >
> > The only reason the warning exists is because it is undefined behaviour
> > (not implementation-defined or anything).  The reason it is that in the
> > standard is that it is hard to implement and even describe for machines
> > that are not two's complement.  However relevant that is today :-)
> 
> Could gcc follow the clang behavior then and skip the warning and
> sanitizer for this case when -fno-strict-overflow or -fwrapv are used?

As I said, we have this implementation choice documented as
  As an extension to the C language, GCC does not use the latitude
  given in C99 and C11 only to treat certain aspects of signed '<<'
  as undefined.  However, '-fsanitize=shift' (and
  '-fsanitize=undefined') will diagnose such cases.  They are also
  diagnosed where constant expressions are required.
but that is not at all what we implement currently, we warn much more
often.  Constant expressions are required only in a few places (#if
condition, bitfield length, (non-variable) array length, enumeration
declarations, _Alignas, _Static_assert, case labels, most initialisers);
not places you will see code like this problem normally.

So imo we should just never do this by default, not just if the nasty
-fwrapv or nastier -fno-strict-overflow is used, just like we suggest
in our own documentation.  The only valid reason -Wshift-negative-value
is in -Wextra is it warns for situations that always are undefined
behaviour (even if not in GCC).

Could you open a GCC PR for this?  The current situation is quite
suboptimal, and what we document as our implementation choice is much
more useful!

Segher