Add segfault catcher. #1053

azteca1998 · 2025-01-15T23:37:25Z

Simplified version of #959. Does not attempt to grow the stack, just catches segfaults and returns an error.

Checklist

Linked to Github Issue
Unit tests added
Integration tests added.
This change requires new documentation.
- Documentation has been added/updated.

github-actions · 2025-01-16T00:22:09Z

Benchmark results Main vs HEAD.

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_insert.cairo (JIT)`	3.836 ± 0.024	3.811	3.893	1.02 ± 0.01
`base dict_insert.cairo (AOT)`	3.769 ± 0.026	3.726	3.798	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head dict_insert.cairo (JIT)`	3.892 ± 0.052	3.835	3.993	1.02 ± 0.01
`head dict_insert.cairo (AOT)`	3.798 ± 0.020	3.768	3.839	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_snapshot.cairo (JIT)`	3.734 ± 0.022	3.694	3.768	1.02 ± 0.01
`base dict_snapshot.cairo (AOT)`	3.649 ± 0.024	3.616	3.692	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head dict_snapshot.cairo (JIT)`	3.767 ± 0.022	3.734	3.795	1.03 ± 0.01
`head dict_snapshot.cairo (AOT)`	3.668 ± 0.030	3.610	3.724	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base factorial_2M.cairo (JIT)`	4.075 ± 0.028	4.024	4.125	1.01 ± 0.01
`base factorial_2M.cairo (AOT)`	4.049 ± 0.020	4.011	4.079	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head factorial_2M.cairo (JIT)`	4.114 ± 0.023	4.089	4.167	1.01 ± 0.01
`head factorial_2M.cairo (AOT)`	4.066 ± 0.020	4.030	4.102	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base fib_2M.cairo (JIT)`	3.626 ± 0.023	3.590	3.659	1.01 ± 0.01
`base fib_2M.cairo (AOT)`	3.584 ± 0.022	3.544	3.613	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head fib_2M.cairo (JIT)`	3.656 ± 0.015	3.633	3.679	1.01 ± 0.01
`head fib_2M.cairo (AOT)`	3.633 ± 0.016	3.613	3.659	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search.cairo (JIT)`	3.812 ± 0.023	3.773	3.848	1.04 ± 0.01
`base linear_search.cairo (AOT)`	3.667 ± 0.016	3.644	3.686	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head linear_search.cairo (JIT)`	3.846 ± 0.017	3.826	3.871	1.03 ± 0.01
`head linear_search.cairo (AOT)`	3.734 ± 0.017	3.694	3.763	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base logistic_map.cairo (JIT)`	3.898 ± 0.022	3.865	3.939	1.05 ± 0.01
`base logistic_map.cairo (AOT)`	3.728 ± 0.015	3.706	3.757	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head logistic_map.cairo (JIT)`	3.960 ± 0.031	3.909	4.011	1.04 ± 0.01
`head logistic_map.cairo (AOT)`	3.791 ± 0.014	3.769	3.810	1.00

github-actions · 2025-01-16T00:30:13Z

Benchmarking results

Benchmark for program `dict_insert`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	20.517 ± 0.139	20.415	20.874	5.49 ± 0.05
`cairo-native (embedded AOT)`	3.739 ± 0.018	3.700	3.766	1.00
`cairo-native (embedded JIT using LLVM's ORC Engine)`	3.798 ± 0.019	3.770	3.821	1.02 ± 0.01

Benchmark for program `dict_snapshot`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	5.750 ± 0.043	5.672	5.794	1.60 ± 0.02
`cairo-native (embedded AOT)`	3.594 ± 0.025	3.563	3.650	1.00
`cairo-native (embedded JIT using LLVM's ORC Engine)`	3.677 ± 0.012	3.657	3.695	1.02 ± 0.01

Benchmark for program `factorial_2M`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	14.203 ± 0.104	14.059	14.454	3.55 ± 0.03
`cairo-native (embedded AOT)`	4.000 ± 0.024	3.970	4.044	1.00
`cairo-native (embedded JIT using LLVM's ORC Engine)`	4.040 ± 0.036	4.015	4.138	1.01 ± 0.01

Benchmark for program `fib_2M`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	14.094 ± 0.034	14.040	14.143	3.97 ± 0.03
`cairo-native (embedded AOT)`	3.551 ± 0.027	3.508	3.589	1.00
`cairo-native (embedded JIT using LLVM's ORC Engine)`	3.578 ± 0.021	3.544	3.610	1.01 ± 0.01

Benchmark for program `linear_search`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	5.733 ± 0.026	5.679	5.781	1.57 ± 0.01
`cairo-native (embedded AOT)`	3.641 ± 0.013	3.624	3.660	1.00
`cairo-native (embedded JIT using LLVM's ORC Engine)`	3.772 ± 0.031	3.736	3.844	1.04 ± 0.01

Benchmark for program `logistic_map`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	5.678 ± 0.041	5.621	5.759	1.54 ± 0.02
`cairo-native (embedded AOT)`	3.684 ± 0.024	3.659	3.748	1.00
`cairo-native (embedded JIT using LLVM's ORC Engine)`	3.884 ± 0.038	3.850	3.967	1.05 ± 0.01

codecov-commenter · 2025-01-16T00:36:31Z

Codecov Report

Attention: Patch coverage is 57.69231% with 22 lines in your changes missing coverage. Please review.

Project coverage is 80.87%. Comparing base (066cda5) to head (d005b80).

