We need to be able to cast from virObjectEventPtr to one of
its many subclasses. Some of these subclasses have 8 byte
alignment on 32-bit platforms, but virObjectEventPtr only
has 4 byte alignment.
Previously the virObject base class had 8 byte alignment
but this dropped to 4 byte when converted to inherit from
GObject. This introduces cast alignment warnings on 32-bit:
../../src/conf/domain_event.c: In function 'virDomainEventDispatchDefaultFunc':
../../src/conf/domain_event.c:1656:30: error: cast increases required alignment of target type [-Werror=cast-align]
1656 | rtcChangeEvent = (virDomainEventRTCChangePtr)event;
| ^
../../src/conf/domain_event.c:1785:34: error: cast increases required alignment of target type [-Werror=cast-align]
1785 | balloonChangeEvent = (virDomainEventBalloonChangePtr)event;
| ^
../../src/conf/domain_event.c:1896:35: error: cast increases required alignment of target type [-Werror=cast-align]
1896 | blockThresholdEvent = (virDomainEventBlockThresholdPtr)event;
| ^
../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventDispatchFunc':
../../src/conf/domain_event.c:1974:24: error: cast increases required alignment of target type [-Werror=cast-align]
1974 | qemuMonitorEvent = (virDomainQemuMonitorEventPtr)event;
| ^
../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventFilter':
../../src/conf/domain_event.c:2179:20: error: cast increases required alignment of target type [-Werror=cast-align]
2179 | monitorEvent = (virDomainQemuMonitorEventPtr) event;
| ^
Forcing 8-byte alignment on virObjectEventPtr removes the
alignment increase during casts to subclasses.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
Technically a build-breaker, but since we don't have any existing
usage of __attribute__((aligned)), I wanted to get a second opinion
on this approach.
One alternative approach would be to switch one of the current "int"
fields in virObjectEvent to "long long".
src/conf/object_event_private.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h
index b31441c53a..126464a9a5 100644
--- a/src/conf/object_event_private.h
+++ b/src/conf/object_event_private.h
@@ -42,7 +42,7 @@ typedef void
virConnectObjectEventGenericCallback cb,
void *cbopaque);
-struct _virObjectEvent {
+struct __attribute__((aligned(4))) _virObjectEvent {
virObject parent;
int eventID;
virObjectMeta meta;
--
2.24.1
On Wed, Jun 03, 2020 at 11:22:47AM +0100, Daniel P. Berrangé wrote:
> We need to be able to cast from virObjectEventPtr to one of
> its many subclasses. Some of these subclasses have 8 byte
> alignment on 32-bit platforms, but virObjectEventPtr only
> has 4 byte alignment.
>
> Previously the virObject base class had 8 byte alignment
> but this dropped to 4 byte when converted to inherit from
> GObject. This introduces cast alignment warnings on 32-bit:
>
> ../../src/conf/domain_event.c: In function 'virDomainEventDispatchDefaultFunc':
> ../../src/conf/domain_event.c:1656:30: error: cast increases required alignment of target type [-Werror=cast-align]
> 1656 | rtcChangeEvent = (virDomainEventRTCChangePtr)event;
> | ^
> ../../src/conf/domain_event.c:1785:34: error: cast increases required alignment of target type [-Werror=cast-align]
> 1785 | balloonChangeEvent = (virDomainEventBalloonChangePtr)event;
> | ^
> ../../src/conf/domain_event.c:1896:35: error: cast increases required alignment of target type [-Werror=cast-align]
> 1896 | blockThresholdEvent = (virDomainEventBlockThresholdPtr)event;
> | ^
> ../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventDispatchFunc':
> ../../src/conf/domain_event.c:1974:24: error: cast increases required alignment of target type [-Werror=cast-align]
> 1974 | qemuMonitorEvent = (virDomainQemuMonitorEventPtr)event;
> | ^
> ../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventFilter':
> ../../src/conf/domain_event.c:2179:20: error: cast increases required alignment of target type [-Werror=cast-align]
> 2179 | monitorEvent = (virDomainQemuMonitorEventPtr) event;
> | ^
>
> Forcing 8-byte alignment on virObjectEventPtr removes the
> alignment increase during casts to subclasses.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>
> Technically a build-breaker, but since we don't have any existing
> usage of __attribute__((aligned)), I wanted to get a second opinion
> on this approach.
>
> One alternative approach would be to switch one of the current "int"
> fields in virObjectEvent to "long long".
I personally prefer using __attribute__. We already use that GCC
extension so I don't see any reason not using another part of that
extension.
>
> src/conf/object_event_private.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h
> index b31441c53a..126464a9a5 100644
> --- a/src/conf/object_event_private.h
> +++ b/src/conf/object_event_private.h
> @@ -42,7 +42,7 @@ typedef void
> virConnectObjectEventGenericCallback cb,
> void *cbopaque);
>
> -struct _virObjectEvent {
> +struct __attribute__((aligned(4))) _virObjectEvent {
> virObject parent;
> int eventID;
> virObjectMeta meta;
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
On Wed, Jun 3, 2020 at 12:29 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> We need to be able to cast from virObjectEventPtr to one of
> its many subclasses. Some of these subclasses have 8 byte
> alignment on 32-bit platforms, but virObjectEventPtr only
> has 4 byte alignment.
>
> Previously the virObject base class had 8 byte alignment
> but this dropped to 4 byte when converted to inherit from
> GObject. This introduces cast alignment warnings on 32-bit:
>
> ../../src/conf/domain_event.c: In function 'virDomainEventDispatchDefaultFunc':
> ../../src/conf/domain_event.c:1656:30: error: cast increases required alignment of target type [-Werror=cast-align]
> 1656 | rtcChangeEvent = (virDomainEventRTCChangePtr)event;
> | ^
> ../../src/conf/domain_event.c:1785:34: error: cast increases required alignment of target type [-Werror=cast-align]
> 1785 | balloonChangeEvent = (virDomainEventBalloonChangePtr)event;
> | ^
> ../../src/conf/domain_event.c:1896:35: error: cast increases required alignment of target type [-Werror=cast-align]
> 1896 | blockThresholdEvent = (virDomainEventBlockThresholdPtr)event;
> | ^
> ../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventDispatchFunc':
> ../../src/conf/domain_event.c:1974:24: error: cast increases required alignment of target type [-Werror=cast-align]
> 1974 | qemuMonitorEvent = (virDomainQemuMonitorEventPtr)event;
> | ^
> ../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventFilter':
> ../../src/conf/domain_event.c:2179:20: error: cast increases required alignment of target type [-Werror=cast-align]
> 2179 | monitorEvent = (virDomainQemuMonitorEventPtr) event;
> | ^
>
> Forcing 8-byte alignment on virObjectEventPtr removes the
> alignment increase during casts to subclasses.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>
> Technically a build-breaker, but since we don't have any existing
> usage of __attribute__((aligned)), I wanted to get a second opinion
> on this approach.
>
> One alternative approach would be to switch one of the current "int"
> fields in virObjectEvent to "long long".
>
> src/conf/object_event_private.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h
> index b31441c53a..126464a9a5 100644
> --- a/src/conf/object_event_private.h
> +++ b/src/conf/object_event_private.h
> @@ -42,7 +42,7 @@ typedef void
> virConnectObjectEventGenericCallback cb,
> void *cbopaque);
>
> -struct _virObjectEvent {
> +struct __attribute__((aligned(4))) _virObjectEvent {
> virObject parent;
> int eventID;
> virObjectMeta meta;
> --
> 2.24.1
>
This is the same error I'm seeing with my patches in CI when
converting virStorageSource to GObject except no alignment value seems
to fix it.
--
Rafael Fonseca
On 6/3/20 12:22 PM, Daniel P. Berrangé wrote:
> We need to be able to cast from virObjectEventPtr to one of
> its many subclasses. Some of these subclasses have 8 byte
> alignment on 32-bit platforms, but virObjectEventPtr only
> has 4 byte alignment.
>
> Previously the virObject base class had 8 byte alignment
> but this dropped to 4 byte when converted to inherit from
> GObject. This introduces cast alignment warnings on 32-bit:
>
> ../../src/conf/domain_event.c: In function 'virDomainEventDispatchDefaultFunc':
> ../../src/conf/domain_event.c:1656:30: error: cast increases required alignment of target type [-Werror=cast-align]
> 1656 | rtcChangeEvent = (virDomainEventRTCChangePtr)event;
> | ^
> ../../src/conf/domain_event.c:1785:34: error: cast increases required alignment of target type [-Werror=cast-align]
> 1785 | balloonChangeEvent = (virDomainEventBalloonChangePtr)event;
> | ^
> ../../src/conf/domain_event.c:1896:35: error: cast increases required alignment of target type [-Werror=cast-align]
> 1896 | blockThresholdEvent = (virDomainEventBlockThresholdPtr)event;
> | ^
> ../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventDispatchFunc':
> ../../src/conf/domain_event.c:1974:24: error: cast increases required alignment of target type [-Werror=cast-align]
> 1974 | qemuMonitorEvent = (virDomainQemuMonitorEventPtr)event;
> | ^
> ../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventFilter':
> ../../src/conf/domain_event.c:2179:20: error: cast increases required alignment of target type [-Werror=cast-align]
> 2179 | monitorEvent = (virDomainQemuMonitorEventPtr) event;
> | ^
>
> Forcing 8-byte alignment on virObjectEventPtr removes the
> alignment increase during casts to subclasses.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>
> Technically a build-breaker, but since we don't have any existing
> usage of __attribute__((aligned)), I wanted to get a second opinion
> on this approach.
>
> One alternative approach would be to switch one of the current "int"
> fields in virObjectEvent to "long long".
>
> src/conf/object_event_private.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h
> index b31441c53a..126464a9a5 100644
> --- a/src/conf/object_event_private.h
> +++ b/src/conf/object_event_private.h
> @@ -42,7 +42,7 @@ typedef void
> virConnectObjectEventGenericCallback cb,
> void *cbopaque);
>
> -struct _virObjectEvent {
> +struct __attribute__((aligned(4))) _virObjectEvent {
> virObject parent;
> int eventID;
> virObjectMeta meta;
>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal
On 6/3/20 5:22 AM, Daniel P. Berrangé wrote:
> Forcing 8-byte alignment on virObjectEventPtr removes the
> alignment increase during casts to subclasses.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>
> Technically a build-breaker, but since we don't have any existing
> usage of __attribute__((aligned)), I wanted to get a second opinion
> on this approach.
>
> One alternative approach would be to switch one of the current "int"
> fields in virObjectEvent to "long long".
>
> -struct _virObjectEvent {
> +struct __attribute__((aligned(4))) _virObjectEvent {
> virObject parent;
> int eventID;
> virObjectMeta meta;
As in this, although it makes the struct larger on 32-bit platforms
(which may in turn affect cache usage):
struct _virObjectEvent {
virObject parent;
long long eventID;
...
Another possibility: use the extension of unnamed unions (but if we're
going to rely on gcc extensions, __attribute__ is nicer than unnamed
unions):
struct _virObjectEvent {
union {
virObject parent;
long long alignment_;
};
int eventID;
...
We already limit ourselves to gcc and clang because of
__attribute__((cleanup)), so I don't see any problem with your approach.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
© 2016 - 2024 Red Hat, Inc.