address review: ArcProgram alias, profiling deadlock/FFI/instrumentation fixes

Review-driven fixes to the ContractExecutor libfunc-profiling path:

- ArcProgram alias (orizi review on src/executor.rs): replace the bare
  `pub use ...::Program` re-export with `pub type ArcProgram = Arc<Program>`,
  since profiling/sierra-emu consumers only ever hold the program as a shared
  handle. Gated on `any(sierra-emu, with-libfunc-profiling)` so EmuContractInfo
  consumers can name the field type too, and adopted for EmuContractInfo.program,
  AotWithProgram.program, and the run_with_profile callback.

- Self-deadlock: run_with_libfunc_profile held the non-reentrant process-wide
  PROFILE_LOCK across self.run, which re-enters this function on the same thread
  whenever a profiled contract calls another contract -- a guaranteed hang. Take
  the lock only at the outermost profiled frame per thread (tracked via a
  thread-local PROFILE_DEPTH); nested frames inherit the outer frame's lock, and
  the existing per-call old_trace_id save/restore keeps the global trace-id symbol
  correct across nesting. Cross-thread isolation is preserved (the outermost frame
  holds the lock for the whole run tree). A ProfileDepthGuard decrements the depth
  on every exit path including unwind.

- FFI panic-safety: ProfilerImpl::push_stmt is a `pub extern "C" fn` invoked
  directly from compiled Cairo code; its `.lock().unwrap()` would unwind across the
  C ABI on a poisoned mutex. Recover from poison instead, matching the lock handling
  in run_with_libfunc_profile.

- Instrumented compiler binary: add a `with-libfunc-profiling` feature to
  starknet-native-compile forwarding to `cairo-native/with-libfunc-profiling`, so an
  instrumented binary (whose emitted .so carries the profiler symbol that
  run_with_libfunc_profile looks up) can be built. A binary built without it produces
  .so files with no profiler symbol, making every profiled call fail.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: avi-starkware

binaries/starknet-native-compile/Cargo.toml

@@ -7,6 +7,13 @@ edition.workspace = true
 license.workspace = true
 repository.workspace = true
 
+[features]
+# Builds an instrumented compiler whose emitted `.so` carries the libfunc-profiling
+# symbols that `cairo_native::AotContractExecutor::run_with_libfunc_profile` looks up.
+# Install with `cargo install ... --features with-libfunc-profiling` to profile native
+# execution; a binary built without it produces `.so`s with no profiler symbol.
+with-libfunc-profiling = ["cairo-native/with-libfunc-profiling"]
+
 [dependencies]
 anyhow.workspace = true
 cairo-lang-sierra.workspace = true

src/executor.rs

@@ -28,10 +28,11 @@ use crate::{
     values::Value,
 };
 use bumpalo::Bump;
