Ensure that called setters are properly resolved so that errors in setters propagate up the Rust stack correctly.

The previous system for handling setters would execute the setter and then return a value to indicate whether or not the caller needed to resolve a stack frame. However, no caller of `Property.set` actually did this. Ergo, errors in setters and getters would not resolve up the stack at the correct time.

This problem also exists in AVM1 but is far less noticable as AVM1 only has two very uncommon runtime errors and very few movies use `throw`.
This commit is contained in:
David Wendt 2020-02-28 20:55:09 -05:00
parent b8106d24d2
commit 54b792ef3a
4 changed files with 56 additions and 40 deletions

View File

@ -451,7 +451,7 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<ReturnValue<'gc>, Error> {
self.0 self.0
.write(context.gc_context) .write(context.gc_context)
.base .base
@ -465,7 +465,7 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<ReturnValue<'gc>, Error> {
self.0 self.0
.write(context.gc_context) .write(context.gc_context)
.base .base

View File

@ -68,6 +68,9 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
} }
/// Set a property on this specific object. /// Set a property on this specific object.
///
/// This function returns a `ReturnValue` which should be resolved. The
/// resulting `Value` is unimportant and should be discarded.
fn set_property_local( fn set_property_local(
self, self,
reciever: Object<'gc>, reciever: Object<'gc>,
@ -75,7 +78,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error>; ) -> Result<ReturnValue<'gc>, Error>;
/// Set a property by it's QName. /// Set a property by it's QName.
fn set_property( fn set_property(
@ -87,7 +90,10 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<(), Error> {
if self.has_own_virtual_setter(name) { if self.has_own_virtual_setter(name) {
return self.set_property_local(reciever, name, value, avm, context); self.set_property_local(reciever, name, value, avm, context)?
.resolve(avm, context)?;
return Ok(());
} }
let mut proto = self.proto(); let mut proto = self.proto();
@ -102,10 +108,17 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
proto = my_proto.proto(); proto = my_proto.proto();
} }
reciever.set_property_local(reciever, name, value, avm, context) reciever
.set_property_local(reciever, name, value, avm, context)?
.resolve(avm, context)?;
Ok(())
} }
/// Init a property on this specific object. /// Init a property on this specific object.
///
/// This function returns a `ReturnValue` which should be resolved. The
/// resulting `Value` is unimportant and should be discarded.
fn init_property_local( fn init_property_local(
self, self,
reciever: Object<'gc>, reciever: Object<'gc>,
@ -113,7 +126,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error>; ) -> Result<ReturnValue<'gc>, Error>;
/// Init a property by it's QName. /// Init a property by it's QName.
fn init_property( fn init_property(
@ -125,7 +138,10 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<(), Error> {
if self.has_own_virtual_setter(name) { if self.has_own_virtual_setter(name) {
return self.init_property_local(reciever, name, value, avm, context); self.init_property_local(reciever, name, value, avm, context)?
.resolve(avm, context)?;
return Ok(());
} }
let mut proto = self.proto(); let mut proto = self.proto();
@ -140,7 +156,11 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
proto = my_proto.proto(); proto = my_proto.proto();
} }
reciever.init_property_local(reciever, name, value, avm, context) reciever
.init_property_local(reciever, name, value, avm, context)?
.resolve(avm, context)?;
Ok(())
} }
/// Retrieve a slot by it's index. /// Retrieve a slot by it's index.

View File

