[Qemu-devel] [PATCH] Fix compilation on GCC 4.5

[Qemu-devel] [PATCH] Fix compilation on GCC 4.5

[Søren Sandmann]

From: Søren Sandmann Pedersen <ssp@redhat.com>

Apparently GCC 4.5 still warns about "value computed not used" even
with __attribute__((unused)). Fix this by only doing the compile time
check on gcc > 4.5.

Signed-off-by: Soren Sandmann <ssp@redhat.com>
---

I need this patch to get qemu to compile with GCC 4.5, but I'm not
sure if 4.5 is the right compiler version to check against.

 osdep.h |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/osdep.h b/osdep.h
index cb213e0..df89552 100644
--- a/osdep.h
+++ b/osdep.h
@@ -41,8 +41,9 @@ typedef signed int              int_fast16_t;
 #endif
 
 /* Convert from a base type to a parent type, with compile time checking.  */
-#ifdef __GNUC__
-#define DO_UPCAST(type, field, dev) ( __extension__ ( { \
+#if defined (__GNUC__) && \
+    (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 5))
+#define DO_UPCAST(type, field, dev) ( __extension__ ( {	\
     char __attribute__((unused)) offset_must_be_zero[ \
         -offsetof(type, field)]; \
     container_of(dev, type, field);}))
-- 
1.7.4

Re: [Qemu-devel] [PATCH] Fix compilation on GCC 4.5

[Stefan Weil]

Am 03.10.2012 20:04, schrieb Søren Sandmann:
> From: Søren Sandmann Pedersen<ssp@redhat.com>
>
> Apparently GCC 4.5 still warns about "value computed not used" even
> with __attribute__((unused)). Fix this by only doing the compile time
> check on gcc>  4.5.
>
> Signed-off-by: Soren Sandmann<ssp@redhat.com>
> ---
>
> I need this patch to get qemu to compile with GCC 4.5, but I'm not
> sure if 4.5 is the right compiler version to check against.
>
>   osdep.h |    5 +++--
>   1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/osdep.h b/osdep.h
> index cb213e0..df89552 100644
> --- a/osdep.h
> +++ b/osdep.h
> @@ -41,8 +41,9 @@ typedef signed int              int_fast16_t;
>   #endif
>
>   /* Convert from a base type to a parent type, with compile time checking.  */
> -#ifdef __GNUC__
> -#define DO_UPCAST(type, field, dev) ( __extension__ ( { \
> +#if defined (__GNUC__)&&  \
> +    (__GNUC__>  4 || (__GNUC__ == 4&&  __GNUC_MINOR__>  5))
> +#define DO_UPCAST(type, field, dev) ( __extension__ ( {	\
>       char __attribute__((unused)) offset_must_be_zero[ \
>           -offsetof(type, field)]; \
>       container_of(dev, type, field);}))

Hello,

I don't see any warnings with gcc Debian 4.4.5-8.

Could you please provide more information on your environment
and the warnings which you get?

Here is a macro which simplifies the version check:

#if QEMU_GNUC_PREREQ(4, 6)

Regards

Stefan Weil

Re: [Qemu-devel] [PATCH] Fix compilation on GCC 4.5

[Søren Sandmann]

Stefan Weil <sw@weilnetz.de> writes:

> I don't see any warnings with gcc Debian 4.4.5-8.
>
> Could you please provide more information on your environment
> and the warnings which you get?
>
> Here is a macro which simplifies the version check:
>
> #if QEMU_GNUC_PREREQ(4, 6)
>
> Regards
>
> Stefan Weil

With this compiler:

    dhcp-100-3-184:~/vertigo/qemu% gcc -v
    Using built-in specs.
    COLLECT_GCC=/usr/bin/gcc
    COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.5.1/lto-wrapper
    Target: x86_64-redhat-linux
    Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
    --infodir=/usr/share/info
    --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap
    --enable-shared --enable-threads=posix --enable-checking=release
    --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions
    --enable-gnu-unique-object --enable-linker-build-id
    --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,lto
    --enable-plugin --enable-java-awt=gtk --disable-dssi
    --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre
    --enable-libgcj-multifile --enable-java-maintainer-mode
    --with-ecj-jar=/usr/share/java/eclipse-ecj.jar
    --disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic
    --with-arch_32=i686 --build=x86_64-redhat-linux
    Thread model: posix
    gcc version 4.5.1 20100924 (Red Hat 4.5.1-4) (GCC) 