-// Re-exported so libfunc-profiling consumers (e.g. blockifier) can refer to the
-// program type without adding a direct `cairo-lang-sierra` dependency.
-#[cfg(feature = "with-libfunc-profiling")]
-pub use cairo_lang_sierra::program::Program;
+// Profiling and sierra-emu consumers (e.g. blockifier) only ever hold the Sierra
+// program as a shared handle. Export the `Arc` alias so they can name it without
+// taking a direct `cairo-lang-sierra` dependency.
+#[cfg(any(feature = "sierra-emu", feature = "with-libfunc-profiling"))]
+pub type ArcProgram = std::sync::Arc<cairo_lang_sierra::program::Program>;
 use cairo_lang_sierra::{
     extensions::{
         circuit::CircuitTypeConcrete,

src/executor/contract_executor.rs

@@ -5,8 +5,6 @@
 //! `H: StarknetSyscallHandler` -- sierra-emu and cairo-native re-export the trait from
 //! `cairo-native-syscalls`, so no adapter is needed.
 
-#[cfg(any(feature = "sierra-emu", feature = "with-libfunc-profiling"))]
-use cairo_lang_sierra::program::Program;
 #[cfg(feature = "sierra-emu")]
 use cairo_lang_starknet_classes::compiler_version::VersionId;
 #[cfg(feature = "sierra-emu")]
@@ -20,6 +18,8 @@ use crate::error::Error;
 use crate::error::Result;
 use crate::execution_result::ContractExecutionResult;
 use crate::executor::AotContractExecutor;
+#[cfg(any(feature = "sierra-emu", feature = "with-libfunc-profiling"))]
+use crate::executor::ArcProgram;
 #[cfg(feature = "with-libfunc-profiling")]
 use crate::metadata::profiler::Profile;
 use crate::starknet::StarknetSyscallHandler;
@@ -43,7 +43,7 @@ pub enum ContractExecutor {
 #[cfg(feature = "sierra-emu")]
 #[derive(Debug, Clone)]
 pub struct EmuContractInfo {
-    pub program: Arc<Program>,
+    pub program: ArcProgram,
     pub entry_points: ContractEntryPoints,
     pub sierra_version: VersionId,
 }
@@ -55,7 +55,7 @@ pub struct EmuContractInfo {
 #[derive(Debug)]
 pub struct AotWithProgram {
     pub executor: AotContractExecutor,
-    pub program: Arc<Program>,
+    pub program: ArcProgram,
 }
 
 impl From<AotContractExecutor> for ContractExecutor {
@@ -143,7 +143,7 @@ impl ContractExecutor {
     }
 
     /// Like [`Self::run`] but, for the `AotWithProgram` variant, hands the captured
-    /// libfunc profile -- together with the `Arc<Program>` the executor was paired with --
+    /// libfunc profile -- together with the `ArcProgram` the executor was paired with --
     /// to `on_profile` after the call returns successfully. The program is included so
     /// callers don't have to keep their own copy around just to resolve libfunc samples.
     /// For other variants this is identical to `run` and `on_profile` is never invoked.
@@ -159,7 +159,7 @@ impl ContractExecutor {
     ) -> Result<ContractExecutionResult>
     where
         H: StarknetSyscallHandler,
-        F: FnOnce(Profile, Arc<Program>),
+        F: FnOnce(Profile, ArcProgram),
     {
         match self {
             ContractExecutor::AotWithProgram(AotWithProgram { executor, program }) => {

src/executor/libfunc_profile.rs

@@ -3,6 +3,7 @@
 //! Available under the `with-libfunc-profiling` feature (gated at the `mod`
 //! declaration in `src/executor.rs`).
 
+use std::cell::Cell;
 use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::{Arc, Mutex};
 
@@ -16,11 +17,23 @@ use crate::metadata::profiler::{Profile, ProfilerBinding, ProfilerImpl, LIBFUNC_
 use crate::starknet::StarknetSyscallHandler;
 use crate::utils::BuiltinCosts;
 
-/// Process-wide lock that serializes calls into [`AotContractExecutor::run_with_libfunc_profile`].
-/// The profiler hot-swaps a process-global symbol (`cairo_native__profiler__profile_id`);
-/// concurrent callers would race on that write and on the [`LIBFUNC_PROFILE`] slot bookkeeping.
+/// Process-wide lock that serializes *top-level* profiled runs across threads. The
+/// profiler hot-swaps a process-global symbol (`cairo_native__profiler__profile_id`);
+/// a concurrent thread would race on that write and on the [`LIBFUNC_PROFILE`] slot
+/// bookkeeping. It is acquired only by the outermost profiled frame on each thread
+/// (see [`PROFILE_DEPTH`]); nested same-thread calls -- a profiled contract invoking
+/// another contract -- re-enter without re-locking, which would otherwise deadlock
+/// this non-reentrant mutex.
 static PROFILE_LOCK: Mutex<()> = Mutex::new(());
 
+thread_local! {
+    /// Nesting depth of profiled runs on the current thread. Only the outermost frame
+    /// (depth 0) takes [`PROFILE_LOCK`]; deeper frames rely on the lock the outer frame
+    /// already holds. The per-call `old_trace_id` save/restore keeps the global trace-id
+    /// symbol correct across nesting without any additional locking.
+    static PROFILE_DEPTH: Cell<usize> = const { Cell::new(0) };
+}
+
 impl AotContractExecutor {
     /// Run the entrypoint with libfunc-level profiling instrumentation.
     ///
@@ -60,10 +73,17 @@ impl AotContractExecutor {
         H: StarknetSyscallHandler,
         F: FnOnce(Profile),
     {
-        // Serialize against concurrent profile calls. Recover from a poisoned lock --
-        // we don't have invariants on the protected state itself; the lock only gates
-        // access to the global trace-id symbol.
-        let _profile_lock = PROFILE_LOCK.lock().unwrap_or_else(|e| e.into_inner());
+        // Acquire the cross-thread lock only at the outermost profiled frame on this
+        // thread. A profiled contract that calls another contract re-enters this
+        // function on the same thread; re-locking the non-reentrant `PROFILE_LOCK`
+        // there would self-deadlock, so nested frames inherit the outer frame's lock.
+        // Recover from a poisoned lock -- it only gates access to the global trace-id
+        // symbol, on which we hold no data invariants.
+        let _profile_lock = PROFILE_DEPTH
+            .with(|depth| depth.get() == 0)
+            .then(|| PROFILE_LOCK.lock().unwrap_or_else(|e| e.into_inner()));
+        PROFILE_DEPTH.with(|depth| depth.set(depth.get() + 1));
+        let _depth_guard = ProfileDepthGuard;
 
         // Look up the profile-id symbol before touching any global state. If the
         // executor wasn't compiled with libfunc-profiling instrumentation, the
@@ -147,3 +167,13 @@ impl Drop for ProfilerGuard {
         }
     }
 }
+
+/// Decrements [`PROFILE_DEPTH`] on every exit path (including unwind) so the
+/// outermost-frame lock bookkeeping stays correct even if the profiled run panics.
+struct ProfileDepthGuard;
+
+impl Drop for ProfileDepthGuard {
+    fn drop(&mut self) {
+        PROFILE_DEPTH.with(|depth| depth.set(depth.get().saturating_sub(1)));
+    }
+}

src/metadata/profiler.rs

@@ -362,7 +362,11 @@ impl ProfilerImpl {
 
     // Push a profiler frame
     pub extern "C" fn push_stmt(profile_id: u64, statement_idx: u64, tick_delta: u64) {
-        let mut profiler = LIBFUNC_PROFILE.lock().unwrap();
+        // Invoked directly from compiled/JIT'd Cairo code across the C ABI: a panic
+        // here (e.g. `.unwrap()` on a poisoned mutex) would unwind into machine code
+        // that cannot handle it. Recover from poison instead of panicking, matching
+        // the lock handling in `run_with_libfunc_profile`.
+        let mut profiler = LIBFUNC_PROFILE.lock().unwrap_or_else(|e| e.into_inner());
 
         let Some(profiler) = profiler.get_mut(&profile_id) else {
             eprintln!("Could not find libfunc profiler!");