Re: [Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member
On 4/8/19 10:12 PM, Marc-André Lureau wrote:
> The GCC9 compiler complains about QXL code that takes the address of
> members of the 'struct QXLReleaseRing' which is marked packed:
>
> CC hw/display/qxl.o
> /home/elmarco/src/qemu/hw/display/qxl.c: In function ‘init_qxl_ram’:
> /home/elmarco/src/qemu/hw/display/qxl.c:50:19: warning: taking address of
> packed member of ‘struct QXLReleaseRing_ring_el’ may result in an unaligned
> pointer value [-Waddress-of-packed-member]
>50 | ret = &(r)->items[prod].el;
> \
> | ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c:429:5: note: in expansion of macro
> ‘SPICE_RING_PROD_ITEM’
> 429 | SPICE_RING_PROD_ITEM(d, &d->ram->release_ring, item);
> | ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c: In function ‘qxl_push_free_res’:
> /home/elmarco/src/qemu/hw/display/qxl.c:50:19: warning: taking address of
> packed member of ‘struct QXLReleaseRing_ring_el’ may result in an unaligned
> pointer value [-Waddress-of-packed-member]
>50 | ret = &(r)->items[prod].el;
> \
> | ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c:762:5: note: in expansion of macro
> ‘SPICE_RING_PROD_ITEM’
> 762 | SPICE_RING_PROD_ITEM(d, ring, item);
> | ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c: In function
> ‘interface_release_resource’:
> /home/elmarco/src/qemu/hw/display/qxl.c:50:19: warning: taking address of
> packed member of ‘struct QXLReleaseRing_ring_el’ may result in an unaligned
> pointer value [-Waddress-of-packed-member]
>50 | ret = &(r)->items[prod].el;
> \
> | ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c:795:5: note: in expansion of macro
> ‘SPICE_RING_PROD_ITEM’
> 795 | SPICE_RING_PROD_ITEM(qxl, ring, item);
> | ^~~~
>
> Replace pointer usage by direct structure/array access instead.
>
> Signed-off-by: Marc-André Lureau
Tested-by: Philippe Mathieu-Daudé
> ---
> hw/display/qxl.c | 83 +---
> 1 file changed, 50 insertions(+), 33 deletions(-)
>
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index c8ce5781e0..12d83dd6f1 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -39,29 +39,49 @@
> * abort we just qxl_set_guest_bug and set the return to NULL. Still
> * it may happen as a result of emulator bug as well.
> */
> -#undef SPICE_RING_PROD_ITEM
> -#define SPICE_RING_PROD_ITEM(qxl, r, ret) { \
> -uint32_t prod = (r)->prod & SPICE_RING_INDEX_MASK(r); \
> -if (prod >= ARRAY_SIZE((r)->items)) { \
> -qxl_set_guest_bug(qxl, "SPICE_RING_PROD_ITEM indices mismatch " \
> - "%u >= %zu", prod, ARRAY_SIZE((r)->items)); \
> -ret = NULL; \
> -} else {\
> -ret = &(r)->items[prod].el; \
> -} \
> +#define SPICE_RING_GET_CHECK(qxl, r, field) ({ \
> +field = (r)->field & SPICE_RING_INDEX_MASK(r); \
> +bool mismatch = field >= ARRAY_SIZE((r)->items);\
> +if (mismatch) { \
> +qxl_set_guest_bug(qxl, "SPICE_RING_GET %s indices mismatch "\
> + "%u >= %zu", stringify(field), field, \
> + ARRAY_SIZE((r)->items)); \
> +} \
> +!mismatch; \
> +})
> +
> +static inline uint64_t
> +qxl_release_ring_get_prod(PCIQXLDevice *qxl)
> +{
> +struct QXLReleaseRing *ring = &qxl->ram->release_ring;
> +uint32_t prod;
> +bool ok = SPICE_RING_GET_CHECK(qxl, ring, prod);
> +assert(ok);
> +
> +return ring->items[prod].el;
> +}
> +
> +static inline bool
> +qxl_release_ring_set_prod(PCIQXLDevice *qxl, uint64_t val)
> +{
> +struct QXLReleaseRing *ring = &qxl->ram->release_ring;
> +uint32_t prod;
> +bool ok = SPICE_RING_GET_CHECK(qxl, ring, prod);
> +if (ok) {
> +ring->items[prod].el = val;
> }
> +return ok;
> +}
>
> #undef SPICE_RING_CONS_ITEM
> -#define SPICE_RING_CONS_ITEM(qxl, r, ret) { \
> -uint32_t cons = (r)->cons & SPICE_RING_INDEX_MASK(r); \
> -if (cons >= ARRAY_SIZE((r)->items)) { \
> -qxl_set_guest_bug(qxl, "SPICE_RING_CONS_ITEM indices mismatc
[Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member
The GCC9 compiler complains about QXL code that takes the address of
members of the 'struct QXLReleaseRing' which is marked packed:
CC hw/display/qxl.o
/home/elmarco/src/qemu/hw/display/qxl.c: In function ‘init_qxl_ram’:
/home/elmarco/src/qemu/hw/display/qxl.c:50:19: warning: taking address of
packed member of ‘struct QXLReleaseRing_ring_el’ may result in an unaligned
pointer value [-Waddress-of-packed-member]
50 | ret = &(r)->items[prod].el;
\
| ^~~~
/home/elmarco/src/qemu/hw/display/qxl.c:429:5: note: in expansion of macro
‘SPICE_RING_PROD_ITEM’
429 | SPICE_RING_PROD_ITEM(d, &d->ram->release_ring, item);
| ^~~~
/home/elmarco/src/qemu/hw/display/qxl.c: In function ‘qxl_push_free_res’:
/home/elmarco/src/qemu/hw/display/qxl.c:50:19: warning: taking address of
packed member of ‘struct QXLReleaseRing_ring_el’ may result in an unaligned
pointer value [-Waddress-of-packed-member]
50 | ret = &(r)->items[prod].el;
\
| ^~~~
/home/elmarco/src/qemu/hw/display/qxl.c:762:5: note: in expansion of macro
‘SPICE_RING_PROD_ITEM’
762 | SPICE_RING_PROD_ITEM(d, ring, item);
| ^~~~
/home/elmarco/src/qemu/hw/display/qxl.c: In function
‘interface_release_resource’:
/home/elmarco/src/qemu/hw/display/qxl.c:50:19: warning: taking address of
packed member of ‘struct QXLReleaseRing_ring_el’ may result in an unaligned
pointer value [-Waddress-of-packed-member]
50 | ret = &(r)->items[prod].el;
\
| ^~~~
/home/elmarco/src/qemu/hw/display/qxl.c:795:5: note: in expansion of macro
‘SPICE_RING_PROD_ITEM’
795 | SPICE_RING_PROD_ITEM(qxl, ring, item);
| ^~~~
Replace pointer usage by direct structure/array access instead.
Signed-off-by: Marc-André Lureau
---
hw/display/qxl.c | 83 +---
1 file changed, 50 insertions(+), 33 deletions(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index c8ce5781e0..12d83dd6f1 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -39,29 +39,49 @@
* abort we just qxl_set_guest_bug and set the return to NULL. Still
* it may happen as a result of emulator bug as well.
*/
-#undef SPICE_RING_PROD_ITEM
-#define SPICE_RING_PROD_ITEM(qxl, r, ret) { \
-uint32_t prod = (r)->prod & SPICE_RING_INDEX_MASK(r); \
-if (prod >= ARRAY_SIZE((r)->items)) { \
-qxl_set_guest_bug(qxl, "SPICE_RING_PROD_ITEM indices mismatch " \
- "%u >= %zu", prod, ARRAY_SIZE((r)->items)); \
-ret = NULL; \
-} else {\
-ret = &(r)->items[prod].el; \
-} \
+#define SPICE_RING_GET_CHECK(qxl, r, field) ({ \
+field = (r)->field & SPICE_RING_INDEX_MASK(r); \
+bool mismatch = field >= ARRAY_SIZE((r)->items);\
+if (mismatch) { \
+qxl_set_guest_bug(qxl, "SPICE_RING_GET %s indices mismatch "\
+ "%u >= %zu", stringify(field), field, \
+ ARRAY_SIZE((r)->items)); \
+} \
+!mismatch; \
+})
+
+static inline uint64_t
+qxl_release_ring_get_prod(PCIQXLDevice *qxl)
+{
+struct QXLReleaseRing *ring = &qxl->ram->release_ring;
+uint32_t prod;
+bool ok = SPICE_RING_GET_CHECK(qxl, ring, prod);
+assert(ok);
+
+return ring->items[prod].el;
+}
+
+static inline bool
+qxl_release_ring_set_prod(PCIQXLDevice *qxl, uint64_t val)
+{
+struct QXLReleaseRing *ring = &qxl->ram->release_ring;
+uint32_t prod;
+bool ok = SPICE_RING_GET_CHECK(qxl, ring, prod);
+if (ok) {
+ring->items[prod].el = val;
}
+return ok;
+}
#undef SPICE_RING_CONS_ITEM
-#define SPICE_RING_CONS_ITEM(qxl, r, ret) { \
-uint32_t cons = (r)->cons & SPICE_RING_INDEX_MASK(r); \
-if (cons >= ARRAY_SIZE((r)->items)) { \
-qxl_set_guest_bug(qxl, "SPICE_RING_CONS_ITEM indices mismatch " \
- "%u >= %zu", cons, ARRAY_SIZE((r)->items)); \
-ret = NULL; \
-} else {\
-ret = &(r)->items[cons].el;
