Re: s390-ccw: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]

[Stefano Garzarella]

On Thu, Apr 22, 2021 at 06:54:21PM +0200, Philippe Mathieu-Daudé wrote:
> On 4/22/21 6:07 PM, Thomas Huth wrote:
> > On 22/04/2021 17.52, Stefano Garzarella wrote:
> > > On Thu, Apr 22, 2021 at 05:38:26PM +0200, Thomas Huth wrote:
> > > > On 22/04/2021 16.31, Philippe Mathieu-Daudé wrote:
> > > > > On 4/22/21 2:41 PM, Christian Borntraeger wrote:
> > > > > > On 22.04.21 13:47, Thomas Huth wrote:
> > > > > > > On 22/04/2021 13.24, Philippe Mathieu-Daudé wrote:
> > > > > > > > On 4/22/21 12:30 PM, Peter Maydell wrote:
> > > > > > > > > On Thu, 22 Apr 2021 at 11:18, Daniel P. Berrangé
> > > > > > > > > <berrange@redhat.com> wrote:
> > > > >
> > > > > > > > This silents the warning however:
> > > > > > > >
> > > > > > > > -- >8 --
> > > > > > > > diff --git a/pc-bios/s390-ccw/libc.h b/pc-bios/s390-ccw/libc.h
> > > > > > > > index bcdc45732d..2dea399904 100644
> > > > > > > > --- a/pc-bios/s390-ccw/libc.h
> > > > > > > > +++ b/pc-bios/s390-ccw/libc.h
> > > > > > > > @@ -19,6 +19,8 @@ typedef unsigned short     uint16_t;
> > > > > > > >   typedef unsigned int       uint32_t;
> > > > > > > >   typedef unsigned long long uint64_t;
> > > > > > > >
> > > > > > > > +#pragma GCC diagnostic push
> > > > > > > > +#pragma GCC diagnostic ignored "-Wstringop-overflow"
> > > > > > > >   static inline void *memset(void *s, int c, size_t n)
> > > > > > > >   {
> > > > > > > >       size_t i;
> > > > > > > > @@ -30,6 +32,7 @@ static inline void *memset(void *s, int c,
> > > > > > > > size_t n)
> > > > > > > >
> > > > > > > >       return s;
> > > > > > > >   }
> > > > > > > > +#pragma GCC diagnostic pop
> > > > > > >
> > > > > > > Honestly, that compiler "bug" sounds like it could trigger at any
> > > > > > > other spot in the bios code, too, since we are doing lots of direct
> > > > > > > accesses to low memory there. I think it's likely best if we shut it
> > > > > > > off with -Wno-stringop-overflow in the pc-bios/s390-ccw/Makefile ...
> > > > > > > could you please try to add it there?
> > > > >
> > > > > We are good with:
> > > > >
> > > > > -- >8 --
> > > > > diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> > > > > index 29fd9019b8..dcec09b6f9 100644
> > > > > --- a/pc-bios/s390-ccw/Makefile
> > > > > +++ b/pc-bios/s390-ccw/Makefile
> > > > > @@ -8,6 +8,9 @@ CFLAGS = -O2 -g
> > > > >  quiet-command = $(if $(V),$1,$(if $(2),@printf "  %-7s %s\n" $2 $3 &&
> > > > > $1, @$1))
> > > > >  cc-option = $(if $(shell $(CC) $1 -S -o /dev/null -xc /dev/null >
> > > > > /dev/null \
> > > > >               2>&1 && echo OK), $1, $2)
> > > > > +cc-c-option = $(if $(shell $(CC) $1 $2 -c -o /dev/null -xc /dev/null \
> > > > > +                >/dev/null 2>&1 && echo OK), $2, $3)
> > > > >
> > > > >  VPATH_SUFFIXES = %.c %.h %.S %.m %.mak %.sh %.rc Kconfig% %.json.in
> > > > >  set-vpath = $(if $1,$(foreach PATTERN,$(VPATH_SUFFIXES),$(eval vpath
> > > > > $(PATTERN) $1)))
> > > > > @@ -30,6 +33,7 @@ OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o
> > > > > menu.o \
> > > > >           virtio.o virtio-scsi.o virtio-blkdev.o libc.o cio.o
> > > > > dasd-ipl.o
> > > > >
> > > > >  QEMU_CFLAGS := -Wall $(filter -W%, $(QEMU_CFLAGS))
> > > > > +QEMU_CFLAGS += $(call cc-c-option, $(QEMU_CFLAGS),
> > > > > -Wno-stringop-overflow)
> > > > >  QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks
> > > >
> > > > I think you don't need the "cc-c-option" magic here. Just add the
> > > > -Wno-stringop-overflow. All supported versions of GCC and Clang
> > > > simply silently ignore unknown -Wno-something options (try e.g. "gcc
> > > > -Wno-yadayadayada" on the command line).
> > >
> > > Clang seems not to like it. It's always a mess to do anything for gcc
> > > and clang :-(
>
> I didn't know we could build s390-ccw with clang.
>
> Not with clang-12:
>
> clang version 12.0.0 (Fedora 12.0.0-0.3.rc1.fc34)
>
> warning: unknown warning option '-Wold-style-declaration'; did you mean
> '-Wout-of-line-declaration'? [-Wunknown-warning-option]
> warning: unknown warning option '-Wimplicit-fallthrough=2'; did you mean
> '-Wimplicit-fallthrough'? [-Wunknown-warning-option]
> warning: unknown warning option '-Wno-stringop-overflow'; did you mean
> '-Wno-shift-overflow'? [-Wunknown-warning-option]
> warning: unknown warning option '-Wold-style-declaration'; did you mean
> '-Wout-of-line-declaration'? [-Wunknown-warning-option]
> warning: unknown warning option '-Wimplicit-fallthrough=2'; did you mean
> '-Wimplicit-fallthrough'? [-Wunknown-warning-option]
> warning: unknown warning option '-Wno-stringop-overflow'; did you mean
> '-Wno-shift-overflow'? [-Wunknown-warning-option]
> error: unknown target CPU 'z900'
> note: valid target CPU values are: arch8, z10, arch9, z196, arch10,
> zEC12, arch11, z13, arch12, z14, arch13, z15

Ah, I didn't know that, I just tried the unknown -Wno-yadayadayada :-)

> > > Maybe we can set also `-Wno-unknown-warning-option`.
> > >
> > > $ clang -Wno-stringop-overflow -Werror main.c
> > > error: unknown warning option '-Wno-stringop-overflow'; did you mean
> > > '-Wno-shift-overflow'? [-Werror,-Wunknown-warning-option]
> >
> > D'oh, I was pretty sure that Clang behaves the same with regards to
> > -Wno-something, but seems I was wrong, sorry.

Np :-) I'm always confused between gcc and clang...

> > So I think we should simply keep this v1 of Philippe's patch.

Yep, I agree.

Thanks,
Stefano