diff mbox series

[10/16] rust: introduce alternative implementation of offset_of!

Message ID 20241015131735.518771-11-pbonzini@redhat.com (mailing list archive)
State New
Headers show
Series rust: allow older versions of rustc and bindgen | expand

Commit Message

Paolo Bonzini Oct. 15, 2024, 1:17 p.m. UTC
offset_of! was stabilized in Rust 1.77.0.  Use an alternative implemenation
that was found on the Rust forums, and whose author agreed to license as
MIT for use in QEMU.

The alternative allows only one level of field access, but apart
from this can be used just by replacing core::mem::offset_of! with
qemu_api::offset_of!.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs  |  91 ++++++++++++-------------
 rust/qemu-api/meson.build         |  12 ++--
 rust/qemu-api/src/device_class.rs |   8 +--
 rust/qemu-api/src/lib.rs          |   4 ++
 rust/qemu-api/src/offset_of.rs    | 106 ++++++++++++++++++++++++++++++
 rust/qemu-api/src/tests.rs        |  11 ++--
 6 files changed, 176 insertions(+), 56 deletions(-)
 create mode 100644 rust/qemu-api/src/offset_of.rs

Comments

Junjie Mao Oct. 17, 2024, 5:07 a.m. UTC | #1
Paolo Bonzini <pbonzini@redhat.com> writes:

> offset_of! was stabilized in Rust 1.77.0.  Use an alternative implemenation
> that was found on the Rust forums, and whose author agreed to license as
> MIT for use in QEMU.
>
> The alternative allows only one level of field access, but apart
> from this can be used just by replacing core::mem::offset_of! with
> qemu_api::offset_of!.

How about a macro like this (which essentially comes from memoffset
crate [1])? It has the same use as core::mem::offset_of! (except the
same single-level limitation) and does not need wrapping structures with
with_offsets!.

macro_rules! offset_of {
    ($parent:ty, $field:tt) => {{
	let uninit = std::mem::MaybeUninit::<$parent>::uninit();
	let base = uninit.as_ptr();
	// SAFETY:
	//
	// MaybeUninit<$parent> has the same size and alignment as $parent, so
	// projection to $field is in bound.
	//
	// addr_of! does not create intermediate references to the uninitialized
	// memory, thus no UB is involved.
	let field = unsafe { std::ptr::addr_of!((*base).$field) };
	// SAFETY:
	//
	// Both base and field point to the MaybeUninit<$parent> and are casted
	// to u8 for calculating their distance.
	unsafe { field.cast::<u8>().offset_from(base.cast::<u8>()) as usize }
    }};
}

[1] https://docs.rs/memoffset/latest/memoffset/

--
Best Regards
Junjie Mao
Paolo Bonzini Oct. 17, 2024, 8:44 a.m. UTC | #2
On Thu, Oct 17, 2024 at 7:35 AM Junjie Mao <junjie.mao@hotmail.com> wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> > offset_of! was stabilized in Rust 1.77.0.  Use an alternative implemenation
> > that was found on the Rust forums, and whose author agreed to license as
> > MIT for use in QEMU.
> >
> > The alternative allows only one level of field access, but apart
> > from this can be used just by replacing core::mem::offset_of! with
> > qemu_api::offset_of!.
>
> How about a macro like this (which essentially comes from memoffset
> crate [1])? It has the same use as core::mem::offset_of! (except the
> same single-level limitation) and does not need wrapping structures with
> with_offsets!.

Unfortunately offset_from is not available in const context, and
offset_of! is needed to fill in the Property and VMStateDescription
arrays.

That said, I noticed now that declare_properties! does not declare the
resulting array as const, so that would be possible. But if
declare_properties could use a non-mut static, that would be better.

Paolo

> macro_rules! offset_of {
>     ($parent:ty, $field:tt) => {{
>         let uninit = std::mem::MaybeUninit::<$parent>::uninit();
>         let base = uninit.as_ptr();
>         // SAFETY:
>         //
>         // MaybeUninit<$parent> has the same size and alignment as $parent, so
>         // projection to $field is in bound.
>         //
>         // addr_of! does not create intermediate references to the uninitialized
>         // memory, thus no UB is involved.
>         let field = unsafe { std::ptr::addr_of!((*base).$field) };
>         // SAFETY:
>         //
>         // Both base and field point to the MaybeUninit<$parent> and are casted
>         // to u8 for calculating their distance.
>         unsafe { field.cast::<u8>().offset_from(base.cast::<u8>()) as usize }
>     }};
> }
>
> [1] https://docs.rs/memoffset/latest/memoffset/
>
> --
> Best Regards
> Junjie Mao
>
Junjie Mao Oct. 18, 2024, 3:01 a.m. UTC | #3
Paolo Bonzini <pbonzini@redhat.com> writes:

