older gccs and case labels producing integer constants

older gccs and case labels producing integer constants

[Borislav Petkov]

Hi folks,

I'm starting to see failures like this on allmodconfig builds:

sound/usb/midi.c: In function ‘snd_usbmidi_out_endpoint_create’:
sound/usb/midi.c:1389:2: error: case label does not reduce to an integer constant
  case (((0xfc08) << 16) | (0x0101)):
  ^~~~

(The case statement is a macro but it evaluates to what I have there)

and that thing fails with

$ gcc --version
gcc (SUSE Linux) 7.5.0

although it doesn't have any problems building with newer compilers.

I'm presuming older gccs consider those case statements signed ints and
the following fixes it:

  case ((((unsigned int)0xfc08) << 16) | (0x0101)):

and I guess we can whack the couple of occurrences but what I'm
wondering is why does this work with newer gccs?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: older gccs and case labels producing integer constants

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

On Tue, 5 Apr 2022, Borislav Petkov wrote:

> Hi folks,
> 
> I'm starting to see failures like this on allmodconfig builds:
> 
> sound/usb/midi.c: In function ‘snd_usbmidi_out_endpoint_create’:
> sound/usb/midi.c:1389:2: error: case label does not reduce to an integer constant
>   case (((0xfc08) << 16) | (0x0101)):
>   ^~~~
> 
> (The case statement is a macro but it evaluates to what I have there)
> 
> and that thing fails with
> 
> $ gcc --version
> gcc (SUSE Linux) 7.5.0
> 
> although it doesn't have any problems building with newer compilers.
> 
> I'm presuming older gccs consider those case statements signed ints and
> the following fixes it:
> 
>   case ((((unsigned int)0xfc08) << 16) | (0x0101)):
> 
> and I guess we can whack the couple of occurrences but what I'm
> wondering is why does this work with newer gccs?

I tried

void foo (int i)
{
  switch (i)
    {
      case (((0xfc08) << 16) | (0x0101)):;
    }
}

also with 'unsigned int i' but that's accepted with GCC 7.  So
what do you switch on?

> Thx.
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

Re: older gccs and case labels producing integer constants

[Borislav Petkov]

On Tue, Apr 05, 2022 at 11:58:33AM +0200, Richard Biener wrote:
> also with 'unsigned int i' but that's accepted with GCC 7.  So
> what do you switch on?

Ah, right, so it must be some of the gazillion switches. Below's a dump
from building the asm version of that same file.

Can you recognize which one might be causing this?

# GNU C11 (SUSE Linux) version 7.5.0 (x86_64-suse-linux)
#       compiled by GNU C version 7.5.0, GMP version 6.1.2, MPFR version 4.0.2-p6, MPC version 1.1.0, isl version isl-0.18-GMP

# GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
# options passed:  -nostdinc -I ./arch/x86/include
# -I ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi
# -I ./arch/x86/include/generated/uapi -I ./include/uapi
# -I ./include/generated/uapi -D __KERNEL__ -D CC_USING_FENTRY -D MODULE
# -D KBUILD_BASENAME="midi" -D KBUILD_MODNAME="snd_usbmidi_lib"
# -D __KBUILD_MODNAME=kmod_snd_usbmidi_lib
# -include ./include/linux/compiler-version.h
# -include ./include/linux/kconfig.h
# -include ./include/linux/compiler_types.h -MMD sound/usb/.midi.s.d
# sound/usb/midi.c -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64
# -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3
# -mskip-rax-setup -mtune=generic -mno-red-zone -mcmodel=kernel
# -mindirect-branch=thunk-extern -mindirect-branch-register -mrecord-mcount
# -mfentry -march=x86-64 -auxbase-strip sound/usb/midi.s -O2 -Wall -Wundef
# -Werror=strict-prototypes -Wno-trigraphs
# -Werror=implicit-function-declaration -Werror=implicit-int
# -Werror=return-type -Wno-format-security -Wno-sign-compare
# -Wno-frame-address -Wformat-truncation=0 -Wformat-overflow=0
# -Wframe-larger-than=2048 -Werror -Wimplicit-fallthrough=5 -Wno-main
# -Wno-unused-but-set-variable -Wunused-const-variable=0
# -Wdeclaration-after-statement -Wvla -Wno-pointer-sign
# -Wstringop-overflow=0 -Wno-restrict -Wno-maybe-uninitialized
# -Werror=date-time -Werror=incompatible-pointer-types
# -Werror=designated-init -std=gnu11 -p -fno-strict-aliasing -fno-common
# -fshort-wchar -fno-PIE -falign-jumps=1 -falign-loops=1
# -fno-asynchronous-unwind-tables -fno-jump-tables
# -fno-delete-null-pointer-checks -fno-reorder-blocks -fno-ipa-cp-clone
# -fno-partial-inlining -fstack-protector-strong
# -fno-stack-clash-protection -fno-inline-functions-called-once
# -falign-functions=64 -fno-strict-overflow -fstack-check=no
# -fconserve-stack -fsanitize=bounds -fsanitize=shift
# -fsanitize=integer-divide-by-zero -fsanitize=bool -fsanitize=enum
# -fsanitize-coverage=trace-pc -fverbose-asm
# --param allow-store-data-races=0
# options enabled:  -falign-labels -fauto-inc-dec -fbranch-count-reg
# -fcaller-saves -fchkp-check-incomplete-type -fchkp-check-read
# -fchkp-check-write -fchkp-instrument-calls -fchkp-narrow-bounds
# -fchkp-optimize -fchkp-store-bounds -fchkp-use-static-bounds
# -fchkp-use-static-const-bounds -fchkp-use-wrappers -fcode-hoisting
# -fcombine-stack-adjustments -fcompare-elim -fcprop-registers
# -fcrossjumping -fcse-follow-jumps -fdefer-pop -fdevirtualize
# -fdevirtualize-speculatively -fdwarf2-cfi-asm -fearly-inlining
# -feliminate-unused-debug-types -fexpensive-optimizations
# -fforward-propagate -ffp-int-builtin-inexact -ffunction-cse -fgcse
# -fgcse-lm -fgnu-runtime -fgnu-unique -fguess-branch-probability
# -fhoist-adjacent-loads -fident -fif-conversion -fif-conversion2
# -findirect-inlining -finline -finline-atomics -finline-small-functions
# -fipa-bit-cp -fipa-cp -fipa-icf -fipa-icf-functions -fipa-icf-variables
# -fipa-profile -fipa-pure-const -fipa-reference
# -fipa-reference-addressable -fipa-sra -fipa-stack-alignment -fipa-vrp
# -fira-hoist-pressure -fira-share-save-slots -fira-share-spill-slots
# -fisolate-erroneous-paths-dereference -fivopts -fkeep-static-consts
# -fleading-underscore -flifetime-dse -flra-remat -flto-odr-type-merging
# -fmath-errno -fmerge-constants -fmerge-debug-strings
# -fmove-loop-invariants -fomit-frame-pointer -foptimize-sibling-calls
# -foptimize-strlen -fpeephole -fpeephole2 -fplt -fprefetch-loop-arrays
# -fprofile -free -freg-struct-return -freorder-functions
# -frerun-cse-after-loop -fsanitize-coverage=trace-pc
# -fsched-critical-path-heuristic -fsched-dep-count-heuristic
# -fsched-group-heuristic -fsched-interblock -fsched-last-insn-heuristic
# -fsched-rank-heuristic -fsched-spec -fsched-spec-insn-heuristic
# -fsched-stalled-insns-dep -fschedule-fusion -fschedule-insns2
# -fsemantic-interposition -fshow-column -fshrink-wrap
# -fshrink-wrap-separate -fsigned-zeros -fsplit-ivs-in-unroller
# -fsplit-wide-types -fssa-backprop -fssa-phiopt -fstack-protector-strong
# -fstdarg-opt -fstore-merging -fstrict-volatile-bitfields -fsync-libcalls
# -fthread-jumps -ftoplevel-reorder -ftrapping-math -ftree-bit-ccp
# -ftree-builtin-call-dce -ftree-ccp -ftree-ch -ftree-coalesce-vars
# -ftree-copy-prop -ftree-cselim -ftree-dce -ftree-dominator-opts
# -ftree-dse -ftree-forwprop -ftree-fre -ftree-loop-if-convert
# -ftree-loop-im -ftree-loop-ivcanon -ftree-loop-optimize
# -ftree-parallelize-loops= -ftree-phiprop -ftree-pre -ftree-pta
# -ftree-reassoc -ftree-scev-cprop -ftree-sink -ftree-slsr -ftree-sra
# -ftree-switch-conversion -ftree-tail-merge -ftree-ter -ftree-vrp
# -funit-at-a-time -fverbose-asm -fzero-initialized-in-bss
# -m128bit-long-double -m64 -malign-stringops -mavx256-split-unaligned-load
# -mavx256-split-unaligned-store -mfentry -mfxsr -mglibc -mieee-fp
# -mindirect-branch-register -mlong-double-80 -mno-fancy-math-387
# -mno-red-zone -mno-sse4 -mpush-args -mrecord-mcount -mskip-rax-setup
# -mtls-direct-seg-refs -mvzeroupper

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: older gccs and case labels producing integer constants

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 1835 bytes --]