This is the error I get:

    make  BUILD_DIR=/home/ssp/vertigo/qemu -C i386-softmmu V="1"
    TARGET_DIR="i386-softmmu/" all
    make[1]: Entering directory `/home/ssp/vertigo/qemu/i386-softmmu'
    gcc -I/home/ssp/vertigo/qemu/slirp -I. -I/home/ssp/vertigo/qemu
    -I/home/ssp/vertigo/qemu/fpu -I/home/ssp/vertigo/qemu/libcacard
    -I/home/ssp/vertigo/qemu/linux-headers -I/home/ssp/vertigo/qemu/tcg
    -I/home/ssp/vertigo/qemu/tcg/i386  -Werror -fPIE -DPIE -m64
    -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
    -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings
    -Wmissing-prototypes -fno-strict-aliasing  -fstack-protector-all
    -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs
    -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
    -Wold-style-declaration -Wold-style-definition -Wtype-limits
    -I/usr/include/libpng12   -I/usr/include/spice-server
    -I/usr/include/pixman-1 -I/usr/include/spice-1   -I/usr/include/nss3
    -I/usr/include/nspr4   -pthread -I/usr/include/glib-2.0
    -I/usr/lib64/glib-2.0/include   -DHAS_AUDIO -DHAS_AUDIO_CHOICE
    -DTARGET_PHYS_ADDR_BITS=64 -I../linux-headers
    -I.. -I/home/ssp/vertigo/qemu/target-i386 -DNEED_CPU_H
    -I/home/ssp/vertigo/qemu/include    -I/usr/include/libpng12   -pthread
    -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -MMD -MP -MT
    hw/vfio_pci.o -MF hw/vfio_pci.d -O2 -D_FORTIFY_SOURCE=2 -g  -c -o
    hw/vfio_pci.o /home/ssp/vertigo/qemu/hw/vfio_pci.c
    cc1: warnings being treated as errors
    /home/ssp/vertigo/qemu/hw/vfio_pci.c: In function ‘vfio_bar_write’:
    /home/ssp/vertigo/qemu/hw/vfio_pci.c:604:84: error: value computed is not used
    /home/ssp/vertigo/qemu/hw/vfio_pci.c: In function ‘vfio_bar_read’:
    /home/ssp/vertigo/qemu/hw/vfio_pci.c:644:84: error: value computed is
    not used
    make[1]: *** [hw/vfio_pci.o] Error 1
    make[1]: Leaving directory `/home/ssp/vertigo/qemu/i386-softmmu'
    make: *** [subdir-i386-softmmu] Error 2

Re: [Qemu-devel] [PATCH] Fix compilation on GCC 4.5

[Stefan Weil]

