diff --git a/core/build_playerglobal/src/lib.rs b/core/build_playerglobal/src/lib.rs index 1edeca775d22..a0014d2b74e0 100644 --- a/core/build_playerglobal/src/lib.rs +++ b/core/build_playerglobal/src/lib.rs @@ -461,7 +461,7 @@ fn write_native_table(data: &[u8], out_dir: &Path) -> Result, Box Result, Box Result, Box Result, Box Activation<'a, 'gc> { fn op_call_method( &mut self, - index: u32, + index: usize, arg_count: u32, push_return_value: bool, ) -> Result<(), Error<'gc>> { @@ -1628,7 +1628,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { Ok(()) } - fn op_get_slot(&mut self, index: u32) -> Result<(), Error<'gc>> { + fn op_get_slot(&mut self, index: usize) -> Result<(), Error<'gc>> { let stack_top = self.stack.stack_top(); let object = stack_top @@ -1647,7 +1647,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { Ok(()) } - fn op_set_slot(&mut self, index: u32) -> Result<(), Error<'gc>> { + fn op_set_slot(&mut self, index: usize) -> Result<(), Error<'gc>> { let value = self.pop_stack(); let object = self .pop_stack() @@ -1660,7 +1660,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { Ok(()) } - fn op_set_slot_no_coerce(&mut self, index: u32) -> Result<(), Error<'gc>> { + fn op_set_slot_no_coerce(&mut self, index: usize) -> Result<(), Error<'gc>> { let value = self.pop_stack(); let object = self .pop_stack() @@ -1673,7 +1673,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { Ok(()) } - fn op_set_global_slot(&mut self, index: u32) -> Result<(), Error<'gc>> { + fn op_set_global_slot(&mut self, index: usize) -> Result<(), Error<'gc>> { let value = self.pop_stack(); self.global_scope() @@ -1712,7 +1712,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { Ok(()) } - fn op_construct_slot(&mut self, index: u32, arg_count: u32) -> Result<(), Error<'gc>> { + fn op_construct_slot(&mut self, index: usize, arg_count: u32) -> Result<(), Error<'gc>> { let args = self.stack.get_args(arg_count as usize); let source = self .pop_stack() diff --git a/core/src/avm2/class.rs b/core/src/avm2/class.rs index 2c6b7f7a9991..48cdce9e8434 100644 --- a/core/src/avm2/class.rs +++ b/core/src/avm2/class.rs @@ -9,7 +9,7 @@ use crate::avm2::object::{scriptobject_allocator, ClassObject, Object}; use crate::avm2::script::TranslationUnit; use crate::avm2::traits::{Trait, TraitKind}; use crate::avm2::value::Value; -use crate::avm2::vtable::VTable; +use crate::avm2::vtable::{VTable, VTableInitError}; use crate::avm2::Error; use crate::avm2::Multiname; use crate::avm2::Namespace; @@ -321,9 +321,13 @@ impl<'gc> Class<'gc> { let c_class = Class(Gc::new(mc, c_class)); i_class.link_with_c_class(mc, c_class); - i_class.init_vtable_with_interfaces(context, Box::new([])); + i_class + .init_vtable_with_interfaces(context, Box::new([])) + .expect("Specialized vector has no traits on itself"); - c_class.init_vtable_with_interfaces(context, Box::new([])); + c_class + .init_vtable_with_interfaces(context, Box::new([])) + .expect("Specialized vector has no traits on itself"); let write = unlock!(Gc::write(mc, this.0), ClassData, cell); write.borrow_mut().applications.insert(Some(param), i_class); @@ -828,11 +832,22 @@ impl<'gc> Class<'gc> { Ok(()) } + /// Initialize the vtable and interfaces of this Class with an empty vtable + /// and no interfaces. This is useful for classes that are known to not have + /// any traits or interfaces. + pub fn init_empty_vtable(self, mc: &Mutation<'gc>) { + let write = Gc::write(mc, self.0); + + let _ = unlock!(write, ClassData, all_interfaces).set(Box::new([])); + let _ = unlock!(write, ClassData, vtable).set(VTable::empty(mc)); + } + /// Initialize the vtable and interfaces of this Class. pub fn init_vtable(self, activation: &mut Activation<'_, 'gc>) -> Result<(), Error<'gc>> { let interfaces = self.gather_interfaces(activation)?; - self.init_vtable_with_interfaces(activation.context, interfaces); + self.init_vtable_with_interfaces(activation.context, interfaces) + .map_err(|e| e.into_avm(activation))?; Ok(()) } @@ -844,7 +859,7 @@ impl<'gc> Class<'gc> { self, context: &mut UpdateContext<'gc>, interfaces: Box<[Class<'gc>]>, - ) { + ) -> Result<(), VTableInitError> { if self.0.traits.get().is_none() { panic!( "Attempted to initialize vtable on a class that did not have its traits loaded yet" @@ -863,7 +878,9 @@ impl<'gc> Class<'gc> { None, self.0.super_class.map(|c| c.vtable()), context, - )); + )?); + + Ok(()) } /// Associate all the methods defined on this class with the specified diff --git a/core/src/avm2/error.rs b/core/src/avm2/error.rs index 04dd8fadeaf0..1525db699aea 100644 --- a/core/src/avm2/error.rs +++ b/core/src/avm2/error.rs @@ -252,7 +252,7 @@ pub fn make_error_1025<'gc>(activation: &mut Activation<'_, 'gc>, index: u32) -> #[cold] pub fn make_error_1026<'gc>( activation: &mut Activation<'_, 'gc>, - slot_id: u32, + slot_id: usize, slot_count: usize, ) -> Error<'gc> { let err = verify_error( diff --git a/core/src/avm2/globals/avmplus.rs b/core/src/avm2/globals/avmplus.rs index e59edb2b4b45..30c9dd42a7c8 100644 --- a/core/src/avm2/globals/avmplus.rs +++ b/core/src/avm2/globals/avmplus.rs @@ -211,8 +211,6 @@ fn describe_internal_body<'gc>( _ => unreachable!(), }; - let trait_metadata = vtable.get_metadata_for_slot(*slot_id); - let variable = ScriptObject::new_object(activation); variable.set_dynamic_property(istr!("name"), prop_name.into(), activation.gc()); variable.set_dynamic_property( @@ -231,7 +229,7 @@ fn describe_internal_body<'gc>( if flags.contains(DescribeTypeFlags::INCLUDE_METADATA) { let metadata_object = ArrayObject::empty(activation); - if let Some(metadata) = trait_metadata { + if let Some(metadata) = vtable.get_metadata_for_slot(*slot_id) { write_metadata(metadata_object, metadata, activation); } variable.set_dynamic_property( @@ -274,8 +272,6 @@ fn describe_internal_body<'gc>( let declared_by_name = declared_by.dollar_removed_name(mc).to_qualified_name(mc); - let trait_metadata = vtable.get_metadata_for_disp(*disp_id); - let method_obj = ScriptObject::new_object(activation); method_obj.set_dynamic_property(istr!("name"), prop_name.into(), activation.gc()); @@ -307,7 +303,7 @@ fn describe_internal_body<'gc>( if flags.contains(DescribeTypeFlags::INCLUDE_METADATA) { let metadata_object = ArrayObject::empty(activation); - if let Some(metadata) = trait_metadata { + if let Some(metadata) = vtable.get_metadata_for_disp(*disp_id) { write_metadata(metadata_object, metadata, activation); } method_obj.set_dynamic_property( diff --git a/core/src/avm2/globals/flash/display/display_object.rs b/core/src/avm2/globals/flash/display/display_object.rs index ca70d41b1da7..5a15d6eb74a9 100644 --- a/core/src/avm2/globals/flash/display/display_object.rs +++ b/core/src/avm2/globals/flash/display/display_object.rs @@ -934,7 +934,7 @@ pub fn object_to_rectangle<'gc>( activation: &mut Activation<'_, 'gc>, object: Object<'gc>, ) -> Result, Error<'gc>> { - const SLOTS: &[u32] = &[ + const SLOTS: &[usize] = &[ rectangle_slots::X, rectangle_slots::Y, rectangle_slots::WIDTH, diff --git a/core/src/avm2/globals/flash/events/mouse_event.rs b/core/src/avm2/globals/flash/events/mouse_event.rs index d8b55e9f61fa..195fc25251db 100644 --- a/core/src/avm2/globals/flash/events/mouse_event.rs +++ b/core/src/avm2/globals/flash/events/mouse_event.rs @@ -40,8 +40,8 @@ pub fn update_after_event<'gc>( pub(super) fn local_to_stage_x<'gc>( activation: &mut Activation<'_, 'gc>, this: Object<'gc>, - slot_x: u32, - slot_y: u32, + slot_x: usize, + slot_y: usize, ) -> Result, Error<'gc>> { if let Some(evt) = this.as_event() { let local_x = this.get_slot(slot_x).coerce_to_number(activation)?; @@ -66,8 +66,8 @@ pub(super) fn local_to_stage_x<'gc>( pub(super) fn local_to_stage_y<'gc>( activation: &mut Activation<'_, 'gc>, this: Object<'gc>, - slot_x: u32, - slot_y: u32, + slot_x: usize, + slot_y: usize, ) -> Result, Error<'gc>> { if let Some(evt) = this.as_event() { let local_x = this.get_slot(slot_x).coerce_to_number(activation)?; diff --git a/core/src/avm2/globals/global_scope.rs b/core/src/avm2/globals/global_scope.rs index 9f9a0350ac92..586db11b1a24 100644 --- a/core/src/avm2/globals/global_scope.rs +++ b/core/src/avm2/globals/global_scope.rs @@ -34,7 +34,9 @@ pub fn create_class<'gc>( // `global` classes have no interfaces, so use `init_vtable_with_interfaces` // and pass an empty list - class.init_vtable_with_interfaces(activation.context, Box::new([])); + class + .init_vtable_with_interfaces(activation.context, Box::new([])) + .map_err(|e| e.into_avm(activation))?; class.mark_builtin_type(BuiltinType::ScriptTraits); diff --git a/core/src/avm2/globals/null.rs b/core/src/avm2/globals/null.rs index 8fe7cc5cd126..35badc0ab17f 100644 --- a/core/src/avm2/globals/null.rs +++ b/core/src/avm2/globals/null.rs @@ -14,9 +14,8 @@ pub fn create_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> { ); class.set_attributes(ClassAttributes::FINAL | ClassAttributes::SEALED); - // The `null` class has no interfaces, so use `init_vtable_with_interfaces` - // and pass an empty list - class.init_vtable_with_interfaces(activation.context, Box::new([])); + // The `null` class has no interfaces or traits, so use `init_empty_vtable` + class.init_empty_vtable(activation.gc()); class } diff --git a/core/src/avm2/globals/void.rs b/core/src/avm2/globals/void.rs index 061d2483d55a..fea8b313eb05 100644 --- a/core/src/avm2/globals/void.rs +++ b/core/src/avm2/globals/void.rs @@ -14,9 +14,8 @@ pub fn create_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> { ); class.set_attributes(ClassAttributes::FINAL | ClassAttributes::SEALED); - // The `void` class has no interfaces, so use `init_vtable_with_interfaces` - // and pass an empty list - class.init_vtable_with_interfaces(activation.context, Box::new([])); + // The `void` class has no interfaces or traits, so use `init_empty_vtable` + class.init_empty_vtable(activation.gc()); class } diff --git a/core/src/avm2/object.rs b/core/src/avm2/object.rs index f8178e3cf38e..bcba2cfc8c50 100644 --- a/core/src/avm2/object.rs +++ b/core/src/avm2/object.rs @@ -399,7 +399,7 @@ pub trait TObject<'gc>: 'gc + Collect<'gc> + Debug + Into> + Clone + /// Retrieve a slot by its index. #[no_dynamic] #[inline(always)] - fn get_slot(self, id: u32) -> Value<'gc> { + fn get_slot(self, id: usize) -> Value<'gc> { let base = self.base(); base.get_slot(id) @@ -409,7 +409,7 @@ pub trait TObject<'gc>: 'gc + Collect<'gc> + Debug + Into> + Clone + #[no_dynamic] fn set_slot( self, - id: u32, + id: usize, value: Value<'gc>, activation: &mut Activation<'_, 'gc>, ) -> Result<(), Error<'gc>> { @@ -422,7 +422,7 @@ pub trait TObject<'gc>: 'gc + Collect<'gc> + Debug + Into> + Clone + } #[no_dynamic] - fn set_slot_no_coerce(self, id: u32, value: Value<'gc>, mc: &Mutation<'gc>) { + fn set_slot_no_coerce(self, id: usize, value: Value<'gc>, mc: &Mutation<'gc>) { let base = self.base(); base.set_slot(id, value, mc); @@ -580,7 +580,7 @@ pub trait TObject<'gc>: 'gc + Collect<'gc> + Debug + Into> + Clone + fn install_bound_method( &self, mc: &Mutation<'gc>, - disp_id: u32, + disp_id: usize, function: FunctionObject<'gc>, ) { let base = self.base(); @@ -691,7 +691,7 @@ pub trait TObject<'gc>: 'gc + Collect<'gc> + Debug + Into> + Clone + } #[no_dynamic] - fn get_bound_method(&self, id: u32) -> Option> { + fn get_bound_method(&self, id: usize) -> Option> { let base = self.base(); base.get_bound_method(id) } diff --git a/core/src/avm2/object/class_object.rs b/core/src/avm2/object/class_object.rs index 55eb2df93f6a..4e41ee350a33 100644 --- a/core/src/avm2/object/class_object.rs +++ b/core/src/avm2/object/class_object.rs @@ -218,6 +218,7 @@ impl<'gc> ClassObject<'gc> { self.superclass_object().map(|cls| cls.instance_vtable()), activation.context, ); + let vtable = vtable.expect("ClassObject VTable should be valid"); unlock!(Gc::write(mc, self.0), ClassObjectData, instance_vtable).set(vtable); } @@ -254,6 +255,7 @@ impl<'gc> ClassObject<'gc> { Some(class_classobject.instance_vtable()), activation.gc(), ); + let class_vtable = class_vtable.expect("ClassObject VTable should be valid"); self.set_vtable(activation.gc(), class_vtable); } @@ -606,7 +608,7 @@ impl<'gc> ClassObject<'gc> { self, activation: &mut Activation<'_, 'gc>, receiver: Object<'gc>, - disp_id: u32, + disp_id: usize, arguments: FunctionArgs<'_, 'gc>, ) -> Result, Error<'gc>> { let full_method = self.instance_vtable().get_full_method(disp_id).unwrap(); diff --git a/core/src/avm2/object/script_object.rs b/core/src/avm2/object/script_object.rs index af53ccf43e02..495a488b5041 100644 --- a/core/src/avm2/object/script_object.rs +++ b/core/src/avm2/object/script_object.rs @@ -154,21 +154,13 @@ impl<'gc> ScriptObjectData<'gc> { proto: Option>, vtable: VTable<'gc>, ) -> Self { - let default_slots = vtable.default_slots(); + let slot_table = vtable.slot_table(); // We use `iter` and `collect` rather than setting elements of a Box<[]> // or pushing to a Vec for better performance - let slots = default_slots + let slots = slot_table .iter() - .map(|value| { - if let Some(value) = value { - Lock::new(*value) - } else { - // FIXME this case throws a VerifyError during vtable - // construction in Flash Player - Lock::new(Value::Undefined) - } - }) + .map(|slot_info| Lock::new(slot_info.default_value)) .collect::>(); ScriptObjectData { @@ -298,22 +290,18 @@ impl<'gc> ScriptObjectWrapper<'gc> { } #[inline(always)] - pub fn get_slot(self, id: u32) -> Value<'gc> { + pub fn get_slot(self, id: usize) -> Value<'gc> { self.0 .slots - .get(id as usize) + .get(id) .cloned() .map(|s| s.get()) .expect("Slot index out of bounds") } /// Set a slot by its index. - pub fn set_slot(self, id: u32, value: Value<'gc>, mc: &Mutation<'gc>) { - let slot = self - .0 - .slots - .get(id as usize) - .expect("Slot index out of bounds"); + pub fn set_slot(self, id: usize, value: Value<'gc>, mc: &Mutation<'gc>) { + let slot = self.0.slots.get(id).expect("Slot index out of bounds"); Gc::write(mc, self.0); // SAFETY: We just triggered a write barrier on the Gc. @@ -322,8 +310,8 @@ impl<'gc> ScriptObjectWrapper<'gc> { } /// Retrieve a bound method from the method table. - pub fn get_bound_method(self, id: u32) -> Option> { - self.bound_methods().get(id as usize).and_then(|v| *v) + pub fn get_bound_method(self, id: usize) -> Option> { + self.bound_methods().get(id).and_then(|v| *v) } pub fn has_own_dynamic_property(self, name: &Multiname<'gc>) -> bool { @@ -387,16 +375,16 @@ impl<'gc> ScriptObjectWrapper<'gc> { pub fn install_bound_method( self, mc: &Mutation<'gc>, - disp_id: u32, + disp_id: usize, function: FunctionObject<'gc>, ) { let mut bound_methods = self.bound_methods_mut(mc); - if bound_methods.len() <= disp_id as usize { - bound_methods.resize_with(disp_id as usize + 1, Default::default); + if bound_methods.len() <= disp_id { + bound_methods.resize_with(disp_id + 1, Default::default); } - *bound_methods.get_mut(disp_id as usize).unwrap() = Some(function); + *bound_methods.get_mut(disp_id).unwrap() = Some(function); } /// Get the `Class` for this object. @@ -415,10 +403,7 @@ impl<'gc> ScriptObjectWrapper<'gc> { pub fn set_vtable(&self, mc: &Mutation<'gc>, vtable: VTable<'gc>) { // Make sure both vtables have the same number of slots - assert_eq!( - self.vtable().default_slots().len(), - vtable.default_slots().len() - ); + assert_eq!(self.vtable().slot_count(), vtable.slot_count()); unlock!(Gc::write(mc, self.0), ScriptObjectData, vtable).set(vtable); } diff --git a/core/src/avm2/op.rs b/core/src/avm2/op.rs index bb94462ccfd0..0bc291461fda 100644 --- a/core/src/avm2/op.rs +++ b/core/src/avm2/op.rs @@ -32,7 +32,7 @@ pub enum Op<'gc> { num_args: u32, }, CallMethod { - index: u32, + index: usize, num_args: u32, push_return_value: bool, }, @@ -92,7 +92,7 @@ pub enum Op<'gc> { num_args: u32, }, ConstructSlot { - index: u32, + index: usize, num_args: u32, }, ConstructSuper { @@ -174,7 +174,7 @@ pub enum Op<'gc> { }, GetSlot { // note: 0-indexed, as opposed to FP. - index: u32, + index: usize, }, GetSuper { multiname: Gc<'gc, Multiname<'gc>>, @@ -289,7 +289,7 @@ pub enum Op<'gc> { RShift, SetGlobalSlot { // note: 0-indexed, as opposed to FP. - index: u32, + index: usize, }, SetLocal { index: u32, @@ -307,11 +307,11 @@ pub enum Op<'gc> { }, SetSlot { // note: 0-indexed, as opposed to FP. - index: u32, + index: usize, }, SetSlotNoCoerce { // note: 0-indexed, as opposed to FP. - index: u32, + index: usize, }, SetSuper { multiname: Gc<'gc, Multiname<'gc>>, diff --git a/core/src/avm2/optimizer/type_aware.rs b/core/src/avm2/optimizer/type_aware.rs index bbf98bad188d..a8ac734b9ae4 100644 --- a/core/src/avm2/optimizer/type_aware.rs +++ b/core/src/avm2/optimizer/type_aware.rs @@ -1435,7 +1435,7 @@ fn abstract_interpret_ops<'gc>( return Err(make_error_1026( activation, slot_id + 1, - vtable.default_slots().len(), + vtable.slot_count(), )); }; @@ -1465,7 +1465,7 @@ fn abstract_interpret_ops<'gc>( return Err(make_error_1026( activation, slot_id + 1, - vtable.default_slots().len(), + vtable.slot_count(), )); }; diff --git a/core/src/avm2/property.rs b/core/src/avm2/property.rs index a0e77116ae88..ec16e40c1dc3 100644 --- a/core/src/avm2/property.rs +++ b/core/src/avm2/property.rs @@ -14,10 +14,19 @@ use super::class::Class; #[derive(Debug, Collect, Clone, Copy)] #[collect(no_drop)] pub enum Property { - Virtual { get: Option, set: Option }, - Method { disp_id: u32 }, - Slot { slot_id: u32 }, - ConstSlot { slot_id: u32 }, + Virtual { + get: Option, + set: Option, + }, + Method { + disp_id: usize, + }, + Slot { + slot_id: usize, + }, + ConstSlot { + slot_id: usize, + }, } /// The type of a `Slot`/`ConstSlot` property, represented @@ -118,29 +127,29 @@ impl<'gc> PropertyClass<'gc> { } impl Property { - pub fn new_method(disp_id: u32) -> Self { + pub fn new_method(disp_id: usize) -> Self { Property::Method { disp_id } } - pub fn new_getter(disp_id: u32) -> Self { + pub fn new_getter(disp_id: usize) -> Self { Property::Virtual { get: Some(disp_id), set: None, } } - pub fn new_setter(disp_id: u32) -> Self { + pub fn new_setter(disp_id: usize) -> Self { Property::Virtual { get: None, set: Some(disp_id), } } - pub fn new_slot(slot_id: u32) -> Self { + pub fn new_slot(slot_id: usize) -> Self { Property::Slot { slot_id } } - pub fn new_const_slot(slot_id: u32) -> Self { + pub fn new_const_slot(slot_id: usize) -> Self { Property::ConstSlot { slot_id } } } diff --git a/core/src/avm2/script.rs b/core/src/avm2/script.rs index 09d5fdef0f56..d6f51157c1a0 100644 --- a/core/src/avm2/script.rs +++ b/core/src/avm2/script.rs @@ -468,6 +468,7 @@ impl<'gc> Script<'gc> { Some(object_class.instance_vtable()), mc, ); + let global_obj_vtable = global_obj_vtable.map_err(|e| e.into_avm(activation))?; // Script initializers are always run in "interpreter mode" let script_init_assoc = MethodAssociation::classbound(global_class, true); diff --git a/core/src/avm2/traits.rs b/core/src/avm2/traits.rs index c2ca5211eacb..1bd76b5aeafe 100644 --- a/core/src/avm2/traits.rs +++ b/core/src/avm2/traits.rs @@ -74,29 +74,29 @@ pub enum TraitKind<'gc> { /// A data field on an object instance that can be read from and written /// to. Slot { - slot_id: u32, + slot_id: usize, type_name: Option>>, default_value: Value<'gc>, domain: Domain<'gc>, }, /// A method on an object that can be called. - Method { disp_id: u32, method: Method<'gc> }, + Method { disp_id: usize, method: Method<'gc> }, /// A getter property on an object that can be read. - Getter { disp_id: u32, method: Method<'gc> }, + Getter { disp_id: usize, method: Method<'gc> }, /// A setter property on an object that can be written. - Setter { disp_id: u32, method: Method<'gc> }, + Setter { disp_id: usize, method: Method<'gc> }, /// A class property on an object that can be used to construct more /// objects. - Class { slot_id: u32, class: Class<'gc> }, + Class { slot_id: usize, class: Class<'gc> }, /// A data field on an object that is always a particular value, and cannot /// be overridden. Const { - slot_id: u32, + slot_id: usize, type_name: Option>>, default_value: Value<'gc>, domain: Domain<'gc>, @@ -143,7 +143,7 @@ impl<'gc> Trait<'gc> { name, attributes: trait_attribs_from_abc_traits(abc_trait), kind: TraitKind::Slot { - slot_id, + slot_id: slot_id as usize, type_name, default_value, domain: unit.domain(), @@ -155,7 +155,7 @@ impl<'gc> Trait<'gc> { name, attributes: trait_attribs_from_abc_traits(abc_trait), kind: TraitKind::Method { - disp_id, + disp_id: disp_id as usize, method: unit.load_method(method, false, activation)?, }, metadata: Metadata::from_abc_index(activation, unit, &abc_trait.metadata)?, @@ -164,7 +164,7 @@ impl<'gc> Trait<'gc> { name, attributes: trait_attribs_from_abc_traits(abc_trait), kind: TraitKind::Getter { - disp_id, + disp_id: disp_id as usize, method: unit.load_method(method, false, activation)?, }, metadata: Metadata::from_abc_index(activation, unit, &abc_trait.metadata)?, @@ -173,7 +173,7 @@ impl<'gc> Trait<'gc> { name, attributes: trait_attribs_from_abc_traits(abc_trait), kind: TraitKind::Setter { - disp_id, + disp_id: disp_id as usize, method: unit.load_method(method, false, activation)?, }, metadata: Metadata::from_abc_index(activation, unit, &abc_trait.metadata)?, @@ -182,7 +182,7 @@ impl<'gc> Trait<'gc> { name, attributes: trait_attribs_from_abc_traits(abc_trait), kind: TraitKind::Class { - slot_id, + slot_id: slot_id as usize, class: unit.load_class(class.0, activation)?, }, metadata: Metadata::from_abc_index(activation, unit, &abc_trait.metadata)?, @@ -199,7 +199,7 @@ impl<'gc> Trait<'gc> { name, attributes: trait_attribs_from_abc_traits(abc_trait), kind: TraitKind::Const { - slot_id, + slot_id: slot_id as usize, type_name, default_value, domain: unit.domain(), @@ -230,19 +230,8 @@ impl<'gc> Trait<'gc> { self.attributes.contains(TraitAttributes::OVERRIDE) } - pub fn set_attributes(&mut self, attribs: TraitAttributes) { - self.attributes = attribs; - } - - /// Convenience chaining method that adds the override flag to a trait. - pub fn with_override(mut self) -> Self { - self.attributes |= TraitAttributes::OVERRIDE; - - self - } - /// Get the slot ID of this trait. - pub fn slot_id(&self) -> Option { + pub fn slot_id(&self) -> Option { match self.kind { TraitKind::Slot { slot_id, .. } => Some(slot_id), TraitKind::Method { .. } => None, @@ -253,20 +242,8 @@ impl<'gc> Trait<'gc> { } } - /// Set the slot ID of this trait. - pub fn set_slot_id(&mut self, id: u32) { - match &mut self.kind { - TraitKind::Slot { slot_id, .. } => *slot_id = id, - TraitKind::Method { .. } => {} - TraitKind::Getter { .. } => {} - TraitKind::Setter { .. } => {} - TraitKind::Class { slot_id, .. } => *slot_id = id, - TraitKind::Const { slot_id, .. } => *slot_id = id, - } - } - /// Get the dispatch ID of this trait. - pub fn disp_id(&self) -> Option { + pub fn disp_id(&self) -> Option { match self.kind { TraitKind::Slot { .. } => None, TraitKind::Method { disp_id, .. } => Some(disp_id), @@ -277,18 +254,6 @@ impl<'gc> Trait<'gc> { } } - /// Set the dispatch ID of this trait. - pub fn set_disp_id(&mut self, id: u32) { - match &mut self.kind { - TraitKind::Slot { .. } => {} - TraitKind::Method { disp_id, .. } => *disp_id = id, - TraitKind::Getter { disp_id, .. } => *disp_id = id, - TraitKind::Setter { disp_id, .. } => *disp_id = id, - TraitKind::Class { .. } => {} - TraitKind::Const { .. } => {} - } - } - /// Get the method contained within this trait, if it has one. pub fn as_method(&self) -> Option> { match &self.kind { diff --git a/core/src/avm2/value.rs b/core/src/avm2/value.rs index 912849ddc41e..e6d80ec05f6a 100644 --- a/core/src/avm2/value.rs +++ b/core/src/avm2/value.rs @@ -1266,7 +1266,7 @@ impl<'gc> Value<'gc> { /// This method will panic if called on null or undefined. pub fn call_method( &self, - id: u32, + id: usize, arguments: &[Value<'gc>], activation: &mut Activation<'_, 'gc>, ) -> Result, Error<'gc>> { @@ -1275,7 +1275,7 @@ impl<'gc> Value<'gc> { pub fn call_method_with_args( &self, - id: u32, + id: usize, arguments: FunctionArgs<'_, 'gc>, activation: &mut Activation<'_, 'gc>, ) -> Result, Error<'gc>> { diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index 449fa7f729cf..f1e7c17efcaa 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -862,11 +862,7 @@ fn translate_op<'gc>( AbcOp::SetLocal { index } => Op::SetLocal { index }, AbcOp::Kill { index } => Op::Kill { index }, AbcOp::Call { num_args } => Op::Call { num_args }, - AbcOp::CallMethod { index, num_args } => Op::CallMethod { - index, - num_args, - push_return_value: true, - }, + AbcOp::CallMethod { .. } => unreachable!(), AbcOp::CallProperty { index, num_args } => { let multiname = pool_multiname(activation, translation_unit, index)?; @@ -1035,8 +1031,12 @@ fn translate_op<'gc>( Op::GetDescendants { multiname } } // Turn 1-based representation into 0-based representation - AbcOp::GetSlot { index } => Op::GetSlot { index: index - 1 }, - AbcOp::SetSlot { index } => Op::SetSlot { index: index - 1 }, + AbcOp::GetSlot { index } => Op::GetSlot { + index: index as usize - 1, + }, + AbcOp::SetSlot { index } => Op::SetSlot { + index: index as usize - 1, + }, AbcOp::GetGlobalSlot { index } => { let first_op = if activation.outer().is_empty() { Op::GetScopeObject { index: 0 } @@ -1045,9 +1045,16 @@ fn translate_op<'gc>( }; // GetGlobalSlot is split into two ops - return Ok((first_op, Some(Op::GetSlot { index }))); + return Ok(( + first_op, + Some(Op::GetSlot { + index: index as usize - 1, + }), + )); } - AbcOp::SetGlobalSlot { index } => Op::SetGlobalSlot { index: index - 1 }, + AbcOp::SetGlobalSlot { index } => Op::SetGlobalSlot { + index: index as usize - 1, + }, AbcOp::Construct { num_args } => Op::Construct { num_args }, AbcOp::ConstructProp { index, num_args } => { diff --git a/core/src/avm2/vtable.rs b/core/src/avm2/vtable.rs index 4e98b45cd375..819ee2faadd9 100644 --- a/core/src/avm2/vtable.rs +++ b/core/src/avm2/vtable.rs @@ -1,4 +1,5 @@ use crate::avm2::activation::Activation; +use crate::avm2::error::{make_error_1107, Error}; use crate::avm2::metadata::Metadata; use crate::avm2::method::Method; use crate::avm2::object::{ClassObject, FunctionObject}; @@ -7,7 +8,7 @@ use crate::avm2::property_map::PropertyMap; use crate::avm2::scope::ScopeChain; use crate::avm2::traits::{Trait, TraitKind}; use crate::avm2::value::Value; -use crate::avm2::{Class, Error, Multiname, Namespace, QName}; +use crate::avm2::{Class, Multiname, Namespace, QName}; use crate::context::UpdateContext; use crate::string::{AvmString, StringContext}; use gc_arena::barrier::{field, unlock}; @@ -29,18 +30,15 @@ struct VTableData<'gc> { resolved_traits: PropertyMap<'gc, Property>, /// Use hashmaps for the metadata tables because metadata will rarely be present on traits - slot_metadata_table: HashMap]>>, + slot_metadata_table: HashMap]>>, - disp_metadata_table: HashMap]>>, + disp_metadata_table: HashMap]>>, - /// Stores the `PropertyClass` for each slot, - /// indexed by `slot_id` - slot_classes: Box<[Lock>]>, + /// method_table is indexed by `slot_id` + slot_table: Box<[SlotInfo<'gc>]>, /// method_table is indexed by `disp_id` method_table: Box<[ClassBoundMethod<'gc>]>, - - default_slots: Box<[Option>]>, } impl PartialEq for VTable<'_> { @@ -49,6 +47,13 @@ impl PartialEq for VTable<'_> { } } +#[derive(Collect, Clone)] +#[collect(no_drop)] +pub struct SlotInfo<'gc> { + property_class: Lock>, + pub default_value: Value<'gc>, +} + // TODO: it would be nice to remove the Option-ness from `scope` field for this // to be more intuitive and cheaper #[derive(Collect, Clone)] @@ -78,14 +83,14 @@ impl<'gc> VTable<'gc> { scope: Option>, superclass_vtable: Option, mc: &Mutation<'gc>, - ) -> Self { + ) -> Result { let this = Self::init_vtable( defining_class_def, super_class_obj, scope, superclass_vtable, - ); - VTable(Gc::new(mc, this)) + )?; + Ok(VTable(Gc::new(mc, this))) } /// Like `VTable::new`, but also copies properties from the defining class' interfaces. @@ -95,40 +100,46 @@ impl<'gc> VTable<'gc> { scope: Option>, superclass_vtable: Option, context: &UpdateContext<'gc>, - ) -> Self { + ) -> Result { let mut this = Self::init_vtable( defining_class_def, super_class_obj, scope, superclass_vtable, - ); + )?; Self::copy_interface_properties(&mut this, defining_class_def, context); - VTable(Gc::new(context.gc(), this)) + + Ok(VTable(Gc::new(context.gc(), this))) } pub fn resolved_traits(self) -> &'gc PropertyMap<'gc, Property> { &Gc::as_ref(self.0).resolved_traits } - pub fn get_metadata_for_slot(self, slot_id: u32) -> Option<&'gc [Metadata<'gc>]> { + pub fn get_metadata_for_slot(self, slot_id: usize) -> Option<&'gc [Metadata<'gc>]> { Gc::as_ref(self.0) .slot_metadata_table .get(&slot_id) .map(|v| &**v) } - pub fn get_metadata_for_disp(self, disp_id: u32) -> Option<&'gc [Metadata<'gc>]> { + pub fn get_metadata_for_disp(self, disp_id: usize) -> Option<&'gc [Metadata<'gc>]> { Gc::as_ref(self.0) .disp_metadata_table .get(&disp_id) .map(|v| &**v) } - pub fn slot_class_name(self, context: &mut StringContext<'gc>, slot_id: u32) -> AvmString<'gc> { + pub fn slot_class_name( + self, + context: &mut StringContext<'gc>, + slot_id: usize, + ) -> AvmString<'gc> { self.0 - .slot_classes - .get(slot_id as usize) + .slot_table + .get(slot_id) .expect("Invalid slot ID") + .property_class .get() .get_name(context) } @@ -154,21 +165,18 @@ impl<'gc> VTable<'gc> { /// Coerces `value` to the type of the slot with id `slot_id` pub fn coerce_trait_value( self, - slot_id: u32, + slot_id: usize, value: Value<'gc>, activation: &mut Activation<'_, 'gc>, ) -> Result, Error<'gc>> { - let slot_id = slot_id as usize; - let mut slot_class = self.0.slot_classes[slot_id].get(); + let mut slot_class = self.0.slot_table[slot_id].property_class.get(); let (value, changed) = slot_class.coerce(activation, value)?; // Calling coerce modified `PropertyClass` to cache the class lookup, // so store the new value back in the vtable. if changed { - let write = Gc::write(activation.gc(), self.0); - let slots = field!(write, VTableData, slot_classes).as_deref(); - slots[slot_id].unlock().set(slot_class); + self.set_slot_class(activation.gc(), slot_id, slot_class); } Ok(value) } @@ -177,29 +185,35 @@ impl<'gc> VTable<'gc> { self.resolved_traits().get_for_multiname(name).is_some() } - pub fn get_method(self, disp_id: u32) -> Option> { - self.0 - .method_table - .get(disp_id as usize) - .cloned() - .map(|x| x.method) + pub fn get_method(self, disp_id: usize) -> Option> { + self.0.method_table.get(disp_id).cloned().map(|x| x.method) } - pub fn get_full_method(self, disp_id: u32) -> Option<&'gc ClassBoundMethod<'gc>> { - Gc::as_ref(self.0).method_table.get(disp_id as usize) + pub fn get_full_method(self, disp_id: usize) -> Option<&'gc ClassBoundMethod<'gc>> { + Gc::as_ref(self.0).method_table.get(disp_id) } - pub fn default_slots(&self) -> &[Option>] { - &self.0.default_slots + pub fn slot_count(self) -> usize { + self.0.slot_table.len() } - pub fn slot_class(self, slot_id: u32) -> Option> { - self.0.slot_classes.get(slot_id as usize).map(Lock::get) + pub fn slot_table(&self) -> &[SlotInfo<'gc>] { + &self.0.slot_table } - pub fn set_slot_class(self, mc: &Mutation<'gc>, slot_id: u32, value: PropertyClass<'gc>) { - let slots = field!(Gc::write(mc, self.0), VTableData, slot_classes).as_deref(); - slots[slot_id as usize].unlock().set(value); + pub fn slot_class(self, slot_id: usize) -> Option> { + self.0 + .slot_table + .get(slot_id) + .map(|e| e.property_class.get()) + } + + pub fn set_slot_class(self, mc: &Mutation<'gc>, slot_id: usize, value: PropertyClass<'gc>) { + let slots = field!(Gc::write(mc, self.0), VTableData, slot_table).as_deref(); + let slot = &slots[slot_id]; + let slot = unlock!(slot, SlotInfo, property_class); + + slot.set(value); } pub fn replace_scopes_with(self, mc: &Mutation<'gc>, new_scope: ScopeChain<'gc>) { @@ -214,7 +228,7 @@ impl<'gc> VTable<'gc> { super_class_obj: Option>, scope: Option>, superclass_vtable: Option, - ) -> VTableData<'gc> { + ) -> Result, VTableInitError> { // Let's talk about slot_ids and disp_ids. // Specification is one thing, but reality is another. @@ -269,17 +283,23 @@ impl<'gc> VTable<'gc> { let mut resolved_traits = PropertyMap::new(); let mut slot_metadata_table = HashMap::new(); let mut disp_metadata_table = HashMap::new(); + let mut slot_table = Vec::new(); let mut method_table = Vec::new(); - let mut default_slots = Vec::new(); - let mut slot_classes = Vec::new(); + + // Subclasses cannot "override" slots in superclasses, so we only + // maintain the list of slots that were declared by the subclass. At the + // end of this method, we will append this list to `slot_table`. + let mut new_slots = Vec::new(); + let mut first_slot_offset = 0; if let Some(superclass_vtable) = superclass_vtable { resolved_traits = superclass_vtable.resolved_traits().clone(); slot_metadata_table = superclass_vtable.0.slot_metadata_table.clone(); disp_metadata_table = superclass_vtable.0.disp_metadata_table.clone(); - slot_classes.extend_from_slice(&superclass_vtable.0.slot_classes); + slot_table.extend_from_slice(&superclass_vtable.0.slot_table); method_table.extend_from_slice(&superclass_vtable.0.method_table); - default_slots.extend_from_slice(&superclass_vtable.0.default_slots); + + first_slot_offset = superclass_vtable.slot_count(); if let Some(protected_namespace) = defining_class_def.protected_namespace() { if let Some(super_protected_namespace) = superclass_vtable.0.protected_namespace { @@ -309,12 +329,12 @@ impl<'gc> VTable<'gc> { disp_metadata_table.insert(*disp_id, metadata); } - method_table[*disp_id as usize] = entry; + method_table[*disp_id] = entry; } // note: ideally overwriting other property types // should be a VerifyError - _ => { - let disp_id = method_table.len() as u32; + None => { + let disp_id = method_table.len(); method_table.push(entry); resolved_traits .insert(trait_data.name(), Property::new_method(disp_id)); @@ -323,6 +343,9 @@ impl<'gc> VTable<'gc> { disp_metadata_table.insert(disp_id, metadata); } } + _ => unreachable!( + "`Class::validate_class` ensures overridden trait is correct" + ), } } TraitKind::Getter { method, .. } => { @@ -339,10 +362,10 @@ impl<'gc> VTable<'gc> { disp_metadata_table.insert(*disp_id, metadata); } - method_table[*disp_id as usize] = entry; + method_table[*disp_id] = entry; } Some(Property::Virtual { get, .. }) => { - let disp_id = method_table.len() as u32; + let disp_id = method_table.len(); *get = Some(disp_id); method_table.push(entry); @@ -350,8 +373,8 @@ impl<'gc> VTable<'gc> { disp_metadata_table.insert(disp_id, metadata); } } - _ => { - let disp_id = method_table.len() as u32; + None => { + let disp_id = method_table.len(); method_table.push(entry); resolved_traits .insert(trait_data.name(), Property::new_getter(disp_id)); @@ -360,6 +383,9 @@ impl<'gc> VTable<'gc> { disp_metadata_table.insert(disp_id, metadata); } } + _ => unreachable!( + "`Class::validate_class` ensures overridden trait is correct" + ), } } TraitKind::Setter { method, .. } => { @@ -376,10 +402,10 @@ impl<'gc> VTable<'gc> { disp_metadata_table.insert(*disp_id, metadata); } - method_table[*disp_id as usize] = entry; + method_table[*disp_id] = entry; } Some(Property::Virtual { set, .. }) => { - let disp_id = method_table.len() as u32; + let disp_id = method_table.len(); method_table.push(entry); *set = Some(disp_id); @@ -387,8 +413,8 @@ impl<'gc> VTable<'gc> { disp_metadata_table.insert(disp_id, metadata); } } - _ => { - let disp_id = method_table.len() as u32; + None => { + let disp_id = method_table.len(); method_table.push(entry); resolved_traits .insert(trait_data.name(), Property::new_setter(disp_id)); @@ -397,6 +423,9 @@ impl<'gc> VTable<'gc> { disp_metadata_table.insert(disp_id, metadata); } } + _ => unreachable!( + "`Class::validate_class` ensures overridden trait is correct" + ), } } TraitKind::Slot { slot_id, .. } @@ -404,78 +433,95 @@ impl<'gc> VTable<'gc> { | TraitKind::Class { slot_id, .. } => { let slot_id = *slot_id; - let value = trait_to_default_value(trait_data); - let value = Some(value); + let default_value = trait_to_default_value(trait_data); + + let prop_class = match trait_data.kind() { + TraitKind::Slot { + type_name, domain, .. + } + | TraitKind::Const { + type_name, domain, .. + } => PropertyClass::name(*type_name, *domain), + TraitKind::Class { class, .. } => PropertyClass::Class( + class.c_class().expect("Trait should hold an i_class"), + ), + _ => unreachable!(), + }; + + let slot_info = SlotInfo { + property_class: Lock::new(prop_class), + default_value, + }; let new_slot_id = if slot_id == 0 { - default_slots.push(value); - default_slots.len() as u32 - 1 + new_slots.push(Some(slot_info)); + new_slots.len() - 1 } else { // it's non-zero, so let's turn it from 1-based to 0-based. let slot_id = slot_id - 1; - if let Some(Some(_)) = default_slots.get(slot_id as usize) { - // slot_id conflict - default_slots.push(value); - default_slots.len() as u32 - 1 + if slot_id < first_slot_offset { + // Conflict: subclass attempted to use slot id of + // slot in superclass + return Err(VTableInitError::SlotConflict); + } + + // now make the slot id relative to the start of this + // subclass's slots + let slot_id = slot_id - first_slot_offset; + + if let Some(Some(_)) = new_slots.get(slot_id) { + // Conflict: subclass attempted to use slot id that + // it already used + return Err(VTableInitError::SlotConflict); } else { - if slot_id as usize >= default_slots.len() { - default_slots.resize_with(slot_id as usize + 1, Default::default); + if slot_id >= new_slots.len() { + new_slots.resize(slot_id + 1, None); } - default_slots[slot_id as usize] = value; + new_slots[slot_id] = Some(slot_info); + slot_id } }; - if new_slot_id as usize >= slot_classes.len() { - // We will overwrite `PropertyClass::Any` when we process the slots - // with the ids that we just skipped over. - slot_classes - .resize(new_slot_id as usize + 1, Lock::new(PropertyClass::Any)); - } - - let (new_prop, new_class) = match trait_data.kind() { - TraitKind::Slot { - type_name, domain, .. - } => ( - Property::new_slot(new_slot_id), - PropertyClass::name(*type_name, *domain), - ), - TraitKind::Const { - type_name, domain, .. - } => ( - Property::new_const_slot(new_slot_id), - PropertyClass::name(*type_name, *domain), - ), - TraitKind::Class { class, .. } => ( - Property::new_const_slot(new_slot_id), - PropertyClass::Class( - class.c_class().expect("Trait should hold an i_class"), - ), - ), - _ => unreachable!(), - }; - - resolved_traits.insert(trait_data.name(), new_prop); + // Convert new_slot_id to an absolute slot id + let new_slot_id = new_slot_id + first_slot_offset; if let Some(metadata) = trait_data.metadata() { slot_metadata_table.insert(new_slot_id, metadata); } - slot_classes[new_slot_id as usize] = Lock::new(new_class); + let property = match trait_data.kind() { + TraitKind::Slot { .. } => Property::new_slot(new_slot_id), + TraitKind::Const { .. } | TraitKind::Class { .. } => { + Property::new_const_slot(new_slot_id) + } + _ => unreachable!(), + }; + + resolved_traits.insert(trait_data.name(), property); } } } - VTableData { + // Append the new slots to the slot table now + for slot in new_slots { + if let Some(slot) = slot { + slot_table.push(slot); + } else { + // Holes in the slot table throw an error in Flash Player + return Err(VTableInitError::SlotHole); + } + } + + Ok(VTableData { scope, protected_namespace: defining_class_def.protected_namespace(), resolved_traits, slot_metadata_table, disp_metadata_table, + slot_table: slot_table.into_boxed_slice(), method_table: method_table.into_boxed_slice(), - default_slots: default_slots.into_boxed_slice(), - slot_classes: slot_classes.into_boxed_slice(), - } + }) } fn copy_interface_properties( @@ -518,7 +564,7 @@ impl<'gc> VTable<'gc> { self, activation: &mut Activation<'_, 'gc>, receiver: Value<'gc>, - disp_id: u32, + disp_id: usize, ) -> Option> { self.get_full_method(disp_id) .map(|method| Self::bind_method(activation, receiver, method)) @@ -550,6 +596,22 @@ impl<'gc> VTable<'gc> { } } +#[derive(Debug)] +pub enum VTableInitError { + SlotConflict, + SlotHole, +} + +impl VTableInitError { + pub fn into_avm<'gc>(self, activation: &mut Activation<'_, 'gc>) -> Error<'gc> { + match self { + VTableInitError::SlotConflict | VTableInitError::SlotHole => { + make_error_1107(activation) + } + } + } +} + fn trait_to_default_value<'gc>(trait_data: &Trait<'gc>) -> Value<'gc> { match trait_data.kind() { TraitKind::Slot { default_value, .. } => *default_value, diff --git a/tests/tests/swfs/avm2/getglobalslot/output.txt b/tests/tests/swfs/avm2/getglobalslot/output.txt new file mode 100644 index 000000000000..912dd71acda4 --- /dev/null +++ b/tests/tests/swfs/avm2/getglobalslot/output.txt @@ -0,0 +1 @@ +[class Test] diff --git a/tests/tests/swfs/avm2/getglobalslot/test.swf b/tests/tests/swfs/avm2/getglobalslot/test.swf new file mode 100644 index 000000000000..f962e15ebe3c Binary files /dev/null and b/tests/tests/swfs/avm2/getglobalslot/test.swf differ diff --git a/tests/tests/swfs/avm2/getglobalslot/test.toml b/tests/tests/swfs/avm2/getglobalslot/test.toml new file mode 100644 index 000000000000..dbee897f5863 --- /dev/null +++ b/tests/tests/swfs/avm2/getglobalslot/test.toml @@ -0,0 +1 @@ +num_frames = 1