On Tue, 5 Apr 2022, Richard Biener wrote:

> On Tue, 5 Apr 2022, Borislav Petkov wrote:
> 
> > Hi folks,
> > 
> > I'm starting to see failures like this on allmodconfig builds:
> > 
> > sound/usb/midi.c: In function ‘snd_usbmidi_out_endpoint_create’:
> > sound/usb/midi.c:1389:2: error: case label does not reduce to an integer constant
> >   case (((0xfc08) << 16) | (0x0101)):
> >   ^~~~
> > 
> > (The case statement is a macro but it evaluates to what I have there)
> > 
> > and that thing fails with
> > 
> > $ gcc --version
> > gcc (SUSE Linux) 7.5.0
> > 
> > although it doesn't have any problems building with newer compilers.
> > 
> > I'm presuming older gccs consider those case statements signed ints and
> > the following fixes it:
> > 
> >   case ((((unsigned int)0xfc08) << 16) | (0x0101)):
> > 
> > and I guess we can whack the couple of occurrences but what I'm
> > wondering is why does this work with newer gccs?
> 
> I tried
> 
> void foo (int i)
> {
>   switch (i)
>     {
>       case (((0xfc08) << 16) | (0x0101)):;
>     }
> }
> 
> also with 'unsigned int i' but that's accepted with GCC 7.  So
> what do you switch on?

Aha, also

> gcc-7 -S t.c -std=c11 -pedantic -pedantic-errors
t.c: In function 'foo':
t.c:6:7: error: case label is not an integer constant expression 
[-Wpedantic]
       case USB_ID(0xfc08, 0x0101):;
       ^~~~

aber _nur_ mit -std=c11 (oder c99, aber nicht c89) und -pedantic 
-pedantic-errors.

#define USB_ID(v,p) (((v)<<16)|(p))
void foo (unsigned int *i)
{
  switch (*i)
    {
      case USB_ID(0xfc08, 0x0101):;
    }
}

Wird auch mit gcc 11 rejected.  Kanns sein dass mit gcc 7 andere
compiler flags genommen werden?

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

Re: older gccs and case labels producing integer constants

[Borislav Petkov]

On Tue, Apr 05, 2022 at 12:06:45PM +0200, Richard Biener wrote:
> Wird auch mit gcc 11 rejected.  Kanns sein dass mit gcc 7 andere
> compiler flags genommen werden?

Found it:

$ gcc -fsanitize=shift -c switch.c
switch.c: In function ‘foo’:
switch.c:10:7: error: case label does not reduce to an integer constant
       case (((0xfc08) << 16) | (0x0101)):;

$ gcc --version
gcc (SUSE Linux) 7.4.1 20190905 [gcc-7-branch revision 275407]
Copyright (C) 2017 Free Software Foundation, Inc.

Something not fully backported?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: older gccs and case labels producing integer constants

[Borislav Petkov]

On Tue, Apr 05, 2022 at 12:36:58PM +0200, Borislav Petkov wrote:
> On Tue, Apr 05, 2022 at 12:06:45PM +0200, Richard Biener wrote:
> > Wird auch mit gcc 11 rejected.  Kanns sein dass mit gcc 7 andere
> > compiler flags genommen werden?
> 
> Found it:
> 
> $ gcc -fsanitize=shift -c switch.c
> switch.c: In function ‘foo’:
> switch.c:10:7: error: case label does not reduce to an integer constant
>        case (((0xfc08) << 16) | (0x0101)):;
> 
> $ gcc --version
> gcc (SUSE Linux) 7.4.1 20190905 [gcc-7-branch revision 275407]
> Copyright (C) 2017 Free Software Foundation, Inc.
> 
> Something not fully backported?

Ok, not really:

gcc-10 -fsanitize=shift -c switch.c
switch.c: In function ‘foo’:
switch.c:10:7: error: case label does not reduce to an integer constant
   10 |       case (((0xfc08) << 16) | (0x0101)):;
      |       ^~~~

BUT!