Am 03.10.2012 21:49, schrieb Søren Sandmann:
> Stefan Weil<sw@weilnetz.de>  writes:
>
>> I don't see any warnings with gcc Debian 4.4.5-8.
>>
>> Could you please provide more information on your environment
>> and the warnings which you get?
>>
>> Here is a macro which simplifies the version check:
>>
>> #if QEMU_GNUC_PREREQ(4, 6)
>>
>> Regards
>>
>> Stefan Weil
>
> With this compiler:
>
>      dhcp-100-3-184:~/vertigo/qemu% gcc -v
>      Using built-in specs.
>      COLLECT_GCC=/usr/bin/gcc
>      COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.5.1/lto-wrapper
>      Target: x86_64-redhat-linux
>      Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
>      --infodir=/usr/share/info
>      --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap
>      --enable-shared --enable-threads=posix --enable-checking=release
>      --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions
>      --enable-gnu-unique-object --enable-linker-build-id
>      --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,lto
>      --enable-plugin --enable-java-awt=gtk --disable-dssi
>      --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre
>      --enable-libgcj-multifile --enable-java-maintainer-mode
>      --with-ecj-jar=/usr/share/java/eclipse-ecj.jar
>      --disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic
>      --with-arch_32=i686 --build=x86_64-redhat-linux
>      Thread model: posix
>      gcc version 4.5.1 20100924 (Red Hat 4.5.1-4) (GCC)
>
> This is the error I get:
>
>      make  BUILD_DIR=/home/ssp/vertigo/qemu -C i386-softmmu V="1"
>      TARGET_DIR="i386-softmmu/" all
>      make[1]: Entering directory `/home/ssp/vertigo/qemu/i386-softmmu'
>      gcc -I/home/ssp/vertigo/qemu/slirp -I. -I/home/ssp/vertigo/qemu
>      -I/home/ssp/vertigo/qemu/fpu -I/home/ssp/vertigo/qemu/libcacard
>      -I/home/ssp/vertigo/qemu/linux-headers -I/home/ssp/vertigo/qemu/tcg
>      -I/home/ssp/vertigo/qemu/tcg/i386  -Werror -fPIE -DPIE -m64
>      -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
>      -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings
>      -Wmissing-prototypes -fno-strict-aliasing  -fstack-protector-all
>      -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs
>      -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
>      -Wold-style-declaration -Wold-style-definition -Wtype-limits
>      -I/usr/include/libpng12   -I/usr/include/spice-server
>      -I/usr/include/pixman-1 -I/usr/include/spice-1   -I/usr/include/nss3
>      -I/usr/include/nspr4   -pthread -I/usr/include/glib-2.0
>      -I/usr/lib64/glib-2.0/include   -DHAS_AUDIO -DHAS_AUDIO_CHOICE
>      -DTARGET_PHYS_ADDR_BITS=64 -I../linux-headers
>      -I.. -I/home/ssp/vertigo/qemu/target-i386 -DNEED_CPU_H
>      -I/home/ssp/vertigo/qemu/include    -I/usr/include/libpng12   -pthread
>      -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -MMD -MP -MT
>      hw/vfio_pci.o -MF hw/vfio_pci.d -O2 -D_FORTIFY_SOURCE=2 -g  -c -o
>      hw/vfio_pci.o /home/ssp/vertigo/qemu/hw/vfio_pci.c
>      cc1: warnings being treated as errors
>      /home/ssp/vertigo/qemu/hw/vfio_pci.c: In function ‘vfio_bar_write’:
>      /home/ssp/vertigo/qemu/hw/vfio_pci.c:604:84: error: value computed is not used
>      /home/ssp/vertigo/qemu/hw/vfio_pci.c: In function ‘vfio_bar_read’:
>      /home/ssp/vertigo/qemu/hw/vfio_pci.c:644:84: error: value computed is
>      not used
>      make[1]: *** [hw/vfio_pci.o] Error 1
>      make[1]: Leaving directory `/home/ssp/vertigo/qemu/i386-softmmu'
>      make: *** [subdir-i386-softmmu] Error 2

That's strange.

The lines which cause compiler errors look like this:

     vfio_eoi(DO_UPCAST(VFIODevice, bars[bar->nr], bar));

There are more uses of DO_UPCAST without any compiler error:

     VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, pdev);

Neither of both lines creates a compiler error with any of my
compilers (gcc 4.4 up to latest gcc, Linux and MinGW hosts).

When I compile with gcc option -save-temps, I get this code
for the first line:

     vfio_eoi(( __extension__ ( { char __attribute__((unused)) 
offset_must_be_zero[ -__builtin_offsetof (VFIODevice, bars[bar->nr])]; 
({ const typeof(((VFIODevice *) 0)->bars[bar->nr]) *__mptr = (bar); 
(VFIODevice *) ((char *) __mptr - __builtin_offsetof (VFIODevice, 
bars[bar->nr]));});})));

