From a658ead3a5cc6bdbda5b507c51dffb5c92cb1516 Mon Sep 17 00:00:00 2001 From: Esteve Soler Arderiu Date: Thu, 16 Jan 2025 00:35:49 +0100 Subject: [PATCH 1/8] Add `SafeRunner` for catching segfaults. --- src/error.rs | 3 ++ src/executor.rs | 5 ++- src/executor/contract.rs | 5 ++- src/utils.rs | 3 ++ src/utils/safe_runner.rs | 93 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 src/utils/safe_runner.rs diff --git a/src/error.rs b/src/error.rs index 5f5b65546..8f9526953 100644 --- a/src/error.rs +++ b/src/error.rs @@ -90,6 +90,9 @@ pub enum Error { #[error("Failed to parse a Cairo/Sierra program: {0}")] ProgramParser(String), + + #[error("program execution segfaulted")] + ProgramExecutionSegfault, } impl Error { diff --git a/src/executor.rs b/src/executor.rs index 8aaf2d9fe..7a3cfc728 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -223,14 +223,15 @@ fn invoke_dynamic( #[cfg(target_arch = "aarch64")] let mut ret_registers = [0; 4]; - unsafe { + crate::utils::run_safely(|| unsafe { invoke_trampoline( function_ptr, invoke_data.as_ptr().cast(), invoke_data.len() >> 3, ret_registers.as_mut_ptr(), ); - } + }) + .map_err(|_| Error::ProgramExecutionSegfault)?; // If the syscall handler was changed, then reset the previous one. // It's only necessary to restore the pointer if it's been modified i.e. if previous_syscall_handler is Some(...) diff --git a/src/executor/contract.rs b/src/executor/contract.rs index f0a9c17a8..0ea9452ab 100644 --- a/src/executor/contract.rs +++ b/src/executor/contract.rs @@ -436,14 +436,15 @@ impl AotContractExecutor { #[cfg(target_arch = "aarch64")] let mut ret_registers = [0; 4]; - unsafe { + crate::utils::run_safely(|| unsafe { invoke_trampoline( function_ptr, invoke_data.as_ptr().cast(), invoke_data.len() >> 3, ret_registers.as_mut_ptr(), ); - } + }) + .map_err(|_| Error::ProgramExecutionSegfault)?; // Parse final gas. unsafe fn read_value(ptr: &mut NonNull<()>) -> &T { diff --git a/src/utils.rs b/src/utils.rs index 6ac8733c2..aee2c255c 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,9 +1,11 @@ //! # Various utilities +pub use self::safe_runner::setup_safe_runner; pub(crate) use self::{ block_ext::{BlockExt, GepIndex}, program_registry_ext::ProgramRegistryExt, range_ext::RangeExt, + safe_runner::run_safely, }; use crate::{metadata::MetadataStorage, OptLevel}; use cairo_lang_compiler::CompilerConfig; @@ -34,6 +36,7 @@ mod block_ext; pub mod mem_tracing; mod program_registry_ext; mod range_ext; +mod safe_runner; #[cfg(target_os = "macos")] pub const SHARED_LIBRARY_EXT: &str = "dylib"; diff --git a/src/utils/safe_runner.rs b/src/utils/safe_runner.rs new file mode 100644 index 000000000..0a1626388 --- /dev/null +++ b/src/utils/safe_runner.rs @@ -0,0 +1,93 @@ +use libc::{ + c_int, sigaction, sigaltstack, siginfo_t, sigset_t, stack_t, ucontext_t, SA_ONSTACK, + SA_SIGINFO, SIGSEGV, SIGSTKSZ, +}; +use std::{ + cell::UnsafeCell, + mem::MaybeUninit, + ptr::{self, null_mut}, +}; + +extern "C" { + fn setjmp(env: *mut ()) -> c_int; + fn longjmp(env: *mut (), val: c_int) -> !; +} + +thread_local! { + static STATE: UnsafeCell = const { UnsafeCell::new(SafeRunnerState::Inactive) }; + static STACK: UnsafeCell = const { UnsafeCell::new(SignalStack(MaybeUninit::uninit())) }; +} + +type JmpBuf = MaybeUninit<[u8; 1024]>; + +#[repr(align(16))] +struct SignalStack(MaybeUninit<[u8; SIGSTKSZ]>); + +enum SafeRunnerState { + Inactive, + Active(Box), +} + +#[allow(clippy::result_unit_err)] +pub fn setup_safe_runner() -> Result<(), ()> { + unsafe { + let ret = sigaction( + SIGSEGV, + &sigaction { + sa_sigaction: segfault_handler + as *const extern "C" fn(c_int, &siginfo_t, &mut ucontext_t) + as usize, + sa_mask: MaybeUninit::::zeroed().assume_init(), + sa_flags: SA_ONSTACK | SA_SIGINFO, + sa_restorer: None, + }, + null_mut(), + ); + if ret < 0 { + return Err(()); + } + + sigaltstack( + &stack_t { + ss_sp: STACK.with(|x| x.get()).cast(), + ss_flags: 0, + ss_size: SIGSTKSZ, + }, + null_mut(), + ); + } + + Ok(()) +} + +pub fn run_safely(f: impl FnOnce() -> T) -> Result { + let (jmp_buf, prev_state) = STATE.with(|x| unsafe { + let jmp_buf; + let prev_state = ptr::replace( + x.get(), + SafeRunnerState::Active({ + let mut tmp = Box::new(JmpBuf::uninit()); + jmp_buf = tmp.as_mut_ptr(); + tmp + }), + ); + + (jmp_buf, prev_state) + }); + + let jmp_ret = unsafe { setjmp(jmp_buf.cast()) }; + let result = match jmp_ret { + 0 => Ok(f()), + _ => Err(()), + }; + + STATE.with(|x| unsafe { ptr::write(x.get(), prev_state) }); + result +} + +unsafe extern "C" fn segfault_handler(_sig: c_int, _info: &siginfo_t, _context: &mut ucontext_t) { + match STATE.with(|x| &mut *x.get()) { + SafeRunnerState::Inactive => libc::abort(), + SafeRunnerState::Active(jmp_buf) => longjmp(jmp_buf.as_mut_ptr().cast(), 0), + } +} From 428965dc5eea8679775a8cda5cb7ea0072e0ce8d Mon Sep 17 00:00:00 2001 From: Esteve Soler Arderiu Date: Thu, 16 Jan 2025 00:51:13 +0100 Subject: [PATCH 2/8] Add API to manually abort. --- src/error.rs | 5 ++- src/executor.rs | 4 +- src/executor/contract.rs | 4 +- src/utils.rs | 4 +- src/utils/safe_runner.rs | 80 ++++++++++++++++++++++++++-------------- 5 files changed, 60 insertions(+), 37 deletions(-) diff --git a/src/error.rs b/src/error.rs index 8f9526953..dd6108d24 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,6 @@ //! Various error types used thorough the crate. use crate::metadata::gas::GasMetadataError; +use crate::utils::safe_runner::SafeRunnerError; use cairo_lang_sierra::extensions::modules::utils::Range; use cairo_lang_sierra::{ edit_state::EditStateError, ids::ConcreteTypeId, program_registry::ProgramRegistryError, @@ -91,8 +92,8 @@ pub enum Error { #[error("Failed to parse a Cairo/Sierra program: {0}")] ProgramParser(String), - #[error("program execution segfaulted")] - ProgramExecutionSegfault, + #[error(transparent)] + SafeRunner(SafeRunnerError), } impl Error { diff --git a/src/executor.rs b/src/executor.rs index 7a3cfc728..9b5822681 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -223,7 +223,7 @@ fn invoke_dynamic( #[cfg(target_arch = "aarch64")] let mut ret_registers = [0; 4]; - crate::utils::run_safely(|| unsafe { + crate::utils::safe_runner::run_safely(|| unsafe { invoke_trampoline( function_ptr, invoke_data.as_ptr().cast(), @@ -231,7 +231,7 @@ fn invoke_dynamic( ret_registers.as_mut_ptr(), ); }) - .map_err(|_| Error::ProgramExecutionSegfault)?; + .map_err(Error::SafeRunner)?; // If the syscall handler was changed, then reset the previous one. // It's only necessary to restore the pointer if it's been modified i.e. if previous_syscall_handler is Some(...) diff --git a/src/executor/contract.rs b/src/executor/contract.rs index 0ea9452ab..c326a91e4 100644 --- a/src/executor/contract.rs +++ b/src/executor/contract.rs @@ -436,7 +436,7 @@ impl AotContractExecutor { #[cfg(target_arch = "aarch64")] let mut ret_registers = [0; 4]; - crate::utils::run_safely(|| unsafe { + crate::utils::safe_runner::run_safely(|| unsafe { invoke_trampoline( function_ptr, invoke_data.as_ptr().cast(), @@ -444,7 +444,7 @@ impl AotContractExecutor { ret_registers.as_mut_ptr(), ); }) - .map_err(|_| Error::ProgramExecutionSegfault)?; + .map_err(Error::SafeRunner)?; // Parse final gas. unsafe fn read_value(ptr: &mut NonNull<()>) -> &T { diff --git a/src/utils.rs b/src/utils.rs index aee2c255c..75a0a239a 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,11 +1,9 @@ //! # Various utilities -pub use self::safe_runner::setup_safe_runner; pub(crate) use self::{ block_ext::{BlockExt, GepIndex}, program_registry_ext::ProgramRegistryExt, range_ext::RangeExt, - safe_runner::run_safely, }; use crate::{metadata::MetadataStorage, OptLevel}; use cairo_lang_compiler::CompilerConfig; @@ -36,7 +34,7 @@ mod block_ext; pub mod mem_tracing; mod program_registry_ext; mod range_ext; -mod safe_runner; +pub mod safe_runner; #[cfg(target_os = "macos")] pub const SHARED_LIBRARY_EXT: &str = "dylib"; diff --git a/src/utils/safe_runner.rs b/src/utils/safe_runner.rs index 0a1626388..a34e22284 100644 --- a/src/utils/safe_runner.rs +++ b/src/utils/safe_runner.rs @@ -7,6 +7,7 @@ use std::{ mem::MaybeUninit, ptr::{self, null_mut}, }; +use thiserror::Error; extern "C" { fn setjmp(env: *mut ()) -> c_int; @@ -21,6 +22,7 @@ thread_local! { type JmpBuf = MaybeUninit<[u8; 1024]>; #[repr(align(16))] +#[allow(dead_code)] struct SignalStack(MaybeUninit<[u8; SIGSTKSZ]>); enum SafeRunnerState { @@ -28,39 +30,59 @@ enum SafeRunnerState { Active(Box), } -#[allow(clippy::result_unit_err)] -pub fn setup_safe_runner() -> Result<(), ()> { +#[derive(Debug, Error)] +pub enum SafeRunnerError { + #[error("program execution aborted")] + Aborted, + #[error("program execution segfaulted")] + Segfault, +} + +/// Configure the current **process** for the [`SafeRunner`]. +/// +/// Note: It will override the previous signal handler for SIGSEGV. +pub fn setup_safe_runner() { unsafe { - let ret = sigaction( - SIGSEGV, - &sigaction { - sa_sigaction: segfault_handler - as *const extern "C" fn(c_int, &siginfo_t, &mut ucontext_t) - as usize, - sa_mask: MaybeUninit::::zeroed().assume_init(), - sa_flags: SA_ONSTACK | SA_SIGINFO, - sa_restorer: None, - }, - null_mut(), + assert_eq!( + sigaction( + SIGSEGV, + &sigaction { + sa_sigaction: segfault_handler + as *const extern "C" fn(c_int, &siginfo_t, &mut ucontext_t) + as usize, + sa_mask: MaybeUninit::::zeroed().assume_init(), + sa_flags: SA_ONSTACK | SA_SIGINFO, + sa_restorer: None, + }, + null_mut(), + ), + 0, ); - if ret < 0 { - return Err(()); - } - - sigaltstack( - &stack_t { - ss_sp: STACK.with(|x| x.get()).cast(), - ss_flags: 0, - ss_size: SIGSTKSZ, - }, - null_mut(), + assert_eq!( + sigaltstack( + &stack_t { + ss_sp: STACK.with(|x| x.get()).cast(), + ss_flags: 0, + ss_size: SIGSTKSZ, + }, + null_mut(), + ), + 0, ); } +} - Ok(()) +/// Manually trigger the segfault handler, thus aborting the current program. +pub fn abort_safe_runner() -> ! { + unsafe { + match STATE.with(|x| &mut *x.get()) { + SafeRunnerState::Inactive => libc::abort(), + SafeRunnerState::Active(jmp_buf) => longjmp(jmp_buf.as_mut_ptr().cast(), 2), + } + } } -pub fn run_safely(f: impl FnOnce() -> T) -> Result { +pub fn run_safely(f: impl FnOnce() -> T) -> Result { let (jmp_buf, prev_state) = STATE.with(|x| unsafe { let jmp_buf; let prev_state = ptr::replace( @@ -78,7 +100,9 @@ pub fn run_safely(f: impl FnOnce() -> T) -> Result { let jmp_ret = unsafe { setjmp(jmp_buf.cast()) }; let result = match jmp_ret { 0 => Ok(f()), - _ => Err(()), + 1 => Err(SafeRunnerError::Segfault), + 2 => Err(SafeRunnerError::Aborted), + _ => unreachable!(), }; STATE.with(|x| unsafe { ptr::write(x.get(), prev_state) }); @@ -88,6 +112,6 @@ pub fn run_safely(f: impl FnOnce() -> T) -> Result { unsafe extern "C" fn segfault_handler(_sig: c_int, _info: &siginfo_t, _context: &mut ucontext_t) { match STATE.with(|x| &mut *x.get()) { SafeRunnerState::Inactive => libc::abort(), - SafeRunnerState::Active(jmp_buf) => longjmp(jmp_buf.as_mut_ptr().cast(), 0), + SafeRunnerState::Active(jmp_buf) => longjmp(jmp_buf.as_mut_ptr().cast(), 1), } } From fba9cdfaa8aeb77a048092f07041d7875e3cc3bf Mon Sep 17 00:00:00 2001 From: Esteve Soler Arderiu Date: Thu, 16 Jan 2025 01:17:35 +0100 Subject: [PATCH 3/8] Implement `ManagedAllocator`. --- src/utils.rs | 1 + src/utils/allocator.rs | 104 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 src/utils/allocator.rs diff --git a/src/utils.rs b/src/utils.rs index 75a0a239a..e8862a33d 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -30,6 +30,7 @@ use std::{ }; use thiserror::Error; +pub mod allocator; mod block_ext; pub mod mem_tracing; mod program_registry_ext; diff --git a/src/utils/allocator.rs b/src/utils/allocator.rs new file mode 100644 index 000000000..915fb0d50 --- /dev/null +++ b/src/utils/allocator.rs @@ -0,0 +1,104 @@ +use std::{ + alloc::Layout, + cell::UnsafeCell, + collections::{hash_map::Entry, HashMap}, + ptr, +}; + +thread_local! { + static ALLOCATOR: UnsafeCell = UnsafeCell::new(ManagedAllocator::default()); +} + +pub fn register_runtime_symbols(find_symbol: impl Fn(&str) -> Option<*mut ()>) { + if let Some(symbol) = find_symbol("cairo_native__alloc") { + unsafe { + *symbol.cast::<*const ()>() = + impl_alloc as *const extern "C" fn(u64, u64) -> *mut () as *const () + } + } + + if let Some(symbol) = find_symbol("cairo_native__realloc") { + unsafe { + *symbol.cast::<*const ()>() = + impl_realloc as *const extern "C" fn(*mut (), u64) -> *mut () as *const () + } + } + + if let Some(symbol) = find_symbol("cairo_native__free") { + unsafe { + *symbol.cast::<*const ()>() = impl_free as *const extern "C" fn(*mut ()) as *const () + } + } +} + +pub fn run_with_allocator(f: impl FnOnce() -> T) -> T { + let prev_allocator = + ALLOCATOR.with(|x| unsafe { ptr::replace(x.get(), ManagedAllocator::default()) }); + + let result = f(); + + ALLOCATOR.with(|x| unsafe { ptr::write(x.get(), prev_allocator) }); + result +} + +#[derive(Debug, Default)] +struct ManagedAllocator { + allocs: HashMap<*mut u8, Layout>, +} + +impl ManagedAllocator { + pub fn alloc(&mut self, layout: Layout) -> *mut u8 { + let ptr = unsafe { std::alloc::alloc(layout) }; + self.allocs.insert(ptr, layout); + + ptr + } + + pub fn realloc(&mut self, ptr: *mut u8, new_size: usize) -> *mut u8 { + assert!(!ptr.is_null()); + match self.allocs.entry(ptr) { + Entry::Occupied(mut entry) => { + let new_ptr = unsafe { std::alloc::realloc(ptr, *entry.get(), new_size) }; + let new_layout = { + let layout = *entry.get(); + Layout::from_size_align(layout.size(), layout.align()).unwrap() + }; + + if ptr == new_ptr { + entry.insert(new_layout); + } else { + entry.remove(); + self.allocs.insert(new_ptr, new_layout); + } + + new_ptr + } + Entry::Vacant(_) => panic!(), + } + } + + pub fn dealloc(&mut self, ptr: *mut u8) { + let layout = self.allocs.remove(&ptr).unwrap(); + unsafe { std::alloc::dealloc(ptr, layout) } + } +} + +impl Drop for ManagedAllocator { + fn drop(&mut self) { + for (ptr, layout) in self.allocs.drain() { + unsafe { std::alloc::dealloc(ptr, layout) } + } + } +} + +extern "C" fn impl_alloc(size: u64, align: u64) -> *mut () { + // let layout = Layout::from_size_align(size, align).unwrap(); + + todo!() +} + +extern "C" fn impl_realloc(ptr: *mut (), new_size: u64) -> *mut () { + todo!() +} + +extern "C" fn impl_free(ptr: *mut ()) {} From fe68f199dbe08031710aaf3ee4024b06c92520e6 Mon Sep 17 00:00:00 2001 From: Esteve Soler Arderiu Date: Thu, 16 Jan 2025 01:19:00 +0100 Subject: [PATCH 4/8] Make the runners use the custom allocator. --- src/executor.rs | 16 +++++++++------- src/executor/contract.rs | 16 +++++++++------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/executor.rs b/src/executor.rs index 9b5822681..c51fb8195 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -223,13 +223,15 @@ fn invoke_dynamic( #[cfg(target_arch = "aarch64")] let mut ret_registers = [0; 4]; - crate::utils::safe_runner::run_safely(|| unsafe { - invoke_trampoline( - function_ptr, - invoke_data.as_ptr().cast(), - invoke_data.len() >> 3, - ret_registers.as_mut_ptr(), - ); + crate::utils::allocator::run_with_allocator(|| { + crate::utils::safe_runner::run_safely(|| unsafe { + invoke_trampoline( + function_ptr, + invoke_data.as_ptr().cast(), + invoke_data.len() >> 3, + ret_registers.as_mut_ptr(), + ); + }) }) .map_err(Error::SafeRunner)?; diff --git a/src/executor/contract.rs b/src/executor/contract.rs index c326a91e4..8c7ffe47d 100644 --- a/src/executor/contract.rs +++ b/src/executor/contract.rs @@ -436,13 +436,15 @@ impl AotContractExecutor { #[cfg(target_arch = "aarch64")] let mut ret_registers = [0; 4]; - crate::utils::safe_runner::run_safely(|| unsafe { - invoke_trampoline( - function_ptr, - invoke_data.as_ptr().cast(), - invoke_data.len() >> 3, - ret_registers.as_mut_ptr(), - ); + crate::utils::allocator::run_with_allocator(|| { + crate::utils::safe_runner::run_safely(|| unsafe { + invoke_trampoline( + function_ptr, + invoke_data.as_ptr().cast(), + invoke_data.len() >> 3, + ret_registers.as_mut_ptr(), + ); + }) }) .map_err(Error::SafeRunner)?; From 2c4dab78927a15444ec3ff2a44ff3a5370489459 Mon Sep 17 00:00:00 2001 From: Esteve Soler Arderiu Date: Thu, 16 Jan 2025 01:21:49 +0100 Subject: [PATCH 5/8] Add missing stuff. --- src/utils/allocator.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/utils/allocator.rs b/src/utils/allocator.rs index 915fb0d50..4b1b23f6d 100644 --- a/src/utils/allocator.rs +++ b/src/utils/allocator.rs @@ -9,6 +9,11 @@ thread_local! { static ALLOCATOR: UnsafeCell = UnsafeCell::new(ManagedAllocator::default()); } +// TODO: Replace `crate::utils::libc_free`, `crate::utils::libc_malloc`, +// `crate::utils::libc_realloc` with our implementation. +// TODO: Merge runtime crate into library (after #1051). +// TODO: Register runtime symbols (after #1051). + pub fn register_runtime_symbols(find_symbol: impl Fn(&str) -> Option<*mut ()>) { if let Some(symbol) = find_symbol("cairo_native__alloc") { unsafe { From a0c3d41d3e317a33517c2fb3faaff21bd73a9675 Mon Sep 17 00:00:00 2001 From: Esteve Soler Arderiu Date: Thu, 16 Jan 2025 12:17:24 +0100 Subject: [PATCH 6/8] Fix MacOS build. --- src/utils/safe_runner.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils/safe_runner.rs b/src/utils/safe_runner.rs index a34e22284..a94f9717a 100644 --- a/src/utils/safe_runner.rs +++ b/src/utils/safe_runner.rs @@ -52,6 +52,7 @@ pub fn setup_safe_runner() { as usize, sa_mask: MaybeUninit::::zeroed().assume_init(), sa_flags: SA_ONSTACK | SA_SIGINFO, + #[cfg(target_os = "linux")] sa_restorer: None, }, null_mut(), From d005b80a77bc31777f0d2ac8f8c21e153b56124f Mon Sep 17 00:00:00 2001 From: Esteve Soler Arderiu Date: Tue, 4 Feb 2025 18:01:32 +0100 Subject: [PATCH 7/8] Hide everything behind a feature. Fix various bugs triggered on early returns. --- Cargo.toml | 1 + src/error.rs | 4 +- src/executor.rs | 88 +++++++++++++++++++++++++--------------- src/executor/contract.rs | 29 +++++++------ src/runtime.rs | 40 +++++++++--------- src/utils.rs | 44 ++++++++++++-------- 6 files changed, 119 insertions(+), 87 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index deaf6cfa3..dc09c3aa1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ scarb = ["build-cli", "dep:scarb-ui", "dep:scarb-metadata"] with-cheatcode = [] with-debug-utils = [] with-mem-tracing = [] +with-segfault-catcher = [] # the aquamarine dep is only used in docs and cannot be detected as used by cargo udeps [package.metadata.cargo-udeps.ignore] diff --git a/src/error.rs b/src/error.rs index dd6108d24..02b64b19c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,6 +1,5 @@ //! Various error types used thorough the crate. use crate::metadata::gas::GasMetadataError; -use crate::utils::safe_runner::SafeRunnerError; use cairo_lang_sierra::extensions::modules::utils::Range; use cairo_lang_sierra::{ edit_state::EditStateError, ids::ConcreteTypeId, program_registry::ProgramRegistryError, @@ -92,8 +91,9 @@ pub enum Error { #[error("Failed to parse a Cairo/Sierra program: {0}")] ProgramParser(String), + #[cfg(feature = "with-segfault-catcher")] #[error(transparent)] - SafeRunner(SafeRunnerError), + SafeRunner(crate::utils::safe_runner::SafeRunnerError), } impl Error { diff --git a/src/executor.rs b/src/executor.rs index 975f6ef59..d9fa02406 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -9,6 +9,7 @@ use crate::{ error::{panic::ToNativeAssertError, Error}, execution_result::{BuiltinStats, ExecutionResult}, native_panic, + runtime::BUILTIN_COSTS, starknet::{handler::StarknetSyscallHandlerCallbacks, StarknetSyscallHandler}, types::TypeBuilder, utils::{libc_free, BuiltinCosts, RangeExt}, @@ -140,23 +141,13 @@ fn invoke_dynamic( .map(|syscall_handler| StarknetSyscallHandlerCallbacks::new(syscall_handler)); // We only care for the previous syscall handler if we actually modify it #[cfg(feature = "with-cheatcode")] - let previous_syscall_handler = syscall_handler.as_mut().map(|syscall_handler| { - let previous_syscall_handler = crate::starknet::SYSCALL_HANDLER_VTABLE.get(); - let syscall_handler_ptr = std::ptr::addr_of!(*syscall_handler) as *mut (); - crate::starknet::SYSCALL_HANDLER_VTABLE.set(syscall_handler_ptr); - - previous_syscall_handler - }); + let syscall_handler_guard = syscall_handler + .as_mut() + .map(|syscall_handler| SyscallHandlerGuard::install(syscall_handler as *mut _)); - // Order matters, for the libfunc impl - let builtin_costs_stack: [u64; 7] = BuiltinCosts::default().into(); - // Note: the ptr into a slice is valid, it can be used with cast() - // Care should be taken if you dereference it and take the .as_ptr() of the slice, since when you - // deref it, it will be a copy on the stack, so you will get the ptr of the value in the stack. - let builtin_costs: *mut [u64; 7] = Box::into_raw(Box::new(builtin_costs_stack)); // We may be inside a recursive contract, save the possible saved builtin costs to restore it after our call. - let old_builtincosts_ptr = - crate::runtime::cairo_native__set_costs_builtin(builtin_costs.cast()); + let builtin_costs = BuiltinCosts::default(); + let builtin_costs_guard = BuiltinCostsGuard::install(builtin_costs); // Generate argument list. let mut iter = args.iter(); @@ -223,22 +214,24 @@ fn invoke_dynamic( #[cfg(target_arch = "aarch64")] let mut ret_registers = [0; 4]; - crate::utils::safe_runner::run_safely(|| unsafe { + #[allow(unused_mut)] + let mut run_trampoline = || unsafe { invoke_trampoline( function_ptr, invoke_data.as_ptr().cast(), invoke_data.len() >> 3, ret_registers.as_mut_ptr(), ); - }) - .map_err(Error::SafeRunner)?; + }; + #[cfg(feature = "with-segfault-catcher")] + crate::utils::safe_runner::run_safely(run_trampoline).map_err(Error::SafeRunner)?; + #[cfg(not(feature = "with-segfault-catcher"))] + run_trampoline(); - // If the syscall handler was changed, then reset the previous one. - // It's only necessary to restore the pointer if it's been modified i.e. if previous_syscall_handler is Some(...) + // Restore the previous syscall handler and builtin costs. #[cfg(feature = "with-cheatcode")] - if let Some(previous_syscall_handler) = previous_syscall_handler { - crate::starknet::SYSCALL_HANDLER_VTABLE.set(previous_syscall_handler); - } + drop(syscall_handler_guard); + drop(builtin_costs_guard); // Parse final gas. unsafe fn read_value(ptr: &mut NonNull<()>) -> &T { @@ -338,16 +331,6 @@ fn invoke_dynamic( debug_name: None, }); - // Restore the old ptr and get back our builtincost box and free it. - let our_builtincosts_ptr = - crate::runtime::cairo_native__set_costs_builtin(old_builtincosts_ptr); - - if !our_builtincosts_ptr.is_null() && old_builtincosts_ptr.is_aligned() { - unsafe { - let _ = Box::<[u64; 7]>::from_raw(our_builtincosts_ptr.cast_mut().cast()); - }; - } - #[cfg(feature = "with-mem-tracing")] crate::utils::mem_tracing::report_stats(); @@ -358,6 +341,45 @@ fn invoke_dynamic( }) } +#[cfg(feature = "with-cheatcode")] +#[derive(Debug)] +struct SyscallHandlerGuard(*mut ()); + +#[cfg(feature = "with-cheatcode")] +impl SyscallHandlerGuard { + // NOTE: It is the caller's responsibility to ensure that the syscall handler is alive until the + // guard is dropped. + pub fn install(value: *mut T) -> Self { + let previous_value = crate::starknet::SYSCALL_HANDLER_VTABLE.get(); + let syscall_handler_ptr = value as *mut (); + crate::starknet::SYSCALL_HANDLER_VTABLE.set(syscall_handler_ptr); + + Self(previous_value) + } +} + +#[cfg(feature = "with-cheatcode")] +impl Drop for SyscallHandlerGuard { + fn drop(&mut self) { + crate::starknet::SYSCALL_HANDLER_VTABLE.set(self.0); + } +} + +#[derive(Debug)] +struct BuiltinCostsGuard(BuiltinCosts); + +impl BuiltinCostsGuard { + pub fn install(value: BuiltinCosts) -> Self { + Self(BUILTIN_COSTS.replace(value)) + } +} + +impl Drop for BuiltinCostsGuard { + fn drop(&mut self) { + BUILTIN_COSTS.set(self.0); + } +} + /// Parses the result by reading from the return ptr the given type. fn parse_result( type_id: &ConcreteTypeId, diff --git a/src/executor/contract.rs b/src/executor/contract.rs index 1b6853b20..0c6e9e946 100644 --- a/src/executor/contract.rs +++ b/src/executor/contract.rs @@ -36,7 +36,7 @@ use crate::{ context::NativeContext, error::{panic::ToNativeAssertError, Error, Result}, execution_result::{BuiltinStats, ContractExecutionResult}, - executor::invoke_trampoline, + executor::{invoke_trampoline, BuiltinCostsGuard}, metadata::{gas::MetadataComputationConfig, runtime_bindings::setup_runtime}, module::NativeModule, starknet::{handler::StarknetSyscallHandlerCallbacks, StarknetSyscallHandler}, @@ -330,11 +330,11 @@ impl AotContractExecutor { }; let function_ptr = self.find_function_ptr(&function_id, true)?; - let builtin_costs: [u64; 7] = builtin_costs.unwrap_or_default().into(); - + // Initialize syscall handler and builtin costs. // We may be inside a recursive contract, save the possible saved builtin costs to restore it after our call. - let old_builtincosts_ptr = - crate::runtime::cairo_native__set_costs_builtin(builtin_costs.as_ptr()); + let mut syscall_handler = StarknetSyscallHandlerCallbacks::new(&mut syscall_handler); + let builtin_costs = builtin_costs.unwrap_or_default(); + let builtin_costs_guard = BuiltinCostsGuard::install(builtin_costs); // it can vary from contract to contract thats why we need to store/ load it. let builtins_size: usize = self.contract_info.entry_points[&selector] @@ -353,18 +353,13 @@ impl AotContractExecutor { .as_ptr() .to_bytes(&mut invoke_data, |_| unreachable!())?; - let mut syscall_handler = StarknetSyscallHandlerCallbacks::new(&mut syscall_handler); - for b in &self.contract_info.entry_points[&selector].builtins { match b { BuiltinType::Gas => { gas.to_bytes(&mut invoke_data, |_| unreachable!())?; } BuiltinType::BuiltinCosts => { - // todo: check if valid - builtin_costs - .as_ptr() - .to_bytes(&mut invoke_data, |_| unreachable!())?; + builtin_costs.to_bytes(&mut invoke_data, |_| unreachable!())?; } BuiltinType::System => { (&mut syscall_handler as *mut StarknetSyscallHandlerCallbacks<_>) @@ -441,15 +436,19 @@ impl AotContractExecutor { #[cfg(target_arch = "aarch64")] let mut ret_registers = [0; 4]; - crate::utils::safe_runner::run_safely(|| unsafe { + #[allow(unused_mut)] + let mut run_trampoline = || unsafe { invoke_trampoline( function_ptr, invoke_data.as_ptr().cast(), invoke_data.len() >> 3, ret_registers.as_mut_ptr(), ); - }) - .map_err(Error::SafeRunner)?; + }; + #[cfg(feature = "with-segfault-catcher")] + crate::utils::safe_runner::run_safely(run_trampoline).map_err(Error::SafeRunner)?; + #[cfg(not(feature = "with-segfault-catcher"))] + run_trampoline(); // Parse final gas. unsafe fn read_value(ptr: &mut NonNull<()>) -> &T { @@ -577,7 +576,7 @@ impl AotContractExecutor { }; // Restore the original builtin costs pointer. - crate::runtime::cairo_native__set_costs_builtin(old_builtincosts_ptr); + drop(builtin_costs_guard); #[cfg(feature = "with-mem-tracing")] crate::utils::mem_tracing::report_stats(); diff --git a/src/runtime.rs b/src/runtime.rs index 3661953e5..caeba479b 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1,5 +1,6 @@ #![allow(non_snake_case)] +use crate::utils::BuiltinCosts; use cairo_lang_sierra_gas::core_libfunc_cost::{ DICT_SQUASH_REPEATED_ACCESS_COST, DICT_SQUASH_UNIQUE_KEY_COST, }; @@ -22,7 +23,7 @@ use std::{ io::Write, mem::{forget, ManuallyDrop}, os::fd::FromRawFd, - ptr::{self, null, null_mut}, + ptr, rc::Rc, }; use std::{ops::Mul, vec::IntoIter}; @@ -162,7 +163,7 @@ impl Clone for FeltDict { layout: self.layout, elements: if self.mappings.is_empty() { - null_mut() + ptr::null_mut() } else { unsafe { alloc(Layout::from_size_align_unchecked( @@ -255,7 +256,7 @@ pub unsafe extern "C" fn cairo_native__dict_new( mappings: HashMap::default(), layout: Layout::from_size_align_unchecked(size as usize, align as usize), - elements: null_mut(), + elements: ptr::null_mut(), dup_fn, drop_fn, @@ -578,27 +579,24 @@ pub unsafe extern "C" fn cairo_native__libfunc__ec__ec_state_try_finalize_nz( } thread_local! { - // We can use cell because a ptr is copy. - static BUILTIN_COSTS: Cell<*const u64> = const { - Cell::new(null()) + pub(crate) static BUILTIN_COSTS: Cell = const { + // These default values shouldn't be accessible, they will be overriden before entering + // compiled code. + Cell::new(BuiltinCosts { + r#const: 0, + pedersen: 0, + bitwise: 0, + ecop: 0, + poseidon: 0, + add_mod: 0, + mul_mod: 0, + }) }; } -/// Store the gas builtin in the internal thread local. Returns the old pointer, to restore it after execution. -/// Not a runtime metadata method, it should be called before the program is executed. -pub extern "C" fn cairo_native__set_costs_builtin(ptr: *const u64) -> *const u64 { - let old = BUILTIN_COSTS.get(); - BUILTIN_COSTS.set(ptr); - old -} - /// Get the gas builtin from the internal thread local. -pub extern "C" fn cairo_native__get_costs_builtin() -> *const u64 { - if BUILTIN_COSTS.get().is_null() { - // We shouldn't panic here, but we can print a big message. - eprintln!("BUILTIN_COSTS POINTER IS NULL!"); - } - BUILTIN_COSTS.get() +pub extern "C" fn cairo_native__get_costs_builtin() -> *const [u64; 7] { + BUILTIN_COSTS.with(|x| x.as_ptr()) as *const [u64; 7] } // Utility methods for the print runtime function @@ -878,7 +876,7 @@ mod tests { }; let key = Felt::ONE.to_bytes_le(); - let mut ptr = null_mut::(); + let mut ptr = ptr::null_mut::(); assert_eq!( unsafe { cairo_native__dict_get(dict, &key, (&raw mut ptr).cast()) }, diff --git a/src/utils.rs b/src/utils.rs index 7e0e1042f..5d8240cab 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -34,6 +34,7 @@ mod block_ext; pub mod mem_tracing; mod program_registry_ext; mod range_ext; +#[cfg(feature = "with-segfault-catcher")] pub mod safe_runner; #[cfg(target_os = "macos")] @@ -53,7 +54,10 @@ pub static HALF_PRIME: LazyLock = LazyLock::new(|| { .expect("hardcoded half prime constant should be valid") }); +// Order matters, for the libfunc impl +// https://github.com/starkware-libs/sequencer/blob/1b7252f8a30244d39614d7666aa113b81291808e/crates/blockifier/src/execution/entry_point_execution.rs#L208 #[derive(Debug, Clone, Copy, Deserialize, Serialize)] +#[repr(C)] pub struct BuiltinCosts { pub r#const: u64, pub pedersen: u64, @@ -64,22 +68,6 @@ pub struct BuiltinCosts { pub mul_mod: u64, } -impl From for [u64; 7] { - // Order matters, for the libfunc impl - // https://github.com/starkware-libs/sequencer/blob/1b7252f8a30244d39614d7666aa113b81291808e/crates/blockifier/src/execution/entry_point_execution.rs#L208 - fn from(value: BuiltinCosts) -> Self { - [ - value.r#const, - value.pedersen, - value.bitwise, - value.ecop, - value.poseidon, - value.add_mod, - value.mul_mod, - ] - } -} - impl Default for BuiltinCosts { fn default() -> Self { Self { @@ -94,6 +82,30 @@ impl Default for BuiltinCosts { } } +impl crate::arch::AbiArgument for BuiltinCosts { + fn to_bytes( + &self, + buffer: &mut Vec, + find_dict_overrides: impl Copy + + Fn( + &cairo_lang_sierra::ids::ConcreteTypeId, + ) -> ( + Option, + Option, + ), + ) -> crate::error::Result<()> { + self.r#const.to_bytes(buffer, find_dict_overrides)?; + self.pedersen.to_bytes(buffer, find_dict_overrides)?; + self.bitwise.to_bytes(buffer, find_dict_overrides)?; + self.ecop.to_bytes(buffer, find_dict_overrides)?; + self.poseidon.to_bytes(buffer, find_dict_overrides)?; + self.add_mod.to_bytes(buffer, find_dict_overrides)?; + self.mul_mod.to_bytes(buffer, find_dict_overrides)?; + + Ok(()) + } +} + #[cfg(feature = "with-mem-tracing")] #[allow(unused_imports)] pub(crate) use self::mem_tracing::{ From 267146acce4c7405eb1db22ce43deed46d03a89e Mon Sep 17 00:00:00 2001 From: Esteve Soler Arderiu Date: Mon, 10 Feb 2025 15:27:02 +0100 Subject: [PATCH 8/8] Add some documentation. --- src/utils/safe_runner.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/utils/safe_runner.rs b/src/utils/safe_runner.rs index a94f9717a..e8080e350 100644 --- a/src/utils/safe_runner.rs +++ b/src/utils/safe_runner.rs @@ -1,3 +1,21 @@ +//! # Safe runner +//! +//! The safe runner provides a way to run contracts without any risk of crashing due to invalid +//! memory accesses, including stack overflows. The same mechanism can also be used to abort the +//! current Cairo program execution, although it will probably leak some memory. +//! +//! It works by setting a signal handler for the `SIGSEGV` signal, which is generated by the +//! operating system on invalid memory accesses. This signal handler is global, therefore it must be +//! set by the application, rather than the library, by invoking `setup_safe_runner`. +//! +//! Since returning from a `SIGSEGV` signal handler would crash the program, the safe runner will +//! perform a non-local goto using the `setjmp` and `longjmp` functions. The first one configures +//! the jump target while the second performs the goto. +//! +//! Note: The protection is not 100% guaranteed. Since all memory addresses are within the same +//! memory space, some combinations of base addresses plus offsets may end up in a different page +//! allocation. + use libc::{ c_int, sigaction, sigaltstack, siginfo_t, sigset_t, stack_t, ucontext_t, SA_ONSTACK, SA_SIGINFO, SIGSEGV, SIGSTKSZ, @@ -77,12 +95,15 @@ pub fn setup_safe_runner() { pub fn abort_safe_runner() -> ! { unsafe { match STATE.with(|x| &mut *x.get()) { - SafeRunnerState::Inactive => libc::abort(), + SafeRunnerState::Inactive => { + panic!("manual abort triggered from outside a safe runner"); + } SafeRunnerState::Active(jmp_buf) => longjmp(jmp_buf.as_mut_ptr().cast(), 2), } } } +/// Run a closure within a safe runner. pub fn run_safely(f: impl FnOnce() -> T) -> Result { let (jmp_buf, prev_state) = STATE.with(|x| unsafe { let jmp_buf;