[Qemu-devel] [PATCH] pci: Fix compiler warning (MinGW-w64 gcc 4.9)

[Qemu-devel] [PATCH] pci: Fix compiler warning (MinGW-w64 gcc 4.9)

[Stefan Weil]

i686-w64-mingw32-gcc 4.9.1 from Debian Jessie complains:

hw/pci/pci.c:938:29: warning:
 array subscript is above array bounds [-Warray-bounds]

Using g_assert instead of assert fixes this warning.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 hw/pci/pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 48f19a3..34f71dc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -927,8 +927,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
     uint64_t wmask;
     pcibus_t size = memory_region_size(memory);
 
-    assert(region_num >= 0);
-    assert(region_num < PCI_NUM_REGIONS);
+    g_assert(region_num >= 0);
+    g_assert(region_num < PCI_NUM_REGIONS);
     if (size & (size-1)) {
         fprintf(stderr, "ERROR: PCI region size must be pow2 "
                     "type=0x%x, size=0x%"FMT_PCIBUS"\n", type, size);
-- 
2.1.4

Re: [Qemu-devel] [PATCH] pci: Fix compiler warning (MinGW-w64 gcc 4.9)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]

On 05/14/2015 02:38 PM, Stefan Weil wrote:
> i686-w64-mingw32-gcc 4.9.1 from Debian Jessie complains:
> 
> hw/pci/pci.c:938:29: warning:
>  array subscript is above array bounds [-Warray-bounds]
> 
> Using g_assert instead of assert fixes this warning.

Is that because the mingw headers don't properly mark the expansion of
the failed branch of assert() as noreturn, whereas g_assert() does, and
therefore the compiler has more information about what variables must be
if the rest of the function is reached?

> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  hw/pci/pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 48f19a3..34f71dc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -927,8 +927,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
>      uint64_t wmask;
>      pcibus_t size = memory_region_size(memory);
>  
> -    assert(region_num >= 0);
> -    assert(region_num < PCI_NUM_REGIONS);
> +    g_assert(region_num >= 0);
> +    g_assert(region_num < PCI_NUM_REGIONS);

I must say, this is the strangest way I've ever heard of silencing a
compiler warning.  But if it works and my uneducated guess above about
why is correct,

Reviewed-by: Eric Blake <eblake@redhat.com>

Meanwhile, you may want to file a bug to the mingw maintainers that
their header is puny when compared to glibc assert() or to glib's
g_assert, when it comes to giving gcc decent hints.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] pci: Fix compiler warning (MinGW-w64 gcc 4.9)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

On 05/14/2015 03:06 PM, Eric Blake wrote:
> On 05/14/2015 02:38 PM, Stefan Weil wrote:
>> i686-w64-mingw32-gcc 4.9.1 from Debian Jessie complains:
>>
>> hw/pci/pci.c:938:29: warning:
>>  array subscript is above array bounds [-Warray-bounds]
>>
>> Using g_assert instead of assert fixes this warning.
> 
> Is that because the mingw headers don't properly mark the expansion of
> the failed branch of assert() as noreturn, whereas g_assert() does, and
> therefore the compiler has more information about what variables must be
> if the rest of the function is reached?
> 

> 
> Meanwhile, you may want to file a bug to the mingw maintainers that
> their header is puny when compared to glibc assert() or to glib's
> g_assert, when it comes to giving gcc decent hints.

Oh, I was right!

glibc /usr/include/assert.h:
extern void __assert_fail (const char *__assertion, const char *__file,
                           unsigned int __line, const char *__function)
     __THROW __attribute__ ((__noreturn__));
# define assert(expr)                                                   \
  ((expr)                                                               \
   ? __ASSERT_VOID_CAST (0)                                             \
   : __assert_fail (__STRING(expr), __FILE__, __LINE__, __ASSERT_FUNCTION))