Could you please replace the first line by that code and try your compiler?
If it remains silent, I suspect a bad offsetof macro.

Do you compile on Linux? Which distribution or which C library do you use?

Regards

Stefan Weil

Re: [Qemu-devel] [PATCH] Fix compilation on GCC 4.5

[Søren Sandmann]

Stefan Weil <sw@weilnetz.de> writes:

> That's strange.
>
> The lines which cause compiler errors look like this:
>
>     vfio_eoi(DO_UPCAST(VFIODevice, bars[bar->nr], bar));
>
> There are more uses of DO_UPCAST without any compiler error:
>
>     VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
>
> Neither of both lines creates a compiler error with any of my
> compilers (gcc 4.4 up to latest gcc, Linux and MinGW hosts).

Maybe the difference is that bars[bar->nr] is an expression whereas pdev
is just an identifier.

But on closer look, the macro actually looks pretty suspicious to
me. The comment above it that says it does compile time checking, is
probably not true. In the case where 'field' is bars[bar->nr] I'd be
very surprised if the compiler is smart enough to optimize the array
away entirely. It would have to reason like this: The output of
__builtin_offset is always >= 0, so the size of the array can only ever
be negative or zero, which means the array can be deleted because if the
length isn't zero, the program invokes undefined behavior.

It *could* do this, but does it, and is that something qemu should rely
on?

Even if this probably small runtime hit is deemed acceptable, it's
likely surprising to many users of the macro that the 'field' expression
is evaluated twice, so if the "compile time checking" is staying, maybe
it should be done with something like this instead:

diff --git a/osdep.h b/osdep.h
index cb213e0..51ffab0 100644
--- a/osdep.h
+++ b/osdep.h
@@ -42,10 +42,11 @@ typedef signed int              int_fast16_t;
 
 /* Convert from a base type to a parent type, with compile time
 checking.  */
 #ifdef __GNUC__
-#define DO_UPCAST(type, field, dev) ( __extension__ ( { \
-    char __attribute__((unused)) offset_must_be_zero[ \
-        -offsetof(type, field)]; \
-    container_of(dev, type, field);}))
+#define DO_UPCAST(type, field, dev) ( __extension__ ( { \
+    const typeof(((type *) 0)->field) *__mptr = (dev); \
+    ssize_t __offset = - offsetof (type, field); \
+    char __attribute__((unused)) offset_must_be_zero[ __offset ]; \
+    ((type *) ((char *) __mptr + __offset)); } ) )
 #else
 #define DO_UPCAST(type, field, dev) container_of(dev, type, field)
 #endif

This compiles with my GCC, but I haven't tested the resulting binary at
all.

> When I compile with gcc option -save-temps, I get this code
> for the first line:
>
>     vfio_eoi(( __extension__ ( { char __attribute__((unused))
> offset_must_be_zero[ -__builtin_offsetof (VFIODevice, bars[bar->nr])];
> ({ const typeof(((VFIODevice *) 0)->bars[bar->nr]) *__mptr = (bar);
> (VFIODevice *) ((char *) __mptr - __builtin_offsetof (VFIODevice,
> bars[bar->nr]));});})));

I get what appears to be exactly the same:

 vfio_eoi(( __extension__ ( { char __attribute__((unused))
 offset_must_be_zero[ -__builtin_offsetof (VFIODevice, bars[bar->nr])];
 ({ const typeof(((VFIODevice *) 0)->bars[bar->nr]) *__mptr = (bar);
 (VFIODevice *) ((char *) __mptr - __builtin_offsetof (VFIODevice,
 bars[bar->nr]));});})));

> Could you please replace the first line by that code and try your compiler?
> If it remains silent, I suspect a bad offsetof macro.

Replacing the vfio_eoi[...] lines with the expansion that you get
doesn't change anything. It still produces a warning which becomes an
error due to -Werror.

> Do you compile on Linux? Which distribution or which C library do you
> use?

Fedora 14.

dhcp-100-3-184:~% rpm -q glibc
glibc-2.13-2.x86_64
glibc-2.13-2.i686

