Merge #290

290: Replace `*mut ()` with a new `Register` wrapper in `RawSyscalls`. r=hudson-ayers a=jrvanwhy

This change makes it easier to replace the `*mut ()` type in `RawSyscalls` with another type. I also removed `/platform` from `.gitignore`, as it was making `git` ignore new files in the `libtock_platform` crate.

This is an alternative to #289, which achieves the same goal by using a type alias.

Co-authored-by: Johnathan Van Why <[email protected]>

Author: bors[bot]

.gitignore

@@ -1,3 +1,2 @@
 /Cargo.lock
-/platform
 /target

platform/src/lib.rs

@@ -5,6 +5,7 @@ mod async_traits;
 mod command_return;
 mod error_code;
 mod raw_syscalls;
+mod register;
 pub mod return_variant;
 mod syscalls;
 mod syscalls_impl;
@@ -14,6 +15,7 @@ pub use async_traits::{CallbackContext, FreeCallback, Locator, MethodCallback};
 pub use command_return::CommandReturn;
 pub use error_code::ErrorCode;
 pub use raw_syscalls::RawSyscalls;
+pub use register::Register;
 pub use return_variant::ReturnVariant;
 pub use syscalls::Syscalls;
 pub use yield_types::YieldNoWaitReturn;

platform/src/raw_syscalls.rs

@@ -1,6 +1,8 @@
 // TODO: Implement `libtock_unittest`, which is referenced in the comment on
 // `RawSyscalls`.
 
+use crate::Register;
+
 /// `RawSyscalls` allows a fake Tock kernel to be injected into components for
 /// unit testing. It is implemented by `libtock_runtime::TockSyscalls` and
 /// `libtock_unittest::fake::Kernel`. **Components should not use `RawSyscalls`
@@ -19,50 +21,44 @@
 // Theoretically, RawSyscalls could consist of a single raw system call. To
 // start, something like this should work:
 //
-//   unsafe fn syscall<const CLASS: usize>([usize; 4]) -> [usize; 4];
-//
-// However, this will not work with Miri's -Zmiri-track-raw-pointers flag, as it
-// causes pointers passed to the kernel via the Allow system calls to be
-// untagged. In order to work with -Zmiri-track-raw-pointers, we need to pass
-// pointers for the register values. Rust's closest analogue to C's void pointer
-// is *mut () or *const (); we use *mut () because it is shorter:
+//   unsafe fn syscall<const CLASS: usize>([Reg; 4]) -> [Reg; 4];
 //
-//   unsafe fn syscall<const CLASS: usize>([*mut (); 4]) -> [*mut (); 4];
+// Note: Reg is an abbreviation of Register.
 //
 // Using a single system call has a major inefficiency. The single raw system
 // call would need to clobber every register that any system call can clobber.
 // Yield has a far longer clobber list than most system calls, so this would be
 // inefficient for the majority of system calls. As a result, we can split yield
 // out into its own function, giving the following API:
 //
-//   unsafe fn yield([*mut (); 4]) -> [*mut (); 4];
-//   unsafe fn syscall<const CLASS: usize>([*mut (); 4]) -> [*mut (); 4];
+//   unsafe fn yield([Reg; 4]) -> [Reg; 4];
+//   unsafe fn syscall<const CLASS: usize>([Reg; 4]) -> [Reg; 4];
 //
 // There is one significant inefficiency remaining. Many system calls, such as
 // memop's "get RAM start address" operation, do not need to set all four
 // arguments. The compiler cannot optimize away this inefficiency, so to remove
 // it we need to split the system calls up based on the number of arguments they
 // take:
 //
-//   unsafe fn yield0([*mut (); 0]) -> [*mut (); 4];
-//   unsafe fn yield1([*mut (); 1]) -> [*mut (); 4];
-//   unsafe fn yield2([*mut (); 2]) -> [*mut (); 4];
-//   unsafe fn yield3([*mut (); 3]) -> [*mut (); 4];
-//   unsafe fn yield4([*mut (); 4]) -> [*mut (); 4];
-//   unsafe fn syscall0<const CLASS: usize>([*mut (); 0]) -> [*mut (); 4];
-//   unsafe fn syscall1<const CLASS: usize>([*mut (); 1]) -> [*mut (); 4];
-//   unsafe fn syscall2<const CLASS: usize>([*mut (); 2]) -> [*mut (); 4];
-//   unsafe fn syscall3<const CLASS: usize>([*mut (); 3]) -> [*mut (); 4];
-//   unsafe fn syscall4<const CLASS: usize>([*mut (); 4]) -> [*mut (); 4];
+//   unsafe fn yield0([Reg; 0]) -> [Reg; 4];
+//   unsafe fn yield1([Reg; 1]) -> [Reg; 4];
+//   unsafe fn yield2([Reg; 2]) -> [Reg; 4];
+//   unsafe fn yield3([Reg; 3]) -> [Reg; 4];
+//   unsafe fn yield4([Reg; 4]) -> [Reg; 4];
+//   unsafe fn syscall0<const CLASS: usize>([Reg; 0]) -> [Reg; 4];
+//   unsafe fn syscall1<const CLASS: usize>([Reg; 1]) -> [Reg; 4];
+//   unsafe fn syscall2<const CLASS: usize>([Reg; 2]) -> [Reg; 4];
+//   unsafe fn syscall3<const CLASS: usize>([Reg; 3]) -> [Reg; 4];
+//   unsafe fn syscall4<const CLASS: usize>([Reg; 4]) -> [Reg; 4];
 //
 // However, not all of these are used! If we remove the system calls that are
 // unused, we are left with the following:
 //