vs mingw /usr/i686-w64-mingw32/sys-root/mingw/include/assert.h:
extern void __cdecl
_assert (const char *_Message, const char *_File, unsigned _Line);
#define assert(_Expression) \
 (void) \
 ((!!(_Expression)) || \
  (_assert(#_Expression,__FILE__,__LINE__),0))

even though mingw has in that same file:
  void __cdecl _Exit(int) __MINGW_ATTRIB_NORETURN;

so it is indeed a bug in the mingw headers that __MINGW_ATTRIB_NORETURN
was not attached to _assert's declaration.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] pci: Fix compiler warning (MinGW-w64 gcc 4.9)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On 05/14/2015 03:16 PM, Eric Blake wrote:
>> Meanwhile, you may want to file a bug to the mingw maintainers that
>> their header is puny when compared to glibc assert() or to glib's
>> g_assert, when it comes to giving gcc decent hints.
> 
> Oh, I was right!

> 
> so it is indeed a bug in the mingw headers that __MINGW_ATTRIB_NORETURN
> was not attached to _assert's declaration.

Filed a Fedora bug at
https://bugzilla.redhat.com/show_bug.cgi?id=1221811 but of course it
will take a while for fixed headers to propagate to all distros, so this
patch is fine for qemu in the meantime.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] pci: Fix compiler warning (MinGW-w64 gcc 4.9)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1842 bytes --]

On 05/14/2015 03:16 PM, Eric Blake wrote:
> On 05/14/2015 03:06 PM, Eric Blake wrote:
>> On 05/14/2015 02:38 PM, Stefan Weil wrote:
>>> i686-w64-mingw32-gcc 4.9.1 from Debian Jessie complains:
>>>
>>> hw/pci/pci.c:938:29: warning:
>>>  array subscript is above array bounds [-Warray-bounds]
>>>
>>> Using g_assert instead of assert fixes this warning.
>>
>> Is that because the mingw headers don't properly mark the expansion of
>> the failed branch of assert() as noreturn, whereas g_assert() does, and
>> therefore the compiler has more information about what variables must be
>> if the rest of the function is reached?
>>
> 
>>
>> Meanwhile, you may want to file a bug to the mingw maintainers that
>> their header is puny when compared to glibc assert() or to glib's
>> g_assert, when it comes to giving gcc decent hints.
> 
> Oh, I was right!
> 
> glibc /usr/include/assert.h:

> vs mingw /usr/i686-w64-mingw32/sys-root/mingw/include/assert.h:

and glib's version in /usr/include/glib-2.0/glib/gtestutils.h:
#define g_assert(expr)                  G_STMT_START { \
                                             if G_LIKELY (expr) ; else \
                                               g_assertion_message_expr
(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \

 #expr); \
                                        } G_STMT_END

GLIB_AVAILABLE_IN_ALL
void    g_assertion_message_expr        (const char     *domain,
                                         const char     *file,
                                         int             line,
                                         const char     *func,
                                         const char     *expr)
G_GNUC_NORETURN;


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] pci: Fix compiler warning (MinGW-w64 gcc 4.9)

[Markus Armbruster]

Stefan Weil <sw@weilnetz.de> writes:

> i686-w64-mingw32-gcc 4.9.1 from Debian Jessie complains:
>
> hw/pci/pci.c:938:29: warning:
>  array subscript is above array bounds [-Warray-bounds]
>
> Using g_assert instead of assert fixes this warning.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  hw/pci/pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 48f19a3..34f71dc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -927,8 +927,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
>      uint64_t wmask;
>      pcibus_t size = memory_region_size(memory);
>  
> -    assert(region_num >= 0);
> -    assert(region_num < PCI_NUM_REGIONS);
> +    g_assert(region_num >= 0);
> +    g_assert(region_num < PCI_NUM_REGIONS);
>      if (size & (size-1)) {
>          fprintf(stderr, "ERROR: PCI region size must be pow2 "
>                      "type=0x%x, size=0x%"FMT_PCIBUS"\n", type, size);

This is obviously a bug in that version of MinGW.  Have you reported it?
Do we really want to work around it?

Re: [Qemu-devel] [PATCH] pci: Fix compiler warning (MinGW-w64 gcc 4.9)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 930 bytes --]

On 05/15/2015 02:00 AM, Markus Armbruster wrote:
> Stefan Weil <sw@weilnetz.de> writes:
> 
>> i686-w64-mingw32-gcc 4.9.1 from Debian Jessie complains:
>>

> 
> This is obviously a bug in that version of MinGW.  Have you reported it?

I don't know if Debian has a bug filed against mingw yet, but I
mentioned elsewhere in the thread that I filed a bug for Fedora which is
also impacted.

> Do we really want to work around it?

That, I'm not sure.  It takes a while for updated mingw headers to
propagate, and the fix is small enough that it won't hurt to leave the
fix in place even after updated headers are available.  But since the
problem is a missing attribute in the header causing a compiler warning,
it could be worked around by turning off compiler warnings until a new
header is present.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]