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

[Christian Borntraeger <borntraeger@de.ibm.com>]

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:
>>>>
>>>> On Thu, Apr 22, 2021 at 06:47:30AM +0200, Thomas Huth wrote:
>>>>> On 22/04/2021 06.18, Philippe Mathieu-Daudé wrote:
>>>>>> Hi Thomas, Daniel, Stefano,
>>>>>>
>>>>>> Regarding the following warning (GCC 11 on Fedora 34):
>>>>>>
>>>>>> In file included from pc-bios/s390-ccw/main.c:11:
>>>>>>
>>>>>> In function ‘memset’,
>>>>>>
>>>>>>       inlined from ‘boot_setup’ at pc-bios/s390-ccw/main.c:185:5,
>>>>>>
>>>>>>       inlined from ‘main’ at pc-bios/s390-ccw/main.c:288:5:
>>>>>>
>>>>>> pc-bios/s390-ccw/libc.h:28:14: warning: writing 1 byte into a region of
>>>>>> size 0 [-Wstringop-overflow=]
>>>>>>
>>>>>>      28 |         p[i] = c;
>>>>>>
>>>>>>         |         ~~~~~^~~
>>>>>>
>>>>>> Daniel were right on IRC:
>>>>>>
>>>>>> danpb: it is from a call  memset((char *)S390EP, 0, 6)     where  S390EP
>>>>>> is just a constant address 0x10008
>>>>>> danpb: the compiler doesn't now how big that is, so it seems to assume
>>>>>> it is zero length
>>>>>>
>>>>>> This is a known GCC issue:
>>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
>>>>>> "gcc-11 -Warray-bounds or -Wstringop-overread warning when accessing a
>>>>>> pointer from integer literal"
>>>>>
>>>>>   Hi Philippe,
>>>>>
>>>>> thanks for following up with the gcc bugzilla!
>>>>>
>>>>> ... so the problem is that GCC thinks we're in fact dereferencing a NULL
>>>>> pointer at offset 0x10008 here? Wow, that's ... crazy.
>>>>>
>>>>> Not sure what to do now - wait for the bug to get resolved? Compile the
>>>>> s390-ccw bios with -Wno-stringop-overread ? Add "volatiles" here and there
>>>>> to hope that these silence the compiler warnings? ... I tend to wait for the
>>>>> bug ticket to see whether the GCC folks change the behavior of the compiler
>>>>> again, but I'm open for other suggestions.
>>>>
>>>> Assuming it is just this one place in the code ,then we should just
>>>> use "pragma" to temporarily disable/re-enable that single warning flag
>>>> either side of the problem.
>>>
>>> The gcc bug report suggests that use of 'volatile' also sidesteps
>>> the warning. Is that a sensible approach here ?
>>
>> I'm not sure I got it right... I tried:
>>
>> -    memset((char *)S390EP, 0, 6);
>> +    memset((char *)(volatile char *)S390EP, 0, 6);
>>
>> But no change (still -Wstringop-overflow=).
>>
>> If I use:
>>
>> -    memset((char *)S390EP, 0, 6);
>> +    memset((volatile char *)S390EP, 0, 6);
>>
>> I still have -Wstringop-overflow=, but also:
>>
>> pc-bios/s390-ccw/main.c:185:12: warning: passing argument 1 of ‘memset’
>> discards ‘volatile’ qualifier from pointer target type
>> [-Wdiscarded-qualifiers]
>> pc-bios/s390-ccw/libc.h:22:34: note: expected ‘void *’ but argument is
>> of type ‘volatile char *’
> 
> Yeah, the warning happens in the memset(), so it likely doesn't help to change the parameter here.
> 
>> 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?

Ack. This warning does more harm than good in here.