diff --git a/crates/pindakaas-derive/src/lib.rs b/crates/pindakaas-derive/src/lib.rs index 59058f8325..3d2abf83f8 100644 --- a/crates/pindakaas-derive/src/lib.rs +++ b/crates/pindakaas-derive/src/lib.rs @@ -93,109 +93,82 @@ pub fn ipasir_solver_derive(input: TokenStream) -> TokenStream { quote!() }; - let (term_callback, term_drop) = if opts.term_callback { + let term_callback = if opts.term_callback { let term_cb = match opts.term_callback_ident { Some(x) => quote! { self. #x }, None => quote! { self.term_cb }, }; - ( - quote! { - impl crate::solver::TermCallback for #ident { - fn set_terminate_callback crate::solver::SlvTermSignal + 'static>( - &mut self, - cb: Option, - ) { - if let Some(mut cb) = cb { - let wrapped_cb = move || -> std::ffi::c_int { - match cb() { - crate::solver::SlvTermSignal::Continue => std::ffi::c_int::from(0), - crate::solver::SlvTermSignal::Terminate => std::ffi::c_int::from(1), - } - }; - let trampoline = crate::solver::libloading::get_trampoline0(&wrapped_cb); - let layout = std::alloc::Layout::for_value(&wrapped_cb); - let data = Box::leak(Box::new(wrapped_cb)) as *mut _ as *mut std::ffi::c_void; - if layout.size() != 0 { - // Otherwise nothing was leaked. - #term_cb = Some((data, layout)); - } - unsafe { - #krate::ipasir_set_terminate( - #ptr, - data, - Some(trampoline), - ) - } - } else { - if let Some((ptr, layout)) = #term_cb .take() { - unsafe { std::alloc::dealloc(ptr as *mut _, layout) }; + quote! { + impl crate::solver::TermCallback for #ident { + fn set_terminate_callback crate::solver::SlvTermSignal + 'static>( + &mut self, + cb: Option, + ) { + if let Some(mut cb) = cb { + let wrapped_cb = move || -> std::ffi::c_int { + match cb() { + crate::solver::SlvTermSignal::Continue => std::ffi::c_int::from(0), + crate::solver::SlvTermSignal::Terminate => std::ffi::c_int::from(1), } - unsafe { #krate::ipasir_set_terminate(#ptr, std::ptr::null_mut(), None) } + }; + let trampoline = crate::solver::libloading::get_trampoline0(&wrapped_cb); + #term_cb = crate::solver::libloading::FFIPointer::new(wrapped_cb); + unsafe { + #krate::ipasir_set_terminate( + #ptr, + #term_cb .get_ptr(), + Some(trampoline), + ) } + } else { + #term_cb = crate::solver::libloading::FFIPointer::default(); + unsafe { #krate::ipasir_set_terminate(#ptr, std::ptr::null_mut(), None) } } } - }, - quote! { - if let Some((ptr, layout)) = #term_cb .take() { - unsafe { std::alloc::dealloc(ptr as *mut _, layout) }; - } - }, - ) + } + } } else { - (quote!(), quote!()) + quote!() }; - let (learn_callback, learn_drop) = if opts.learn_callback { + let learn_callback = if opts.learn_callback { let learn_cb = match opts.learn_callback_ident { Some(x) => quote! { self. #x }, None => quote! { self.learn_cb }, }; - ( - quote! { - impl crate::solver::LearnCallback for #ident { - fn set_learn_callback) + 'static>( - &mut self, - cb: Option, - ) { - const MAX_LEN: std::ffi::c_int = 512; - if let Some(mut cb) = cb { - let wrapped_cb = move |clause: *const i32| { - let mut iter = crate::solver::libloading::ExplIter(clause) - .map(|i: i32| crate::Lit(std::num::NonZeroI32::new(i).unwrap())); - cb(&mut iter) - }; - let trampoline = crate::solver::libloading::get_trampoline1(&wrapped_cb); - let layout = std::alloc::Layout::for_value(&wrapped_cb); - let data = Box::leak(Box::new(wrapped_cb)) as *mut _ as *mut std::ffi::c_void; - if layout.size() != 0 { - // Otherwise nothing was leaked. - #learn_cb = Some((data, layout)); - } - unsafe { - #krate::ipasir_set_learn( - #ptr, - data, - MAX_LEN, - Some(trampoline), - ) - } - } else { - if let Some((ptr, layout)) = #learn_cb .take() { - unsafe { std::alloc::dealloc(ptr as *mut _, layout) }; - } - unsafe { #krate::ipasir_set_learn(#ptr, std::ptr::null_mut(), MAX_LEN, None) } + + quote! { + impl crate::solver::LearnCallback for #ident { + fn set_learn_callback) + 'static>( + &mut self, + cb: Option, + ) { + const MAX_LEN: std::ffi::c_int = 512; + if let Some(mut cb) = cb { + let wrapped_cb = move |clause: *const i32| { + let mut iter = crate::solver::libloading::ExplIter(clause) + .map(|i: i32| crate::Lit(std::num::NonZeroI32::new(i).unwrap())); + cb(&mut iter) + }; + let trampoline = crate::solver::libloading::get_trampoline1(&wrapped_cb); + #learn_cb = crate::solver::libloading::FFIPointer::new(wrapped_cb); + unsafe { + #krate::ipasir_set_learn( + #ptr, + #learn_cb .get_ptr(), + MAX_LEN, + Some(trampoline), + ) } + } else { + #learn_cb = crate::solver::libloading::FFIPointer::default(); + unsafe { #krate::ipasir_set_learn(#ptr, std::ptr::null_mut(), MAX_LEN, None) } } } - }, - quote! { - if let Some((ptr, layout)) = #learn_cb .take() { - unsafe { std::alloc::dealloc(ptr as *mut _, layout) }; - } - }, - ) + } + } } else { - (quote!(), quote!()) + quote!() }; let sol_ident = format_ident!("{}Sol", ident); @@ -210,8 +183,7 @@ pub fn ipasir_solver_derive(input: TokenStream) -> TokenStream { #[cfg(feature = "ipasir-up")] pub(crate) struct #prop_ident { /// Rust Propagator Storage - prop: *mut c_void, - drop_prop: fn(*mut c_void), + prop: crate::solver::libloading::FFIPointer, access_prop: fn(*mut c_void) -> *mut dyn std::any::Any, /// C Wrapper Object wrapper: *mut std::ffi::c_void, @@ -221,7 +193,6 @@ pub fn ipasir_solver_derive(input: TokenStream) -> TokenStream { impl Drop for #prop_ident { fn drop(&mut self) { unsafe { #krate::ipasir_prop_release(self.wrapper) }; - (self.drop_prop)(self.prop); } } @@ -229,16 +200,13 @@ pub fn ipasir_solver_derive(input: TokenStream) -> TokenStream { impl #prop_ident { pub(crate) fn new(prop: P, slv: #actions_ident) -> Self { // Construct wrapping structures - let prop = crate::solver::libloading::IpasirPropStore::new(prop, slv); - let drop_prop = |x: *mut std::ffi::c_void| { - let prop = unsafe { Box::

::from_raw(x as *mut P) }; - drop(prop); - }; - let access_prop = |x: *mut std::ffi::c_void| { - x as *mut P as *mut dyn std::any::Any + let store = crate::solver::libloading::IpasirPropStore::new(prop, slv); + let prop = crate::solver::libloading::FFIPointer::new(store); + let access_prop = |x: *mut std::ffi::c_void| -> *mut dyn std::any::Any { + let store = unsafe{ &mut *(x as *mut crate::solver::libloading::IpasirPropStore) } ; + (&mut store.prop) as *mut dyn std::any::Any }; - let data = Box::leak(Box::new(prop)) as *mut _ as *mut std::ffi::c_void; - let wrapper = unsafe { #krate::ipasir_prop_init(data as *mut std::ffi::c_void) }; + let wrapper = unsafe { #krate::ipasir_prop_init(prop.get_ptr()) }; // Set function pointers for methods unsafe { #krate::ipasir_prop_set_notify_assignment(wrapper, Some(crate::solver::libloading::ipasir_notify_assignment_cb::)) }; @@ -251,7 +219,7 @@ pub fn ipasir_solver_derive(input: TokenStream) -> TokenStream { unsafe { #krate::ipasir_prop_set_has_external_clause(wrapper, Some(crate::solver::libloading::ipasir_has_external_clause_cb::)) }; unsafe { #krate::ipasir_prop_set_add_external_clause_lit(wrapper, Some(crate::solver::libloading::ipasir_add_external_clause_lit_cb::)) }; - Self { prop: data, drop_prop, access_prop, wrapper, } + Self { prop, access_prop, wrapper, } } } @@ -288,14 +256,14 @@ pub fn ipasir_solver_derive(input: TokenStream) -> TokenStream { #[cfg(feature = "ipasir-up")] impl crate::solver::PropagatorAccess for #ident { fn propagator(&self) -> Option<&P> { - #prop_member.as_ref().map(|p| unsafe { &*(p.access_prop)(p.prop) } .downcast_ref()).flatten() + #prop_member.as_ref().map(|p| unsafe { &*(p.access_prop)(p.prop.get_ptr()) } .downcast_ref()).flatten() } } #[cfg(feature = "ipasir-up")] impl crate::solver::MutPropagatorAccess for #ident { fn propagator_mut(&mut self) -> Option<&mut P> { - #prop_member.as_ref().map(|p| unsafe { &mut *(p.access_prop)(p.prop) } .downcast_mut()).flatten() + #prop_member.as_ref().map(|p| unsafe { &mut *(p.access_prop)(p.prop.get_ptr()) } .downcast_mut()).flatten() } } @@ -337,7 +305,7 @@ pub fn ipasir_solver_derive(input: TokenStream) -> TokenStream { #sol_ident { ptr: self.ptr, #[cfg(feature = "ipasir-up")] - prop: #prop_member .as_mut().map(|p| p.prop), + prop: #prop_member .as_mut().map(|p| p.prop.get_ptr()), #[cfg(feature = "ipasir-up")] access_prop: #prop_member .as_ref().map(|p| p.access_prop), } @@ -457,8 +425,6 @@ pub fn ipasir_solver_derive(input: TokenStream) -> TokenStream { quote! { impl Drop for #ident { fn drop(&mut self) { - #learn_drop - #term_drop unsafe { #krate::ipasir_release( #ptr ) } } } diff --git a/crates/pindakaas/src/solver/cadical.rs b/crates/pindakaas/src/solver/cadical.rs index f582e3d353..6d62677f19 100644 --- a/crates/pindakaas/src/solver/cadical.rs +++ b/crates/pindakaas/src/solver/cadical.rs @@ -1,7 +1,6 @@ #[cfg(feature = "ipasir-up")] use std::sync::{Arc, Mutex}; use std::{ - alloc::Layout, ffi::{c_void, CString}, fmt, }; @@ -10,7 +9,7 @@ use pindakaas_cadical::{ccadical_copy, ccadical_phase, ccadical_unphase}; use pindakaas_derive::IpasirSolver; use super::VarFactory; -use crate::Lit; +use crate::{solver::libloading::FFIPointer, Lit}; #[derive(IpasirSolver)] #[ipasir(krate = pindakaas_cadical, assumptions, learn_callback, term_callback, ipasir_up)] @@ -24,9 +23,9 @@ pub struct Cadical { #[cfg(feature = "ipasir-up")] vars: Arc>, /// The callback used when a clause is learned. - learn_cb: Option<(*mut c_void, Layout)>, + learn_cb: FFIPointer, /// The callback used to check whether the solver should terminate. - term_cb: Option<(*mut c_void, Layout)>, + term_cb: FFIPointer, #[cfg(feature = "ipasir-up")] /// The external propagator called by the solver @@ -41,8 +40,8 @@ impl Default for Cadical { vars: VarFactory::default(), #[cfg(feature = "ipasir-up")] vars: Arc::default(), - learn_cb: None, - term_cb: None, + learn_cb: FFIPointer::default(), + term_cb: FFIPointer::default(), #[cfg(feature = "ipasir-up")] prop: None, } @@ -59,8 +58,8 @@ impl Clone for Cadical { Self { ptr, vars, - learn_cb: None, - term_cb: None, + learn_cb: FFIPointer::default(), + term_cb: FFIPointer::default(), #[cfg(feature = "ipasir-up")] prop: None, } diff --git a/crates/pindakaas/src/solver/intel_sat.rs b/crates/pindakaas/src/solver/intel_sat.rs index ce694dafc9..bb48c3cc87 100644 --- a/crates/pindakaas/src/solver/intel_sat.rs +++ b/crates/pindakaas/src/solver/intel_sat.rs @@ -1,8 +1,9 @@ -use std::{alloc::Layout, ffi::c_void}; +use std::ffi::c_void; use pindakaas_derive::IpasirSolver; use super::VarFactory; +use crate::solver::libloading::FFIPointer; #[derive(Debug, IpasirSolver)] #[ipasir(krate = pindakaas_intel_sat, assumptions, learn_callback, term_callback)] @@ -12,9 +13,9 @@ pub struct IntelSat { /// The variable factory for this solver. vars: VarFactory, /// The callback used when a clause is learned. - learn_cb: Option<(*mut c_void, Layout)>, + learn_cb: FFIPointer, /// The callback used to check whether the solver should terminate. - term_cb: Option<(*mut c_void, Layout)>, + term_cb: FFIPointer, } impl Default for IntelSat { @@ -22,8 +23,8 @@ impl Default for IntelSat { Self { ptr: unsafe { pindakaas_intel_sat::ipasir_init() }, vars: VarFactory::default(), - term_cb: None, - learn_cb: None, + term_cb: FFIPointer::default(), + learn_cb: FFIPointer::default(), } } } diff --git a/crates/pindakaas/src/solver/libloading.rs b/crates/pindakaas/src/solver/libloading.rs index cf41e3210b..fc5230221c 100644 --- a/crates/pindakaas/src/solver/libloading.rs +++ b/crates/pindakaas/src/solver/libloading.rs @@ -1,7 +1,6 @@ #[cfg(feature = "ipasir-up")] use std::collections::VecDeque; use std::{ - alloc::{self, Layout}, ffi::{c_char, c_int, c_void, CStr}, num::NonZeroI32, ptr, @@ -87,8 +86,8 @@ impl IpasirLibrary { IpasirSolver { slv: (self.ipasir_init_sym().unwrap())(), vars: VarFactory::default(), - learn_cb: None, - term_cb: None, + learn_cb: FFIPointer::default(), + term_cb: FFIPointer::default(), signature_fn: self.ipasir_signature_sym().unwrap(), release_fn: self.ipasir_release_sym().unwrap(), add_fn: self.ipasir_add_sym().unwrap(), @@ -129,9 +128,9 @@ pub struct IpasirSolver<'lib> { vars: VarFactory, /// The callback used when a clause is learned. - learn_cb: Option<(*mut c_void, Layout)>, + learn_cb: FFIPointer, /// The callback used to check whether the solver should terminate. - term_cb: Option<(*mut c_void, Layout)>, + term_cb: FFIPointer, signature_fn: Symbol<'lib, extern "C" fn() -> *const c_char>, release_fn: Symbol<'lib, extern "C" fn(*mut c_void)>, @@ -157,14 +156,6 @@ pub struct IpasirSolver<'lib> { impl<'lib> Drop for IpasirSolver<'lib> { fn drop(&mut self) { - // Drop the termination callback. - if let Some((ptr, layout)) = self.term_cb.take() { - unsafe { alloc::dealloc(ptr as *mut _, layout) }; - } - // Drop the learning callback. - if let Some((ptr, layout)) = self.learn_cb.take() { - unsafe { alloc::dealloc(ptr as *mut _, layout) }; - } // Release the solver. (self.release_fn)(self.slv); } @@ -299,7 +290,7 @@ impl FailedAssumtions for IpasirFailed<'_> { } impl<'lib> TermCallback for IpasirSolver<'lib> { - fn set_terminate_callback SlvTermSignal>(&mut self, cb: Option) { + fn set_terminate_callback SlvTermSignal + 'static>(&mut self, cb: Option) { if let Some(mut cb) = cb { let wrapped_cb = move || -> c_int { match cb() { @@ -307,45 +298,33 @@ impl<'lib> TermCallback for IpasirSolver<'lib> { SlvTermSignal::Terminate => c_int::from(1), } }; - let trampoline = get_trampoline0(&wrapped_cb); - let layout = Layout::for_value(&wrapped_cb); - let data = Box::leak(Box::new(wrapped_cb)) as *mut _ as *mut std::ffi::c_void; - if layout.size() != 0 { - // Otherwise nothing was leaked. - self.term_cb = Some((data, layout)); - } - (self.set_terminate_fn)(self.slv, data, Some(trampoline)); + self.term_cb = FFIPointer::new(wrapped_cb); + (self.set_terminate_fn)(self.slv, self.term_cb.get_ptr(), Some(trampoline)); } else { - if let Some((ptr, layout)) = self.term_cb.take() { - unsafe { alloc::dealloc(ptr as *mut _, layout) }; - } + self.term_cb = FFIPointer::default(); (self.set_terminate_fn)(self.slv, ptr::null_mut(), None); } } } impl<'lib> LearnCallback for IpasirSolver<'lib> { - fn set_learn_callback)>(&mut self, cb: Option) { + fn set_learn_callback) + 'static>( + &mut self, + cb: Option, + ) { const MAX_LEN: std::ffi::c_int = 512; if let Some(mut cb) = cb { - let wrapped_cb = |clause: *const i32| { + let wrapped_cb = move |clause: *const i32| { let mut iter = ExplIter(clause).map(|i: i32| Lit(NonZeroI32::new(i).unwrap())); cb(&mut iter) }; let trampoline = get_trampoline1(&wrapped_cb); - let layout = Layout::for_value(&wrapped_cb); - let data = Box::leak(Box::new(wrapped_cb)) as *mut _ as *mut std::ffi::c_void; - if layout.size() != 0 { - // Otherwise nothing was leaked. - self.learn_cb = Some((data, layout)); - } - (self.set_learn_fn)(self.slv, data, MAX_LEN, Some(trampoline)); + self.learn_cb = FFIPointer::new(wrapped_cb); + (self.set_learn_fn)(self.slv, self.learn_cb.get_ptr(), MAX_LEN, Some(trampoline)); } else { - if let Some((ptr, layout)) = self.learn_cb.take() { - unsafe { alloc::dealloc(ptr as *mut _, layout) }; - } + self.learn_cb = FFIPointer::default(); (self.set_learn_fn)(self.slv, ptr::null_mut(), MAX_LEN, None); } } @@ -415,6 +394,53 @@ impl IpasirPropStore { } } +// --- Helpers for C interface --- +pub(crate) fn get_drop_fn(_: &T) -> fn(*mut std::ffi::c_void) { + |ptr: *mut std::ffi::c_void| { + let b = unsafe { Box::::from_raw(ptr as *mut T) }; + drop(b); + } +} + +#[derive(Debug, PartialEq)] +pub(crate) struct FFIPointer { + pub(crate) ptr: *mut std::ffi::c_void, + pub(crate) drop_fn: fn(*mut std::ffi::c_void), +} + +impl FFIPointer { + pub(crate) fn new(obj: T) -> Self { + let drop_fn = get_drop_fn(&obj); + let ptr = Box::leak(Box::new(obj)) as *mut _ as *mut std::ffi::c_void; + Self { ptr, drop_fn } + } + + /// Get the FFI pointer to the contained object + /// + /// # WARNING + /// This pointer is only valid until the FFIPointer object is dropped. + pub(crate) fn get_ptr(&self) -> *mut std::ffi::c_void { + self.ptr + } +} + +impl Default for FFIPointer { + fn default() -> Self { + Self { + ptr: std::ptr::null_mut(), + drop_fn: |_: *mut std::ffi::c_void| {}, + } + } +} + +impl Drop for FFIPointer { + fn drop(&mut self) { + if !self.ptr.is_null() { + (self.drop_fn)(self.ptr); + } + } +} + // --- Callback functions for C propagator interface --- #[cfg(feature = "ipasir-up")]