Here is a small program that reproduces the behavior:

    dhcp-100-3-184:~% cat offset.c 

    struct foo
    {
        int bar[17];
    };
    
    int external (void);
    
    int
    main ()
    {
        struct foo baz = { { 0 } };
    
        __extension__ ( {
            char offset_must_be_zero [ - __builtin_offsetof (struct foo,
                 bar[external()])]  __attribute ((unused));
            });
    
        return baz.bar[8];
    }

    dhcp-100-3-184:~% gcc -Werror -W -Wall -Wunused offset.c 
    cc1: warnings being treated as errors
    offset.c: In function ‘main’:
    offset.c:16:25: error: value computed is not used


Søren 

Re: [Qemu-devel] [PATCH] Fix compilation on GCC 4.5

[Richard Henderson]

On 10/03/2012 11:04 AM, Søren Sandmann wrote:
> From: Søren Sandmann Pedersen <ssp@redhat.com>
> 
> Apparently GCC 4.5 still warns about "value computed not used" even
> with __attribute__((unused)). Fix this by only doing the compile time
> check on gcc > 4.5.
> 
> Signed-off-by: Soren Sandmann <ssp@redhat.com>
> ---
> 
> I need this patch to get qemu to compile with GCC 4.5, but I'm not
> sure if 4.5 is the right compiler version to check against.
> 
>  osdep.h |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/osdep.h b/osdep.h
> index cb213e0..df89552 100644
> --- a/osdep.h
> +++ b/osdep.h
> @@ -41,8 +41,9 @@ typedef signed int              int_fast16_t;
>  #endif
>  
>  /* Convert from a base type to a parent type, with compile time checking.  */
> -#ifdef __GNUC__
> -#define DO_UPCAST(type, field, dev) ( __extension__ ( { \
> +#if defined (__GNUC__) && \
> +    (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 5))
> +#define DO_UPCAST(type, field, dev) ( __extension__ ( {	\
>      char __attribute__((unused)) offset_must_be_zero[ \
>          -offsetof(type, field)]; \
>      container_of(dev, type, field);}))

Try moving the attribute to the end of the declaration,
i.e. just before the semi-colon.


r~

Re: [Qemu-devel] [PATCH] Fix compilation on GCC 4.5

[Søren Sandmann]

Richard Henderson <rth@twiddle.net> writes:

>> diff --git a/osdep.h b/osdep.h
>> index cb213e0..df89552 100644
>> --- a/osdep.h
>> +++ b/osdep.h
>> @@ -41,8 +41,9 @@ typedef signed int              int_fast16_t;
>>  #endif
>>  
>>  /* Convert from a base type to a parent type, with compile time checking.  */
>> -#ifdef __GNUC__
>> -#define DO_UPCAST(type, field, dev) ( __extension__ ( { \
>> +#if defined (__GNUC__) && \
>> +    (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 5))
>> +#define DO_UPCAST(type, field, dev) ( __extension__ ( {	\
>>      char __attribute__((unused)) offset_must_be_zero[ \
>>          -offsetof(type, field)]; \
>>      container_of(dev, type, field);}))
>
> Try moving the attribute to the end of the declaration,
> i.e. just before the semi-colon.

The patch below doesn't make any difference. 

Søren

diff --git a/osdep.h b/osdep.h
index cb213e0..1a36c20 100644
--- a/osdep.h
+++ b/osdep.h
@@ -43,8 +43,8 @@ typedef signed int              int_fast16_t;
 /* Convert from a base type to a parent type, with compile time
 checking.  */
 #ifdef __GNUC__
 #define DO_UPCAST(type, field, dev) ( __extension__ ( { \
-    char __attribute__((unused)) offset_must_be_zero[ \
-        -offsetof(type, field)]; \
+    char offset_must_be_zero[ \
+        -offsetof(type, field)] __attribute__((unused)) ; \
     container_of(dev, type, field);}))
 #else
 #define DO_UPCAST(type, field, dev) container_of(dev, type, field)