-//   unsafe fn yield1([*mut (); 1]) -> [*mut (); 4];
-//   unsafe fn yield2([*mut (); 2]) -> [*mut (); 4];
-//   unsafe fn syscall1<const CLASS: usize>([*mut (); 1]) -> [*mut (); 4];
-//   unsafe fn syscall2<const CLASS: usize>([*mut (); 2]) -> [*mut (); 4];
-//   unsafe fn syscall4<const CLASS: usize>([*mut (); 4]) -> [*mut (); 4];
+//   unsafe fn yield1([Reg; 1]) -> [Reg; 4];
+//   unsafe fn yield2([Reg; 2]) -> [Reg; 4];
+//   unsafe fn syscall1<const CLASS: usize>([Reg; 1]) -> [Reg; 4];
+//   unsafe fn syscall2<const CLASS: usize>([Reg; 2]) -> [Reg; 4];
+//   unsafe fn syscall4<const CLASS: usize>([Reg; 4]) -> [Reg; 4];
 //
 // These system calls are refined further individually, which is documented on
 // a per-function basis.
@@ -86,7 +82,7 @@ pub unsafe trait RawSyscalls {
     /// # Safety
     /// yield1 may only be used for yield operations that do not return a value.
     /// It is exactly as safe as the underlying system call.
-    unsafe fn yield1(_: [*mut (); 1]);
+    unsafe fn yield1(_: [Register; 1]);
 
     // yield2 can only be used to call `yield-no-wait`. `yield-no-wait` does not
     // return any values, so to simplify the assembly we omit return arguments.
@@ -106,7 +102,7 @@ pub unsafe trait RawSyscalls {
     /// # Safety
     /// yield2 may only be used for yield operations that do not return a value.
     /// It has the same safety invariants as the underlying system call.
-    unsafe fn yield2(_: [*mut (); 2]);
+    unsafe fn yield2(_: [Register; 2]);
 
     // syscall1 is only used to invoke Memop operations. Because there are no
     // Memop commands that set r2 or r3, raw_syscall1 only needs to return r0
@@ -135,7 +131,7 @@ pub unsafe trait RawSyscalls {
     /// This directly makes a system call. It can only be used for core kernel
     /// system calls that accept 1 argument and only overwrite r0 and r1 on
     /// return. It is unsafe any time the underlying system call is unsafe.
-    unsafe fn syscall1<const CLASS: usize>(_: [*mut (); 1]) -> [*mut (); 2];
+    unsafe fn syscall1<const CLASS: usize>(_: [Register; 1]) -> [Register; 2];
 
     // syscall2 is used to invoke Exit as well as Memop operations that take an
     // argument. Memop does not currently use more than 2 registers for its
@@ -160,7 +156,7 @@ pub unsafe trait RawSyscalls {
     /// `syscall2` directly makes a system call. It can only be used for core
     /// kernel system calls that accept 2 arguments and only overwrite r0 and r1
     /// on return. It is unsafe any time the underlying system call is unsafe.
-    unsafe fn syscall2<const CLASS: usize>(_: [*mut (); 2]) -> [*mut (); 2];
+    unsafe fn syscall2<const CLASS: usize>(_: [Register; 2]) -> [Register; 2];
 
     // syscall4 should:
     //     1. Call the syscall class specified by CLASS.
@@ -183,5 +179,5 @@ pub unsafe trait RawSyscalls {
     /// `syscall4` must NOT be used to invoke yield. Otherwise, it has the same
     /// safety invariants as the underlying system call, which varies depending
     /// on the system call class.
-    unsafe fn syscall4<const CLASS: usize>(_: [*mut (); 4]) -> [*mut (); 4];
+    unsafe fn syscall4<const CLASS: usize>(_: [Register; 4]) -> [Register; 4];
 }

platform/src/register.rs

@@ -0,0 +1,89 @@
+/// In order to work with Miri's `-Zmiri-track-raw-pointers` flag, we cannot
+/// pass pointers to the kernel through `usize` values (as casting to and from
+/// `usize` drops the pointer`s tag). Instead, `RawSyscalls` uses the `Register`
+/// type. `Register` wraps a raw pointer type that keeps that tags around. User
+/// code should not depend on the particular type of pointer that `Register`
+/// wraps, but instead use the conversion functions in this module.
+#[derive(Clone, Copy, Debug)]
+pub struct Register(pub *mut ());
+
+// -----------------------------------------------------------------------------
+// Conversions to Register
+// -----------------------------------------------------------------------------
+
+impl From<u32> for Register {
+    fn from(value: u32) -> Register {
+        Register(value as *mut ())
+    }
+}
+
+impl From<usize> for Register {
+    fn from(value: usize) -> Register {
+        Register(value as *mut ())
+    }
+}
+
+impl<T> From<*mut T> for Register {
+    fn from(value: *mut T) -> Register {
+        Register(value as *mut ())
+    }
+}
+
+impl<T> From<*const T> for Register {
+    fn from(value: *const T) -> Register {
+        Register(value as *mut ())
+    }
+}
+
+// -----------------------------------------------------------------------------
+// Infallible conversions from Register
+// -----------------------------------------------------------------------------
+
+// If we implement From<u32> on Register, then we automatically get a
+// TryFrom<Error = Infallible> implementation, which conflicts with our fallible
+// TryFrom implementation. We could choose to not implement TryFrom and instead
+// add a fallible accessor (something like "expect_u32"), but that seems
+// confusing. Instead, we use an inherent method for the Register -> u32
+// infallible conversion.
+impl Register {
+    /// Casts this register to a u32, truncating it if it is larger than
+    /// u32::MAX. This conversion should be avoided in host-based test code; use
+    /// the `TryFrom<Register> for u32` implementation instead.
+    pub fn as_u32(self) -> u32 {
+        self.0 as u32
+    }
+}
+
+impl From<Register> for usize {
+    fn from(register: Register) -> usize {
+        register.0 as usize
+    }
+}
+
+impl<T> From<Register> for *mut T {
+    fn from(register: Register) -> *mut T {
+        register.0 as *mut T
+    }
+}
+
+impl<T> From<Register> for *const T {
+    fn from(register: Register) -> *const T {
+        register.0 as *const T
+    }
+}
+
+// -----------------------------------------------------------------------------
+// Fallible conversions from Register
+// -----------------------------------------------------------------------------
+
+/// Converts a `Register` to a `u32`. Returns an error if the `Register`'s value
+/// is larger than `u32::MAX`. This is intended for use in host-based tests; in
+/// Tock process binary code, use Register::as_u32 instead.
+impl core::convert::TryFrom<Register> for u32 {
+    type Error = core::num::TryFromIntError;
+
+    fn try_from(register: Register) -> Result<u32, core::num::TryFromIntError> {
+        use core::convert::TryInto;
+        (register.0 as usize).try_into()
+    }
+}

platform/src/syscalls_impl.rs

@@ -19,7 +19,7 @@ impl<S: RawSyscalls> Syscalls for S {
             // Flag can be uninitialized here because the kernel promises to
             // only write to it, not read from it. MaybeUninit guarantees that
             // it is safe to write a YieldNoWaitReturn into it.
-            Self::yield2([yield_op::NO_WAIT as *mut (), flag.as_mut_ptr() as *mut ()]);
+            Self::yield2([yield_op::NO_WAIT.into(), flag.as_mut_ptr().into()]);
 
             // yield-no-wait guarantees it sets (initializes) flag before
             // returning.
@@ -32,7 +32,7 @@ impl<S: RawSyscalls> Syscalls for S {
         // requirement. The yield-wait system call cannot trigger undefined
         // behavior on its own in any other way.
         unsafe {
-            Self::yield1([yield_op::WAIT as *mut ()]);
+            Self::yield1([yield_op::WAIT.into()]);
         }
     }
 }

runtime/src/syscalls_impl_arm.rs

@@ -1,7 +1,7 @@
-use libtock_platform::RawSyscalls;
+use libtock_platform::{RawSyscalls, Register};
 
 unsafe impl RawSyscalls for crate::TockSyscalls {
-    unsafe fn yield1([r0]: [*mut (); 1]) {
+    unsafe fn yield1([Register(r0)]: [Register; 1]) {
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::yield1
         unsafe {
@@ -22,7 +22,7 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
         }
     }
 
-    unsafe fn yield2([r0, r1]: [*mut (); 2]) {
+    unsafe fn yield2([Register(r0), Register(r1)]: [Register; 2]) {
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::yield2
         unsafe {
@@ -43,7 +43,7 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
         }
     }
 
-    unsafe fn syscall1<const CLASS: usize>([mut r0]: [*mut (); 1]) -> [*mut (); 2] {
+    unsafe fn syscall1<const CLASS: usize>([Register(mut r0)]: [Register; 1]) -> [Register; 2] {
         let r1;
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::syscall1
@@ -55,10 +55,12 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
                  options(preserves_flags, nostack, nomem),
             );
         }
-        [r0, r1]
+        [Register(r0), Register(r1)]
     }
 
-    unsafe fn syscall2<const CLASS: usize>([mut r0, mut r1]: [*mut (); 2]) -> [*mut (); 2] {
+    unsafe fn syscall2<const CLASS: usize>(
+        [Register(mut r0), Register(mut r1)]: [Register; 2],
+    ) -> [Register; 2] {
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::syscall2
         unsafe {
@@ -69,12 +71,12 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
                  options(preserves_flags, nostack, nomem)
             );
         }
-        [r0, r1]
+        [Register(r0), Register(r1)]
     }
 
     unsafe fn syscall4<const CLASS: usize>(
-        [mut r0, mut r1, mut r2, mut r3]: [*mut (); 4],
-    ) -> [*mut (); 4] {
+        [Register(mut r0), Register(mut r1), Register(mut r2), Register(mut r3)]: [Register; 4],
+    ) -> [Register; 4] {
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::syscall4
         unsafe {
@@ -87,6 +89,6 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
                  options(preserves_flags, nostack),
             );
         }
-        [r0, r1, r2, r3]
+        [Register(r0), Register(r1), Register(r2), Register(r3)]
     }
 }

runtime/src/syscalls_impl_riscv.rs

@@ -1,10 +1,10 @@
-use libtock_platform::RawSyscalls;
+use libtock_platform::{RawSyscalls, Register};
 
 unsafe impl RawSyscalls for crate::TockSyscalls {
     // This yield implementation is currently limited to RISC-V versions without
     // floating-point registers, as it does not mark them clobbered.
     #[cfg(not(any(target_feature = "d", target_feature = "f")))]
-    unsafe fn yield1([r0]: [*mut (); 1]) {
+    unsafe fn yield1([Register(r0)]: [Register; 1]) {
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::yield1
         unsafe {
@@ -37,7 +37,7 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
     // This yield implementation is currently limited to RISC-V versions without
     // floating-point registers, as it does not mark them clobbered.
     #[cfg(not(any(target_feature = "d", target_feature = "f")))]
-    unsafe fn yield2([r0, r1]: [*mut (); 2]) {
+    unsafe fn yield2([Register(r0), Register(r1)]: [Register; 2]) {
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::yield2
         unsafe {
@@ -67,7 +67,7 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
         }
     }
 
-    unsafe fn syscall1<const CLASS: usize>([mut r0]: [*mut (); 1]) -> [*mut (); 2] {
+    unsafe fn syscall1<const CLASS: usize>([Register(mut r0)]: [Register; 1]) -> [Register; 2] {
         let r1;
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::syscall1
@@ -79,10 +79,12 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
                  options(preserves_flags, nostack, nomem),
             );
         }
-        [r0, r1]
+        [Register(r0), Register(r1)]
     }
 
-    unsafe fn syscall2<const CLASS: usize>([mut r0, mut r1]: [*mut (); 2]) -> [*mut (); 2] {
+    unsafe fn syscall2<const CLASS: usize>(
+        [Register(mut r0), Register(mut r1)]: [Register; 2],
+    ) -> [Register; 2] {
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::syscall2
         unsafe {
@@ -93,12 +95,12 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
                  options(preserves_flags, nostack, nomem)
             );
         }
-        [r0, r1]
+        [Register(r0), Register(r1)]
     }
 
     unsafe fn syscall4<const CLASS: usize>(
-        [mut r0, mut r1, mut r2, mut r3]: [*mut (); 4],
-    ) -> [*mut (); 4] {
+        [Register(mut r0), Register(mut r1), Register(mut r2), Register(mut r3)]: [Register; 4],
+    ) -> [Register; 4] {
         // Safety: This matches the invariants required by the documentation on
         // RawSyscalls::syscall4
         unsafe {
@@ -111,6 +113,6 @@ unsafe impl RawSyscalls for crate::TockSyscalls {
                  options(preserves_flags, nostack),
             );
         }
-        [r0, r1, r2, r3]
+        [Register(r0), Register(r1), Register(r2), Register(r3)]
     }
 }