Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions changelog.d/10592-instanceof-value-kinds.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
`instanceof` no longer segfaults on short inline strings, and `Object.create(proto).constructor`
returns the real constructor. The receiver is now resolved per value kind rather than assumed to be a
heap pointer, with the prototype and class-registry paths updated to match.

The string crash took down ajv, and with it every fastify schema route; the `constructor` defect
crashed lodash's `isEqual`. `new EventEmitter() instanceof EventEmitter` is fixed as a direct
consequence.

A related shape, `class Sub extends EventEmitter {}` followed by `new Sub() instanceof
EventEmitter`, took a separate fix: that call compiles through the dynamic-dispatch instanceof path
(the RHS resolves via a native-module lookup), which never registered or consulted the class-chain
parent edge that `extends Array`/`Map`/`Set`/`Error` subclassing already uses. A subclass instance is
a real object carrying its own class id, not a handle and not prototype-linked to
`EventEmitter.prototype`, so it was invisible to the handle/prototype probes on that path and always
answered `false`. EventEmitter's reserved class id is now a valid `extends` parent, and the
dynamic-dispatch branch delegates to the class-chain walk first, falling back to the prototype walk
for `util.inherits`-style shapes.

A CodeRabbit review pass on this PR also found that `instanceof`'s dynamic-RHS classification (and
`value_is_callable`) trusted the INT32-class-ref tag band alone, without checking the class id was
actually registered. A JS program can construct a `number` sharing that same tag band directly (via
`DataView`), which was then misread as a class reference instead of correctly reaching the
unresolved-RHS `TypeError`. Both sites now go through the same `class_ref_id` helper (which also
checks `is_class_id_registered`) that the rest of the crate already uses for this.
8 changes: 8 additions & 0 deletions crates/perry-codegen/src/expr/instance_misc1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ pub(crate) fn builtin_parent_reserved_class_id(name: &str) -> Option<u32> {
"BigInt64Array" => 0xFFFF0039,
"BigUint64Array" => 0xFFFF003A,
"Function" => 0xFFFF00F0,
// #10556: `class Sub extends EventEmitter {}` — same shape as the
// Array/Map/Set/Error builtins above. Without this edge,
// `new Sub() instanceof EventEmitter` never reaches the class-chain
// walk in `js_instanceof` and falls back to the dynamic-dispatch
// handle/prototype probes in perry-runtime/src/object/instanceof.rs,
// which don't recognize a genuine subclass ObjectHeader. Keep in
// sync with `CLASS_ID_EVENT_EMITTER` there.
"EventEmitter" => 0xFFFF0076,
_ => return None,
})
}
Expand Down
30 changes: 11 additions & 19 deletions crates/perry-runtime/src/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1526,15 +1526,15 @@ fn object_ptr(value: f64) -> Option<*mut ObjectHeader> {
return None;
}
let raw = (bits & crate::value::POINTER_MASK) as usize;
if raw < 0x10000 || crate::buffer::is_registered_buffer(raw) {
if crate::buffer::is_registered_buffer(raw) {
return None;
}
unsafe {
let header =
(raw as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
if (*header).obj_type != crate::gc::GC_TYPE_OBJECT {
return None;
}
// #10556: `x instanceof EventEmitter` asks `is_worker_instance_value`, and a
// native `new EventEmitter()` is a POINTER_TAG registry handle (`0x38000`)
// that sailed over the old `0x10000` floor into a header read.
let header = unsafe { crate::value::addr_class::try_read_gc_header(raw) }?;
if header.obj_type != crate::gc::GC_TYPE_OBJECT {
return None;
}
Some(raw as *mut ObjectHeader)
}
Expand All @@ -1545,18 +1545,10 @@ fn array_ptr(value: f64) -> Option<*mut ArrayHeader> {
return None;
}
let raw = (bits & crate::value::POINTER_MASK) as usize;
if raw < 0x10000 {
return None;
}
unsafe {
let header =
(raw as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
match (*header).obj_type {
crate::gc::GC_TYPE_ARRAY | crate::gc::GC_TYPE_LAZY_ARRAY => {
Some(raw as *mut ArrayHeader)
}
_ => None,
}
let header = unsafe { crate::value::addr_class::try_read_gc_header(raw) }?;
match header.obj_type {
crate::gc::GC_TYPE_ARRAY | crate::gc::GC_TYPE_LAZY_ARRAY => Some(raw as *mut ArrayHeader),
_ => None,
}
}

Expand Down
11 changes: 6 additions & 5 deletions crates/perry-runtime/src/collection_iter_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,13 @@ pub fn is_set_iterator_addr(addr: usize) -> bool {
}

fn iterator_class_id(addr: usize) -> Option<u32> {
if addr < crate::gc::GC_HEADER_SIZE + 0x1000 {
return None;
}
// `util.types.isMapIterator(v)` / `isSetIterator(v)` hand any value's
// candidate address here. The canonical header read rejects the handle
// band and out-of-window bits before touching memory (#10479: the old
// `addr < GC_HEADER_SIZE + 0x1000` floor let a proxy/fetch id through).
unsafe {
let gc_header = (addr - crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
if (*gc_header).obj_type != crate::gc::GC_TYPE_OBJECT {
let header = crate::value::addr_class::try_read_gc_header(addr)?;
if header.obj_type != crate::gc::GC_TYPE_OBJECT {
return None;
}
Some((*(addr as *const ObjectHeader)).class_id)
Expand Down
1 change: 1 addition & 0 deletions crates/perry-runtime/src/object/class_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ pub(crate) use prototype_objects::{
class_prototype_object, ensure_function_prototype_object, function_class_id,
function_value_for_class_id, resolve_proto_chain_field,
resolve_proto_chain_field_with_receiver, resolve_proto_chain_symbol,
synthetic_class_prototype_object,
};
pub use prototype_objects::{
js_set_function_prototype, js_set_prototype_property, NEXT_SYNTHETIC_CLASS_ID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,34 @@ pub(crate) fn ensure_function_prototype_object(
proto_handle.with_mut_ptr::<ObjectHeader, _>(|proto| proto)
}

/// Floor of the synthetic class-id range (see [`NEXT_SYNTHETIC_CLASS_ID`]).
pub(crate) const SYNTHETIC_CLASS_ID_BASE: u32 = 0x8000_0000;

per_test_global! {
/// Synthetic class id allocator for prototype-object classes. High bit
/// set (0x8000_0000+) to keep them separate from codegen-assigned ids
/// (which start from 1 and grow by module). u32 wraparound is not a
/// concern in practice — would require ~2 billion `Function.prototype = X`
/// statements at module init.
pub static NEXT_SYNTHETIC_CLASS_ID: std::sync::atomic::AtomicU32 =
std::sync::atomic::AtomicU32::new(0x8000_0000);
std::sync::atomic::AtomicU32::new(SYNTHETIC_CLASS_ID_BASE);
}

/// The `[[Prototype]]` object recorded for a SYNTHETIC class id — one of the
/// ids `Object.create(proto)` (#809) and `F.prototype = obj` (#711) allocate
/// from [`NEXT_SYNTHETIC_CLASS_ID`]. That link is the authoritative prototype
/// of every instance stamped with the id.
///
/// Unlike [`class_prototype_object`], this refuses a DECLARED class id, whose
/// entry in the same table is the parent CLASS OBJECT of a class-expression
/// subclass (#1788/#6552) rather than a prototype.
pub(crate) fn synthetic_class_prototype_object(class_id: u32) -> *mut ObjectHeader {
if class_id < SYNTHETIC_CLASS_ID_BASE
|| class_id >= NEXT_SYNTHETIC_CLASS_ID.load(std::sync::atomic::Ordering::Relaxed)
{
return std::ptr::null_mut();
}
class_prototype_object(class_id)
}

/// Perform ordinary `.prototype` assignment, then synchronize the synthetic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,38 @@ pub(super) unsafe fn instance_constructor_value(
if let Some(func_value) = super::super::class_registry::function_value_for_class_id(class_id) {
return Some(JSValue::from_bits(func_value.to_bits()));
}
// #10478: an `Object.create(proto)` result is stamped with a synthetic
// class id that only indexes its prototype object
// (`CLASS_PROTOTYPE_OBJECTS`); unlike the function ids above it names no
// class VALUE. Its `constructor` is the inherited `proto.constructor`, so
// read it off that chain. The INT32 synthesis below minted a ClassRef for
// the synthetic id itself (`0x7FFE_0000_8000_0000`): unequal to `Object` /
// `A`, printed as `[object Function]`, and `C instanceof C` segfaulted
// (lodash `isEqual(cloneDeep(x), x)`).
let synthetic_proto = super::super::class_registry::synthetic_class_prototype_object(class_id);
if !synthetic_proto.is_null() {
// The prototype's OWN `constructor` data field answers the common
// shapes directly — `Object.prototype`, a declared `C.prototype`, a
// materialized `F.prototype`, a `{ constructor: F }` literal — so take
// it without the general chain walk, whose implicit-`this` juggling,
// accessor-receiver override and registry probes cost ~3000
// instructions per read. Skipped when an accessor owns the key, which
// must run through the walk to fire with the right receiver.
if get_accessor_descriptor(synthetic_proto as usize, "constructor").is_none() {
if let Some(value) = own_data_field_by_name(synthetic_proto, key) {
if !value.is_undefined() && !value.is_null() {
return Some(value);
}
}
}
let receiver = f64::from_bits(crate::value::js_nanbox_pointer(obj as i64).to_bits());
return Some(
super::super::class_registry::resolve_proto_chain_field_with_receiver(
class_id, key, receiver,
)
.unwrap_or_else(JSValue::undefined),
);
}
if class_id != 0 && is_class_id_registered(class_id) {
let bits = 0x7FFE_0000_0000_0000u64 | (class_id as u64);
return Some(JSValue::from_bits(bits));
Expand Down
118 changes: 74 additions & 44 deletions crates/perry-runtime/src/object/instanceof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ pub(crate) fn value_is_callable(value: f64) -> bool {
// INT32-tagged class references (top 16 bits = 0x7FFE) are callable
// constructors emitted by codegen. `is_pointer()` only checks 0x7FFD,
// so they would fall through to `return false` without this guard.
if (value.to_bits() >> 48) == 0x7FFE {
// `class_ref_id` also requires `is_class_id_registered`, so a
// user-crafted NaN payload sharing this tag band (e.g. via
// `DataView.setFloat64` — a real JS number, not a class ref) is not
// misclassified as callable.
if class_ref_id(value).is_some() {
return true;
}
let jv = crate::JSValue::from_bits(value.to_bits());
Expand Down Expand Up @@ -62,15 +66,13 @@ fn small_native_handle_id(value: f64) -> Option<i64> {
None
}

/// Candidate heap address of an `instanceof` operand; 0 for every primitive.
/// #10479: this used to treat every tag band `>= 0x7FF8` as a pointer, so a
/// 1-5 byte inline string (or an INT32 class ref) reached
/// `object_static_prototype` as a garbage address and segfaulted.
#[inline]
fn value_addr(value: f64) -> usize {
let bits = value.to_bits();
if (bits >> 48) >= 0x7FF8 {
(bits & crate::value::POINTER_MASK) as usize
} else if (bits >> 48) == 0 && bits >= 0x1000 {
bits as usize
} else {
0
}
crate::value::addr_class::object_ref_addr(value)
}

fn recorded_prototype_instanceof_builtin(value: f64, name: &str) -> Option<bool> {
Expand Down Expand Up @@ -99,7 +101,12 @@ fn is_native_module_namespace_value(value: f64, expected: &str) -> bool {
return false;
}
let obj = jv.as_pointer::<ObjectHeader>();
if obj.is_null() {
// #10556: a native `new EventEmitter()` is a POINTER_TAG registry handle
// (`0x38000`), and `x instanceof EventEmitter` asks this probe first — the
// null check alone let it read `class_id` out of unmapped low memory.
let is_object = unsafe { crate::value::addr_class::try_read_gc_header(obj as usize) }
.is_some_and(|header| header.obj_type == crate::gc::GC_TYPE_OBJECT);
if !is_object {
return false;
}
unsafe {
Expand Down Expand Up @@ -245,12 +252,13 @@ pub extern "C" fn js_instanceof_dynamic(value: f64, type_ref: f64) -> f64 {
}
}
let bits = type_ref.to_bits();
let top16 = bits >> 48;
if top16 == 0x7FFE {
let class_id = (bits & 0xFFFF_FFFF) as u32;
if class_id != 0 {
return js_instanceof(value, class_id);
}
// `class_ref_id` requires `is_class_id_registered`, not just the tag —
// a user-crafted NaN payload sharing the 0x7FFE band (a real JS number
// constructed via `DataView.setFloat64`, not a codegen-emitted class
// ref) must fall through to the unresolved-RHS `TypeError` below
// instead of being dispatched into `js_instanceof` as a bogus class id.
if let Some(class_id) = class_ref_id(type_ref) {
return js_instanceof(value, class_id);
}
// #9502: a heap class object's template id identifies its code, not its
// evaluation. Compare the actual prototype objects so sibling evaluations
Expand Down Expand Up @@ -328,9 +336,20 @@ pub extern "C" fn js_instanceof_dynamic(value: f64, type_ref: f64) -> f64 {
return f64::from_bits(crate::value::TAG_TRUE);
}
if module == "events" && method == "EventEmitter" {
// #10556: a genuine subclass instance (`class Sub extends
// EventEmitter {}`) is a real ObjectHeader carrying Sub's own
// class id, not a handle and not prototype-linked to the real
// `EventEmitter.prototype` — so it is invisible to the
// handle/prototype probes below. Delegate to the static path
// first: `js_instanceof` walks the class-chain parent edge that
// codegen registers for `extends EventEmitter`
// (`builtin_parent_reserved_class_id` in
// perry-codegen/src/expr/instance_misc1.rs), and its own
// `CLASS_ID_EVENT_EMITTER` branch already covers the direct
// handle/`util.inherits` cases. Keep the general prototype walk
// as a fallback for shapes neither path reaches.
return f64::from_bits(
if is_event_emitter_instance_value(value)
|| super::tls_constructor_prototype_is_instance_of(value, method.as_str())
if js_instanceof(value, CLASS_ID_EVENT_EMITTER).to_bits() == crate::value::TAG_TRUE
|| ordinary_has_instance_prototype_walk(value, type_ref)
{
crate::value::TAG_TRUE
Expand Down Expand Up @@ -702,11 +721,12 @@ fn ordinary_has_instance_prototype_walk(value: f64, type_ref: f64) -> bool {
// * a heap-allocated string/bigint/symbol gets ToObject-wrapped by
// getPrototypeOf, so the walk climbs the wrapper chain and can spuriously
// match (`Symbol() instanceof Object` wrongly returned `true`).
// Every tag below is checked without dereferencing. Real f64 numbers are
// already answered `false` by the primitive fast paths before this point and
// share tag-space with raw heap pointers (a bare `is_number()` would
// misclassify a module-level object var), so they are intentionally left to
// those paths rather than guarded here.
// Every tag below is checked without dereferencing. Real f64 numbers share
// tag-space with legacy raw heap pointers (a bare `is_number()` would
// misclassify a raw-bitcast object), so a number is only rejected when it
// does not decode as an object address. They are NOT all answered by
// earlier fast paths: a dynamic `1.5 instanceof Number` / `instanceof
// Object` reached this walk, ToObject-wrapped the number and matched.
let scope = crate::gc::RuntimeHandleScope::new();
let value = scope.root_nanbox_f64(value);
let type_ref = scope.root_nanbox_f64(type_ref);
Expand All @@ -718,6 +738,7 @@ fn ordinary_has_instance_prototype_walk(value: f64, type_ref: f64) -> bool {
|| jv.is_int32()
|| jv.is_any_string()
|| jv.is_bigint()
|| (jv.is_number() && value_addr(value.get_nanbox_f64()) == 0)
|| unsafe { crate::symbol::js_is_symbol(value.get_nanbox_f64()) != 0 }
{
return false;
Expand Down Expand Up @@ -1599,27 +1620,15 @@ pub extern "C" fn js_instanceof(value: f64, class_id: u32) -> f64 {
// perspective — must return true without force-materializing.
const CLASS_ID_ARRAY: u32 = 0xFFFF0024;
if class_id == CLASS_ID_ARRAY {
let addr = if jsval.is_pointer() {
(bits & 0x0000_FFFF_FFFF_FFFF) as usize
} else {
let top16 = (bits >> 48) as u16;
if top16 == 0 && bits >= 0x1000 {
bits as usize
} else {
0
}
};
if addr != 0 && addr >= crate::gc::GC_HEADER_SIZE {
let gc_header = (addr - crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
unsafe {
let obj_type = (*gc_header).obj_type;
if obj_type == crate::gc::GC_TYPE_ARRAY || obj_type == crate::gc::GC_TYPE_LAZY_ARRAY
{
return true_val;
}
}
}
return false_val;
// A POINTER_TAG handle id (fetch/zlib/stdlib registries) is not a heap
// address; the canonical header read rejects it instead of probing the
// byte below it.
let is_array = unsafe { crate::value::addr_class::try_read_gc_header(value_addr(value)) }
.is_some_and(|header| {
header.obj_type == crate::gc::GC_TYPE_ARRAY
|| header.obj_type == crate::gc::GC_TYPE_LAZY_ARRAY
});
return if is_array { true_val } else { false_val };
}

// Typed arrays — Int8Array..Float16Array reserved IDs (0xFFFF0030..3B).
Expand Down Expand Up @@ -1783,6 +1792,18 @@ pub extern "C" fn js_instanceof(value: f64, class_id: u32) -> f64 {
if let Some(matches) = recorded_prototype_instanceof_builtin(value, "Error") {
return if matches { true_val } else { false_val };
}
}

// Everything below reads `ObjectHeader::class_id`, which only a
// genuine `GC_TYPE_OBJECT` has. Every other GC type keeps something
// else in that word — an array's `length`, a closure's function
// pointer, a Map's `size` — so `[1, 2] instanceof C` was true whenever
// the length equalled (or chained to) `C`'s class id.
if gc_type != crate::gc::GC_TYPE_OBJECT {
return false_val;
}

if class_id == crate::error::CLASS_ID_ERROR {
let obj_class_id = (*obj_ptr).class_id;
if extends_builtin_error(obj_class_id) {
return true_val;
Expand Down Expand Up @@ -1834,6 +1855,15 @@ mod null_lhs_tests {
f64::from_bits(crate::value::INT32_TAG | 5), // int32 5
f64::from_bits(crate::value::STRING_TAG | 0x1000), // string tag (addr never deref'd)
f64::from_bits(crate::value::BIGINT_TAG | 0x1000), // bigint tag (addr never deref'd)
// #10479: inline SSO strings ("uri", "a") and a synthetic class ref.
f64::from_bits(crate::value::SHORT_STRING_TAG | 0x0300_0069_7275),
f64::from_bits(crate::value::SHORT_STRING_TAG | 0x0100_0000_0061),
f64::from_bits(crate::value::INT32_TAG | 0x8000_0000),
// Ordinary numbers: a dynamic `1.5 instanceof Number` reached the
// walk and matched through the ToObject wrapper.
1.5,
-0.0,
f64::NAN,
];
for lhs in cases {
// A dummy non-object RHS is never consulted for a non-object LHS.
Expand Down
Loading
Loading