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

[Thomas Huth <thuth@redhat.com>]

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?

  Thomas