From 27fb18a15eb98b3b28e83c9acd718484cef2be54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 20 Aug 2026 06:50:32 +0200 Subject: [PATCH] fix(ffi): guard string header ABI revision --- .../8442-string-header-abi-tripwire.md | 7 +++ crates/perry-ffi/src/lib.rs | 16 +++---- crates/perry-ffi/src/types.rs | 43 +++++++++++++++++++ crates/perry-runtime/src/lib.rs | 2 +- crates/perry-runtime/src/string/mod.rs | 16 +++++++ 5 files changed, 75 insertions(+), 9 deletions(-) create mode 100644 changelog.d/8442-string-header-abi-tripwire.md diff --git a/changelog.d/8442-string-header-abi-tripwire.md b/changelog.d/8442-string-header-abi-tripwire.md new file mode 100644 index 0000000000..89c25cd4b1 --- /dev/null +++ b/changelog.d/8442-string-header-abi-tripwire.md @@ -0,0 +1,7 @@ +### fix(ffi): guard the published string-header ABI against runtime drift + +`perry-ffi` now publishes a string-header ABI revision paired with the runtime's +exported revision symbol, and tests pin both revisions and the 20-byte layout. +Out-of-tree native wrappers can fail loudly on an incompatible runtime instead +of reading corrupt string payloads. The borrowed string/byte helpers also now +document that moving-GC borrows must be copied before the next runtime allocation. diff --git a/crates/perry-ffi/src/lib.rs b/crates/perry-ffi/src/lib.rs index 75f75417e0..b2ab7166af 100644 --- a/crates/perry-ffi/src/lib.rs +++ b/crates/perry-ffi/src/lib.rs @@ -56,7 +56,7 @@ pub use async_runtime::{ mod types; pub use types::{ ArrayHeader, BigIntHeader, BufferHeader, ClosureHeader, NativeAsyncCompletion, ObjectHeader, - Promise, StringHeader, BIGINT_LIMBS, OBJECT_HEADER_ABI_REVISION, + Promise, StringHeader, BIGINT_LIMBS, OBJECT_HEADER_ABI_REVISION, STRING_HEADER_ABI_REVISION, }; mod handle; @@ -192,10 +192,10 @@ pub fn alloc_string(s: &str) -> JsString { /// Read a `JsString` as a borrowed `&str`. /// -/// Returns `None` on a null handle or invalid UTF-8. The borrow lives -/// as long as the runtime guarantees the string remains alive — for -/// the simple call-and-copy pattern in most FFI functions, that's the -/// duration of the function call. +/// Returns `None` on a null handle or invalid UTF-8. Because Perry's GC can +/// move strings, the borrow is valid only until the next allocation through +/// the Perry runtime. Copy the contents before calling any runtime function +/// that may allocate. /// /// ```ignore /// #[no_mangle] @@ -219,9 +219,9 @@ pub fn read_string(handle: JsString) -> Option<&'static str> { /// ops, …) that store arbitrary bytes inside a `StringHeader` but /// can't go through [`read_string`]'s UTF-8 validation. /// -/// Returns `None` on a null handle. The borrow lives as long as -/// the runtime guarantees the string remains alive — same lifetime -/// rules as [`read_string`]. +/// Returns `None` on a null handle. Because Perry's GC can move strings, the +/// borrow is valid only until the next allocation through the Perry runtime. +/// Copy the contents before calling any runtime function that may allocate. pub fn read_bytes(handle: JsString) -> Option<&'static [u8]> { if handle.is_null() { return None; diff --git a/crates/perry-ffi/src/types.rs b/crates/perry-ffi/src/types.rs index 387384519d..7032185e39 100644 --- a/crates/perry-ffi/src/types.rs +++ b/crates/perry-ffi/src/types.rs @@ -29,6 +29,26 @@ pub const BIGINT_LIMBS: usize = 16; /// * 3 — `{class_id, parent_class_id, meta}`, 16 bytes on LP64/ILP32 (#8047). pub const OBJECT_HEADER_ABI_REVISION: u32 = 3; +/// Revision of the [`StringHeader`] ABI this crate mirrors. +/// +/// Bump on ANY change to `StringHeader`'s size, field set, field offsets, or +/// representation, and on any change to the meaning of the payload returned by +/// `read_bytes`. Bump `perry_runtime::perry_string_header_abi_revision()` in the +/// same commit — `string_header_abi_revision_matches_the_pinned_layout` fails +/// otherwise. +/// +/// It exists because `perry-ffi` is **published to crates.io**: a wrapper built +/// against an older mirror and linked by `perry compile` against a newer +/// runtime could otherwise read the wrong payload bytes with no diagnostic. An +/// out-of-tree wrapper should assert +/// `perry_ffi::STRING_HEADER_ABI_REVISION == perry_string_header_abi_revision()` +/// (declared `extern "C" fn() -> u32`) once at startup and refuse to run on a +/// mismatch. +/// +/// * 1 — `{utf16_len, byte_len, capacity, refcount, flags}`, 20 bytes; the +/// payload returned by `read_bytes` begins immediately after the header. +pub const STRING_HEADER_ABI_REVISION: u32 = 1; + /// Header for a runtime-allocated JS string. #[repr(C)] pub struct StringHeader { @@ -44,6 +64,8 @@ pub struct StringHeader { pub flags: u32, } +const _: () = assert!(std::mem::size_of::() == 20); + /// Header for a runtime-allocated JS array. #[repr(C)] pub struct ArrayHeader { @@ -169,6 +191,27 @@ mod layout_tests { ); } + /// Pin both copies of the revision and the absolute published layout. The + /// mirror test above catches one-sided struct drift; these assertions also + /// catch both structs changing without the required revision bump. + #[test] + fn string_header_abi_revision_matches_the_pinned_layout() { + assert_eq!(STRING_HEADER_ABI_REVISION, 1); + assert_eq!( + STRING_HEADER_ABI_REVISION, + perry_runtime::perry_string_header_abi_revision(), + "the runtime and the published mirror disagree about the string header ABI \ + revision — bump BOTH, in the same commit, and say so in the \ + changelog: perry-ffi is published to crates.io" + ); + assert_eq!(size_of::(), 20); + assert_eq!(offset_of!(StringHeader, utf16_len), 0); + assert_eq!(offset_of!(StringHeader, byte_len), 4); + assert_eq!(offset_of!(StringHeader, capacity), 8); + assert_eq!(offset_of!(StringHeader, refcount), 12); + assert_eq!(offset_of!(StringHeader, flags), 16); + } + #[test] fn array_header_matches_runtime() { assert_layout!(ArrayHeader, perry_runtime::ArrayHeader); diff --git a/crates/perry-runtime/src/lib.rs b/crates/perry-runtime/src/lib.rs index 5396e7f3ea..b987e016e4 100644 --- a/crates/perry-runtime/src/lib.rs +++ b/crates/perry-runtime/src/lib.rs @@ -285,7 +285,7 @@ pub use object::{object_live_slot_count, perry_object_header_abi_revision}; pub use promise::Promise; pub use regex::RegExpHeader; pub use set::SetHeader; -pub use string::StringHeader; +pub use string::{perry_string_header_abi_revision, StringHeader}; pub use value::JSValue; // Re-export closure module for stdlib to use js_closure_call* functions diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index 08942a2177..49ff3b14f5 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -341,6 +341,22 @@ const STRING_HEADER_ABI_MATCHES_CODEGEN: () = { }; const _: () = STRING_HEADER_ABI_MATCHES_CODEGEN; +/// Revision of the [`StringHeader`] ABI, paired with +/// `perry_ffi::STRING_HEADER_ABI_REVISION`. +/// +/// `perry-ffi` is published to crates.io, and a wrapper compiled against an old +/// mirror linked against a new runtime could otherwise read the wrong payload +/// with no diagnostic. Bump this and the perry-ffi constant together on ANY +/// change to the header's size, field set, field offsets, representation, or +/// the meaning of the payload exposed by `perry_ffi::read_bytes`. +/// +/// * 1 — `{utf16_len, byte_len, capacity, refcount, flags}`, 20 bytes; the +/// byte payload begins immediately after the header. +#[no_mangle] +pub extern "C" fn perry_string_header_abi_revision() -> u32 { + 1 +} + // ── UTF-8 ↔ UTF-16 conversion helpers ────────────────────────────────── /// Count UTF-16 code units for a UTF-8 byte slice. Returns 0 for empty/null.