From 193cfabc667a0ec6c0914d5f6fc3a105960cc6d3 Mon Sep 17 00:00:00 2001 From: Skgland Date: Sun, 7 Dec 2025 21:08:17 +0100 Subject: [PATCH 1/9] detect recursive throw_resource_error and panic instead of causing a segmentation fault --- src/machine/machine_errors.rs | 44 ++++++++++++++++++++++++++++--- src/machine/machine_state.rs | 1 + src/machine/machine_state_impl.rs | 1 + 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/machine/machine_errors.rs b/src/machine/machine_errors.rs index fa6822d0..ead28cd9 100644 --- a/src/machine/machine_errors.rs +++ b/src/machine/machine_errors.rs @@ -778,9 +778,47 @@ impl MachineState { // throw an error pre-allocated in the heap pub(super) fn throw_resource_error(&mut self, err: AllocError) { - self.registers[1] = str_loc_as_cell!(err.resource_error_offset(&mut self.heap)); - self.set_ball(); - self.unwind_stack(); + struct RecursionGuard<'state> { + machine_state: &'state mut MachineState, + } + + impl<'state> RecursionGuard<'state> { + fn new(machine_state: &'state mut MachineState) -> Self { + if machine_state.throwing_resource_error { + panic!("attempted to throw `error(resource_error(memory), [])` while attempting to throw `error(resource_error(memory), [])`"); + } + machine_state.throwing_resource_error = true; + Self { machine_state } + } + } + + impl Drop for RecursionGuard<'_> { + fn drop(&mut self) { + self.machine_state.throwing_resource_error = false; + } + } + + impl std::ops::Deref for RecursionGuard<'_> { + type Target = MachineState; + + fn deref(&self) -> &MachineState { + self.machine_state + } + } + + impl std::ops::DerefMut for RecursionGuard<'_> { + fn deref_mut(&mut self) -> &mut MachineState { + self.machine_state + } + } + + let mut guard = RecursionGuard::new(self); + + let state = &mut *guard; + + state.registers[1] = str_loc_as_cell!(err.resource_error_offset(&mut state.heap)); + state.set_ball(); + state.unwind_stack(); } pub(super) fn throw_exception(&mut self, err: MachineStub) { diff --git a/src/machine/machine_state.rs b/src/machine/machine_state.rs index 452debb2..732678da 100644 --- a/src/machine/machine_state.rs +++ b/src/machine/machine_state.rs @@ -74,6 +74,7 @@ pub struct MachineState { pub(super) cp: usize, pub(super) attr_var_init: AttrVarInitializer, pub(super) fail: bool, + pub throwing_resource_error: bool, pub heap: Heap, pub(super) mode: MachineMode, pub(crate) stack: Stack, diff --git a/src/machine/machine_state_impl.rs b/src/machine/machine_state_impl.rs index ee8a130b..7b777ae6 100644 --- a/src/machine/machine_state_impl.rs +++ b/src/machine/machine_state_impl.rs @@ -67,6 +67,7 @@ impl MachineState { unify_fn: MachineState::unify, bind_fn: MachineState::bind, run_cleaners_fn: |_| false, + throwing_resource_error: false, } } From 2fc08dde1e42b1c2f17804b16a5b7d1cb43439b6 Mon Sep 17 00:00:00 2001 From: Skgland Date: Sun, 7 Dec 2025 21:27:24 +0100 Subject: [PATCH 2/9] prevent multiply with overflow resulting in odd errors/segv --- src/macros.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/macros.rs b/src/macros.rs index f977b882..2eda4980 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -472,7 +472,9 @@ macro_rules! resource_error_call_result { macro_rules! heap_index { ($idx:expr) => { - ($idx) * std::mem::size_of::() + std::mem::size_of::() + .checked_mul($idx) + .unwrap() }; } From d63b0a192c25f9dd33902b8577fc813fea77efec Mon Sep 17 00:00:00 2001 From: Skgland Date: Sun, 7 Dec 2025 21:31:24 +0100 Subject: [PATCH 3/9] use checked multiplication in heap_index! macro --- src/macros.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 2eda4980..4ac590eb 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -471,11 +471,16 @@ macro_rules! resource_error_call_result { } macro_rules! heap_index { - ($idx:expr) => { + ($idx:expr) => {{ + let idx = $idx; std::mem::size_of::() - .checked_mul($idx) - .unwrap() - }; + .checked_mul(idx) + .expect(&format!( + "overflow while calculating heap index {idx} * {} > {}", + std::mem::size_of::(), + usize::MAX, + )) + }}; } macro_rules! cell_index { From 11901b5fdefb6b7ef86eca0fba00d79bae8cbad4 Mon Sep 17 00:00:00 2001 From: Skgland Date: Sun, 7 Dec 2025 21:45:13 +0100 Subject: [PATCH 4/9] handle oob heap index calculation --- src/machine/heap.rs | 2 +- src/macros.rs | 20 +++++++++++++------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/machine/heap.rs b/src/machine/heap.rs index d9f1aa88..3d614ebb 100644 --- a/src/machine/heap.rs +++ b/src/machine/heap.rs @@ -623,7 +623,7 @@ impl Heap { pub fn reserve(&mut self, num_cells: usize) -> Result, AllocError> { let section; - let len = heap_index!(num_cells); + let len = heap_index_checked!(num_cells).ok_or(AllocError)?; loop { unsafe { diff --git a/src/macros.rs b/src/macros.rs index 4ac590eb..a5e16b8b 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -470,16 +470,22 @@ macro_rules! resource_error_call_result { }; } +macro_rules! heap_index_checked { + ($idx:expr) => { + std::mem::size_of::().checked_mul($idx) + }; +} + +pub(crate) use heap_index_checked; + macro_rules! heap_index { ($idx:expr) => {{ let idx = $idx; - std::mem::size_of::() - .checked_mul(idx) - .expect(&format!( - "overflow while calculating heap index {idx} * {} > {}", - std::mem::size_of::(), - usize::MAX, - )) + $crate::macros::heap_index_checked!(idx).expect(&format!( + "overflow while calculating heap index {idx} * {} > {}", + std::mem::size_of::(), + usize::MAX, + )) }}; } From 47e908bf76851c0afd018b30ee8d2ed53fe53316 Mon Sep 17 00:00:00 2001 From: Skgland Date: Sun, 7 Dec 2025 22:59:00 +0100 Subject: [PATCH 5/9] handle overflown in Heap::with_cell_capacity --- src/machine/heap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/machine/heap.rs b/src/machine/heap.rs index 3d614ebb..c93d005b 100644 --- a/src/machine/heap.rs +++ b/src/machine/heap.rs @@ -599,7 +599,7 @@ impl Heap { pub(crate) fn with_cell_capacity(cap: usize) -> Result { let ptr = unsafe { let layout = alloc::Layout::from_size_align( - cap * size_of::(), + heap_index_checked!(cap).ok_or(AllocError)?, size_of::(), ) .unwrap(); From 580572aec6227c4371fad2936f7a7c3c1f32d91e Mon Sep 17 00:00:00 2001 From: Skgland Date: Sun, 7 Dec 2025 23:06:09 +0100 Subject: [PATCH 6/9] don't calculate panic message eagerly --- src/macros.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index a5e16b8b..7e7853dc 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -481,11 +481,13 @@ pub(crate) use heap_index_checked; macro_rules! heap_index { ($idx:expr) => {{ let idx = $idx; - $crate::macros::heap_index_checked!(idx).expect(&format!( - "overflow while calculating heap index {idx} * {} > {}", - std::mem::size_of::(), - usize::MAX, - )) + $crate::macros::heap_index_checked!(idx).unwrap_or_else(|| { + panic!( + "overflow while calculating heap index {idx} * {} > {}", + std::mem::size_of::(), + usize::MAX, + ) + }) }}; } From 616f071cd93b2e6061878a7a581787a861cb2429 Mon Sep 17 00:00:00 2001 From: Skgland Date: Mon, 8 Dec 2025 19:59:43 +0100 Subject: [PATCH 7/9] fix another integer overflow --- src/machine/heap.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/machine/heap.rs b/src/machine/heap.rs index c93d005b..d8c52ac8 100644 --- a/src/machine/heap.rs +++ b/src/machine/heap.rs @@ -1148,7 +1148,9 @@ pub fn sized_iter_to_heap_list>( ) -> Result { if size > 0 { let h = heap.cell_len(); - let mut writer = heap.reserve(1 + 2 * size)?; + // not using checked_add for 1 + as the result of multiplying by 2 will be even and the largest representable usize is odd, + // so the addition cannot overflow + let mut writer = heap.reserve(1 + size.checked_mul(2).ok_or(AllocError)?)?; writer.write_with(|section| { for (idx, value) in values.enumerate() { From 00aba961857b42c695eb4118a3a3586dbe9ca084 Mon Sep 17 00:00:00 2001 From: Skgland Date: Mon, 8 Dec 2025 22:27:24 +0100 Subject: [PATCH 8/9] remove overengineered RecursionGuard --- src/machine/machine_errors.rs | 45 ++++++----------------------------- 1 file changed, 7 insertions(+), 38 deletions(-) diff --git a/src/machine/machine_errors.rs b/src/machine/machine_errors.rs index ead28cd9..420ab6fe 100644 --- a/src/machine/machine_errors.rs +++ b/src/machine/machine_errors.rs @@ -778,47 +778,16 @@ impl MachineState { // throw an error pre-allocated in the heap pub(super) fn throw_resource_error(&mut self, err: AllocError) { - struct RecursionGuard<'state> { - machine_state: &'state mut MachineState, + if self.throwing_resource_error { + panic!("attempted to throw `error(resource_error(memory), [])` while attempting to throw `error(resource_error(memory), [])`"); } + self.throwing_resource_error = true; - impl<'state> RecursionGuard<'state> { - fn new(machine_state: &'state mut MachineState) -> Self { - if machine_state.throwing_resource_error { - panic!("attempted to throw `error(resource_error(memory), [])` while attempting to throw `error(resource_error(memory), [])`"); - } - machine_state.throwing_resource_error = true; - Self { machine_state } - } - } + self.registers[1] = str_loc_as_cell!(err.resource_error_offset(&mut self.heap)); + self.set_ball(); + self.unwind_stack(); - impl Drop for RecursionGuard<'_> { - fn drop(&mut self) { - self.machine_state.throwing_resource_error = false; - } - } - - impl std::ops::Deref for RecursionGuard<'_> { - type Target = MachineState; - - fn deref(&self) -> &MachineState { - self.machine_state - } - } - - impl std::ops::DerefMut for RecursionGuard<'_> { - fn deref_mut(&mut self) -> &mut MachineState { - self.machine_state - } - } - - let mut guard = RecursionGuard::new(self); - - let state = &mut *guard; - - state.registers[1] = str_loc_as_cell!(err.resource_error_offset(&mut state.heap)); - state.set_ball(); - state.unwind_stack(); + self.throwing_resource_error = false; } pub(super) fn throw_exception(&mut self, err: MachineStub) { From 129cca53ed03f979be8d79e07a7bcfda4cbbe0d9 Mon Sep 17 00:00:00 2001 From: Skgland Date: Wed, 10 Dec 2025 00:04:24 +0100 Subject: [PATCH 9/9] throw `error(resource_error(memory), [])` when the list length exceeds usize. --- src/machine/machine_errors.rs | 7 ------- src/machine/system_calls.rs | 5 ++--- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/machine/machine_errors.rs b/src/machine/machine_errors.rs index 420ab6fe..4ed19893 100644 --- a/src/machine/machine_errors.rs +++ b/src/machine/machine_errors.rs @@ -76,7 +76,6 @@ impl ValidType { #[derive(Debug, Clone, Copy)] pub(crate) enum ResourceError { - FiniteMemory(HeapCellValue), OutOfFiles, } @@ -330,12 +329,6 @@ impl MachineState { pub(super) fn resource_error(err: ResourceError) -> MachineError { let stub = match err { - ResourceError::FiniteMemory(size_requested) => { - functor!( - atom!("resource_error"), - [atom_as_cell((atom!("finite_memory"))), cell(size_requested)] - ) - } ResourceError::OutOfFiles => { functor!( atom!("resource_error"), diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index b7a63aad..ac13c2e5 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -4371,7 +4371,6 @@ impl Machine { #[inline(always)] pub(crate) fn det_length_rundown(&mut self) -> CallResult { - let stub_gen = || functor_stub(atom!("length"), 2); let len = self.deref_register(2); let n = match Number::try_from((len, &self.machine_st.arena.f64_tbl)) { @@ -4379,8 +4378,8 @@ impl Machine { Ok(Number::Integer(n)) => match (&*n).try_into() as Result { Ok(n) => n, Err(_) => { - let err = MachineState::resource_error(ResourceError::FiniteMemory(len)); - return Err(self.machine_st.error_form(err, stub_gen())); + self.machine_st.throw_resource_error(AllocError); + return Ok(()); } }, _ => {