From 0ad01930cba2ba526c3cc679564a8146d99089d4 Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Wed, 5 Aug 2026 11:06:13 +0200 Subject: [PATCH 1/2] fix(codegen): class-typed nullish field read throws TypeError (#7153) The class-field guard diamond's fallback funneled a nullish receiver into js_object_get_field_by_name_f64, which answers undefined - so a field read through a class-annotated binding holding undefined/null kept running on a silent wrong value where Node throws TypeError. Mirror the generic path's nullish check (#462) on the cold fallback arm of both the value-context diamond (property_get.rs) and the raw-f64 number-context variant (property_get/helpers.rs), and in their runtime full-outline js_class_field_get_ic (#5391). --- crates/perry-codegen/src/expr/property_get.rs | 38 ++++++++++ .../src/expr/property_get/helpers.rs | 35 ++++++++- .../src/typed_feedback/guards.rs | 17 +++++ ...gap_7153_class_typed_nullish_field_read.ts | 71 +++++++++++++++++++ .../test_gap_repsel_ptr_shape_elements.ts | 13 ++-- 5 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 test-files/test_gap_7153_class_typed_nullish_field_read.ts diff --git a/crates/perry-codegen/src/expr/property_get.rs b/crates/perry-codegen/src/expr/property_get.rs index fe54039b18..1dfbb4afe9 100644 --- a/crates/perry-codegen/src/expr/property_get.rs +++ b/crates/perry-codegen/src/expr/property_get.rs @@ -1602,6 +1602,44 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } ctx.current_block = fallback_idx; + // #7153: the guard rejects a nullish receiver along with + // every other shape miss, but a nullish field read must + // throw TypeError per spec — the by-name lookup below + // answers `undefined` and the program keeps running on a + // silent wrong value. Mirror the generic path's check + // (generic_dispatch.rs); the cost sits on the cold + // fallback arm only. + let (is_null, is_nullish) = { + let blk = ctx.block(); + let is_undef = + blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_UNDEFINED_I64); + let is_null = blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_NULL_I64); + let is_nullish = blk.or(I1, &is_undef, &is_null); + (is_null, is_nullish) + }; + let throw_idx = ctx.new_block("class_field_get.throw_nullish"); + let lookup_idx = ctx.new_block("class_field_get.fallback_lookup"); + let throw_label = ctx.block_label(throw_idx); + let lookup_label = ctx.block_label(lookup_idx); + ctx.block() + .cond_br(&is_nullish, &throw_label, &lookup_label); + + ctx.current_block = throw_idx; + let prop_entry = ctx.strings.entry(key_idx); + let prop_bytes_global = format!("@{}", prop_entry.bytes_global); + let prop_len_str = prop_entry.byte_len.to_string(); + let is_null_i32 = ctx.block().zext(I1, &is_null, I32); + ctx.block().call_void( + "js_throw_type_error_property_access", + &[ + (I32, &is_null_i32), + (PTR, &prop_bytes_global), + (I64, &prop_len_str), + ], + ); + ctx.block().unreachable(); + + ctx.current_block = lookup_idx; let blk = ctx.block(); blk.call_void("js_typed_feedback_record_fallback_call", &[(I64, &site_id)]); let val_fallback_js = blk.call( diff --git a/crates/perry-codegen/src/expr/property_get/helpers.rs b/crates/perry-codegen/src/expr/property_get/helpers.rs index 41acd3ebb7..5e3966d868 100644 --- a/crates/perry-codegen/src/expr/property_get/helpers.rs +++ b/crates/perry-codegen/src/expr/property_get/helpers.rs @@ -13,7 +13,7 @@ use crate::native_value::{ BoundsState, BufferAccessMode, LoweredValue, MaterializationReason, NativeRep, SemanticKind, }; use crate::type_analysis::receiver_class_name; -use crate::types::{DOUBLE, I32, I64, I8, PTR}; +use crate::types::{DOUBLE, I1, I32, I64, I8, PTR}; pub(crate) fn class_has_computed_runtime_members(ctx: &FnCtx<'_>, class_name: &str) -> bool { ctx.classes @@ -695,6 +695,39 @@ pub(crate) fn lower_raw_f64_class_field_get_for_number_context( ); ctx.current_block = fallback_idx; + // #7153: same nullish-receiver check as the value-context diamond in + // property_get.rs — a nullish field read must throw TypeError, not coerce + // `undefined` to NaN and keep running. + let (is_null, is_nullish) = { + let blk = ctx.block(); + let is_undef = blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_UNDEFINED_I64); + let is_null = blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_NULL_I64); + let is_nullish = blk.or(I1, &is_undef, &is_null); + (is_null, is_nullish) + }; + let throw_idx = ctx.new_block("class_field_get_number.throw_nullish"); + let lookup_idx = ctx.new_block("class_field_get_number.fallback_lookup"); + let throw_label = ctx.block_label(throw_idx); + let lookup_label = ctx.block_label(lookup_idx); + ctx.block() + .cond_br(&is_nullish, &throw_label, &lookup_label); + + ctx.current_block = throw_idx; + let prop_entry = ctx.strings.entry(key_idx); + let prop_bytes_global = format!("@{}", prop_entry.bytes_global); + let prop_len_str = prop_entry.byte_len.to_string(); + let is_null_i32 = ctx.block().zext(I1, &is_null, I32); + ctx.block().call_void( + "js_throw_type_error_property_access", + &[ + (I32, &is_null_i32), + (PTR, &prop_bytes_global), + (I64, &prop_len_str), + ], + ); + ctx.block().unreachable(); + + ctx.current_block = lookup_idx; let blk = ctx.block(); blk.call_void("js_typed_feedback_record_fallback_call", &[(I64, &site_id)]); let val_fallback_js = blk.call( diff --git a/crates/perry-runtime/src/typed_feedback/guards.rs b/crates/perry-runtime/src/typed_feedback/guards.rs index dc757e4694..1e09b3f8d0 100644 --- a/crates/perry-runtime/src/typed_feedback/guards.rs +++ b/crates/perry-runtime/src/typed_feedback/guards.rs @@ -761,7 +761,24 @@ pub extern "C" fn js_class_field_get_ic( crate::typed_feedback::js_typed_feedback_record_fallback_call(site_id); let obj_bits = receiver.to_bits(); + // #7153: this function is the full-outline of the codegen class-field-get + // diamond (#5391), so it must mirror the diamond's nullish-receiver check — + // a field read on undefined/null throws TypeError instead of answering + // `undefined` through the by-name lookup. let key_raw = key as u64 & crate::value::POINTER_MASK; + if obj_bits == crate::value::TAG_UNDEFINED || obj_bits == crate::value::TAG_NULL { + let name = unsafe { + crate::object::has_own_helpers::str_from_string_header( + key_raw as *const crate::StringHeader, + ) + } + .unwrap_or(""); + crate::error::js_throw_type_error_property_access( + (obj_bits == crate::value::TAG_NULL) as u32, + name.as_ptr(), + name.len(), + ); + } crate::object::js_object_get_field_by_name_f64( obj_bits as *const ObjectHeader, key_raw as *const crate::StringHeader, diff --git a/test-files/test_gap_7153_class_typed_nullish_field_read.ts b/test-files/test_gap_7153_class_typed_nullish_field_read.ts new file mode 100644 index 0000000000..612852ae64 --- /dev/null +++ b/test-files/test_gap_7153_class_typed_nullish_field_read.ts @@ -0,0 +1,71 @@ +// #7153: a field read through a CLASS-ANNOTATED binding whose runtime value is +// nullish must throw TypeError, exactly like Node — not answer `undefined`. +// +// The bug: `const missing = short[5]` on a `Row[]` refines the binding to +// `Named("Row")`, so the read takes the class-field guard diamond +// (property_get.rs / property_get/helpers.rs). The guard correctly rejects a +// non-pointer receiver, but the fallback funneled straight into +// `js_object_get_field_by_name_f64`, which answers `undefined` for any +// unrecognized receiver — the program kept running on a silent wrong value. +// An `any`-typed binding took the generic dispatch path, which has thrown +// correctly since #462. The reads below are DIRECT (no closure wrapper — a +// capture could reroute the lowering) and every case prints the caught +// error's name and message so the file is byte-exact against Node. + +class Row { + id: number; + name: string; + score: number; + constructor(id: number, name: string, score: number) { + this.id = id; + this.name = name; + this.score = score; + } +} + +const short: Row[] = []; +short.push(new Row(9, "s", 9)); + +// 1. Value context: the plain class-field guard diamond. +const missing = short[5]; +try { + console.log("oob value:", missing.id); +} catch (e) { + console.log("oob value:", (e as Error).constructor.name + ": " + (e as Error).message); +} + +// 2. Number context: the raw-f64 number-context variant of the same diamond +// (`missing.score * 2` routes through the ToNumber-flavored lowering). +try { + console.log("oob number:", missing.score * 2); +} catch (e) { + console.log("oob number:", (e as Error).constructor.name + ": " + (e as Error).message); +} + +// 3. String-typed field on the same nullish receiver. +try { + console.log("oob string field:", missing.name); +} catch (e) { + console.log("oob string field:", (e as Error).constructor.name + ": " + (e as Error).message); +} + +// 4. A null element behind the same class annotation: the message must say +// "null", not "undefined". +const holed: Row[] = []; +holed.push(null as unknown as Row); +const nullish = holed[0]; +try { + console.log("null value:", nullish.id); +} catch (e) { + console.log("null value:", (e as Error).constructor.name + ": " + (e as Error).message); +} +try { + console.log("null number:", nullish.score + 1); +} catch (e) { + console.log("null number:", (e as Error).constructor.name + ": " + (e as Error).message); +} + +// 5. Control: an in-bounds element still reads normally through the same +// lowering (the fix must not disturb the fast path or valid fallbacks). +const present = short[0]; +console.log("in bounds:", present.id, present.name, present.score * 2); diff --git a/test-files/test_gap_repsel_ptr_shape_elements.ts b/test-files/test_gap_repsel_ptr_shape_elements.ts index f3dc32b20d..10600b1a1c 100644 --- a/test-files/test_gap_repsel_ptr_shape_elements.ts +++ b/test-files/test_gap_repsel_ptr_shape_elements.ts @@ -26,13 +26,12 @@ // mutated with `pop`, one with mixed element classes, one read out of // bounds — each still has to produce Node's answer. // -// NOT covered, deliberately: `short[5].id` where the binding is annotated -// `Row`. Node throws `TypeError: Cannot read properties of undefined`; Perry -// prints `undefined` — on `main`, at the base commit, and with -// `PERRY_PTR_SHAPE_LOCALS=0`, so it is an unrelated pre-existing gap and NOT -// this pass's OOB hazard (which E5's in-bounds conjunct is what rules out). -// Asserting `typeof` instead keeps the out-of-bounds read exercised without -// making this file red for someone else's bug. +// NOT covered here: `short[5].id` where the binding is annotated `Row` — +// that was the pre-existing #7153 gap (Perry printed `undefined` where Node +// throws TypeError), fixed and covered by +// `test_gap_7153_class_typed_nullish_field_read.ts`. The `typeof` assert +// below stays: it exercises the out-of-bounds read without a throw, which is +// this pass's concern (E5's in-bounds conjunct). class Row { id: number; From df38afe702b0678863bc8eac0ef418ff67abcdbd Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Wed, 5 Aug 2026 11:06:54 +0200 Subject: [PATCH 2/2] docs(changelog): add 7430 fragment --- changelog.d/7430-class-typed-nullish-field-read.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog.d/7430-class-typed-nullish-field-read.md diff --git a/changelog.d/7430-class-typed-nullish-field-read.md b/changelog.d/7430-class-typed-nullish-field-read.md new file mode 100644 index 0000000000..546522a934 --- /dev/null +++ b/changelog.d/7430-class-typed-nullish-field-read.md @@ -0,0 +1,8 @@ +Fixed a field read through a class-annotated binding holding `undefined`/`null` +silently answering `undefined` instead of throwing `TypeError` (#7153) — e.g. +`short[5].id` on a `Row[]` where index 5 is out of bounds, which Node aborts +with `Cannot read properties of undefined (reading 'id')`. The class-field +guard diamond's fallback (value-context and raw-f64 number-context lowerings, +plus the outlined `js_class_field_get_ic`) now mirrors the generic dispatch +path's nullish-receiver check on the cold fallback arm; the fast path is +unchanged. Covered by `test_gap_7153_class_typed_nullish_field_read.ts`.