when more switches are set with gcc-10 (full gcc cmdline from a kernel
build), then that passes.

But it doesn't pass with gcc-7.

Weird...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: older gccs and case labels producing integer constants

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

On Tue, 5 Apr 2022, Borislav Petkov wrote:

> On Tue, Apr 05, 2022 at 12:36:58PM +0200, Borislav Petkov wrote:
> > On Tue, Apr 05, 2022 at 12:06:45PM +0200, Richard Biener wrote:
> > > Wird auch mit gcc 11 rejected.  Kanns sein dass mit gcc 7 andere
> > > compiler flags genommen werden?
> > 
> > Found it:
> > 
> > $ gcc -fsanitize=shift -c switch.c
> > switch.c: In function ‘foo’:
> > switch.c:10:7: error: case label does not reduce to an integer constant
> >        case (((0xfc08) << 16) | (0x0101)):;
> > 
> > $ gcc --version
> > gcc (SUSE Linux) 7.4.1 20190905 [gcc-7-branch revision 275407]
> > Copyright (C) 2017 Free Software Foundation, Inc.
> > 
> > Something not fully backported?
> 
> Ok, not really:
> 
> gcc-10 -fsanitize=shift -c switch.c
> switch.c: In function ‘foo’:
> switch.c:10:7: error: case label does not reduce to an integer constant
>    10 |       case (((0xfc08) << 16) | (0x0101)):;
>       |       ^~~~
> 
> BUT!
> 
> when more switches are set with gcc-10 (full gcc cmdline from a kernel
> build), then that passes.
> 
> But it doesn't pass with gcc-7.
> 
> Weird...

As was noted in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66880
this is invalid C99+ but compilers are not required to diagnose that
(you get it diagnosed with -pedantic).  -fsanitize=shift exposes
it though since the non-integral-constant gets instrumented.

Richard.

Re: older gccs and case labels producing integer constants

[Borislav Petkov]

On Tue, Apr 05, 2022 at 01:41:09PM +0200, Richard Biener wrote:
> As was noted in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66880
> this is invalid C99+ but compilers are not required to diagnose that
> (you get it diagnosed with -pedantic).  -fsanitize=shift exposes
> it though since the non-integral-constant gets instrumented.

Right, just to close this: I was still unsure which of the cmdline
options would cause it and bisected the kernel (big fat box can build
allmodconfigs in no time :)).

The single change which fixes the whole build is

---
diff --git a/Makefile b/Makefile
index 8c7de9a72ea2..3582089cfeb6 100644
--- a/Makefile
+++ b/Makefile
@@ -523,7 +523,7 @@ KBUILD_CFLAGS   := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \
 		   -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE \
 		   -Werror=implicit-function-declaration -Werror=implicit-int \
 		   -Werror=return-type -Wno-format-security \
-		   -std=gnu11
+		   -std=gnu89
 KBUILD_CPPFLAGS := -D__KERNEL__
 KBUILD_AFLAGS_KERNEL :=
 KBUILD_CFLAGS_KERNEL :=

with that

gcc (SUSE Linux) 7.4.1 20190905 [gcc-7-branch revision 275407]

but as we saw, only -std=gnu11 alone doesn't cause it:

$ gcc -std=gnu11 -o switch.o switch.c
$

And so we had the -fsanitize=shift already enabled since 2020 in the
kernel build and the gnu11 change then triggered the undefined behavior
due to the -fsanitize instrumentation as it was already explained:

$ gcc -std=gnu11 -fsanitize=shift -o switch.o switch.c
switch.c: In function ‘foo’:
switch.c:10:7: error: case label does not reduce to an integer constant
       case (((0xfc08) << 16) | (0x0101)):;
       ^~~~

Ok, now I can sleep at night again.

:-)))

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: older gccs and case labels producing integer constants

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]

On Tue, 5 Apr 2022, Borislav Petkov wrote:

> On Tue, Apr 05, 2022 at 12:06:45PM +0200, Richard Biener wrote:
> > Wird auch mit gcc 11 rejected.  Kanns sein dass mit gcc 7 andere
> > compiler flags genommen werden?
> 
> Found it:
> 
> $ gcc -fsanitize=shift -c switch.c
> switch.c: In function ‘foo’:
> switch.c:10:7: error: case label does not reduce to an integer constant
>        case (((0xfc08) << 16) | (0x0101)):;
> 
> $ gcc --version
> gcc (SUSE Linux) 7.4.1 20190905 [gcc-7-branch revision 275407]
> Copyright (C) 2017 Free Software Foundation, Inc.
> 
> Something not fully backported?

