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

[Stefano Garzarella <sgarzare@redhat.com>]

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