@ -153,10 +153,8 @@ impl<'gc> Property<'gc> {
/// Set a property slot. /// Set a property slot.
/// ///
/// This function returns `true` if the set has completed, or `false` if /// This function returns a `ReturnValue` which should be resolved. The
/// it has not yet occured. If `false`, and you need to run code after the /// resulting `Value` is unimportant and should be discarded.
/// set has occured, you must recursively execute the top-most frame via
/// `run_current_frame`.
/// ///
/// This function cannot set slot properties and will panic if one /// This function cannot set slot properties and will panic if one
/// is encountered. /// is encountered.
@ -166,21 +164,20 @@ impl<'gc> Property<'gc> {
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>, this: Object<'gc>,
new_value: impl Into<Value<'gc>>, new_value: impl Into<Value<'gc>>,
) -> Result<bool, Error> { ) -> Result<ReturnValue<'gc>, Error> {
match self { match self {
Property::Virtual { set, .. } => { Property::Virtual { set, .. } => {
if let Some(function) = set { if let Some(function) = set {
let return_value = function.exec( return function.exec(
Some(this), Some(this),
&[new_value.into()], &[new_value.into()],
avm, avm,
context, context,
this.proto(), this.proto(),
)?; );
Ok(return_value.is_immediate())
} else {
Ok(true)
} }
Ok(Value::Undefined.into())
} }
Property::Stored { Property::Stored {
value, attributes, .. value, attributes, ..
@ -189,7 +186,7 @@ impl<'gc> Property<'gc> {
*value = new_value.into(); *value = new_value.into();
} }
Ok(true) Ok(Value::Undefined.into())
} }
Property::Slot { .. } => panic!("Cannot recursively set slots"), Property::Slot { .. } => panic!("Cannot recursively set slots"),
} }
@ -202,10 +199,8 @@ impl<'gc> Property<'gc> {
/// properties, at least once. Virtual properties with no setter cannot be /// properties, at least once. Virtual properties with no setter cannot be
/// initialized. /// initialized.
/// ///
/// This function returns `true` if the set has completed, or `false` if /// This function returns a `ReturnValue` which should be resolved. The
/// it has not yet occured. If `false`, and you need to run code after the /// resulting `Value` is unimportant and should be discarded.
/// set has occured, you must recursively execute the top-most frame via
/// `run_current_frame`.
/// ///
/// This function cannot initialize slot properties and will panic if one /// This function cannot initialize slot properties and will panic if one
/// is encountered. /// is encountered.
@ -215,26 +210,25 @@ impl<'gc> Property<'gc> {
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>, this: Object<'gc>,
new_value: impl Into<Value<'gc>>, new_value: impl Into<Value<'gc>>,
) -> Result<bool, Error> { ) -> Result<ReturnValue<'gc>, Error> {
match self { match self {
Property::Virtual { set, .. } => { Property::Virtual { set, .. } => {
if let Some(function) = set { if let Some(function) = set {
let return_value = function.exec( return function.exec(
Some(this), Some(this),
&[new_value.into()], &[new_value.into()],
avm, avm,
context, context,
this.proto(), this.proto(),
)?; );
Ok(return_value.is_immediate())
} else {
Ok(true)
} }
Ok(Value::Undefined.into())
} }
Property::Stored { value, .. } => { Property::Stored { value, .. } => {
*value = new_value.into(); *value = new_value.into();
Ok(true) Ok(Value::Undefined.into())
} }
Property::Slot { .. } => panic!("Cannot recursively init slots"), Property::Slot { .. } => panic!("Cannot recursively init slots"),
} }

View File

@ -54,7 +54,7 @@ impl<'gc> TObject<'gc> for ScriptObject<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<ReturnValue<'gc>, Error> {
self.0 self.0
.write(context.gc_context) .write(context.gc_context)
.set_property_local(reciever, name, value, avm, context) .set_property_local(reciever, name, value, avm, context)
@ -67,7 +67,7 @@ impl<'gc> TObject<'gc> for ScriptObject<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<ReturnValue<'gc>, Error> {
self.0 self.0
.write(context.gc_context) .write(context.gc_context)
.init_property_local(reciever, name, value, avm, context) .init_property_local(reciever, name, value, avm, context)
@ -241,20 +241,21 @@ impl<'gc> ScriptObjectData<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<ReturnValue<'gc>, Error> {
if let Some(prop) = self.values.get_mut(name) { if let Some(prop) = self.values.get_mut(name) {
if let Some(slot_id) = prop.slot_id() { if let Some(slot_id) = prop.slot_id() {
self.set_slot(slot_id, value, context.gc_context)?; self.set_slot(slot_id, value, context.gc_context)?;
Ok(Value::Undefined.into())
} else { } else {
prop.set(avm, context, reciever, value)?; prop.set(avm, context, reciever, value)
} }
} else { } else {
//TODO: Not all classes are dynamic like this //TODO: Not all classes are dynamic like this
self.values self.values
.insert(name.clone(), Property::new_dynamic_property(value)); .insert(name.clone(), Property::new_dynamic_property(value));
}
Ok(()) Ok(Value::Undefined.into())
}
} }
pub fn init_property_local( pub fn init_property_local(
@ -264,20 +265,21 @@ impl<'gc> ScriptObjectData<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<ReturnValue<'gc>, Error> {
if let Some(prop) = self.values.get_mut(name) { if let Some(prop) = self.values.get_mut(name) {
if let Some(slot_id) = prop.slot_id() { if let Some(slot_id) = prop.slot_id() {
self.init_slot(slot_id, value, context.gc_context)?; self.init_slot(slot_id, value, context.gc_context)?;
Ok(Value::Undefined.into())
} else { } else {
prop.init(avm, context, reciever, value)?; prop.init(avm, context, reciever, value)
} }
} else { } else {
//TODO: Not all classes are dynamic like this //TODO: Not all classes are dynamic like this
self.values self.values
.insert(name.clone(), Property::new_dynamic_property(value)); .insert(name.clone(), Property::new_dynamic_property(value));
}
Ok(()) Ok(Value::Undefined.into())
}
} }
pub fn delete_property(&mut self, name: &QName) -> bool { pub fn delete_property(&mut self, name: &QName) -> bool {