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
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 <[email protected]>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
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.
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.
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.
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
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
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 <[email protected]>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
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