From e366793204e66425b3544c467f79a1dc9f763c41 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Mon, 12 Jan 2026 02:52:58 -0800 Subject: [PATCH] don't compare function pointers -- they're non-deterministic --- crates/fayalite/src/module.rs | 7 ++- crates/fayalite/src/sim.rs | 86 ++++++++++++++++++++++++++++------- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/crates/fayalite/src/module.rs b/crates/fayalite/src/module.rs index 9d1a0e7..d959182 100644 --- a/crates/fayalite/src/module.rs +++ b/crates/fayalite/src/module.rs @@ -2396,13 +2396,16 @@ impl ModuleBuilder { #[track_caller] pub fn extern_module_simulation_fn< Args: fmt::Debug + Clone + Hash + Eq + Send + Sync + 'static, + F: Copy + Fn(Args, crate::sim::ExternModuleSimulationState) -> Fut + Send + Sync + 'static, Fut: IntoFuture + 'static, >( &self, args: Args, - f: fn(Args, crate::sim::ExternModuleSimulationState) -> Fut, + f: F, ) { - self.extern_module_simulation(crate::sim::SimGeneratorFn { args, f }); + // use for compile-time side-effect + let _ = crate::sim::SimGeneratorFn::::ASSERT_F_IS_ZERO_SIZED; + self.extern_module_simulation(crate::sim::SimGeneratorFn::new(args, f)); } } diff --git a/crates/fayalite/src/sim.rs b/crates/fayalite/src/sim.rs index 7068e44..a59a4c7 100644 --- a/crates/fayalite/src/sim.rs +++ b/crates/fayalite/src/sim.rs @@ -54,7 +54,6 @@ use std::{ hash::Hash, mem, pin::{Pin, pin}, - ptr, rc::Rc, sync::{Arc, Mutex, MutexGuard}, task::Poll, @@ -4065,13 +4064,48 @@ pub trait ExternModuleSimGenerator: Clone + Eq + Hash + Any + Send + Sync + fmt: fn run<'a>(&'a self, sim: ExternModuleSimulationState) -> impl IntoFuture + 'a; } -pub struct SimGeneratorFn { - pub args: Args, - pub f: fn(Args, ExternModuleSimulationState) -> Fut, +/// Type requirements: `F` must be zero-sized, this guarantees that the function called +/// by `F` is entirely determined by the type `F` and not by its value, allowing us to +/// correctly leave `F` out of the stuff used for `Hash` and `Eq`. +/// +/// Note we can't just use function pointers instead -- comparing them is non-deterministic +/// since Rust will duplicate and/or merge functions, even if those functions happen to have +/// different UB but just happen to compile to the same assembly language: +/// +pub struct SimGeneratorFn Fut, Fut> { + args: Args, + f: F, } -impl fmt::Debug for SimGeneratorFn { +impl Fut, Fut> + SimGeneratorFn +{ + pub const ASSERT_F_IS_ZERO_SIZED: () = { + if size_of::() != 0 { + panic!( + "F must be zero-sized -- so it must be a closure with no captures or a function item, it can't be a function pointer" + ); + } + }; + pub const fn new(args: Args, f: F) -> Self { + // use for compile-time side-effect + let _ = Self::ASSERT_F_IS_ZERO_SIZED; + Self { args, f } + } + pub const fn args(&self) -> &Args { + &self.args + } + pub const fn f(&self) -> F { + self.f + } +} + +impl Fut, Fut> fmt::Debug + for SimGeneratorFn +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // use for compile-time side-effect + let _ = Self::ASSERT_F_IS_ZERO_SIZED; let Self { args, f: _ } = self; f.debug_struct("SimGeneratorFn") .field("args", args) @@ -4080,25 +4114,39 @@ impl fmt::Debug for SimGeneratorFn { } } -impl Hash for SimGeneratorFn { +impl Fut, Fut> Hash + for SimGeneratorFn +{ fn hash(&self, state: &mut H) { - let Self { args, f } = self; + // use for compile-time side-effect + let _ = Self::ASSERT_F_IS_ZERO_SIZED; + let Self { args, f: _ } = self; args.hash(state); - f.hash(state); } } -impl Eq for SimGeneratorFn {} +impl Fut, Fut> Eq + for SimGeneratorFn +{ +} -impl PartialEq for SimGeneratorFn { +impl Fut, Fut> PartialEq + for SimGeneratorFn +{ fn eq(&self, other: &Self) -> bool { - let Self { args, f } = self; - *args == other.args && ptr::fn_addr_eq(*f, other.f) + // use for compile-time side-effect + let _ = Self::ASSERT_F_IS_ZERO_SIZED; + let Self { args, f: _ } = self; + *args == other.args } } -impl Clone for SimGeneratorFn { +impl Fut, Fut> Clone + for SimGeneratorFn +{ fn clone(&self) -> Self { + // use for compile-time side-effect + let _ = Self::ASSERT_F_IS_ZERO_SIZED; Self { args: self.args.clone(), f: self.f, @@ -4106,14 +4154,20 @@ impl Clone for SimGeneratorFn { } } -impl Copy for SimGeneratorFn {} +impl Fut, Fut> Copy + for SimGeneratorFn +{ +} impl< - T: fmt::Debug + Clone + Eq + Hash + Send + Sync + 'static, + Args: fmt::Debug + Clone + Eq + Hash + Send + Sync + 'static, + F: Copy + Fn(Args, ExternModuleSimulationState) -> Fut + Send + Sync + 'static, Fut: IntoFuture + 'static, -> ExternModuleSimGenerator for SimGeneratorFn +> ExternModuleSimGenerator for SimGeneratorFn { fn run<'a>(&'a self, sim: ExternModuleSimulationState) -> impl IntoFuture + 'a { + // use for compile-time side-effect + let _ = Self::ASSERT_F_IS_ZERO_SIZED; (self.f)(self.args.clone(), sim) } } -- 2.49.1