From ed1fa4e22f1c6f2bb68f1fc46df8cb5edbde2555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 26 Sep 2026 18:16:19 +0000 Subject: [PATCH 1/2] fix(runtime): a vtable IC hit also requires the method name bytes to match (#11341) --- .../src/object/class_registry/dispatch.rs | 65 ++++++++++++++++--- .../native_call_method/collection_methods.rs | 3 +- .../native_call_method/handle_methods.rs | 3 +- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/crates/perry-runtime/src/object/class_registry/dispatch.rs b/crates/perry-runtime/src/object/class_registry/dispatch.rs index bdb66efc74..99dd3330fd 100644 --- a/crates/perry-runtime/src/object/class_registry/dispatch.rs +++ b/crates/perry-runtime/src/object/class_registry/dispatch.rs @@ -103,14 +103,28 @@ pub(crate) fn class_lookup_surface_gen_bump() { const VTABLE_IC_SIZE: usize = 4096; const VTABLE_IC_MASK: usize = VTABLE_IC_SIZE - 1; - +/// Longest method name `VTABLE_IC` stores (and therefore caches). Longer names +/// take the uncached registry walk. +const VTABLE_IC_NAME_MAX: usize = 24; + +// #11341: the slot is chosen by the name's ADDRESS (cheap, and exact for the +// rodata strings codegen passes), but a hit also requires the name BYTES to +// match. Not every caller's name lives in rodata: the `new Function` / eval +// interpreter passes a transient Rust `String` (`dyn_eval::bridge::call_method`) +// and `js_native_call_method_value` passes the bytes of a GC string. Once such +// a buffer is freed, another method name can be allocated at the same address, +// and an address-only hit then dispatched the call to the FIRST name's method. +// mysql2's generated row parser — `result.two = packet.parseLengthCodedInt()` +// then `result.s = packet.readLengthCodedString()`, run by the interpreter — +// read `'hi'` back as the number 617 that way. #[repr(C)] #[derive(Copy, Clone)] struct VTableICEntry { gen: u64, class_id: u32, - _pad: u32, + name_len: u32, method_name_ptr: usize, + name: [u8; VTABLE_IC_NAME_MAX], func_ptr: usize, param_count: u32, has_synthetic_arguments: u32, @@ -120,8 +134,9 @@ struct VTableICEntry { const EMPTY_VTABLE_IC_ENTRY: VTableICEntry = VTableICEntry { gen: 0, class_id: 0, - _pad: 0, + name_len: 0, method_name_ptr: 0, + name: [0; VTABLE_IC_NAME_MAX], func_ptr: 0, param_count: 0, has_synthetic_arguments: 0, @@ -152,8 +167,9 @@ fn vtable_ic_slot(class_id: u32, method_name_ptr: usize) -> usize { pub(crate) unsafe fn vtable_ic_lookup( class_id: u32, method_name_ptr: usize, + name: &[u8], ) -> Option<(usize, u32, bool, bool)> { - if method_name_ptr == 0 { + if method_name_ptr == 0 || name.len() > VTABLE_IC_NAME_MAX { return None; } let cur_gen = VTABLE_GEN.load(Ordering::Relaxed); @@ -164,6 +180,8 @@ pub(crate) unsafe fn vtable_ic_lookup( if entry.gen == cur_gen && entry.class_id == class_id && entry.method_name_ptr == method_name_ptr + && entry.name_len as usize == name.len() + && entry.name[..name.len()] == *name { Some(( entry.func_ptr, @@ -181,23 +199,27 @@ pub(crate) unsafe fn vtable_ic_lookup( pub(crate) unsafe fn vtable_ic_insert( class_id: u32, method_name_ptr: usize, + name: &[u8], func_ptr: usize, param_count: u32, has_synthetic_arguments: bool, has_rest: bool, ) { - if method_name_ptr == 0 { + if method_name_ptr == 0 || name.len() > VTABLE_IC_NAME_MAX { return; } let cur_gen = VTABLE_GEN.load(Ordering::Relaxed); let slot = vtable_ic_slot(class_id, method_name_ptr); + let mut stored = [0u8; VTABLE_IC_NAME_MAX]; + stored[..name.len()].copy_from_slice(name); VTABLE_IC.with(|cell| { let cache = &mut **cell.get(); cache[slot] = VTableICEntry { gen: cur_gen, class_id, - _pad: 0, + name_len: name.len() as u32, method_name_ptr, + name: stored, func_ptr, param_count, has_synthetic_arguments: if has_synthetic_arguments { 1 } else { 0 }, @@ -237,11 +259,12 @@ pub(crate) unsafe fn vtable_ic_insert( // substitutes for an object-specific check. // // Deliberately SEPARATE from `VTABLE_IC` above: that one is also written from -// the collection dispatcher, and it is keyed on the name's ADDRESS. +// the collection dispatcher, and it picks its slot by the name's ADDRESS (a hit +// there also compares the bytes since #11341). // ============================================================================ // The key is the method-name BYTES, never its address. `VTABLE_IC` above keys -// on the rodata pointer codegen passes, which is stable — but +// its slot on the name pointer codegen passes — but // `js_native_call_method_str_key` reaches the same tower with a name // materialised into a CALLER-STACK scratch buffer (`str_bytes_from_jsvalue` // with a `[u8; SHORT_STRING_MAX_LEN]`), and two different short names can land @@ -729,6 +752,32 @@ mod obj_dispatch_ic_tests { }); } + #[test] + fn vtable_ic_hit_requires_matching_name_bytes_not_only_the_address() { + // #11341: the interpreter (`dyn_eval::bridge::call_method`) and + // `js_native_call_method_value` pass method names that are NOT + // rodata, so a freed name buffer can come back holding a different + // name at the same address. Sabotage it the same way here: one + // backing buffer, two names. + with_stable_gen(&|| { + let mut scratch = *b"parseInt\0\0\0\0\0\0\0"; + let addr = scratch.as_ptr() as usize; + unsafe { vtable_ic_insert(CID, addr, &scratch[..8], 0xEEEE, 1, false, false) }; + if unsafe { vtable_ic_lookup(CID, addr, &scratch[..8]) } + != Some((0xEEEE, 1, false, false)) + { + return false; + } + scratch[..10].copy_from_slice(b"readString"); + assert_eq!( + unsafe { vtable_ic_lookup(CID, addr, &scratch[..10]) }, + None, + "a different name at the same address must MISS" + ); + true + }); + } + #[test] fn a_hit_requires_the_matching_class_id() { with_stable_gen(&|| { diff --git a/crates/perry-runtime/src/object/native_call_method/collection_methods.rs b/crates/perry-runtime/src/object/native_call_method/collection_methods.rs index 718dad127d..fca8c802f2 100644 --- a/crates/perry-runtime/src/object/native_call_method/collection_methods.rs +++ b/crates/perry-runtime/src/object/native_call_method/collection_methods.rs @@ -468,7 +468,7 @@ pub(super) unsafe fn dispatch_raw_pointer( let class_id = (*obj).class_id; if class_id != 0 { if let Some((func_ptr, param_count, has_synthetic_arguments, has_rest)) = - vtable_ic_lookup(class_id, method_name_ptr as usize) + vtable_ic_lookup(class_id, method_name_ptr as usize, method_name.as_bytes()) { let this_i64 = raw_bits as i64; return Some(call_vtable_method( @@ -493,6 +493,7 @@ pub(super) unsafe fn dispatch_raw_pointer( vtable_ic_insert( class_id, method_name_ptr as usize, + method_name.as_bytes(), entry.func_ptr, entry.param_count, entry.has_synthetic_arguments, diff --git a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs index 3848f52788..c59e7f2dea 100644 --- a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs +++ b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs @@ -1007,7 +1007,7 @@ pub(super) unsafe fn dispatch_handle( let class_id = (*obj).class_id; if class_id != 0 { if let Some((func_ptr, param_count, has_synthetic_arguments, has_rest)) = - vtable_ic_lookup(class_id, method_name_ptr as usize) + vtable_ic_lookup(class_id, method_name_ptr as usize, method_name.as_bytes()) { let this_i64 = jsval.as_pointer::() as i64; return Some(call_vtable_method( @@ -1087,6 +1087,7 @@ pub(super) unsafe fn dispatch_handle( vtable_ic_insert( class_id, method_name_ptr as usize, + method_name.as_bytes(), entry.func_ptr, entry.param_count, entry.has_synthetic_arguments, From 01e30fb091fb95a0bcf11d0ad4155fbcde648238 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 26 Sep 2026 20:44:04 +0000 Subject: [PATCH 2/2] changelog: #11432 --- changelog.d/11432-vtable-ic-name-bytes.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/11432-vtable-ic-name-bytes.md diff --git a/changelog.d/11432-vtable-ic-name-bytes.md b/changelog.d/11432-vtable-ic-name-bytes.md new file mode 100644 index 0000000000..0fd04488de --- /dev/null +++ b/changelog.d/11432-vtable-ic-name-bytes.md @@ -0,0 +1,3 @@ +**fix(runtime): a `VTABLE_IC` hit also requires the method name bytes to match, so compiled mysql2 no longer reads `'hi'` as `617` (#11341).** The per-thread vtable inline cache chose and matched its entries by `(class_id, method-name address)`. Not every caller passes a name that lives in rodata. The `new Function` interpreter (`dyn_eval`) passes a transient `String` for each member call, and `js_native_call_method_value` passes a GC string's bytes. Once such a buffer was freed and a different name was allocated at the same address, the cache dispatched the call to the previous name's method. + +mysql2's generated text-row parser calls `packet.parseLengthCodedInt()` and then `packet.readLengthCodedString()`. The second call intermittently ran the first, so a text column read back as a number: 13 of 20 runs of the affected program on main with a `mysql_native_password` user, 0 of 20 with this fix. Entries now store the name bytes (up to 24 bytes, the same limit as `OBJ_DISPATCH_IC`). A new unit test, `vtable_ic_hit_requires_matching_name_bytes_not_only_the_address`, rewrites a buffer in place with a different name and asserts the lookup misses.