* [Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member
@ 2019-04-08 20:12 Marc-André Lureau
2019-04-08 20:12 ` Marc-André Lureau
2019-05-03 16:18 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 4+ messages in thread
From: Marc-André Lureau @ 2019-04-08 20:12 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, Marc-André Lureau
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 <marcandre.lureau@redhat.com>
---
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; \
- } \
- }
+#define SPICE_RING_CONS_ITEM(qxl, r, ret) { \
+ uint32_t cons; \
+ if (!SPICE_RING_GET_CHECK(qxl, r, cons)) { \
+ ret = NULL; \
+ } else { \
+ ret = &(r)->items[cons].el; \
+ } \
+}
#undef ALIGN
#define ALIGN(a, b) (((a) + ((b) - 1)) & ~((b) - 1))
@@ -414,7 +434,6 @@ static void init_qxl_rom(PCIQXLDevice *d)
static void init_qxl_ram(PCIQXLDevice *d)
{
uint8_t *buf;
- uint64_t *item;
buf = d->vga.vram_ptr;
d->ram = (QXLRam *)(buf + le32_to_cpu(d->shadow_rom.ram_header_offset));
@@ -426,9 +445,9 @@ static void init_qxl_ram(PCIQXLDevice *d)
SPICE_RING_INIT(&d->ram->cmd_ring);
SPICE_RING_INIT(&d->ram->cursor_ring);
SPICE_RING_INIT(&d->ram->release_ring);
- SPICE_RING_PROD_ITEM(d, &d->ram->release_ring, item);
- assert(item);
- *item = 0;
+ if (!qxl_release_ring_set_prod(d, 0)) {
+ g_assert_not_reached();
+ }
qxl_ring_set_dirty(d);
}
@@ -732,7 +751,6 @@ static int interface_req_cmd_notification(QXLInstance *sin)
static inline void qxl_push_free_res(PCIQXLDevice *d, int flush)
{
QXLReleaseRing *ring = &d->ram->release_ring;
- uint64_t *item;
int notify;
#define QXL_FREE_BUNCH_SIZE 32
@@ -759,11 +777,9 @@ static inline void qxl_push_free_res(PCIQXLDevice *d, int flush)
if (notify) {
qxl_send_events(d, QXL_INTERRUPT_DISPLAY);
}
- SPICE_RING_PROD_ITEM(d, ring, item);
- if (!item) {
+ if (!qxl_release_ring_set_prod(d, 0)) {
return;
}
- *item = 0;
d->num_free_res = 0;
d->last_release = NULL;
qxl_ring_set_dirty(d);
@@ -775,7 +791,8 @@ static void interface_release_resource(QXLInstance *sin,
{
PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
QXLReleaseRing *ring;
- uint64_t *item, id;
+ uint32_t prod;
+ uint64_t id;
if (ext.group_id == MEMSLOT_GROUP_HOST) {
/* host group -> vga mode update request */
@@ -792,16 +809,16 @@ static void interface_release_resource(QXLInstance *sin,
* pci bar 0, $command.release_info
*/
ring = &qxl->ram->release_ring;
- SPICE_RING_PROD_ITEM(qxl, ring, item);
- if (!item) {
+
+ if (!SPICE_RING_GET_CHECK(qxl, ring, prod)) {
return;
}
- if (*item == 0) {
+ if (qxl_release_ring_get_prod(qxl) == 0) {
/* stick head into the ring */
id = ext.info->id;
ext.info->next = 0;
qxl_ram_set_dirty(qxl, &ext.info->next);
- *item = id;
+ qxl_release_ring_set_prod(qxl, id);
qxl_ring_set_dirty(qxl);
} else {
/* append item to the list */
--
2.21.0.196.g041f5ea1cf
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member
2019-04-08 20:12 [Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member Marc-André Lureau
@ 2019-04-08 20:12 ` Marc-André Lureau
2019-05-03 16:18 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 4+ messages in thread
From: Marc-André Lureau @ 2019-04-08 20:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, kraxel
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 <marcandre.lureau@redhat.com>
---
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; \
- } \
- }
+#define SPICE_RING_CONS_ITEM(qxl, r, ret) { \
+ uint32_t cons; \
+ if (!SPICE_RING_GET_CHECK(qxl, r, cons)) { \
+ ret = NULL; \
+ } else { \
+ ret = &(r)->items[cons].el; \
+ } \
+}
#undef ALIGN
#define ALIGN(a, b) (((a) + ((b) - 1)) & ~((b) - 1))
@@ -414,7 +434,6 @@ static void init_qxl_rom(PCIQXLDevice *d)
static void init_qxl_ram(PCIQXLDevice *d)
{
uint8_t *buf;
- uint64_t *item;
buf = d->vga.vram_ptr;
d->ram = (QXLRam *)(buf + le32_to_cpu(d->shadow_rom.ram_header_offset));
@@ -426,9 +445,9 @@ static void init_qxl_ram(PCIQXLDevice *d)
SPICE_RING_INIT(&d->ram->cmd_ring);
SPICE_RING_INIT(&d->ram->cursor_ring);
SPICE_RING_INIT(&d->ram->release_ring);
- SPICE_RING_PROD_ITEM(d, &d->ram->release_ring, item);
- assert(item);
- *item = 0;
+ if (!qxl_release_ring_set_prod(d, 0)) {
+ g_assert_not_reached();
+ }
qxl_ring_set_dirty(d);
}
@@ -732,7 +751,6 @@ static int interface_req_cmd_notification(QXLInstance *sin)
static inline void qxl_push_free_res(PCIQXLDevice *d, int flush)
{
QXLReleaseRing *ring = &d->ram->release_ring;
- uint64_t *item;
int notify;
#define QXL_FREE_BUNCH_SIZE 32
@@ -759,11 +777,9 @@ static inline void qxl_push_free_res(PCIQXLDevice *d, int flush)
if (notify) {
qxl_send_events(d, QXL_INTERRUPT_DISPLAY);
}
- SPICE_RING_PROD_ITEM(d, ring, item);
- if (!item) {
+ if (!qxl_release_ring_set_prod(d, 0)) {
return;
}
- *item = 0;
d->num_free_res = 0;
d->last_release = NULL;
qxl_ring_set_dirty(d);
@@ -775,7 +791,8 @@ static void interface_release_resource(QXLInstance *sin,
{
PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
QXLReleaseRing *ring;
- uint64_t *item, id;
+ uint32_t prod;
+ uint64_t id;
if (ext.group_id == MEMSLOT_GROUP_HOST) {
/* host group -> vga mode update request */
@@ -792,16 +809,16 @@ static void interface_release_resource(QXLInstance *sin,
* pci bar 0, $command.release_info
*/
ring = &qxl->ram->release_ring;
- SPICE_RING_PROD_ITEM(qxl, ring, item);
- if (!item) {
+
+ if (!SPICE_RING_GET_CHECK(qxl, ring, prod)) {
return;
}
- if (*item == 0) {
+ if (qxl_release_ring_get_prod(qxl) == 0) {
/* stick head into the ring */
id = ext.info->id;
ext.info->next = 0;
qxl_ram_set_dirty(qxl, &ext.info->next);
- *item = id;
+ qxl_release_ring_set_prod(qxl, id);
qxl_ring_set_dirty(qxl);
} else {
/* append item to the list */
--
2.21.0.196.g041f5ea1cf
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member
2019-04-08 20:12 [Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member Marc-André Lureau
2019-04-08 20:12 ` Marc-André Lureau
@ 2019-05-03 16:18 ` Philippe Mathieu-Daudé
2019-05-03 16:18 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-03 16:18 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: kraxel
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 <marcandre.lureau@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> 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; \
> - } \
> - }
> +#define SPICE_RING_CONS_ITEM(qxl, r, ret) { \
> + uint32_t cons; \
> + if (!SPICE_RING_GET_CHECK(qxl, r, cons)) { \
> + ret = NULL; \
> + } else { \
> + ret = &(r)->items[cons].el; \
> + } \
> +}
>
> #undef ALIGN
> #define ALIGN(a, b) (((a) + ((b) - 1)) & ~((b) - 1))
> @@ -414,7 +434,6 @@ static void init_qxl_rom(PCIQXLDevice *d)
> static void init_qxl_ram(PCIQXLDevice *d)
> {
> uint8_t *buf;
> - uint64_t *item;
>
> buf = d->vga.vram_ptr;
> d->ram = (QXLRam *)(buf + le32_to_cpu(d->shadow_rom.ram_header_offset));
> @@ -426,9 +445,9 @@ static void init_qxl_ram(PCIQXLDevice *d)
> SPICE_RING_INIT(&d->ram->cmd_ring);
> SPICE_RING_INIT(&d->ram->cursor_ring);
> SPICE_RING_INIT(&d->ram->release_ring);
> - SPICE_RING_PROD_ITEM(d, &d->ram->release_ring, item);
> - assert(item);
> - *item = 0;
> + if (!qxl_release_ring_set_prod(d, 0)) {
> + g_assert_not_reached();
> + }
> qxl_ring_set_dirty(d);
> }
>
> @@ -732,7 +751,6 @@ static int interface_req_cmd_notification(QXLInstance *sin)
> static inline void qxl_push_free_res(PCIQXLDevice *d, int flush)
> {
> QXLReleaseRing *ring = &d->ram->release_ring;
> - uint64_t *item;
> int notify;
>
> #define QXL_FREE_BUNCH_SIZE 32
> @@ -759,11 +777,9 @@ static inline void qxl_push_free_res(PCIQXLDevice *d, int flush)
> if (notify) {
> qxl_send_events(d, QXL_INTERRUPT_DISPLAY);
> }
> - SPICE_RING_PROD_ITEM(d, ring, item);
> - if (!item) {
> + if (!qxl_release_ring_set_prod(d, 0)) {
> return;
> }
> - *item = 0;
> d->num_free_res = 0;
> d->last_release = NULL;
> qxl_ring_set_dirty(d);
> @@ -775,7 +791,8 @@ static void interface_release_resource(QXLInstance *sin,
> {
> PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
> QXLReleaseRing *ring;
> - uint64_t *item, id;
> + uint32_t prod;
> + uint64_t id;
>
> if (ext.group_id == MEMSLOT_GROUP_HOST) {
> /* host group -> vga mode update request */
> @@ -792,16 +809,16 @@ static void interface_release_resource(QXLInstance *sin,
> * pci bar 0, $command.release_info
> */
> ring = &qxl->ram->release_ring;
> - SPICE_RING_PROD_ITEM(qxl, ring, item);
> - if (!item) {
> +
> + if (!SPICE_RING_GET_CHECK(qxl, ring, prod)) {
> return;
> }
> - if (*item == 0) {
> + if (qxl_release_ring_get_prod(qxl) == 0) {
> /* stick head into the ring */
> id = ext.info->id;
> ext.info->next = 0;
> qxl_ram_set_dirty(qxl, &ext.info->next);
> - *item = id;
> + qxl_release_ring_set_prod(qxl, id);
> qxl_ring_set_dirty(qxl);
> } else {
> /* append item to the list */
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member
2019-05-03 16:18 ` Philippe Mathieu-Daudé
@ 2019-05-03 16:18 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-03 16:18 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: kraxel
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 <marcandre.lureau@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> 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; \
> - } \
> - }
> +#define SPICE_RING_CONS_ITEM(qxl, r, ret) { \
> + uint32_t cons; \
> + if (!SPICE_RING_GET_CHECK(qxl, r, cons)) { \
> + ret = NULL; \
> + } else { \
> + ret = &(r)->items[cons].el; \
> + } \
> +}
>
> #undef ALIGN
> #define ALIGN(a, b) (((a) + ((b) - 1)) & ~((b) - 1))
> @@ -414,7 +434,6 @@ static void init_qxl_rom(PCIQXLDevice *d)
> static void init_qxl_ram(PCIQXLDevice *d)
> {
> uint8_t *buf;
> - uint64_t *item;
>
> buf = d->vga.vram_ptr;
> d->ram = (QXLRam *)(buf + le32_to_cpu(d->shadow_rom.ram_header_offset));
> @@ -426,9 +445,9 @@ static void init_qxl_ram(PCIQXLDevice *d)
> SPICE_RING_INIT(&d->ram->cmd_ring);
> SPICE_RING_INIT(&d->ram->cursor_ring);
> SPICE_RING_INIT(&d->ram->release_ring);
> - SPICE_RING_PROD_ITEM(d, &d->ram->release_ring, item);
> - assert(item);
> - *item = 0;
> + if (!qxl_release_ring_set_prod(d, 0)) {
> + g_assert_not_reached();
> + }
> qxl_ring_set_dirty(d);
> }
>
> @@ -732,7 +751,6 @@ static int interface_req_cmd_notification(QXLInstance *sin)
> static inline void qxl_push_free_res(PCIQXLDevice *d, int flush)
> {
> QXLReleaseRing *ring = &d->ram->release_ring;
> - uint64_t *item;
> int notify;
>
> #define QXL_FREE_BUNCH_SIZE 32
> @@ -759,11 +777,9 @@ static inline void qxl_push_free_res(PCIQXLDevice *d, int flush)
> if (notify) {
> qxl_send_events(d, QXL_INTERRUPT_DISPLAY);
> }
> - SPICE_RING_PROD_ITEM(d, ring, item);
> - if (!item) {
> + if (!qxl_release_ring_set_prod(d, 0)) {
> return;
> }
> - *item = 0;
> d->num_free_res = 0;
> d->last_release = NULL;
> qxl_ring_set_dirty(d);
> @@ -775,7 +791,8 @@ static void interface_release_resource(QXLInstance *sin,
> {
> PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
> QXLReleaseRing *ring;
> - uint64_t *item, id;
> + uint32_t prod;
> + uint64_t id;
>
> if (ext.group_id == MEMSLOT_GROUP_HOST) {
> /* host group -> vga mode update request */
> @@ -792,16 +809,16 @@ static void interface_release_resource(QXLInstance *sin,
> * pci bar 0, $command.release_info
> */
> ring = &qxl->ram->release_ring;
> - SPICE_RING_PROD_ITEM(qxl, ring, item);
> - if (!item) {
> +
> + if (!SPICE_RING_GET_CHECK(qxl, ring, prod)) {
> return;
> }
> - if (*item == 0) {
> + if (qxl_release_ring_get_prod(qxl) == 0) {
> /* stick head into the ring */
> id = ext.info->id;
> ext.info->next = 0;
> qxl_ram_set_dirty(qxl, &ext.info->next);
> - *item = id;
> + qxl_release_ring_set_prod(qxl, id);
> qxl_ring_set_dirty(qxl);
> } else {
> /* append item to the list */
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-03 16:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-08 20:12 [Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member Marc-André Lureau
2019-04-08 20:12 ` Marc-André Lureau
2019-05-03 16:18 ` Philippe Mathieu-Daudé
2019-05-03 16:18 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).