> On Thu, Oct 17, 2024 at 7:35 AM Junjie Mao <junjie.mao@hotmail.com> wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> > offset_of! was stabilized in Rust 1.77.0.  Use an alternative implemenation
>> > that was found on the Rust forums, and whose author agreed to license as
>> > MIT for use in QEMU.
>> >
>> > The alternative allows only one level of field access, but apart
>> > from this can be used just by replacing core::mem::offset_of! with
>> > qemu_api::offset_of!.
>>
>> How about a macro like this (which essentially comes from memoffset
>> crate [1])? It has the same use as core::mem::offset_of! (except the
>> same single-level limitation) and does not need wrapping structures with
>> with_offsets!.
>
> Unfortunately offset_from is not available in const context, and
> offset_of! is needed to fill in the Property and VMStateDescription
> arrays.
>
> That said, I noticed now that declare_properties! does not declare the
> resulting array as const, so that would be possible. But if
> declare_properties could use a non-mut static, that would be better.

Agree.

Then how about converting with_offsets! to a derive attribute
(e.g. #[derive(offsets)])? The current approach introduces one more
level of indentation. When we later upgrade the minimal supported
version of Rust and switch to std::mem::offset_of!, we'll need a large
diff to adjust the indentation which may be annoying to rebase upon. An
attribute seems easier to manage.

I can help draft the macro early next week if you think that is valuable.

Junjie Mao

>
> Paolo
>
>> macro_rules! offset_of {
>>     ($parent:ty, $field:tt) => {{
>>         let uninit = std::mem::MaybeUninit::<$parent>::uninit();
>>         let base = uninit.as_ptr();
>>         // SAFETY:
>>         //
>>         // MaybeUninit<$parent> has the same size and alignment as $parent, so
>>         // projection to $field is in bound.
>>         //
>>         // addr_of! does not create intermediate references to the uninitialized
>>         // memory, thus no UB is involved.
>>         let field = unsafe { std::ptr::addr_of!((*base).$field) };
>>         // SAFETY:
>>         //
>>         // Both base and field point to the MaybeUninit<$parent> and are casted
>>         // to u8 for calculating their distance.
>>         unsafe { field.cast::<u8>().offset_from(base.cast::<u8>()) as usize }
>>     }};
>> }
>>
>> [1] https://docs.rs/memoffset/latest/memoffset/
diff mbox series

Patch

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 55d933ee5e9..f331a13b5f1 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -13,6 +13,7 @@ 
     bindings::{self, *},
     c_str,
     definitions::ObjectImpl,
+    with_offsets,
 };
 
 use crate::{
@@ -28,50 +29,52 @@ 
 /// QEMU sourced constant.
 pub const PL011_FIFO_DEPTH: usize = 16_usize;
 
-#[repr(C)]
-#[derive(Debug, qemu_api_macros::Object)]
-/// PL011 Device Model in QEMU
-pub struct PL011State {
-    pub parent_obj: SysBusDevice,
-    pub iomem: MemoryRegion,
-    #[doc(alias = "fr")]
-    pub flags: registers::Flags,
-    #[doc(alias = "lcr")]
-    pub line_control: registers::LineControl,
-    #[doc(alias = "rsr")]
-    pub receive_status_error_clear: registers::ReceiveStatusErrorClear,
-    #[doc(alias = "cr")]
-    pub control: registers::Control,
-    pub dmacr: u32,
-    pub int_enabled: u32,
-    pub int_level: u32,
-    pub read_fifo: [u32; PL011_FIFO_DEPTH],
-    pub ilpr: u32,
-    pub ibrd: u32,
-    pub fbrd: u32,
-    pub ifl: u32,
-    pub read_pos: usize,
-    pub read_count: usize,
-    pub read_trigger: usize,
-    #[doc(alias = "chr")]
-    pub char_backend: CharBackend,
-    /// QEMU interrupts
-    ///
-    /// ```text
-    ///  * sysbus MMIO region 0: device registers
-    ///  * sysbus IRQ 0: `UARTINTR` (combined interrupt line)
-    ///  * sysbus IRQ 1: `UARTRXINTR` (receive FIFO interrupt line)
-    ///  * sysbus IRQ 2: `UARTTXINTR` (transmit FIFO interrupt line)
-    ///  * sysbus IRQ 3: `UARTRTINTR` (receive timeout interrupt line)
-    ///  * sysbus IRQ 4: `UARTMSINTR` (momem status interrupt line)
-    ///  * sysbus IRQ 5: `UARTEINTR` (error interrupt line)
-    /// ```
-    #[doc(alias = "irq")]
-    pub interrupts: [qemu_irq; 6usize],
-    #[doc(alias = "clk")]
-    pub clock: NonNull<Clock>,
-    #[doc(alias = "migrate_clk")]
-    pub migrate_clock: bool,
+with_offsets! {
+    #[repr(C)]
+    #[derive(Debug, qemu_api_macros::Object)]
+    /// PL011 Device Model in QEMU
+    pub struct PL011State {
+        pub parent_obj: SysBusDevice,
+        pub iomem: MemoryRegion,
+        #[doc(alias = "fr")]
+        pub flags: registers::Flags,
+        #[doc(alias = "lcr")]
+        pub line_control: registers::LineControl,
+        #[doc(alias = "rsr")]
+        pub receive_status_error_clear: registers::ReceiveStatusErrorClear,
+        #[doc(alias = "cr")]
+        pub control: registers::Control,
+        pub dmacr: u32,
+        pub int_enabled: u32,
+        pub int_level: u32,
+        pub read_fifo: [u32; PL011_FIFO_DEPTH],
+        pub ilpr: u32,
+        pub ibrd: u32,
+        pub fbrd: u32,
+        pub ifl: u32,
+        pub read_pos: usize,
+        pub read_count: usize,
+        pub read_trigger: usize,
+        #[doc(alias = "chr")]
+        pub char_backend: CharBackend,
+        /// QEMU interrupts
+        ///
+        /// ```text
+        ///  * sysbus MMIO region 0: device registers
+        ///  * sysbus IRQ 0: `UARTINTR` (combined interrupt line)
+        ///  * sysbus IRQ 1: `UARTRXINTR` (receive FIFO interrupt line)
+        ///  * sysbus IRQ 2: `UARTTXINTR` (transmit FIFO interrupt line)
+        ///  * sysbus IRQ 3: `UARTRTINTR` (receive timeout interrupt line)
+        ///  * sysbus IRQ 4: `UARTMSINTR` (momem status interrupt line)
+        ///  * sysbus IRQ 5: `UARTEINTR` (error interrupt line)
+        /// ```
+        #[doc(alias = "irq")]
+        pub interrupts: [qemu_irq; 6usize],
+        #[doc(alias = "clk")]
+        pub clock: NonNull<Clock>,
+        #[doc(alias = "migrate_clk")]
+        pub migrate_clock: bool,
+    }
 }
 
 impl ObjectImpl for PL011State {
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index b55931c6490..57f813fc8f9 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -1,3 +1,9 @@ 
+_qemu_api_cfg = ['--cfg', 'MESON']
+# _qemu_api_cfg += ['--cfg', 'feature="allocator"']
+if rustc.version().version_compare('>=1.77.0')
+  _qemu_api_cfg += ['--cfg', 'has_offset_of']
+endif
+
 _qemu_api_rs = static_library(
   'qemu_api',
   structured_sources(
@@ -6,16 +12,14 @@  _qemu_api_rs = static_library(
       'src/c_str.rs',
       'src/definitions.rs',
       'src/device_class.rs',
+      'src/offset_of.rs',
       'src/tests.rs',
     ],
     {'.' : bindings_rs},
   ),
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
   rust_abi: 'rust',
-  rust_args: [
-    '--cfg', 'MESON',
-    # '--cfg', 'feature="allocator"',
-  ],
+  rust_args: _qemu_api_cfg,
   dependencies: [
     qemu_api_macros,
   ],
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index 871063d4a92..d4fa544df39 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -26,7 +26,7 @@  macro_rules! device_class_init {
 
 #[macro_export]
 macro_rules! define_property {
-    ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr, default = $defval:expr$(,)*) => {
+    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr, default = $defval:expr$(,)*) => {
         $crate::bindings::Property {
             name: {
                 #[used]
@@ -34,7 +34,7 @@  macro_rules! define_property {
                 _TEMP.as_ptr()
             },
             info: $prop,
-            offset: ::core::mem::offset_of!($state, $field)
+            offset: $crate::offset_of!($state, $field)
                 .try_into()
                 .expect("Could not fit offset value to type"),
             bitnr: 0,
@@ -47,7 +47,7 @@  macro_rules! define_property {
             link_type: ::core::ptr::null(),
         }
     };
-    ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr$(,)*) => {
+    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr$(,)*) => {
         $crate::bindings::Property {
             name: {
                 #[used]
@@ -55,7 +55,7 @@  macro_rules! define_property {
                 _TEMP.as_ptr()
             },
             info: $prop,
-            offset: ::core::mem::offset_of!($state, $field)
+            offset: $crate::offset_of!($state, $field)
                 .try_into()
                 .expect("Could not fit offset value to type"),
             bitnr: 0,
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 9b2483fbfa3..082f1addb10 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -30,6 +30,7 @@  unsafe impl Sync for bindings::VMStateDescription {}
 pub mod c_str;
 pub mod definitions;
 pub mod device_class;
+pub mod offset_of;
 
 #[cfg(test)]
 mod tests;
@@ -167,3 +168,6 @@  unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
         }
     }
 }
+
+#[cfg(has_offset_of)]
+pub use std::mem::offset_of;
diff --git a/rust/qemu-api/src/offset_of.rs b/rust/qemu-api/src/offset_of.rs
new file mode 100644
index 00000000000..4e1de373674
--- /dev/null
+++ b/rust/qemu-api/src/offset_of.rs
@@ -0,0 +1,106 @@ 
+// SPDX-License-Identifier: MIT
+
+/// This macro provides the same functionality as `core::mem::offset_of`,
+/// except that only one level of field access is supported.  The declaration
+/// of the struct must be wrapped with `with_offsets! { }`.
+///
+/// It is needed because `offset_of!` was only stabilized in Rust 1.77.
+#[cfg(not(has_offset_of))]
+#[macro_export]
+macro_rules! offset_of {
+    ($Container:ty, $field:ident) => {
+        <$Container>::offset_to.$field
+    };
+}
+
+/// A wrapper for struct declarations, that allows using `offset_of!` in
+/// versions of Rust prior to 1.77
+#[macro_export]
+macro_rules! with_offsets {
+    // source: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=10a22a9b8393abd7b541d8fc844bc0df
+    // used under MIT license with permission of Yandros aka Daniel Henry-Mantilla
+    (
+        #[repr(C)]
+        $(#[$struct_meta:meta])*
+        $struct_vis:vis
+        struct $StructName:ident {
+            $(
+                $(#[$field_meta:meta])*
+                $field_vis:vis
+                $field_name:ident : $field_ty:ty
+            ),*
+            $(,)?
+        }
+    ) => (
+        #[repr(C)]
+        $(#[$struct_meta])*
+        $struct_vis
+        struct $StructName {
+            $(
+                $(#[$field_meta])*
+                $field_vis
+                $field_name : $field_ty ,
+            )*
+        }
+
+        #[cfg(not(has_offset_of))]
+        #[allow(nonstandard_style)]
+        const _: () = {
+            pub
+            struct StructOffsets {
+                $(
+                    $field_vis
+                    $field_name: usize,
+                )*
+            }
+            struct Helper;
+            impl $StructName {
+                pub
+                const offset_to: StructOffsets = StructOffsets {
+                    $(
+                        $field_name: Helper::$field_name,
+                    )*
+                };
+            }
+            const END_OF_PREV_FIELD: usize = 0;
+            $crate::with_offsets! {
+                @names [ $($field_name)* ]
+                @tys [ $($field_ty ,)*]
+            }
+        };
+    );
+
+    (
+        @names []
+        @tys []
+    ) => ();
+
+    (
+        @names [$field_name:ident $($other_names:tt)*]
+        @tys [$field_ty:ty , $($other_tys:tt)*]
+    ) => (
+        impl Helper {
+            const $field_name: usize = {
+                let align =
+                    std::mem::align_of::<$field_ty>()
+                ;
+                let trail =
+                    END_OF_PREV_FIELD % align
+                ;
+                0   + END_OF_PREV_FIELD
+                    + (align - trail)
+                        * [1, 0][(trail == 0) as usize]
+            };
+        }
+        const _: () = {
+            const END_OF_PREV_FIELD: usize =
+                Helper::$field_name +
+                std::mem::size_of::<$field_ty>()
+            ;
+            $crate::with_offsets! {
+                @names [$($other_names)*]
+                @tys [$($other_tys)*]
+            }
+        };
+    );
+}
diff --git a/rust/qemu-api/src/tests.rs b/rust/qemu-api/src/tests.rs
index d34b8d24187..b582f27baa1 100644
--- a/rust/qemu-api/src/tests.rs
+++ b/rust/qemu-api/src/tests.rs
@@ -4,6 +4,7 @@ 
 
 use crate::{
     bindings::*, c_str, declare_properties, define_property, device_class_init, vm_state_description,
+    with_offsets,
 };
 
 #[test]
@@ -15,10 +16,12 @@  fn test_device_decl_macros() {
         unmigratable: true,
     }
 
-    #[repr(C)]
-    pub struct DummyState {
-        pub char_backend: CharBackend,
-        pub migrate_clock: bool,
+    with_offsets! {
+        #[repr(C)]
+        pub struct DummyState {
+            pub char_backend: CharBackend,
+            pub migrate_clock: bool,
+        }
     }
 
     declare_properties! {