diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 0039aa12..e6df2c65 100644 --- a/.github/workflows/pr.yaml +++ b/.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 diff --git a/falco_plugin/Cargo.toml b/falco_plugin/Cargo.toml index 381c3c0e..c0e2ad07 100644 --- a/falco_plugin/Cargo.toml +++ b/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] diff --git a/falco_plugin/src/async_event/async_handler.rs b/falco_plugin/src/async_event/async_handler.rs index 0b34e07e..9563d7f4 100644 --- a/falco_plugin/src/async_event/async_handler.rs +++ b/falco_plugin/src/async_event/async_handler.rs @@ -32,7 +32,6 @@ 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 { @@ -40,6 +39,7 @@ impl AsyncHandler { } { Ok(()) => Ok(()), Err(e) => { + let err_ptr = err.as_ptr(); let msg = try_str_from_ptr(&err_ptr)?; Err(e).context(msg.to_string()) } diff --git a/falco_plugin/src/source/event_batch.rs b/falco_plugin/src/source/event_batch.rs index 7156a869..9b7a4803 100644 --- a/falco_plugin/src/source/event_batch.rs +++ b/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()) } } diff --git a/falco_plugin/src/source/wrappers.rs b/falco_plugin/src/source/wrappers.rs index 95d20872..819b8b7e 100644 --- a/falco_plugin/src/source/wrappers.rs +++ b/falco_plugin/src/source/wrappers.rs @@ -241,9 +241,9 @@ pub unsafe extern "C" fn plugin_next_batch( })); 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) => { diff --git a/falco_plugin/src/tables/data.rs b/falco_plugin/src/tables/data.rs index bc9ba4fa..73233867 100644 --- a/falco_plugin/src/tables/data.rs +++ b/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`, 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 diff --git a/falco_plugin/src/tables/export/field/table.rs b/falco_plugin/src/tables/export/field/table.rs index 7bcfd9ec..578dc2e5 100644 --- a/falco_plugin/src/tables/export/field/table.rs +++ b/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 HasMetadata for Box> +impl HasMetadata for Table where K: Key + Ord, K: Borrow<::Borrowed>, @@ -20,6 +21,6 @@ where type Metadata = RefShared>; fn new_with_metadata(tag: &'static CStr, meta: &Self::Metadata) -> Result { - Ok(Box::new(Table::new_with_metadata(tag, meta)?)) + Ok(Table::wrap(TableData::new_with_metadata(tag, meta)?)) } } diff --git a/falco_plugin/src/tables/export/field_value/table.rs b/falco_plugin/src/tables/export/field_value/table.rs index fc681e80..0ceb349a 100644 --- a/falco_plugin/src/tables/export/field_value/table.rs +++ b/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 seal::Sealed for Box> +impl seal::Sealed for Table where K: Key + Ord, K: Borrow<::Borrowed>, @@ -18,7 +18,7 @@ where { } -impl FieldValue for Box> +impl FieldValue for Table where K: Key + Ord, K: Borrow<::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 StaticField for Box> +impl StaticField for Table where K: Key + Ord, K: Borrow<::Borrowed>, @@ -53,7 +53,7 @@ where const READONLY: bool = true; } -impl TryFrom for Box> +impl TryFrom for Table where K: Key + Ord, K: Borrow<::Borrowed>, diff --git a/falco_plugin/src/tables/export/mod.rs b/falco_plugin/src/tables/export/mod.rs index 8cd8a2d7..2723bb55 100644 --- a/falco_plugin/src/tables/export/mod.rs +++ b/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>`, as it makes no sense +//! except for nested tables. These just need to be a `Table`, 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>, +//! exported_table: export::Table, //! } //! //! // implement the base::Plugin trait @@ -71,6 +71,7 @@ mod metadata; mod ref_shared; mod static_field_specialization; mod table; +mod table_box; mod tables_input; mod vtable; mod wrappers; @@ -78,7 +79,7 @@ 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)] diff --git a/falco_plugin/src/tables/export/ref_shared.rs b/falco_plugin/src/tables/export/ref_shared.rs index 09e123f3..a3cde59a 100644 --- a/falco_plugin/src/tables/export/ref_shared.rs +++ b/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 ). +/// 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, + 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` pub type RefCounted = lock_api::RwLock; diff --git a/falco_plugin/src/tables/export/table.rs b/falco_plugin/src/tables/export/table.rs index 4144253c..0929f3c0 100644 --- a/falco_plugin/src/tables/export/table.rs +++ b/falco_plugin/src/tables/export/table.rs @@ -14,29 +14,19 @@ use crate::tables::{FieldTypeId, Key}; use crate::FailureReason; use falco_plugin_api::{ss_plugin_state_data, ss_plugin_table_fieldinfo}; use std::borrow::Borrow; +use std::cell::UnsafeCell; use std::collections::BTreeMap; use std::ffi::CStr; use std::fmt::{Debug, Formatter}; -/// # A table exported to other plugins +/// The inner data storage for an exported table. /// -/// An instance of this type can be exposed to other plugins via -/// [`tables::TablesInput::add_table`](`crate::tables::TablesInput::add_table`) -/// -/// The generic parameters are: key type and entry type. The key type is anything -/// usable as a table key, while the entry type is a type that can be stored in the table. -/// You can obtain such a type by `#[derive]`ing Entry on a struct describing all the table fields. -/// -/// Supported key types include: -/// - integer types (u8/i8, u16/i16, u32/i32, u64/i64) -/// - [`crate::tables::import::Bool`] (an API equivalent of bool) -/// - &CStr (spelled as just `CStr` when used as a generic argument) -/// -/// See [`crate::tables::export`] for details. -/// -/// The implementation is thread-safe when the `thread-safe-tables` feature is enabled. +/// This type holds the actual table data (entries, metadata, field descriptors). +/// Users should not interact with this type directly; use [`super::Table`] instead, +/// which wraps `TableData` and provides the public API. #[must_use] -pub struct Table +#[doc(hidden)] +pub struct TableData where K: Key + Ord, K: Borrow<::Borrowed>, @@ -45,14 +35,14 @@ where E::Metadata: TableMetadata, { name: &'static CStr, - field_descriptors: Vec, + field_descriptors: UnsafeCell>, metadata: RefShared>, data: RefShared>>>, pub(crate) vtable: RefCounted>>, } -impl Debug for Table +impl Debug for TableData where K: Key + Ord + Debug, K: Borrow<::Borrowed>, @@ -72,7 +62,7 @@ where type TableMetadataType = RefShared::Metadata>>; pub(crate) type TableEntryType = RefGuard>; -impl Table +impl TableData where K: Key + Ord, K: Borrow<::Borrowed>, @@ -89,7 +79,7 @@ where ) -> Result { let table = Self { name: tag, - field_descriptors: vec![], + field_descriptors: UnsafeCell::new(vec![]), metadata: metadata.clone(), data: new_shared_ref(BTreeMap::new()), @@ -103,7 +93,7 @@ where pub fn new(name: &'static CStr) -> Result { Ok(Self { name, - field_descriptors: vec![], + field_descriptors: UnsafeCell::new(vec![]), metadata: new_shared_ref(ExtensibleEntryMetadata::new()?), data: new_shared_ref(BTreeMap::new()), @@ -159,7 +149,7 @@ where /// /// The iteration continues until all entries are visited or the closure returns false. // TODO(upstream) the closure cannot store away the entry but we could use explicit docs - pub fn iterate_entries(&mut self, mut func: F) -> bool + pub fn iterate_entries(&self, mut func: F) -> bool where F: FnMut(&mut TableEntryType) -> bool, { @@ -172,12 +162,12 @@ where } /// Remove all entries from the table. - pub fn clear(&mut self) { + pub fn clear(&self) { self.data.write().clear() } /// Erase an entry by key. - pub fn erase(&mut self, key: &Q) -> Option> + pub fn erase(&self, key: &Q) -> Option> where K: Borrow, Q: Ord + ?Sized, @@ -226,7 +216,7 @@ where } /// Attach an entry to a table key - pub fn insert(&mut self, key: &Q, entry: TableEntryType) -> Option> + pub fn insert(&self, key: &Q, entry: TableEntryType) -> Option> where K: Borrow, Q: Ord + ToOwned + ?Sized, @@ -264,10 +254,15 @@ where } /// Return a list of fields as a slice of raw FFI objects - pub fn list_fields(&mut self) -> &[ss_plugin_table_fieldinfo] { - self.field_descriptors.clear(); - self.field_descriptors.extend(self.metadata.list_fields()); - self.field_descriptors.as_slice() + pub fn list_fields(&self) -> &[ss_plugin_table_fieldinfo] { + // SAFETY: `list_fields` is never called re-entrantly; the slice is valid + // for the lifetime of `&self` because `field_descriptors` is owned by self. + unsafe { + let v = &mut *self.field_descriptors.get(); + v.clear(); + v.extend(self.metadata.list_fields()); + std::slice::from_raw_parts(v.as_ptr(), v.len()) + } } /// Return a field descriptor for a particular field @@ -281,12 +276,14 @@ where /// Add a new field to the table pub fn add_field( - &mut self, + &self, name: &CStr, field_type: FieldTypeId, read_only: bool, ) -> Option { - self.metadata.add_field(name, field_type, read_only) + self.metadata + .write_arc() + .add_field(name, field_type, read_only) } } diff --git a/falco_plugin/src/tables/export/table_box.rs b/falco_plugin/src/tables/export/table_box.rs new file mode 100644 index 00000000..28f61b6f --- /dev/null +++ b/falco_plugin/src/tables/export/table_box.rs @@ -0,0 +1,303 @@ +use crate::tables::export::entry::extensible::ExtensibleEntry; +use crate::tables::export::table::{TableData, TableEntryType}; +use crate::tables::export::traits::{Entry, TableMetadata}; +use crate::tables::export::{FieldRef, HasMetadata, RefShared}; +use crate::tables::{FieldTypeId, Key}; +use falco_plugin_api::{ss_plugin_state_data, ss_plugin_table_fieldinfo, ss_plugin_table_input}; +use std::borrow::Borrow; +use std::collections::BTreeMap; +use std::ffi::CStr; +use std::fmt::{Debug, Formatter}; +use std::ops::{Deref, DerefMut}; +use std::ptr::NonNull; + +/// # A table exported to other plugins +/// +/// An instance of this type can be exposed to other plugins via +/// [`tables::TablesInput::add_table`](`crate::tables::TablesInput::add_table`) +/// +/// The generic parameters are: key type and entry type. The key type is anything +/// usable as a table key, while the entry type is a type that can be stored in the table. +/// You can obtain such a type by `#[derive]`ing Entry on a struct describing all the table fields. +/// +/// Supported key types include: +/// - integer types (u8/i8, u16/i16, u32/i32, u64/i64) +/// - [`crate::tables::import::Bool`] (an API equivalent of bool) +/// - &CStr (spelled as just `CStr` when used as a generic argument) +/// +/// See [`crate::tables::export`] for details. +/// +/// The implementation is thread-safe when the `thread-safe-tables` feature is enabled. +/// +/// Internally, this uses `NonNull` instead of `Box` to avoid Miri's Stacked Borrows +/// transitive retagging, which would conflict with FFI callbacks that access the table +/// through raw pointers. +pub struct Table +where + K: Key + Ord, + K: Borrow<::Borrowed>, + ::Borrowed: Ord + ToOwned, + E: Entry, + E::Metadata: TableMetadata, +{ + ptr: NonNull>, +} + +impl Table +where + K: Key + Ord, + K: Borrow<::Borrowed>, + ::Borrowed: Ord + ToOwned, + E: Entry, + E::Metadata: TableMetadata, +{ + /// Wrap a `TableData` into a `Table`. + pub(crate) fn wrap(value: TableData) -> Self { + let ptr = Box::into_raw(Box::new(value)); + // SAFETY: Box::into_raw never returns null + Self { + ptr: unsafe { NonNull::new_unchecked(ptr) }, + } + } + + /// Returns a raw mutable pointer to the contained data without creating a reference. + pub(crate) fn as_mut_ptr(this: &Self) -> *mut TableData { + this.ptr.as_ptr() + } + + /// Get or create the vtable for this table, for use in FFI. + pub(crate) fn get_vtable(&self) -> *mut ss_plugin_table_input { + let table_ptr = Self::as_mut_ptr(self); + (**self).get_vtable_with_ptr(table_ptr) + } + + /// Create a new table + pub fn new(name: &'static CStr) -> Result { + Ok(Self::wrap(TableData::new(name)?)) + } + + /// Create a new table using provided metadata + /// + /// This is only expected to be used by the derive macro. + pub fn new_with_metadata( + tag: &'static CStr, + metadata: &::Metadata, + ) -> Result { + Ok(Self::wrap(TableData::new_with_metadata(tag, metadata)?)) + } + + /// Get an accessor to the underlying data + /// + /// This method returns a reference to the underlying BTreeMap, containing all the table's data. + /// It can be useful for: + /// - accessing the table from a different thread (with the `thread-safe-tables` feature enabled) + /// - bypassing the table API for convenience or more control over locking + /// + /// To actually access the BTreeMap, you first need to lock the returned object for reading + /// (`data.read()`) or writing (`data.write()`). + pub fn data(&self) -> RefShared>>> { + (**self).data() + } + + /// Return the table name. + pub fn name(&self) -> &'static CStr { + (**self).name() + } + + /// Return the number of entries in the table. + pub fn size(&self) -> usize { + (**self).size() + } + + /// Get an entry corresponding to a particular key. + pub fn lookup(&self, key: &Q) -> Option> + where + K: Borrow, + Q: Ord + ?Sized, + { + (**self).lookup(key) + } + + /// Get the value for a field in an entry. + pub fn get_field_value( + &self, + entry: &TableEntryType, + field: &crate::tables::export::field_descriptor::FieldDescriptor, + out: &mut ss_plugin_state_data, + ) -> Result<(), anyhow::Error> { + (**self).get_field_value(entry, field, out) + } + + /// Execute a closure on all entries in the table with read-only access. + /// + /// The iteration continues until all entries are visited or the closure returns false. + // TODO(upstream) the closure cannot store away the entry but we could use explicit docs + pub fn iterate_entries(&self, func: F) -> bool + where + F: FnMut(&mut TableEntryType) -> bool, + { + (**self).iterate_entries(func) + } + + /// Remove all entries from the table. + pub fn clear(&mut self) { + (**self).clear() + } + + /// Erase an entry by key. + pub fn erase(&mut self, key: &Q) -> Option> + where + K: Borrow, + Q: Ord + ?Sized, + { + (**self).erase(key) + } + + /// Create a new table entry. + /// + /// This is a detached entry that can be later inserted into the table using [`Table::insert`]. + pub fn create_entry(&self) -> Result, anyhow::Error> { + (**self).create_entry() + } + + /// Return a closure for creating table entries + /// + /// The `Table` object itself cannot be shared between threads safely even with + /// the `thread-safe-tables` feature enabled, but almost full functionality can be achieved + /// using two objects that can: + /// 1. The underlying BTreeMap, obtained from [Table::data] + /// 2. A closure capable of creating a new entry (returned from this function) + /// + /// The only functionality missing is listing table fields, and until a use case comes along, + /// it's likely to remain unimplemented. + /// + /// The entry obtained by calling the closure returned from `create_entry_fn` can be later + /// inserted into the table e.g. by calling [BTreeMap::insert]. + /// + /// To actually access the entry's fields, you first need to lock the returned object for reading + /// (`data.read()`) or writing (`data.write()`). + pub fn create_entry_fn( + &self, + ) -> impl Fn() -> Result>, anyhow::Error> + use { + (**self).create_entry_fn() + } + + /// Attach an entry to a table key + pub fn insert(&mut self, key: &Q, entry: TableEntryType) -> Option> + where + K: Borrow, + Q: Ord + ToOwned + ?Sized, + { + (**self).insert(key, entry) + } + + /// Write a value to a field of an entry + pub fn write( + &self, + entry: &mut TableEntryType, + field: &crate::tables::export::field_descriptor::FieldDescriptor, + value: &ss_plugin_state_data, + ) -> Result<(), anyhow::Error> { + (**self).write(entry, field, value) + } + + /// Return a list of fields as a slice of raw FFI objects + pub fn list_fields(&self) -> &[ss_plugin_table_fieldinfo] { + (**self).list_fields() + } + + /// Return a field descriptor for a particular field + /// + /// The requested `field_type` must match the actual type of the field + pub fn get_field(&self, name: &CStr, field_type: FieldTypeId) -> Option { + (**self).get_field(name, field_type) + } + + /// Add a new field to the table + pub fn add_field( + &self, + name: &CStr, + field_type: FieldTypeId, + read_only: bool, + ) -> Option { + (**self).add_field(name, field_type, read_only) + } +} + +impl Deref for Table +where + K: Key + Ord, + K: Borrow<::Borrowed>, + ::Borrowed: Ord + ToOwned, + E: Entry, + E::Metadata: TableMetadata, +{ + type Target = TableData; + fn deref(&self) -> &TableData { + // SAFETY: the pointer is valid as long as self is alive + unsafe { self.ptr.as_ref() } + } +} + +impl DerefMut for Table +where + K: Key + Ord, + K: Borrow<::Borrowed>, + ::Borrowed: Ord + ToOwned, + E: Entry, + E::Metadata: TableMetadata, +{ + fn deref_mut(&mut self) -> &mut TableData { + // SAFETY: the pointer is valid as long as self is alive and we have &mut self + unsafe { self.ptr.as_mut() } + } +} + +impl Drop for Table +where + K: Key + Ord, + K: Borrow<::Borrowed>, + ::Borrowed: Ord + ToOwned, + E: Entry, + E::Metadata: TableMetadata, +{ + fn drop(&mut self) { + // SAFETY: we own the allocation and it hasn't been freed + unsafe { + drop(Box::from_raw(self.ptr.as_ptr())); + } + } +} + +impl Debug for Table +where + K: Key + Ord + Debug, + K: Borrow<::Borrowed>, + ::Borrowed: Ord + ToOwned, + E: Entry + Debug, + E::Metadata: TableMetadata + Debug, +{ + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + (**self).fmt(f) + } +} + +// SAFETY: Table has the same semantics as Box +unsafe impl Send for Table +where + K: Key + Ord + Send, + K: Borrow<::Borrowed>, + ::Borrowed: Ord + ToOwned, + E: Entry + Send, + E::Metadata: TableMetadata, +{ +} +unsafe impl Sync for Table +where + K: Key + Ord + Sync, + K: Borrow<::Borrowed>, + ::Borrowed: Ord + ToOwned, + E: Entry + Sync, + E::Metadata: TableMetadata, +{ +} diff --git a/falco_plugin/src/tables/export/tables_input.rs b/falco_plugin/src/tables/export/tables_input.rs index 6b8a00f3..65433c50 100644 --- a/falco_plugin/src/tables/export/tables_input.rs +++ b/falco_plugin/src/tables/export/tables_input.rs @@ -12,10 +12,10 @@ use std::borrow::Borrow; impl TablesInput<'_> { /// # Export a table to the Falco plugin API /// - /// This method returns a Box, which you need to store in your plugin instance + /// This method returns a [`Table`], which you need to store in your plugin instance /// even if you don't intend to use the table yourself (the table is destroyed when /// going out of scope, which will lead to crashes in plugins using your table). - pub fn add_table(&self, table: Table) -> Result>, anyhow::Error> + pub fn add_table(&self, table: Table) -> Result, anyhow::Error> where K: Key + Ord, K: Borrow<::Borrowed>, @@ -27,8 +27,7 @@ impl TablesInput<'_> { let mut writer_vtable_ext = writer_vtable::(); let mut fields_vtable_ext = fields_vtable::(); - let mut table = Box::new(table); - let table_ptr = table.as_mut() as *mut Table; + let table_ptr = Table::as_mut_ptr(&table); // Note: we lend the ss_plugin_table_input to the FFI api and do not need // to hold on to it (everything is copied out), but the name field is copied diff --git a/falco_plugin/src/tables/export/vtable.rs b/falco_plugin/src/tables/export/vtable.rs index 32a582ef..8f8563cb 100644 --- a/falco_plugin/src/tables/export/vtable.rs +++ b/falco_plugin/src/tables/export/vtable.rs @@ -1,7 +1,7 @@ use crate::tables::export::entry::table_metadata::traits::TableMetadata; use crate::tables::export::entry::traits::Entry; +use crate::tables::export::table::TableData; use crate::tables::export::wrappers::{fields_vtable, reader_vtable, writer_vtable}; -use crate::tables::export::Table; use crate::tables::Key; use falco_plugin_api::{ ss_plugin_state_type, ss_plugin_table_fields_vtable, ss_plugin_table_fields_vtable_ext, @@ -9,15 +9,20 @@ use falco_plugin_api::{ ss_plugin_table_writer_vtable, ss_plugin_table_writer_vtable_ext, }; use std::borrow::Borrow; +use std::cell::UnsafeCell; pub(crate) struct Vtable { + pub(crate) inner: UnsafeCell, +} + +pub(crate) struct VtableInner { pub(crate) input: ss_plugin_table_input, reader_ext: ss_plugin_table_reader_vtable_ext, writer_ext: ss_plugin_table_writer_vtable_ext, fields_ext: ss_plugin_table_fields_vtable_ext, } -impl Table +impl TableData where K: Key + Ord, K: Borrow<::Borrowed>, @@ -25,15 +30,22 @@ where E: Entry, E::Metadata: TableMetadata, { - #[allow(clippy::borrowed_box)] - pub(crate) fn get_boxed_vtable(self: &Box) -> *mut ss_plugin_table_input { - let table_ptr = self.as_ref() as *const Table as *mut Table; + /// Get or create the vtable for this table. + /// + /// `table_ptr` must be a raw pointer to `self` with write provenance + /// (e.g. from `Table::as_mut_ptr`). This is necessary because the FFI layer + /// will use the stored pointer for mutable access. + pub(crate) fn get_vtable_with_ptr( + &self, + table_ptr: *mut TableData, + ) -> *mut ss_plugin_table_input { let mut vtable_place = self.vtable.write(); - if let Some(ref mut vtable) = *vtable_place { + if let Some(ref vtable) = *vtable_place { + let inner = vtable.inner.get(); // the ss_plugin_table_t value should never change - debug_assert_eq!(vtable.input.table, table_ptr.cast()); - return &mut vtable.input as *mut _; + debug_assert_eq!(unsafe { (*inner).input.table }, table_ptr.cast()); + return unsafe { std::ptr::addr_of_mut!((*inner).input) }; } let reader_vtable_ext = reader_vtable::(); @@ -68,21 +80,29 @@ where fields_ext: std::ptr::null_mut(), }; - let mut vtable = Box::new(Vtable { - input: table_input, - reader_ext: reader_vtable_ext, - writer_ext: writer_vtable_ext, - fields_ext: fields_vtable_ext, + let vtable = Box::new(Vtable { + inner: UnsafeCell::new(VtableInner { + input: table_input, + reader_ext: reader_vtable_ext, + writer_ext: writer_vtable_ext, + fields_ext: fields_vtable_ext, + }), }); - // we can init these fields only now, when the target struct is allocated on the heap - vtable.input.reader_ext = &mut vtable.reader_ext as *mut _; - vtable.input.writer_ext = &mut vtable.writer_ext as *mut _; - vtable.input.fields_ext = &mut vtable.fields_ext as *mut _; - - let ptr = &mut vtable.input as *mut _; + // Store the vtable first, then set up self-referential pointers + // through the stored Box's UnsafeCell to preserve pointer provenance. *vtable_place = Some(vtable); - ptr + let inner = vtable_place.as_ref().unwrap().inner.get(); + unsafe { + let reader_ext_ptr = std::ptr::addr_of_mut!((*inner).reader_ext); + let writer_ext_ptr = std::ptr::addr_of_mut!((*inner).writer_ext); + let fields_ext_ptr = std::ptr::addr_of_mut!((*inner).fields_ext); + (*inner).input.reader_ext = reader_ext_ptr; + (*inner).input.writer_ext = writer_ext_ptr; + (*inner).input.fields_ext = fields_ext_ptr; + } + + unsafe { std::ptr::addr_of_mut!((*inner).input) } } } diff --git a/falco_plugin/src/tables/export/wrappers.rs b/falco_plugin/src/tables/export/wrappers.rs index 1570070e..a31a0f52 100644 --- a/falco_plugin/src/tables/export/wrappers.rs +++ b/falco_plugin/src/tables/export/wrappers.rs @@ -2,7 +2,7 @@ use crate::error::ffi_result::FfiResult; use crate::tables::export::entry::table_metadata::traits::TableMetadata; use crate::tables::export::entry::traits::Entry; use crate::tables::export::field_descriptor::FieldDescriptor; -use crate::tables::export::table::{Table, TableEntryType}; +use crate::tables::export::table::{TableData, TableEntryType}; use crate::tables::{FieldTypeId, Key}; use falco_plugin_api::{ ss_plugin_bool, ss_plugin_rc, ss_plugin_rc_SS_PLUGIN_FAILURE, ss_plugin_rc_SS_PLUGIN_SUCCESS, @@ -25,7 +25,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return std::ptr::null_mut(); }; table.name().as_ptr() @@ -42,7 +42,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return 0; }; table.size() as u64 @@ -63,7 +63,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return std::ptr::null_mut(); }; let Some(key) = key.as_ref() else { @@ -93,7 +93,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return ss_plugin_rc_SS_PLUGIN_FAILURE; }; let Some(entry) = (entry as *mut TableEntryType).as_mut() else { @@ -142,7 +142,7 @@ where return 0; }; unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return 0; }; @@ -165,7 +165,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return ss_plugin_rc_SS_PLUGIN_FAILURE; }; table.clear(); @@ -186,7 +186,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return ss_plugin_rc_SS_PLUGIN_FAILURE; }; let Some(key) = key.as_ref() else { @@ -212,7 +212,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return std::ptr::null_mut(); }; @@ -241,7 +241,7 @@ where } unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return std::ptr::null_mut(); }; let Some(key) = key.as_ref() else { @@ -272,7 +272,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return ss_plugin_rc_SS_PLUGIN_FAILURE; }; let Some(entry) = (entry as *mut TableEntryType).as_mut() else { @@ -301,7 +301,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return std::ptr::null_mut(); }; let fields = table.list_fields(); @@ -324,7 +324,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return std::ptr::null_mut(); }; let Some(data_type) = FieldTypeId::from_usize(data_type as usize) else { @@ -356,7 +356,7 @@ where E::Metadata: TableMetadata, { unsafe { - let Some(table) = (table as *mut Table).as_mut() else { + let Some(table) = (table as *const TableData).as_ref() else { return std::ptr::null_mut(); }; let Some(data_type) = FieldTypeId::from_usize(data_type as usize) else { diff --git a/falco_plugin/src/tables/import/table/raw.rs b/falco_plugin/src/tables/import/table/raw.rs index b96d6e7b..d38905e2 100644 --- a/falco_plugin/src/tables/import/table/raw.rs +++ b/falco_plugin/src/tables/import/table/raw.rs @@ -103,7 +103,7 @@ impl RawTable { )?; let raw_field = unsafe { field - .as_mut() + .as_ref() .ok_or_else(|| anyhow::anyhow!("Failed to get table field {:?}", name)) .with_last_error(&tables_input.last_error)?; field @@ -135,7 +135,7 @@ impl RawTable { )?; let raw_field = unsafe { field - .as_mut() + .as_ref() .ok_or_else(|| anyhow::anyhow!("Failed to add table field {:?}", name)) .with_last_error(&tables_input.last_error)?; field diff --git a/falco_plugin_runner/src/plugin/async_event.rs b/falco_plugin_runner/src/plugin/async_event.rs index 8b10a378..1b76321d 100644 --- a/falco_plugin_runner/src/plugin/async_event.rs +++ b/falco_plugin_runner/src/plugin/async_event.rs @@ -8,13 +8,17 @@ use std::collections::VecDeque; use std::ffi::{c_char, CStr}; use std::sync::{Arc, Mutex}; +struct AsyncOwner { + async_events: Vec, + event_queue: Arc>>>, +} + pub struct AsyncPlugin { plugin: *mut ss_plugin_t, api: *const plugin_api__bindgen_ty_4, - async_events: Vec, + owner: Box, last_event: Option>, - event_queue: Arc>>>, } impl AsyncPlugin { @@ -35,9 +39,11 @@ impl AsyncPlugin { Self { plugin, api, - async_events, + owner: Box::new(AsyncOwner { + async_events, + event_queue: Arc::new(Mutex::new(VecDeque::new())), + }), last_event: None, - event_queue: Arc::new(Mutex::new(VecDeque::new())), } } @@ -45,8 +51,10 @@ impl AsyncPlugin { unsafe { &*self.api } } - fn owner(&mut self) -> *mut ss_plugin_owner_t { - self as *mut _ as *mut _ + fn owner(&self) -> *mut ss_plugin_owner_t { + (&*self.owner as *const AsyncOwner) + .cast_mut() + .cast::() } pub fn on_capture_start(&mut self) -> Result<(), ss_plugin_rc> { @@ -78,7 +86,7 @@ impl AsyncPlugin { } pub fn next_event(&mut self) -> Result<*mut ss_plugin_event, ss_plugin_rc> { - self.last_event = self.event_queue.lock().unwrap().pop_front(); + self.last_event = self.owner.event_queue.lock().unwrap().pop_front(); match &self.last_event { Some(evt) => Ok(evt.as_ptr().cast::().cast_mut()), None => Err(ss_plugin_rc_SS_PLUGIN_TIMEOUT), @@ -140,7 +148,7 @@ unsafe fn async_handler_inner( err: *mut c_char, ) -> i32 { let err = unsafe { std::slice::from_raw_parts_mut(err as *mut _, PLUGIN_MAX_ERRLEN as usize) }; - let owner = unsafe { &mut *(owner as *mut AsyncPlugin) }; + let owner = unsafe { &*(owner as *const AsyncOwner) }; let evt_len = unsafe { (*event).len as usize }; let event = event as *const u8; diff --git a/falco_plugin_runner/src/plugin/mod.rs b/falco_plugin_runner/src/plugin/mod.rs index 628b56a7..94c1d013 100644 --- a/falco_plugin_runner/src/plugin/mod.rs +++ b/falco_plugin_runner/src/plugin/mod.rs @@ -185,11 +185,12 @@ impl Plugin { self.api().__bindgen_anon_5.capture_open.is_some() // ... etc. } - fn owner(&self) -> *mut ss_plugin_owner_t { - self as *const _ as *mut ss_plugin_owner_t + fn owner(&mut self) -> *mut ss_plugin_owner_t { + self as *mut _ as *mut ss_plugin_owner_t } fn init(&mut self, config: &CStr) -> anyhow::Result<()> { + let owner = self.owner(); let tables = self.tables.borrow(); let tables_input = falco_plugin_api::ss_plugin_init_tables_input { list_tables: Some(list_tables), @@ -203,7 +204,7 @@ impl Plugin { let input = falco_plugin_api::ss_plugin_init_input { config: config.as_ptr(), - owner: self.owner(), + owner, get_owner_last_error: Some(get_last_owner_error), tables: &tables_input, log_fn: Some(log), @@ -298,22 +299,33 @@ impl Plugin { })?; } + let rollback_source = |this: &mut Self| { + if let Some(ref mut source) = this.source { + let _ = source.on_capture_stop(); + } + }; + if let Some(ref mut async_event) = self.async_event { - async_event.on_capture_start().map_err(|e| { - anyhow!( + if let Err(e) = async_event.on_capture_start() { + rollback_source(self); + return Err(anyhow!( "failed to notify async capture start, rc {e}, err {:?}", self.last_error() - ) - })?; + )); + } } if let Some(ref mut capture_listen) = self.capture_listen { - capture_listen.on_capture_start().map_err(|e| { - anyhow!( + if let Err(e) = capture_listen.on_capture_start() { + if let Some(ref mut async_event) = self.async_event { + let _ = async_event.on_capture_stop(); + } + rollback_source(self); + return Err(anyhow!( "failed to notify capture_listen plugin, rc {e}, err {:?}", self.last_error() - ) - })?; + )); + } } self.capturing = true; @@ -537,7 +549,7 @@ pub unsafe extern "C" fn get_table( } let name = unsafe { CStr::from_ptr(name) }; - let tables = owner.tables.borrow(); + let mut tables = owner.tables.borrow_mut(); match tables.get_table(name, key_type) { Some(table) => table as *const _ as *mut _, None => std::ptr::null_mut(), diff --git a/falco_plugin_runner/src/tables.rs b/falco_plugin_runner/src/tables.rs index 1e294a67..5265da38 100644 --- a/falco_plugin_runner/src/tables.rs +++ b/falco_plugin_runner/src/tables.rs @@ -8,9 +8,36 @@ use falco_plugin_api::{ use std::collections::btree_map::Entry; use std::collections::BTreeMap; use std::ffi::{c_char, CStr, CString}; +use std::ptr::NonNull; + +/// A non-retagging Box-like wrapper to avoid Miri's Stacked Borrows issues. +/// Unlike Box, moving this struct does not transitively retag the pointee. +struct RawBox(NonNull); + +impl RawBox { + fn new(val: ss_plugin_table_input) -> Self { + Self(unsafe { NonNull::new_unchecked(Box::into_raw(Box::new(val))) }) + } + fn as_ptr(&self) -> *mut ss_plugin_table_input { + self.0.as_ptr() + } +} + +impl std::ops::Deref for RawBox { + type Target = ss_plugin_table_input; + fn deref(&self) -> &ss_plugin_table_input { + unsafe { self.0.as_ref() } + } +} + +impl Drop for RawBox { + fn drop(&mut self) { + unsafe { drop(Box::from_raw(self.0.as_ptr())) } + } +} pub struct Tables { - tables: BTreeMap>, + tables: BTreeMap, reader_ext_store: Vec, writer_ext_store: Vec, fields_ext_store: Vec, @@ -20,14 +47,14 @@ pub struct Tables { macro_rules! delegate_table_method { ($table:expr => $vtable:ident . $method:ident or $error:expr) => {{ let table_input = $table as *mut ss_plugin_table_input; - let table_input = unsafe { table_input.as_mut() }; - let Some(table_input) = table_input else { + if table_input.is_null() { #[allow(clippy::unused_unit)] return $error; - }; + } let vtable = unsafe { - let Some(vtable) = table_input.$vtable.as_ref() else { + let vtable_ptr = std::ptr::addr_of!((*table_input).$vtable); + let Some(vtable) = (*vtable_ptr).as_ref() else { #[allow(clippy::unused_unit)] return $error; }; @@ -39,7 +66,7 @@ macro_rules! delegate_table_method { return $error; }; - (method, table_input.table) + (method, unsafe { (*table_input).table }) }}; } @@ -254,15 +281,15 @@ impl Tables { } pub fn get_table( - &self, + &mut self, name: &CStr, key_type: ss_plugin_state_type, - ) -> Option<&ss_plugin_table_input> { + ) -> Option<*mut ss_plugin_table_input> { let table = self.tables.get(name)?; if table.key_type != key_type { return None; } - Some(table) + Some(table.as_ptr()) } pub fn add_table(&mut self, name: &CStr, table_input: &ss_plugin_table_input) -> ss_plugin_rc { @@ -292,7 +319,7 @@ impl Tables { table_input.writer_ext = writer_ext; table_input.fields_ext = fields_ext; - entry.insert(Box::new(table_input)); + entry.insert(RawBox::new(table_input)); self.table_info_cache.clear(); ss_plugin_rc_SS_PLUGIN_SUCCESS } diff --git a/falco_plugin_tests/Cargo.toml b/falco_plugin_tests/Cargo.toml index 39d4bc95..5e55cfd2 100644 --- a/falco_plugin_tests/Cargo.toml +++ b/falco_plugin_tests/Cargo.toml @@ -43,7 +43,7 @@ harness = false anyhow = "1.0.88" cxx = { version = "1.0.124", features = ["c++17"] } falco_event_schema = { version = "0.5.0", path = "../falco_event_schema", features = ["derive_deftly"] } -falco_plugin = { version = "0.5.0", path = "../falco_plugin", features = ["thread-safe-tables"] } +falco_plugin = { version = "0.5.0", path = "../falco_plugin", default-features = false, features = ["thread-safe-tables"] } falco_plugin_runner = { version = "0.5.0", path = "../falco_plugin_runner" } log = "0.4.22" typed-path = "0.11.0" diff --git a/falco_plugin_tests/benches/plugin_custom_tables.rs b/falco_plugin_tests/benches/plugin_custom_tables.rs index 0133507c..8db229cf 100644 --- a/falco_plugin_tests/benches/plugin_custom_tables.rs +++ b/falco_plugin_tests/benches/plugin_custom_tables.rs @@ -38,7 +38,7 @@ struct ImportedCustomMetadata { struct CustomTableApi { #[allow(unused)] - exported_custom_table: Box>, + exported_custom_table: export::Table, imported_custom_table: import::Table, insert_val2_on_parse: bool, @@ -123,7 +123,7 @@ static_plugin!(CUSTOM_TABLE_API = CustomTableApi); struct CustomTableDirect { #[allow(unused)] - exported_custom_table: Box>, + exported_custom_table: export::Table, } impl Plugin for CustomTableDirect { @@ -267,7 +267,7 @@ fn bench_plugin_custom_table_extract_only(c: &mut Criterion) { g.throughput(Throughput::Elements(NUM_EVENTS as u64)); bench_plugin_custom_table_extract_only_impl::(&mut g); - #[cfg(have_libsinsp)] + #[cfg(all(have_libsinsp, not(miri)))] bench_plugin_custom_table_extract_only_impl::(&mut g); g.finish(); @@ -365,7 +365,7 @@ fn bench_plugin_custom_table_insert_and_extract(c: &mut Criterion) { bench_plugin_custom_table_insert_and_extract_impl::( &mut g, ); - #[cfg(have_libsinsp)] + #[cfg(all(have_libsinsp, not(miri)))] bench_plugin_custom_table_insert_and_extract_impl::(&mut g); g.finish(); diff --git a/falco_plugin_tests/benches/plugin_extract_static.rs b/falco_plugin_tests/benches/plugin_extract_static.rs index 27b2b005..7aa600a5 100644 --- a/falco_plugin_tests/benches/plugin_extract_static.rs +++ b/falco_plugin_tests/benches/plugin_extract_static.rs @@ -73,7 +73,7 @@ fn plugin_extract_static(c: &mut Criterion) { g.throughput(Throughput::Elements(NUM_EVENTS as u64)); plugin_extract_static_impl::(&mut g); - #[cfg(have_libsinsp)] + #[cfg(all(have_libsinsp, not(miri)))] plugin_extract_static_impl::(&mut g); g.finish(); diff --git a/falco_plugin_tests/benches/plugin_source_batch.rs b/falco_plugin_tests/benches/plugin_source_batch.rs index 1dc68f74..270f9eb6 100644 --- a/falco_plugin_tests/benches/plugin_source_batch.rs +++ b/falco_plugin_tests/benches/plugin_source_batch.rs @@ -41,7 +41,7 @@ fn plugin_source_batch(c: &mut Criterion) { g.throughput(Throughput::Elements(NUM_EVENTS as u64)); bench_plugin_source_batch_impl::(&mut g); - #[cfg(have_libsinsp)] + #[cfg(all(have_libsinsp, not(miri)))] bench_plugin_source_batch_impl::(&mut g); g.finish(); diff --git a/falco_plugin_tests/benches/plugin_source_parse_noop.rs b/falco_plugin_tests/benches/plugin_source_parse_noop.rs index ca20ecfc..621e8990 100644 --- a/falco_plugin_tests/benches/plugin_source_parse_noop.rs +++ b/falco_plugin_tests/benches/plugin_source_parse_noop.rs @@ -67,7 +67,7 @@ fn plugin_source_parse_noop(c: &mut Criterion) { g.throughput(Throughput::Elements(NUM_EVENTS as u64)); bench_plugin_source_parse_noop_impl::(&mut g); - #[cfg(have_libsinsp)] + #[cfg(all(have_libsinsp, not(miri)))] bench_plugin_source_parse_noop_impl::(&mut g); g.finish(); diff --git a/falco_plugin_tests/benches/plugin_threadinfo.rs b/falco_plugin_tests/benches/plugin_threadinfo.rs index cce72fc3..f0d27ff7 100644 --- a/falco_plugin_tests/benches/plugin_threadinfo.rs +++ b/falco_plugin_tests/benches/plugin_threadinfo.rs @@ -110,7 +110,7 @@ impl ParsePlugin for ParseThreadInfoSetCustomField { static_plugin!(PARSE_THREADINFO_SET_CUSTOM_FIELD = ParseThreadInfoSetCustomField); -#[cfg_attr(not(have_libsinsp), allow(unused))] +#[cfg_attr(any(not(have_libsinsp), miri), allow(unused))] fn bench_plugin_threadinfo_tid(g: &mut BenchmarkGroup) { let (mut driver, _plugin) = init_plugin::(&BATCHED_EMPTY_EVENT, c"1").unwrap(); let extract_plugin = driver.register_plugin(&EXTRACT_THREADINFO, c"").unwrap(); @@ -139,7 +139,7 @@ fn bench_plugin_threadinfo_tid(g: &mut BenchmarkG ); } -#[cfg_attr(not(have_libsinsp), allow(unused))] +#[cfg_attr(any(not(have_libsinsp), miri), allow(unused))] fn bench_plugin_threadinfo_missing_custom_field( g: &mut BenchmarkGroup, ) { @@ -171,7 +171,7 @@ fn bench_plugin_threadinfo_missing_custom_field( ); } -#[cfg_attr(not(have_libsinsp), allow(unused))] +#[cfg_attr(any(not(have_libsinsp), miri), allow(unused))] fn bench_plugin_threadinfo_only_set_custom_field( g: &mut BenchmarkGroup, ) { @@ -200,7 +200,7 @@ fn bench_plugin_threadinfo_only_set_custom_field( ); } -#[cfg_attr(not(have_libsinsp), allow(unused))] +#[cfg_attr(any(not(have_libsinsp), miri), allow(unused))] fn bench_plugin_threadinfo_custom_field(g: &mut BenchmarkGroup) { let (mut driver, _plugin) = init_plugin::(&BATCHED_EMPTY_EVENT, c"1").unwrap(); driver @@ -236,7 +236,7 @@ fn plugin_threadinfo(c: &mut Criterion) { let mut g = c.benchmark_group("plugin_threadinfo"); g.throughput(Throughput::Elements(NUM_EVENTS as u64)); - #[cfg(have_libsinsp)] + #[cfg(all(have_libsinsp, not(miri)))] { crate::bench_plugin_threadinfo_tid::(&mut g); crate::bench_plugin_threadinfo_missing_custom_field::( diff --git a/falco_plugin_tests/src/bin/dump_raw_events.rs b/falco_plugin_tests/src/bin/dump_raw_events.rs index 0e9349f8..3516e581 100644 --- a/falco_plugin_tests/src/bin/dump_raw_events.rs +++ b/falco_plugin_tests/src/bin/dump_raw_events.rs @@ -37,12 +37,12 @@ impl ParsePlugin for DumperPlugin { static_plugin!(DUMPER_PLUGIN = DumperPlugin); -#[cfg(not(have_libsinsp))] +#[cfg(any(not(have_libsinsp), miri))] fn main() { panic!("libsinsp not available"); } -#[cfg(have_libsinsp)] +#[cfg(all(have_libsinsp, not(miri)))] fn main() { use falco_plugin_tests::CapturingTestDriver; use falco_plugin_tests::SavefileTestDriver; diff --git a/falco_plugin_tests/src/lib.rs b/falco_plugin_tests/src/lib.rs index 250ec0e6..e677a9dd 100644 --- a/falco_plugin_tests/src/lib.rs +++ b/falco_plugin_tests/src/lib.rs @@ -2,7 +2,7 @@ //! //! This crate isn't really intended for public use, except maybe as a collection of sample plugins. -#[cfg(have_libsinsp)] +#[cfg(all(have_libsinsp, not(miri)))] pub mod ffi; use std::ffi::CStr; @@ -26,20 +26,22 @@ pub fn init_plugin( #[macro_export] macro_rules! instantiate_tests { - ($($func:ident);*) => { + ($($(#[$meta:meta])* $func:ident);*) => { mod native { $( #[test] + $(#[$meta])* fn $func() { super::$func::<$crate::native::Driver>() } )* } - #[cfg(have_libsinsp)] + #[cfg(all(have_libsinsp, not(miri)))] mod ffi { $( #[test] + $(#[$meta])* fn $func() { super::$func::<$crate::ffi::Driver>() } @@ -51,7 +53,7 @@ macro_rules! instantiate_tests { #[macro_export] macro_rules! instantiate_sinsp_tests { ($($func:ident);*) => { - #[cfg(have_libsinsp)] + #[cfg(all(have_libsinsp, not(miri)))] mod ffi { $( #[test] diff --git a/falco_plugin_tests/src/plugin_collection/parse/remaining_into_nested_table.rs b/falco_plugin_tests/src/plugin_collection/parse/remaining_into_nested_table.rs index 96ce0dec..0cc961e3 100644 --- a/falco_plugin_tests/src/plugin_collection/parse/remaining_into_nested_table.rs +++ b/falco_plugin_tests/src/plugin_collection/parse/remaining_into_nested_table.rs @@ -11,7 +11,7 @@ use falco_plugin::tables::TablesInput; use std::ffi::CStr; struct ParseIntoNestedTable { - remaining_table: Box, + remaining_table: RemainingEntryTable, } impl Plugin for ParseIntoNestedTable { diff --git a/falco_plugin_tests/src/plugin_collection/parse/remaining_into_table_api.rs b/falco_plugin_tests/src/plugin_collection/parse/remaining_into_table_api.rs index 866c45c0..830d5c98 100644 --- a/falco_plugin_tests/src/plugin_collection/parse/remaining_into_table_api.rs +++ b/falco_plugin_tests/src/plugin_collection/parse/remaining_into_table_api.rs @@ -14,7 +14,7 @@ use std::ffi::CStr; struct ParseIntoTableApiPlugin { #[allow(unused)] - remaining_table: Box, + remaining_table: RemainingEntryTable, remaining_table_import: RemainingCounterImportTable, } diff --git a/falco_plugin_tests/src/plugin_collection/parse/remaining_into_table_direct.rs b/falco_plugin_tests/src/plugin_collection/parse/remaining_into_table_direct.rs index d7fc1286..835082eb 100644 --- a/falco_plugin_tests/src/plugin_collection/parse/remaining_into_table_direct.rs +++ b/falco_plugin_tests/src/plugin_collection/parse/remaining_into_table_direct.rs @@ -11,7 +11,7 @@ use falco_plugin::tables::TablesInput; use std::ffi::CStr; struct ParseIntoTableDirectPlugin { - remaining_table: Box, + remaining_table: RemainingEntryTable, } impl Plugin for ParseIntoTableDirectPlugin { diff --git a/falco_plugin_tests/src/plugin_collection/tables/remaining_export.rs b/falco_plugin_tests/src/plugin_collection/tables/remaining_export.rs index 474781d2..875fc190 100644 --- a/falco_plugin_tests/src/plugin_collection/tables/remaining_export.rs +++ b/falco_plugin_tests/src/plugin_collection/tables/remaining_export.rs @@ -6,7 +6,7 @@ pub type RemainingEntryTable = export::Table; pub struct RemainingCounter { pub remaining: export::Public, pub readonly: export::Readonly, - pub countdown: Box, + pub countdown: CountdownTable, } pub type CountdownTable = export::Table; diff --git a/falco_plugin_tests/tests/async_tables.rs b/falco_plugin_tests/tests/async_tables.rs index 97573f95..55303ccd 100644 --- a/falco_plugin_tests/tests/async_tables.rs +++ b/falco_plugin_tests/tests/async_tables.rs @@ -27,7 +27,7 @@ struct DummyAsyncPlugin { task: Arc, thread: Option>>, - table: Box>, + table: export::Table, } impl Plugin for DummyAsyncPlugin { diff --git a/falco_plugin_tests/tests/capture_listen_resched.rs b/falco_plugin_tests/tests/capture_listen_resched.rs index ceac3e55..813049d2 100644 --- a/falco_plugin_tests/tests/capture_listen_resched.rs +++ b/falco_plugin_tests/tests/capture_listen_resched.rs @@ -116,5 +116,10 @@ mod tests { } } - instantiate_tests!(test_listen); + // Miri: ignored because this test relies on real-time threading (background thread scheduling, + // sleeps, and wall-clock timeouts) that Miri cannot simulate reliably. + instantiate_tests!( + #[cfg_attr(miri, ignore)] + test_listen + ); } diff --git a/falco_plugin_tests/tests/capture_listen_run_forever.rs b/falco_plugin_tests/tests/capture_listen_run_forever.rs index 54eeb2c9..0f808f82 100644 --- a/falco_plugin_tests/tests/capture_listen_run_forever.rs +++ b/falco_plugin_tests/tests/capture_listen_run_forever.rs @@ -129,5 +129,10 @@ mod tests { } } - instantiate_tests!(test_listen); + // Miri: ignored because this test relies on real-time threading (background thread scheduling, + // sleeps, and wall-clock timeouts) that Miri cannot simulate reliably. + instantiate_tests!( + #[cfg_attr(miri, ignore)] + test_listen + ); } diff --git a/falco_plugin_tests/tests/scap.rs b/falco_plugin_tests/tests/scap.rs index b53d0539..ef08260f 100644 --- a/falco_plugin_tests/tests/scap.rs +++ b/falco_plugin_tests/tests/scap.rs @@ -59,7 +59,7 @@ impl ParsePlugin for DummyPlugin { static_plugin!(PARSE_API = DummyPlugin); #[cfg(test)] -#[cfg_attr(not(have_libsinsp), allow(dead_code))] +#[cfg_attr(any(not(have_libsinsp), miri), allow(dead_code))] mod tests { use falco_plugin_tests::{ init_plugin, instantiate_sinsp_tests, CapturingTestDriver, SavefileTestDriver, ScapStatus, diff --git a/falco_plugin_tests/tests/scap_import_table.rs b/falco_plugin_tests/tests/scap_import_table.rs index f70c939f..f74dbd05 100644 --- a/falco_plugin_tests/tests/scap_import_table.rs +++ b/falco_plugin_tests/tests/scap_import_table.rs @@ -151,7 +151,7 @@ impl ParsePlugin for DummyPlugin { static_plugin!(PARSE_API = DummyPlugin); #[cfg(test)] -#[cfg_attr(not(have_libsinsp), allow(dead_code))] +#[cfg_attr(any(not(have_libsinsp), miri), allow(dead_code))] mod tests { use crate::TEST_DONE; use falco_plugin_tests::{ diff --git a/falco_plugin_tests/tests/scap_import_table_bad_key_type.rs b/falco_plugin_tests/tests/scap_import_table_bad_key_type.rs index cb46532f..4e1ab6ff 100644 --- a/falco_plugin_tests/tests/scap_import_table_bad_key_type.rs +++ b/falco_plugin_tests/tests/scap_import_table_bad_key_type.rs @@ -73,7 +73,7 @@ impl ParsePlugin for DummyPlugin { static_plugin!(PARSE_API = DummyPlugin); #[cfg(test)] -#[cfg_attr(not(have_libsinsp), allow(dead_code))] +#[cfg_attr(any(not(have_libsinsp), miri), allow(dead_code))] mod tests { use falco_plugin_tests::{init_plugin, instantiate_sinsp_tests, TestDriver};