Stefan Weil <s...@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