Probably early vs. late folding change in the frontend.  So yes,
don't use -fsanitize=... ;)

Richard.

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

Re: older gccs and case labels producing integer constants

[Jakub Jelinek]

On Tue, Apr 05, 2022 at 12:36:58PM +0200, Borislav Petkov wrote:
> On Tue, Apr 05, 2022 at 12:06:45PM +0200, Richard Biener wrote:
> > Wird auch mit gcc 11 rejected.  Kanns sein dass mit gcc 7 andere
> > compiler flags genommen werden?
> 
> Found it:
> 
> $ gcc -fsanitize=shift -c switch.c
> switch.c: In function ‘foo’:
> switch.c:10:7: error: case label does not reduce to an integer constant
>        case (((0xfc08) << 16) | (0x0101)):;
> 
> $ gcc --version
> gcc (SUSE Linux) 7.4.1 20190905 [gcc-7-branch revision 275407]
> Copyright (C) 2017 Free Software Foundation, Inc.
> 
> Something not fully backported?

That is rejected with -fsanitize=shift even on current trunk (in C, C++ is
fine).
C++ constexpr code has cases for ubsan builtins and internal functions,
but C just doesn't handle those apparently.

	Jakub

Re: older gccs and case labels producing integer constants

[Jakub Jelinek]

On Wed, Apr 06, 2022 at 11:53:17AM +0200, Jakub Jelinek wrote:
> On Tue, Apr 05, 2022 at 12:36:58PM +0200, Borislav Petkov wrote:
> > On Tue, Apr 05, 2022 at 12:06:45PM +0200, Richard Biener wrote:
> > > Wird auch mit gcc 11 rejected.  Kanns sein dass mit gcc 7 andere
> > > compiler flags genommen werden?
> > 
> > Found it:
> > 
> > $ gcc -fsanitize=shift -c switch.c
> > switch.c: In function ‘foo’:
> > switch.c:10:7: error: case label does not reduce to an integer constant
> >        case (((0xfc08) << 16) | (0x0101)):;
> > 
> > $ gcc --version
> > gcc (SUSE Linux) 7.4.1 20190905 [gcc-7-branch revision 275407]
> > Copyright (C) 2017 Free Software Foundation, Inc.
> > 
> > Something not fully backported?
> 
> That is rejected with -fsanitize=shift even on current trunk (in C, C++ is
> fine).
> C++ constexpr code has cases for ubsan builtins and internal functions,
> but C just doesn't handle those apparently.

But I think the error is actually correct.
In C99 and later, for signed left shift the rule for x << y is that
there is UB if (similarly to all C family) if y is negative or greater or
equal to precision of promoted x, but for C99 also when
((unsigned_typeof_x) x >> (precision_of_x - 1 - y)) != 0.
That is the case above, 0xfc08 is signed int and 0xfc08 << 16 is
0xfc080000 where (0xfc08 >> 15) is 1 and so it is UB.
In C99 and later you need:
	case (int)(((0xfc08U) << 16) | (0x0101)):;
or so.
Note, C++ has different rules (and C++20 and later only has the
y non-negative and less than precision requirement and nothing else).

	Jakub

Re: older gccs and case labels producing integer constants

[Peter Zijlstra]

On Tue, Apr 05, 2022 at 11:50:35AM +0200, Borislav Petkov wrote:
> Hi folks,
> 
> I'm starting to see failures like this on allmodconfig builds:
> 
> sound/usb/midi.c: In function ‘snd_usbmidi_out_endpoint_create’:
> sound/usb/midi.c:1389:2: error: case label does not reduce to an integer constant
>   case (((0xfc08) << 16) | (0x0101)):
>   ^~~~
> 
> (The case statement is a macro but it evaluates to what I have there)
> 
> and that thing fails with
> 
> $ gcc --version
> gcc (SUSE Linux) 7.5.0
> 
> although it doesn't have any problems building with newer compilers.
> 
> I'm presuming older gccs consider those case statements signed ints and
> the following fixes it:
> 
>   case ((((unsigned int)0xfc08) << 16) | (0x0101)):
> 
> and I guess we can whack the couple of occurrences but what I'm
> wondering is why does this work with newer gccs?

IIRC GCC-8 fixed a bunch of -wrapv issues. Could be this is one of them
I suppose.

Re: older gccs and case labels producing integer constants

