diff --git a/.github/workflows/benchmark-pr.yml b/.github/workflows/benchmark-pr.yml index 91f5b02ac..f6254e5e2 100644 --- a/.github/workflows/benchmark-pr.yml +++ b/.github/workflows/benchmark-pr.yml @@ -12,6 +12,13 @@ on: - 'executor/**' - 'bin/cli/**' - 'tooling/ethrex-fixtures/**' + # syscalls is linked into the guest ELF this job builds, so a change confined to + # it changes the bytes proven — a guest allocator swap moves cycles on every + # workload. Without it main's baseline would stay stale until some prover file + # happened to change, and the comparison guard would suppress the table until + # then. pr_main.yaml:99 already hashes 'syscalls/**' into the guest-ELF cache + # key; the two lists must agree on what rebuilds the guest. + - 'syscalls/**' # A baseline is only valid for the workload it measured, and the Makefile is # what defines that workload: it names the block and pins the URL and sha256 # of the .bin this job fetches. Without it a repointed block would leave diff --git a/crypto/ethrex-crypto/Cargo.lock b/crypto/ethrex-crypto/Cargo.lock index ec809fff9..fab277e4b 100644 --- a/crypto/ethrex-crypto/Cargo.lock +++ b/crypto/ethrex-crypto/Cargo.lock @@ -79,7 +79,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "62945a2f7e6de02a31fe400aa489f0e0f5b2502e69f95f853adb82a96c7a6b60" dependencies = [ "quote", - "syn 2.0.118", + "syn", ] [[package]] @@ -92,7 +92,7 @@ dependencies = [ "num-traits", "proc-macro2", "quote", - "syn 2.0.118", + "syn", ] [[package]] @@ -131,7 +131,7 @@ checksum = "213888f660fddcca0d257e88e54ac05bca01885f258ccdf695bafd77031bb69d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn", ] [[package]] @@ -162,12 +162,6 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4c7f02d4ea65f2c1853089ffd8d2787bdbc63de2f0d29dedbcf8ccdfa0ccd4cf" -[[package]] -name = "base64" -version = "0.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" - [[package]] name = "bitvec" version = "1.1.1" @@ -214,12 +208,6 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" -[[package]] -name = "const-default" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b396d1f76d455557e1218ec8066ae14bba60b4b36ecd55577ba979f5db7ecaa" - [[package]] name = "const-oid" version = "0.9.6" @@ -313,7 +301,7 @@ dependencies = [ "enum-ordinalize", "proc-macro2", "quote", - "syn 2.0.118", + "syn", ] [[package]] @@ -340,18 +328,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "embedded-alloc" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f2de9133f68db0d4627ad69db767726c99ff8585272716708227008d3f1bddd" -dependencies = [ - "const-default", - "critical-section", - "linked_list_allocator", - "rlsf", -] - [[package]] name = "embedded-hal" version = "1.0.0" @@ -375,7 +351,7 @@ checksum = "8ca9601fb2d62598ee17836250842873a413586e5d7ed88b356e38ddbb0ec631" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn", ] [[package]] @@ -563,7 +539,6 @@ dependencies = [ name = "lambda-vm-syscalls" version = "0.1.0" dependencies = [ - "embedded-alloc", "getrandom 0.2.17", "getrandom 0.3.4", "lazy_static", @@ -584,12 +559,6 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" -[[package]] -name = "linked_list_allocator" -version = "0.10.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b23ac50abb8261cb38c6e2a7192d3302e0836dac1628f6a93b82b4fad185897" - [[package]] name = "num-bigint" version = "0.4.6" @@ -804,7 +773,7 @@ checksum = "7d323d13972c1b104aa036bc692cd08b822c8bbf23d79a27c526095856499799" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn", ] [[package]] @@ -813,31 +782,12 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8188909339ccc0c68cfb5a04648313f09621e8b87dc03095454f1a11f6c5d436" -[[package]] -name = "rlsf" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1646a59a9734b8b7a0ac51689388a60fe1625d4b956348e9de07591a1478457a" -dependencies = [ - "cfg-if", - "const-default", - "libc", - "rustversion", - "svgbobdoc", -] - [[package]] name = "rustc-hex" version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3e75f6a532d0fd9f7f13144f392b6ad56a32696bfcd9c78f797f16bbb6f072d6" -[[package]] -name = "rustversion" -version = "1.0.22" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" - [[package]] name = "sec1" version = "0.7.3" @@ -884,30 +834,6 @@ version = "2.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" -[[package]] -name = "svgbobdoc" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2c04b93fc15d79b39c63218f15e3fdffaa4c227830686e3b7c5f41244eb3e50" -dependencies = [ - "base64", - "proc-macro2", - "quote", - "syn 1.0.109", - "unicode-width", -] - -[[package]] -name = "syn" -version = "1.0.109" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" -dependencies = [ - "proc-macro2", - "quote", - "unicode-ident", -] - [[package]] name = "syn" version = "2.0.118" @@ -951,7 +877,7 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn", ] [[package]] @@ -962,7 +888,7 @@ checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn", ] [[package]] @@ -998,12 +924,6 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" -[[package]] -name = "unicode-width" -version = "0.1.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" - [[package]] name = "version_check" version = "0.9.5" @@ -1057,7 +977,7 @@ checksum = "1ae7f38b72ec2a254e2b87ef277cf2cd4fb97cbebf944faa6f33354da0867930" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn", ] [[package]] @@ -1077,5 +997,5 @@ checksum = "3c50655cbb0fe3fc43170059e702f1ce5e19b84cec58dc87b037a09935c2f328" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn", ] diff --git a/syscalls/Cargo.toml b/syscalls/Cargo.toml index fb2459255..6bcd8d1a8 100644 --- a/syscalls/Cargo.toml +++ b/syscalls/Cargo.toml @@ -12,8 +12,11 @@ lazy_static = "1.5.0" rand = "0.9.2" # Doug Lea's malloc, behind `dlmalloc-alloc`: slower than the default bump allocator on # every workload measured, but it reclaims freed memory, so it is the allocator to pick -# for continuations. `critical-section` gives the Sync a #[global_allocator] static -# needs; its single-hart impl comes from `riscv` above. See `src/allocator.rs`. +# for a guest whose cumulative allocation has no per-execution bound. Not a +# continuations criterion: continuations are a prover-side split of a single guest +# execution and change nothing about what the guest allocates. `critical-section` gives +# the Sync a #[global_allocator] static needs; its single-hart impl comes from `riscv` +# above. See `src/allocator.rs`. dlmalloc = { version = "0.2.14", default-features = false, optional = true } critical-section = { version = "1.2", optional = true } diff --git a/syscalls/src/allocator.rs b/syscalls/src/allocator.rs index 80162c6a5..7c733ac9d 100644 --- a/syscalls/src/allocator.rs +++ b/syscalls/src/allocator.rs @@ -50,11 +50,30 @@ mod imp { #[cfg_attr(target_arch = "riscv64", global_allocator)] static ALLOC: BumpAlloc = BumpAlloc; + /// Idempotent: a later call must not rewind the cursor over live allocations. See + /// `init_allocator` for why that would be silent corruption and why nothing calls + /// this twice today. `HEAP_END` doubles as the initialized flag -- `init_allocator` + /// always passes the nonzero `MAX_MEMORY_SIZE`. pub fn init(heap_start: usize, heap_end: usize) { + let initialized = HEAP_END.load(Ordering::Relaxed) != 0; + debug_assert!( + !initialized, + "allocator init called twice; the cursor would rewind over live allocations" + ); + if initialized { + return; + } HEAP_POS.store(heap_start, Ordering::Relaxed); HEAP_END.store(heap_end, Ordering::Relaxed); } + // Test-only: `init` is idempotent, so the tests must clear the flag to re-point the + // global cursor at their own heap. + #[cfg(test)] + fn reset() { + HEAP_END.store(0, Ordering::Relaxed); + } + unsafe impl GlobalAlloc for BumpAlloc { unsafe fn alloc(&self, layout: Layout) -> *mut u8 { let align = layout.align(); @@ -99,6 +118,7 @@ mod imp { // Zeroed, like guest memory: reads of never-written heap return 0 there. let base = unsafe { std::alloc::alloc_zeroed(l) }; assert!(!base.is_null()); + reset(); init(base as usize, base as usize + bytes); guard } @@ -166,10 +186,16 @@ mod imp { unsafe { BumpAlloc.alloc(l) }.is_null(), "handed out memory past HEAP_END" ); - // An absurd size declines too. It declines on the bounds check rather than - // on the `checked_add`: `Layout` requires size rounded up to align to fit in - // `isize::MAX`, so a size that would overflow the cursor arithmetic can't be - // constructed in the first place. + // An absurd size declines too, and on the bounds check rather than on the + // `checked_add`. The `Layout` invariant alone does not get you there: it + // gives `size <= isize::MAX - (align - 1)`, and with + // `aligned <= pos + align - 1` that bounds + // `aligned + size <= pos + isize::MAX` -- which is `< 2^64` only if + // `pos < 2^63`. The second half comes from the cursor being heap-bounded: + // `alloc` stores `new_pos` only when `new_pos <= HEAP_END`, so + // `pos <= HEAP_END`, and on the guest that is `MAX_MEMORY_SIZE` = + // 0xC000_0000. The `checked_add` stays: it keeps the no-overflow argument + // local to `alloc` instead of resting on both of those. let huge = layout(isize::MAX as usize - 7, 8); assert!(unsafe { BumpAlloc.alloc(huge) }.is_null()); } @@ -179,6 +205,7 @@ mod imp { #[test] fn uninitialized_allocator_hands_out_nothing() { let _guard = HEAP_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + reset(); init(0, 0); assert!(unsafe { BumpAlloc.alloc(layout(1, 1)) }.is_null()); } @@ -253,14 +280,28 @@ mod imp { fn allocates_zeros(&self) -> bool { // Guest memory is zero-initialized and this provider never reuses a segment, - // so system-fresh bytes read as 0. dlmalloc consults this only in - // `calloc_must_clear` = `!allocates_zeros() || !mmapped(chunk)`, i.e. it may - // skip calloc's memset only for a chunk it marked mmapped. Two independent - // reasons that is safe here: the Rust port has no mmap path at all (nothing - // ever sets the mmapped marker, so calloc always zeroes), and even if it - // grew one, freeing an mmapped chunk whose system `free` declines drops the - // chunk instead of re-binning it — so a recycled block is never mmapped. - // Locked by `calloc_zeroes_recycled_dirty_blocks` below. + // so system-fresh bytes read as 0. + // + // This setting is INERT, not a performance win. dlmalloc consults it only + // through `calloc_must_clear(ptr)` = + // `!allocates_zeros() || !mmapped(Chunk::from_mem(ptr))`, and `mmapped` is + // not a marker bit anyone sets — it is `(*p).head & INUSE == 0`, the absence + // of both in-use bits (dlmalloc 0.2.14 `src/dlmalloc.rs:1805`). Every path + // that returns a pointer to a caller goes through `set_inuse` / + // `set_inuse_and_pinuse` / `set_size_and_pinuse_of_inuse_chunk`, all of which + // set `CINUSE`, and `calloc_must_clear` is only ever evaluated on a user + // pointer. So no *user* chunk is ever `mmapped`, `calloc_must_clear` is + // always true, `calloc` always memsets, and flipping this to `false` would + // change nothing. + // + // Flagless heads do exist, so don't reason from "nothing is ever mmapped": + // `init_top` (dlmalloc.rs:789) writes a segment-end sentinel with + // `head = top_foot_size()` = 80 on 64-bit, and `80 & INUSE == 0`, so that + // sentinel *is* `mmapped()`-true. Harmless — it is never returned to a + // caller, so it never reaches `calloc_must_clear`. + // + // Kept `true` for correctness-by-construction if upstream ever grows an mmap + // path. Locked by `calloc_zeroes_recycled_dirty_blocks` below. true } @@ -272,6 +313,12 @@ mod imp { // Dlmalloc is Send but !Sync, so it can't sit in a static directly. A single-hart // critical section serializes access and supplies the Sync a #[global_allocator] // static requires. Its single-hart implementation comes from the `riscv` crate. + // + // An initialized `Dlmalloc` is address-sensitive and must never be moved: + // `smallbin_at` returns a pointer into `self.smallbins` and `init_bins` writes + // self-pointers into that array, so relocating it after first use — into a `Box`, a + // `OnceCell`, or a local — silently corrupts the bins. Safe as a `static`; the note + // is for whoever refactors this. static DLMALLOC: Mutex>> = Mutex::new(RefCell::new(Dlmalloc::new_with_allocator(BumpSystem))); @@ -280,11 +327,31 @@ mod imp { #[cfg_attr(target_arch = "riscv64", global_allocator)] static ALLOC: DlGlobal = DlGlobal; + /// Idempotent: a later call must not rewind the segment cursor, which would hand + /// dlmalloc segments overlapping ones it is already using. See `init_allocator` for + /// the full argument and for why nothing calls this twice today. `HEAP_END` doubles + /// as the initialized flag -- `init_allocator` always passes the nonzero + /// `MAX_MEMORY_SIZE`. pub fn init(heap_start: usize, heap_end: usize) { + let initialized = HEAP_END.load(Ordering::Relaxed) != 0; + debug_assert!( + !initialized, + "allocator init called twice; the segment cursor would rewind over live segments" + ); + if initialized { + return; + } HEAP_POS.store(heap_start, Ordering::Relaxed); HEAP_END.store(heap_end, Ordering::Relaxed); } + // Test-only: `init` is idempotent, so the tests must clear the flag to re-point the + // global segment cursor at their own heap. + #[cfg(test)] + fn reset() { + HEAP_END.store(0, Ordering::Relaxed); + } + unsafe impl GlobalAlloc for DlGlobal { unsafe fn alloc(&self, layout: Layout) -> *mut u8 { critical_section::with(|cs| unsafe { @@ -346,7 +413,12 @@ mod imp { // Zeroed, like guest memory: reads of never-written heap return 0 there. let base = unsafe { std::alloc::alloc_zeroed(layout) }; assert!(!base.is_null()); + reset(); init(base as usize, base as usize + bytes); + // Moved out by value, which is only sound because it is untouched: an + // initialized `Dlmalloc` is address-sensitive (see the `DLMALLOC` static). + // `new_with_allocator` is const and `init_bins` runs on first malloc, which + // has not happened yet. (guard, Dlmalloc::new_with_allocator(BumpSystem)) } @@ -455,6 +527,7 @@ mod imp { #[test] fn uninitialized_provider_hands_out_nothing() { let _guard = HEAP_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + reset(); init(0, 0); assert!(BumpSystem.alloc(1).0.is_null()); } @@ -490,6 +563,23 @@ mod imp { } } +/// Points the guest allocator at `[_end, MAX_MEMORY_SIZE)`. +/// +/// Must run exactly once per execution, and `imp::init` enforces that by ignoring any +/// later call rather than trusting its callers. A second call rewinds the cursor back +/// over live allocations, and because the bump arm's `alloc_zeroed` skips the memset -- +/// sound only because bump never re-serves a region -- the next `alloc_zeroed` would +/// then hand back dirty bytes. The guest would compute on garbage and the prover would +/// produce a perfectly valid proof of that wrong execution: no crash, no diagnostic, +/// which is why this is guarded rather than merely documented. +/// +/// What makes it once today is an entry-point flag, not the call sites. The six guests +/// that call this explicitly all also override the ELF entry with +/// `-C link-arg=-e -C link-arg=main` in their `.cargo/config.toml`, so `_start` -- the +/// only other caller, in `src/entrypoint.rs` -- never runs for them; guests that do +/// enter through `_start` never call it explicitly. A guest that dropped `-e main` while +/// keeping its explicit call would therefore call this twice, which is why the guard +/// lives in `imp::init` rather than in a comment here. pub fn init_allocator() { unsafe extern "C" { static _end: u8; diff --git a/tooling/ethrex-block-converter/Cargo.lock b/tooling/ethrex-block-converter/Cargo.lock index a8268a857..8ad77716b 100644 --- a/tooling/ethrex-block-converter/Cargo.lock +++ b/tooling/ethrex-block-converter/Cargo.lock @@ -463,12 +463,6 @@ dependencies = [ "digest", ] -[[package]] -name = "const-default" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b396d1f76d455557e1218ec8066ae14bba60b4b36ecd55577ba979f5db7ecaa" - [[package]] name = "const-oid" version = "0.9.6" @@ -796,18 +790,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "embedded-alloc" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f2de9133f68db0d4627ad69db767726c99ff8585272716708227008d3f1bddd" -dependencies = [ - "const-default", - "critical-section", - "linked_list_allocator", - "rlsf", -] - [[package]] name = "embedded-hal" version = "1.0.0" @@ -1662,7 +1644,6 @@ dependencies = [ name = "lambda-vm-syscalls" version = "0.1.0" dependencies = [ - "embedded-alloc", "getrandom 0.2.17", "getrandom 0.3.4", "lazy_static", @@ -1717,12 +1698,6 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6d2cec3eae94f9f509c767b45932f1ada8350c4bdb85af2fcab4a3c14807981" -[[package]] -name = "linked_list_allocator" -version = "0.10.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b23ac50abb8261cb38c6e2a7192d3302e0836dac1628f6a93b82b4fad185897" - [[package]] name = "lock_api" version = "0.4.14" @@ -2406,18 +2381,6 @@ dependencies = [ "rustc-hex", ] -[[package]] -name = "rlsf" -version = "0.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07393724337be2ee43a9d86164df4505746874a3fa65913374bc6d6a92314362" -dependencies = [ - "cfg-if", - "const-default", - "libc", - "rustversion", -] - [[package]] name = "rustc-hash" version = "2.1.3"