From e1226a010189756b071e95fa7d6b3e4a325f5fee Mon Sep 17 00:00:00 2001 From: Pierre Le Fevre Date: Thu, 21 May 2026 22:45:49 +0200 Subject: [PATCH] Throw TypeError for nullish property reads (isu issue 248) --- .isu/issues.json | 2 +- crates/js/src/jit/compiler.rs | 2 + crates/js/src/jit/helpers.rs | 12 +- crates/js/src/vm.rs | 220 ++++++++++++++++++++++++++++++---- 4 files changed, 207 insertions(+), 29 deletions(-) diff --git a/.isu/issues.json b/.isu/issues.json index 592d42a..58f540e 100644 --- a/.isu/issues.json +++ b/.isu/issues.json @@ -2996,7 +2996,7 @@ ], "assigned": [], "author": "piefev", - "state": "open", + "state": "closed", "created_at": "2026-05-21T18:43:29Z" }, { diff --git a/crates/js/src/jit/compiler.rs b/crates/js/src/jit/compiler.rs index 3b23187..62004e8 100644 --- a/crates/js/src/jit/compiler.rs +++ b/crates/js/src/jit/compiler.rs @@ -538,6 +538,7 @@ impl BaselineJit { name_idx as u64, ic_idx as u64, ); + self.emit_check_result(exception_label); } Op::SetPropertyByName => { let obj = read_reg(code, &mut ip); @@ -562,6 +563,7 @@ impl BaselineJit { obj as u64, key as u64, ); + self.emit_check_result(exception_label); } Op::SetProperty => { let obj = read_reg(code, &mut ip); diff --git a/crates/js/src/jit/helpers.rs b/crates/js/src/jit/helpers.rs index 04f6f79..7c3674e 100644 --- a/crates/js/src/jit/helpers.rs +++ b/crates/js/src/jit/helpers.rs @@ -367,8 +367,10 @@ pub extern "C" fn jit_helper_get_prop_name( ) -> u64 { let vm = unsafe { &mut *vm }; // This delegates to the VM's property access logic with inline cache support. - vm.jit_get_property_by_name(dst as u16, obj as u16, name_idx as u16, ic_idx as u16); - OK + match vm.jit_get_property_by_name(dst as u16, obj as u16, name_idx as u16, ic_idx as u16) { + Ok(()) => OK, + Err(_) => EXCEPTION, + } } #[no_mangle] @@ -387,8 +389,10 @@ pub extern "C" fn jit_helper_set_prop_name( #[no_mangle] pub extern "C" fn jit_helper_get_property(vm: *mut Vm, dst: u32, obj: u32, key: u32) -> u64 { let vm = unsafe { &mut *vm }; - vm.jit_get_property(dst as u16, obj as u16, key as u16); - OK + match vm.jit_get_property(dst as u16, obj as u16, key as u16) { + Ok(()) => OK, + Err(_) => EXCEPTION, + } } #[no_mangle] diff --git a/crates/js/src/vm.rs b/crates/js/src/vm.rs index 3e8b7a7..8dddb5e 100644 --- a/crates/js/src/vm.rs +++ b/crates/js/src/vm.rs @@ -855,6 +855,17 @@ impl RuntimeError { } } +fn nullish_property_read_error(receiver: &Value, key: &str) -> Option { + let receiver_name = match receiver { + Value::Undefined => "undefined", + Value::Null => "null", + _ => return None, + }; + Some(RuntimeError::type_error(format!( + "Cannot read properties of {receiver_name} (reading '{key}')" + ))) +} + // ── Property access helpers ────────────────────────────────── /// Get an object's own property value using IC, also returning shape+slot for IC @@ -4143,17 +4154,22 @@ impl Vm { let key_r = Self::read_reg(&mut self.frames[fi]); let base = self.frames[fi].base; let key = self.registers[base + key_r as usize].to_js_string(&self.gc); + let receiver = self.registers[base + obj_r as usize].clone(); + if let Some(err) = nullish_property_read_error(&receiver, &key) { + self.throw_runtime_error(err)?; + continue; + } // Save gc_ref for DOM interception. - let obj_gc_ref = match self.registers[base + obj_r as usize] { - Value::Object(r) | Value::Function(r) => Some(r), + let obj_gc_ref = match &receiver { + Value::Object(r) | Value::Function(r) => Some(*r), _ => None, }; - let val = match self.registers[base + obj_r as usize] { + let val = match &receiver { Value::Object(gc_ref) => { - gc_get_property(&self.gc, gc_ref, &key, &self.shapes) + gc_get_property(&self.gc, *gc_ref, &key, &self.shapes) } - Value::Function(gc_ref) => self.get_function_property(gc_ref, &key), - Value::String(ref s) => { + Value::Function(gc_ref) => self.get_function_property(*gc_ref, &key), + Value::String(s) => { let v = string_get_property(s, &key); if matches!(v, Value::Undefined) { self.string_prototype @@ -4311,23 +4327,28 @@ impl Vm { if !ic_hit { let key = self.frames[fi].func.names[name_idx].clone(); - let obj_gc_ref = match self.registers[base + obj_r as usize] { - Value::Object(r) | Value::Function(r) => Some(r), + let receiver = self.registers[base + obj_r as usize].clone(); + if let Some(err) = nullish_property_read_error(&receiver, &key) { + self.throw_runtime_error(err)?; + continue; + } + let obj_gc_ref = match &receiver { + Value::Object(r) | Value::Function(r) => Some(*r), _ => None, }; - let val = match self.registers[base + obj_r as usize] { + let val = match &receiver { Value::Object(gc_ref) => { // Try own-property lookup for IC update. let (val, own_slot) = - gc_get_property_ic(&self.gc, gc_ref, &key, &self.shapes); + gc_get_property_ic(&self.gc, *gc_ref, &key, &self.shapes); if let Some((shape, slot_index)) = own_slot { self.frames[fi].func.inline_caches[ic_idx] .update(shape, slot_index); } val } - Value::Function(gc_ref) => self.get_function_property(gc_ref, &key), - Value::String(ref s) => { + Value::Function(gc_ref) => self.get_function_property(*gc_ref, &key), + Value::String(s) => { let v = string_get_property(s, &key); if matches!(v, Value::Undefined) { self.string_prototype @@ -4847,6 +4868,15 @@ impl Vm { false } + fn throw_runtime_error(&mut self, err: RuntimeError) -> Result<(), RuntimeError> { + let err_val = err.to_value(&mut self.gc, &mut self.shapes); + if self.handle_exception(err_val) { + Ok(()) + } else { + Err(err) + } + } + /// Register a native function as a global. pub fn define_native( &mut self, @@ -5111,7 +5141,13 @@ impl Vm { // ── JIT helper methods (called by extern "C" helpers) ──────── /// Property access with inline cache support (for JIT helpers). - pub fn jit_get_property_by_name(&mut self, dst: Reg, obj_r: Reg, name_idx: u16, ic_idx: u16) { + pub fn jit_get_property_by_name( + &mut self, + dst: Reg, + obj_r: Reg, + name_idx: u16, + ic_idx: u16, + ) -> Result<(), RuntimeError> { let fi = self.frames.len() - 1; let base = self.frames[fi].base; @@ -5132,21 +5168,25 @@ impl Vm { if !ic_hit { let key = self.frames[fi].func.names[name_idx as usize].clone(); - let obj_gc_ref = match self.registers[base + obj_r as usize] { - Value::Object(r) | Value::Function(r) => Some(r), + let receiver = self.registers[base + obj_r as usize].clone(); + if let Some(err) = nullish_property_read_error(&receiver, &key) { + return Err(err); + } + let obj_gc_ref = match &receiver { + Value::Object(r) | Value::Function(r) => Some(*r), _ => None, }; - let val = match self.registers[base + obj_r as usize] { + let val = match &receiver { Value::Object(gc_ref) => { - let (val, own_slot) = gc_get_property_ic(&self.gc, gc_ref, &key, &self.shapes); + let (val, own_slot) = gc_get_property_ic(&self.gc, *gc_ref, &key, &self.shapes); if let Some((shape, slot_index)) = own_slot { self.frames[fi].func.inline_caches[ic_idx as usize] .update(shape, slot_index); } val } - Value::Function(gc_ref) => self.get_function_property(gc_ref, &key), - Value::String(ref s) => { + Value::Function(gc_ref) => self.get_function_property(*gc_ref, &key), + Value::String(s) => { let v = string_get_property(s, &key); if matches!(v, Value::Undefined) { self.string_prototype @@ -5174,6 +5214,7 @@ impl Vm { }; self.registers[base + dst as usize] = val; } + Ok(()) } /// Property set with inline cache support (for JIT helpers). @@ -5249,7 +5290,12 @@ impl Vm { } /// Dynamic property get (for JIT helpers). - pub fn jit_get_property(&mut self, dst: Reg, obj_r: Reg, key_r: Reg) { + pub fn jit_get_property( + &mut self, + dst: Reg, + obj_r: Reg, + key_r: Reg, + ) -> Result<(), RuntimeError> { let fi = self.frames.len() - 1; let base = self.frames[fi].base; let key = match &self.registers[base + key_r as usize] { @@ -5263,10 +5309,14 @@ impl Vm { } other => other.to_js_string(&self.gc), }; - let val = match self.registers[base + obj_r as usize] { - Value::Object(r) => gc_get_property(&self.gc, r, &key, &self.shapes), - Value::Function(r) => self.get_function_property(r, &key), - Value::String(ref s) => { + let receiver = self.registers[base + obj_r as usize].clone(); + if let Some(err) = nullish_property_read_error(&receiver, &key) { + return Err(err); + } + let val = match &receiver { + Value::Object(r) => gc_get_property(&self.gc, *r, &key, &self.shapes), + Value::Function(r) => self.get_function_property(*r, &key), + Value::String(s) => { let v = string_get_property(s, &key); if matches!(v, Value::Undefined) { self.string_prototype @@ -5279,6 +5329,7 @@ impl Vm { _ => Value::Undefined, }; self.registers[base + dst as usize] = val; + Ok(()) } /// Dynamic property set (for JIT helpers). @@ -5915,6 +5966,12 @@ mod tests { vm.execute(&func) } + fn assert_type_error(result: Result, message: &str) { + let err = result.expect_err("expected TypeError"); + assert_eq!(err.kind, ErrorKind::TypeError); + assert_eq!(err.message, message); + } + // ── Value tests ───────────────────────────────────────── #[test] @@ -6182,6 +6239,121 @@ mod tests { } } + #[test] + fn get_property_on_undefined_throws_type_error() { + let mut b = BytecodeBuilder::new("".into(), 0); + b.func.register_count = 3; + let key = b.add_constant(Constant::String("foo".into())); + b.emit_reg(Op::LoadUndefined, 0); + b.emit_reg_u16(Op::LoadConst, 1, key); + b.emit_reg3(Op::GetProperty, 2, 0, 1); + b.emit_reg(Op::Return, 2); + let func = b.finish(); + + let mut vm = Vm::new(); + assert_type_error( + vm.execute(&func), + "Cannot read properties of undefined (reading 'foo')", + ); + } + + #[test] + fn get_property_on_null_throws_type_error() { + let mut b = BytecodeBuilder::new("".into(), 0); + b.func.register_count = 3; + let key = b.add_constant(Constant::String("bar".into())); + b.emit_reg(Op::LoadNull, 0); + b.emit_reg_u16(Op::LoadConst, 1, key); + b.emit_reg3(Op::GetProperty, 2, 0, 1); + b.emit_reg(Op::Return, 2); + let func = b.finish(); + + let mut vm = Vm::new(); + assert_type_error( + vm.execute(&func), + "Cannot read properties of null (reading 'bar')", + ); + } + + #[test] + fn get_property_by_name_on_undefined_throws_type_error() { + let mut b = BytecodeBuilder::new("".into(), 0); + b.func.register_count = 2; + let name = b.add_name("foo"); + b.emit_reg(Op::LoadUndefined, 0); + b.emit_get_prop_name(1, 0, name); + b.emit_reg(Op::Return, 1); + let func = b.finish(); + + let mut vm = Vm::new(); + assert_type_error( + vm.execute(&func), + "Cannot read properties of undefined (reading 'foo')", + ); + } + + #[test] + fn get_property_by_name_on_null_throws_type_error() { + let mut b = BytecodeBuilder::new("".into(), 0); + b.func.register_count = 2; + let name = b.add_name("bar"); + b.emit_reg(Op::LoadNull, 0); + b.emit_get_prop_name(1, 0, name); + b.emit_reg(Op::Return, 1); + let func = b.finish(); + + let mut vm = Vm::new(); + assert_type_error( + vm.execute(&func), + "Cannot read properties of null (reading 'bar')", + ); + } + + #[test] + fn nullish_property_read_is_catchable() { + match eval( + "var key = 'bar'; + var out = ''; + try { undefined.foo; } catch (e) { out = out + e.name + ':' + e.message; } + try { null[key]; } catch (e) { out = out + '|' + e.name + ':' + e.message; } + out", + ) + .expect("execute") + { + Value::String(s) => assert_eq!( + s, + "TypeError:Cannot read properties of undefined (reading 'foo')|\ + TypeError:Cannot read properties of null (reading 'bar')" + ), + v => panic!("expected string, got {v:?}"), + } + } + + #[test] + fn get_property_by_name_ic_miss_on_nullish_throws_type_error() { + match eval( + "var out = ''; + for (var i = 0; i < 4; i = i + 1) { + var obj = i < 2 ? {foo: i} : (i === 2 ? undefined : null); + try { + out = out + obj.foo; + } catch (e) { + out = out + e.message + '|'; + } + } + out", + ) + .expect("execute") + { + Value::String(s) => assert_eq!( + s, + "01Cannot read properties of undefined (reading 'foo')|\ + Cannot read properties of null (reading 'foo')|" + ), + v => panic!("expected string, got {v:?}"), + } + } + #[test] fn test_typeof_operator() { let mut b = BytecodeBuilder::new("".into(), 0); -- 2.51.2