[Michael Matz]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

Hey,

On Tue, 5 Apr 2022, Peter Zijlstra wrote:

> > sound/usb/midi.c: In function ‘snd_usbmidi_out_endpoint_create’:
> > sound/usb/midi.c:1389:2: error: case label does not reduce to an integer constant
> >   case (((0xfc08) << 16) | (0x0101)):
> >   ^~~~
> 
> IIRC GCC-8 fixed a bunch of -wrapv issues. Could be this is one of them 
> I suppose.

Or better said, later GCCs returned back to the old behaviour of rejecting 
this only with -pedantic even in the presence of -fsanitize.  But 
pedantically speaking (ahem!) it really isn't conforming c99 (which the 
compilation flags claim) , and in this case it seems easy enough to make 
the construct actually be conforming in the kernel sources, so that should 
perhaps be done?


Ciao,
Michael.

Re: older gccs and case labels producing integer constants

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

On Tue, 5 Apr 2022, Michael Matz wrote:

> Hey,
> 
> On Tue, 5 Apr 2022, Peter Zijlstra wrote:
> 
> > > sound/usb/midi.c: In function ‘snd_usbmidi_out_endpoint_create’:
> > > sound/usb/midi.c:1389:2: error: case label does not reduce to an integer constant
> > >   case (((0xfc08) << 16) | (0x0101)):
> > >   ^~~~
> > 
> > IIRC GCC-8 fixed a bunch of -wrapv issues. Could be this is one of them 
> > I suppose.
> 
> Or better said, later GCCs returned back to the old behaviour of rejecting 
> this only with -pedantic even in the presence of -fsanitize.

Only that it doesn't:

#define USB_ID(v,p) (((v)<<16)|(p))
void foo (unsigned int *i)
{
  switch (*i)
    {
      case USB_ID(0xfc08, 0x0101):;
    }
}

> gcc-11 -S t.c -std=c99 -fsanitize=shift
t.c: In function 'foo':
t.c:6:7: error: case label does not reduce to an integer constant
    6 |       case USB_ID(0xfc08, 0x0101):;
      |       ^~~~

for some reason it might fail to sanitize the case label for the
full testcase but clearly it doesn't so on purpose.

> But 
> pedantically speaking (ahem!) it really isn't conforming c99 (which the 
> compilation flags claim) , and in this case it seems easy enough to make 
> the construct actually be conforming in the kernel sources, so that should 
> perhaps be done?

Indeed.  Simply cast vendor/product to (unsigned) in the USB_ID
macro to avoid arithmetic shifts.

Richard.

Re: older gccs and case labels producing integer constants

[Borislav Petkov]

On Tue, Apr 05, 2022 at 02:53:18PM +0200, Richard Biener wrote:
> Indeed.  Simply cast vendor/product to (unsigned) in the USB_ID
> macro to avoid arithmetic shifts.

Yeah, Boris is going to turn on his editor and do patches... :-)

Thx guys!

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: older gccs and case labels producing integer constants

[Jakub Jelinek]

On Tue, Apr 05, 2022 at 02:23:31PM +0200, Peter Zijlstra wrote:
> On Tue, Apr 05, 2022 at 11:50:35AM +0200, Borislav Petkov wrote:
> > Hi folks,
> > 
> > I'm starting to see failures like this on allmodconfig builds:
> > 
> > sound/usb/midi.c: In function ‘snd_usbmidi_out_endpoint_create’:
> > sound/usb/midi.c:1389:2: error: case label does not reduce to an integer constant
> >   case (((0xfc08) << 16) | (0x0101)):
> >   ^~~~
> > 
> > (The case statement is a macro but it evaluates to what I have there)
> > 
> > and that thing fails with
> > 
> > $ gcc --version
> > gcc (SUSE Linux) 7.5.0
> > 
> > although it doesn't have any problems building with newer compilers.
> > 
> > I'm presuming older gccs consider those case statements signed ints and
> > the following fixes it:
> > 
> >   case ((((unsigned int)0xfc08) << 16) | (0x0101)):
> > 
> > and I guess we can whack the couple of occurrences but what I'm
> > wondering is why does this work with newer gccs?
> 
> IIRC GCC-8 fixed a bunch of -wrapv issues. Could be this is one of them
> I suppose.

If we are talking about -fsanitize=shift -fwrapv, then that is
https://gcc.gnu.org/PR68418 , i.e. it was fixed already for GCC 6.

	Jakub