Files with missing lines	Patch %	Lines
src/utils.rs	0.00%	20 Missing ⚠️
src/executor/contract.rs	88.88%	1 Missing ⚠️
src/runtime.rs	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1053      +/-   ##
==========================================
- Coverage   80.93%   80.87%   -0.06%     
==========================================
  Files         108      108              
  Lines       29605    29599       -6     
==========================================
- Hits        23961    23939      -22     
- Misses       5644     5660      +16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

edg-l

Is it possible to add a test that segfaults? Or too tricky?

azteca1998 · 2025-01-20T17:25:58Z

It is technically possible, but it'd require a way to initialize the signal handler globally when testing, potentially messing up the test harness's signal handler (ex. when using nextest).

Instead, if we just initialized every (segfaulting) test then we'd end up initializing it multiple times, which could also cause problems.

JulianGCalderon · 2025-02-03T20:54:18Z

src/utils/safe_runner.rs

Could you add some comments explaining how it works? For someone not familiar with the signal or de jmp API (like me), its a bit tricky to understand.

Is this implementation threadsafe?

I hadn't really considered thread safety. I think it should be safe because the jmp_buf is a thread local, so it shouldn't be possible to transfer it across thread boundaries.

src/utils/safe_runner.rs

JulianGCalderon · 2025-02-03T21:02:20Z

src/executor/contract.rs

-        unsafe {
+        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)?;


Should we gate this under a feature flag? Or maybe provide an API instead of using directly?

JulianGCalderon · 2025-02-03T21:10:49Z

src/utils/safe_runner.rs

+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(), 1),


The 1 value is then returned as jmp_ret in the setjmp call, right? If so, could you add a small comment mentioning it?

Yes, it does exactly that. Even if you pass a 0 to longjmp it'll return a 1. In other words, the only time it'll return 0 is when setjmp is initializing stuff, but never when returning from a longjmp.

src/utils/safe_runner.rs

… returns.

azteca1998 added 2 commits January 16, 2025 00:35

Add SafeRunner for catching segfaults.

a658ead

Add API to manually abort.

428965d

azteca1998 marked this pull request as ready for review January 15, 2025 23:52

azteca1998 requested review from edg-l, igaray, jrchatruc, entropidelic, fmoletta, Oppen, pefontana and gabrielbosio as code owners January 15, 2025 23:52

Fix MacOS build.

a0c3d41

edg-l added the review-ready A PR that is ready for review label Jan 20, 2025

edg-l previously approved these changes Jan 20, 2025

View reviewed changes

JulianGCalderon reviewed Feb 3, 2025

View reviewed changes

azteca1998 dismissed edg-l’s stale review via 2bbdaff February 4, 2025 17:06

azteca1998 added 2 commits February 4, 2025 18:23

Merge branch 'main' into add-segfault-catcher

d21fd0b

Hide everything behind a feature. Fix various bugs triggered on early…

d005b80

… returns.

azteca1998 force-pushed the add-segfault-catcher branch from 2bbdaff to d005b80 Compare February 4, 2025 17:27

edg-l approved these changes Feb 6, 2025

View reviewed changes

This was referenced Feb 6, 2025

Add SafeRunner to catch SIGSEGV and grow the stack automagically. #959

Closed

Add managed allocator. #1054

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add segfault catcher. #1053

Add segfault catcher. #1053

azteca1998 commented Jan 15, 2025

github-actions bot commented Jan 16, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025 •

edited

Loading

codecov-commenter commented Jan 16, 2025 •

edited

Loading

edg-l left a comment •

edited

Loading

azteca1998 commented Jan 20, 2025 •

edited

Loading

JulianGCalderon Feb 3, 2025 •

edited

Loading

azteca1998 Feb 4, 2025

JulianGCalderon Feb 3, 2025

JulianGCalderon Feb 3, 2025

azteca1998 Feb 4, 2025

Add segfault catcher. #1053

Are you sure you want to change the base?

Add segfault catcher. #1053

Conversation

azteca1998 commented Jan 15, 2025

Checklist

github-actions bot commented Jan 16, 2025 • edited Loading

Benchmark results Main vs HEAD.

Base

Head

Base

Head

Base

Head

Base

Head

Base

Head

Base

Head

github-actions bot commented Jan 16, 2025 • edited Loading

Benchmarking results

Benchmark for program dict_insert

Benchmark for program dict_snapshot

Benchmark for program factorial_2M

Benchmark for program fib_2M

Benchmark for program linear_search

Benchmark for program logistic_map

codecov-commenter commented Jan 16, 2025 • edited Loading

Codecov Report

edg-l left a comment • edited Loading

Choose a reason for hiding this comment

azteca1998 commented Jan 20, 2025 • edited Loading

JulianGCalderon Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

azteca1998 Feb 4, 2025

Choose a reason for hiding this comment

JulianGCalderon Feb 3, 2025

Choose a reason for hiding this comment

JulianGCalderon Feb 3, 2025

Choose a reason for hiding this comment

azteca1998 Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 16, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025 •

edited

Loading

Benchmark for program `dict_insert`

Benchmark for program `dict_snapshot`

Benchmark for program `factorial_2M`

Benchmark for program `fib_2M`

Benchmark for program `linear_search`

Benchmark for program `logistic_map`

codecov-commenter commented Jan 16, 2025 •

edited

Loading

edg-l left a comment •

edited

Loading

azteca1998 commented Jan 20, 2025 •

edited

Loading

JulianGCalderon Feb 3, 2025 •

edited

Loading