perlmod: introduce MagicValue

It turns out that using `const` for the `MagicSpec` can lead
to multiple instances of it and the `TryFrom` accessing a
different vtable address than the instantiating functions.

This particularly happened when using `declare_magic!`
directly for a `Box<T>` where `T` is not declared in the
same module and is `!Sync`.

Instead, `declare_magic!()` now creates `static`s,
`MagicSpec` loses the associated value so we can safely
force it to be `Send + Sync`, and `MagicValue` is introduced
as the result of the `.with_value()` method call.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

perlmod/src/lib.rs

@@ -59,7 +59,7 @@ pub use raw_value::RawValue;
 
 pub mod magic;
 #[doc(inline)]
-pub use magic::{MagicSpec, MagicTag};
+pub use magic::{MagicSpec, MagicTag, MagicValue};
 
 #[cfg(feature = "exporter")]
 #[doc(inline)]

perlmod/src/macros.rs

@@ -193,8 +193,10 @@ macro_rules! magic_destructor {
 macro_rules! declare_magic {
     ($ty:ty : &$inner:ty as $class:literal) => {
         const CLASSNAME: &str = $class;
-        const MAGIC: $crate::MagicSpec<$ty> =
-            unsafe { perlmod::MagicSpec::new_static(&$crate::MagicTag::<$ty>::DEFAULT) };
+        static MAGIC: $crate::MagicSpec<$ty> = unsafe {
+            static TAG: $crate::MagicTag<$ty> = $crate::MagicTag::<$ty>::DEFAULT;
+            perlmod::MagicSpec::new_static(&TAG)
+        };
 
         impl<'a> ::std::convert::TryFrom<&'a $crate::Value> for &'a $inner {
             type Error = $crate::error::MagicError;

perlmod/src/magic.rs

@@ -121,6 +121,12 @@ unsafe impl<T> Leakable for std::rc::Rc<T> {
 /// A tag for perl magic, see [`MagicSpec`] for its usage.
 pub struct MagicTag<T = ()>(ffi::MGVTBL, PhantomData<T>);
 
+/// It doesn't actually contain a `T`
+unsafe impl<T> Sync for MagicTag<T> {}
+
+/// It doesn't actually contain a `T`
+unsafe impl<T> Send for MagicTag<T> {}
+
 impl<T> MagicTag<T> {
     /// Create a new tag. See [`MagicSpec`] for its usage.
     pub const fn new() -> Self {
@@ -208,13 +214,20 @@ impl<T: Leakable> MagicTag<T> {
 /// ```
 ///
 /// NOTE: Once `const fn` with trait bounds are stable, this will be `where T: Leakable`.
+#[derive(Clone)]
 pub struct MagicSpec<'o, 'v, T> {
     pub(crate) obj: Option<&'o ScalarRef>,
     pub(crate) how: Option<libc::c_int>,
     pub(crate) vtbl: &'v ffi::MGVTBL,
-    pub(crate) ptr: Option<T>,
+    _phantom: PhantomData<T>,
 }
 
+/// It doesn't actually contain a `T`
+unsafe impl<'o, 'v, T> Sync for MagicSpec<'o, 'v, T> {}
+
+/// It doesn't actually contain a `T`
+unsafe impl<'o, 'v, T> Send for MagicSpec<'o, 'v, T> {}
+
 impl<T> MagicSpec<'static, 'static, T> {
     /// Create a new static magic specification from a tag.
     ///
@@ -226,7 +239,7 @@ impl<T> MagicSpec<'static, 'static, T> {
             obj: None,
             how: None,
             vtbl: &vtbl.0,
-            ptr: None,
+            _phantom: PhantomData,
         }
     }
 
@@ -236,7 +249,7 @@ impl<T> MagicSpec<'static, 'static, T> {
             obj: None,
             how: self.how,
             vtbl: self.vtbl,
-            ptr: None,
+            _phantom: PhantomData,
         }
     }
 }
@@ -245,12 +258,19 @@ impl<'o, 'v, T: Leakable> MagicSpec<'o, 'v, T> {
     /// Leak a `T` into a new `MagicSpec`.
     ///
     /// This LEAKS.
-    pub fn with_value(&self, ptr: T) -> Self {
-        MagicSpec {
-            obj: self.obj.clone(),
-            how: self.how.clone(),
-            vtbl: self.vtbl,
+    pub fn with_value<'a>(&'a self, ptr: T) -> MagicValue<'a, 'o, 'v, T> {
+        MagicValue {
+            spec: self,
             ptr: Some(ptr),
         }
     }
 }
+
+/// We want to instantiate `MagicSpec` as `static`, because as `const` the contained values may not
+/// actually end up to be guaranteed the same storage everywhere the `const` is accessed. But in
+/// order to be able to create a `static`, it must be `Sync`, so `MagicSpec` does not contain the
+/// pointer value anymore, instead, a `MagicValue` is instantiated for this.
+pub struct MagicValue<'spec, 'o, 'v, T> {
+    pub(crate) spec: &'spec MagicSpec<'o, 'v, T>,
+    pub(crate) ptr: Option<T>,
+}

perlmod/src/scalar.rs

@@ -7,7 +7,7 @@ use bitflags::bitflags;
 
 use crate::error::MagicError;
 use crate::ffi::{self, SV};
-use crate::magic::{Leakable, MagicSpec};
+use crate::magic::{Leakable, MagicSpec, MagicValue};
 use crate::raw_value;
 use crate::{Error, Value};
 
@@ -439,12 +439,12 @@ impl ScalarRef {
 
     /// Attach a magic tag to this value. This is a more convenient alternative to using
     /// [`add_raw_magic`](ScalarRef::add_raw_magic()) manually.
-    pub fn add_magic<'o, T: Leakable>(&self, spec: MagicSpec<'o, 'static, T>) {
+    pub fn add_magic<'spec, 'o, T: Leakable>(&self, spec: MagicValue<'spec, 'o, 'static, T>) {
         unsafe {
             self.add_raw_magic(
-                spec.obj,
-                spec.how,
-                Some(spec.vtbl),
+                spec.spec.obj,
+                spec.spec.how,
+                Some(spec.spec.vtbl),
                 spec.ptr.map(Leakable::leak).unwrap_or(std::ptr::null()),
                 0,
             )