fix(plugin)!: pass Miri tests

.github/workflows/pr.yaml

@@ -61,3 +61,18 @@ jobs:
         run: cargo clippy --all-features --all-targets
       - name: Render docs
         run: cargo doc --all-features --no-deps
+
+  miri:
+    name: Miri test
+    runs-on: ubuntu-latest
+    env:
+      RUSTFLAGS: -D warnings
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+      - name: Setup Rust
+        uses: dtolnay/rust-toolchain@nightly
+      - name: Install Miri
+        run: rustup component add miri
+      - name: Build and test
+        run: cargo +nightly miri test --no-default-features --features=miri

falco_plugin/Cargo.toml

@@ -36,8 +36,12 @@ unexpected_cfgs = { level = "allow", check-cfg = ['cfg(linkage, values("static")
 [[example]]
 name = "dummy_source"
 crate-type = ["cdylib"]
+required-features = ["dylib-examples"]
 
 [features]
+default = ["dylib-examples"]
+miri = ["thread-safe-tables"]
+dylib-examples = []
 thread-safe-tables = ["dep:parking_lot"]
 
 [dependencies]

falco_plugin/src/async_event/async_handler.rs

@@ -32,14 +32,14 @@ impl AsyncHandler {
     pub fn emit(&self, event: impl EventToBytes) -> Result<(), anyhow::Error> {
         let mut err = [0 as c_char; PLUGIN_MAX_ERRLEN as usize];
         let mut buf = Vec::new();
-        let err_ptr = &err as *const [c_char] as *const c_char;
 
         event.write(&mut buf)?;
         match unsafe {
             (self.raw_handler)(self.owner, buf.as_ptr() as *const _, err.as_mut_ptr()).as_result()
         } {
             Ok(()) => Ok(()),
             Err(e) => {
+                let err_ptr = err.as_ptr();
                 let msg = try_str_from_ptr(&err_ptr)?;
                 Err(e).context(msg.to_string())
             }

falco_plugin/src/source/event_batch.rs

@@ -51,7 +51,7 @@ impl EventBatch<'_> {
         self.pointers.reserve(num_events);
     }
 
-    pub(super) fn get_events(&self) -> &[*const u8] {
-        self.pointers.as_slice()
+    pub(super) fn get_events_ptr_len(&mut self) -> (*mut *const u8, usize) {
+        (self.pointers.as_mut_ptr(), self.pointers.len())
     }
 }

falco_plugin/src/source/wrappers.rs

@@ -241,9 +241,9 @@ pub unsafe extern "C" fn plugin_next_batch<T: SourcePlugin>(
         }));
         match batch_result {
             Ok(()) => {
-                let events = batch.get_events();
-                *nevts = events.len() as u32;
-                *evts = events as *const _ as *mut _;
+                let (events, events_len) = batch.get_events_ptr_len();
+                *nevts = events_len as u32;
+                *evts = events.cast();
                 ss_plugin_rc_SS_PLUGIN_SUCCESS
             }
             Err(e) => {

falco_plugin/src/tables/data.rs

@@ -102,7 +102,7 @@ pub trait Value: TableData {
 
     /// Given a raw table, fetch the field's metadata
     ///
-    /// The only interesting implementation is for `Box<Table>`, which gets all the fields
+    /// The only interesting implementation is for `Table`, which gets all the fields
     /// of a nested table and stores it in the subtable metadata. All others are no-ops.
     ///
     /// # Safety

falco_plugin/src/tables/export/field/table.rs

@@ -3,13 +3,14 @@ use crate::tables::export::entry::table_metadata::traits::TableMetadata;
 use crate::tables::export::entry::traits::Entry;
 use crate::tables::export::metadata::HasMetadata;
 use crate::tables::export::ref_shared::RefShared;
-use crate::tables::export::table::Table;
+use crate::tables::export::table::TableData;
+use crate::tables::export::Table;
 use crate::tables::Key;
 use anyhow::Error;
 use std::borrow::Borrow;
 use std::ffi::CStr;
 
-impl<K, E> HasMetadata for Box<Table<K, E>>
+impl<K, E> HasMetadata for Table<K, E>
 where
     K: Key + Ord,
     K: Borrow<<K as Key>::Borrowed>,
@@ -20,6 +21,6 @@ where
     type Metadata = RefShared<ExtensibleEntryMetadata<E::Metadata>>;
 
     fn new_with_metadata(tag: &'static CStr, meta: &Self::Metadata) -> Result<Self, Error> {
-        Ok(Box::new(Table::new_with_metadata(tag, meta)?))
+        Ok(Table::wrap(TableData::new_with_metadata(tag, meta)?))
     }
 }

falco_plugin/src/tables/export/field_value/table.rs

@@ -3,12 +3,12 @@ use crate::tables::export::entry::traits::Entry;
 use crate::tables::export::field_value::dynamic::DynamicFieldValue;
 use crate::tables::export::field_value::traits::FieldValue;
 use crate::tables::export::field_value::traits::{seal, StaticField};
-use crate::tables::export::table::Table;
+use crate::tables::export::table_box::Table;
 use crate::tables::{FieldTypeId, Key};
 use falco_plugin_api::ss_plugin_state_data;
 use std::borrow::Borrow;
 
-impl<K, E> seal::Sealed for Box<Table<K, E>>
+impl<K, E> seal::Sealed for Table<K, E>
 where
     K: Key + Ord,
     K: Borrow<<K as Key>::Borrowed>,
@@ -18,7 +18,7 @@ where
 {
 }
 
-impl<K, E> FieldValue for Box<Table<K, E>>
+impl<K, E> FieldValue for Table<K, E>
 where
     K: Key + Ord,
     K: Borrow<<K as Key>::Borrowed>,
@@ -34,14 +34,14 @@ where
         if type_id != FieldTypeId::Table {
             anyhow::bail!("Type mismatch, requested {:?}, got table", type_id)
         }
-        let vtable = self.get_boxed_vtable();
+        let vtable = self.get_vtable();
 
         out.table = vtable.cast();
         Ok(())
     }
 }
 
-impl<K, E> StaticField for Box<Table<K, E>>
+impl<K, E> StaticField for Table<K, E>
 where
     K: Key + Ord,
     K: Borrow<<K as Key>::Borrowed>,
@@ -53,7 +53,7 @@ where
     const READONLY: bool = true;
 }
 
-impl<K, E> TryFrom<DynamicFieldValue> for Box<Table<K, E>>
+impl<K, E> TryFrom<DynamicFieldValue> for Table<K, E>
 where
     K: Key + Ord,
     K: Borrow<<K as Key>::Borrowed>,

falco_plugin/src/tables/export/mod.rs

@@ -6,7 +6,7 @@
 //!
 //! Every field in the entry struct must be wrapped in [`Public`](`crate::tables::export::Public`),
 //! [`Private`](`crate::tables::export::Private`) or [`Readonly`](`crate::tables::export::Readonly`),
-//! except for nested tables. These just need to be a `Box<Table<K, E>>`, as it makes no sense
+//! except for nested tables. These just need to be a `Table<K, E>`, as it makes no sense
 //! to have a private nested table and the distinction between writable and readonly is meaningless
 //! for tables (they have no setter to replace the whole table and you can always add/remove
 //! entries from the nested table).
@@ -32,7 +32,7 @@
 //! struct MyPlugin {
 //!     // you can use methods on this instance to access fields bypassing the Falco table API
 //!     // (for performance within your own plugin)
-//!     exported_table: Box<export::Table<u64, ExportedTable>>,
+//!     exported_table: export::Table<u64, ExportedTable>,
 //! }
 //!
 //! // implement the base::Plugin trait
@@ -71,14 +71,15 @@ mod metadata;
 mod ref_shared;
 mod static_field_specialization;
 mod table;
+mod table_box;
 mod tables_input;
 mod vtable;
 mod wrappers;
 
 pub use field::private::Private;
 pub use field::public::Public;
 pub use field::readonly::Readonly;
-pub use table::Table;
+pub use table_box::Table;
 
 // for macro use only
 #[doc(hidden)]

falco_plugin/src/tables/export/ref_shared.rs

@@ -1,11 +1,112 @@
 use std::sync::Arc;
 
-#[cfg(feature = "thread-safe-tables")]
+#[cfg(all(feature = "thread-safe-tables", not(miri)))]
 use parking_lot::RawRwLock as LockImpl;
 
+#[cfg(all(feature = "thread-safe-tables", miri))]
+use miri_lock::StdRawRwLock as LockImpl;
+
 #[cfg(not(feature = "thread-safe-tables"))]
 use refcell_lock_api::raw::CellRwLock as LockImpl;
 
+/// A [`lock_api::RawRwLock`] implementation backed by `std::sync` primitives.
+///
+/// `parking_lot`'s implementation uses integer-to-pointer casts that Miri warns about
+/// (see <https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html>).
+/// When running under Miri we therefore substitute `parking_lot` with this implementation,
+/// which relies only on `std::sync::Mutex` and `std::sync::Condvar` — both of which have
+/// native Miri support.
+#[cfg(all(feature = "thread-safe-tables", miri))]
+mod miri_lock {
+    use std::sync::{Condvar, Mutex};
+
+    struct State {
+        readers: u32,
+        writing: bool,
+    }
+
+    #[allow(missing_debug_implementations)]
+    pub struct StdRawRwLock {
+        state: Mutex<State>,
+        cvar: Condvar,
+    }
+
+    unsafe impl lock_api::RawRwLock for StdRawRwLock {
+        #[allow(clippy::declare_interior_mutable_const)]
+        const INIT: Self = Self {
+            state: Mutex::new(State {
+                readers: 0,
+                writing: false,
+            }),
+            cvar: Condvar::new(),
+        };
+
+        type GuardMarker = lock_api::GuardSend;
+
+        fn lock_shared(&self) {
+            let mut state = self.state.lock().unwrap();
+            loop {
+                if !state.writing {
+                    state.readers += 1;
+                    return;
+                }
+                state = self.cvar.wait(state).unwrap();
+            }
+        }
+
+        fn try_lock_shared(&self) -> bool {
+            let mut state = self.state.lock().unwrap();
+            if !state.writing {
+                state.readers += 1;
+                true
+            } else {
+                false
+            }
+        }
+
+        unsafe fn unlock_shared(&self) {
+            let mut state = self.state.lock().unwrap();
+            state.readers -= 1;
+            if state.readers == 0 {
+                drop(state);
+                self.cvar.notify_all();
+            }
+        }
+
+        fn lock_exclusive(&self) {
+            let mut state = self.state.lock().unwrap();
+            loop {
+                if !state.writing && state.readers == 0 {
+                    state.writing = true;
+                    return;
+                }
+                state = self.cvar.wait(state).unwrap();
+            }
+        }
+
+        fn try_lock_exclusive(&self) -> bool {
+            let mut state = self.state.lock().unwrap();
+            if !state.writing && state.readers == 0 {
+                state.writing = true;
+                true
+            } else {
+                false
+            }
+        }
+
+        unsafe fn unlock_exclusive(&self) {
+            let mut state = self.state.lock().unwrap();
+            state.writing = false;
+            drop(state);
+            self.cvar.notify_all();
+        }
+    }
+
+    // SAFETY: the lock is entirely based on std::sync primitives which are Send+Sync.
+    unsafe impl Send for StdRawRwLock {}
+    unsafe impl Sync for StdRawRwLock {}
+}
+
 /// like `RefCell<T>`
 pub type RefCounted<T> = lock_api::RwLock<LockImpl, T>;