From 2aa5b62b444315869b485deedc8b03a65a8c85a6 Mon Sep 17 00:00:00 2001 From: David Wendt Date: Fri, 25 Oct 2019 23:21:14 -0400 Subject: [PATCH] Make most code that might touch user-defined functions falliable. --- core/src/avm1.rs | 36 +++++++++----- core/src/avm1/activation.rs | 13 ++--- core/src/avm1/function.rs | 13 +++-- core/src/avm1/globals.rs | 40 +++++++++------- core/src/avm1/globals/math.rs | 44 ++++++++++------- core/src/avm1/movie_clip.rs | 30 ++++++------ core/src/avm1/object.rs | 90 +++++++++++++++++++++-------------- core/src/avm1/scope.rs | 15 +++--- core/src/avm1/tests.rs | 6 ++- core/src/avm1/value.rs | 2 +- 10 files changed, 166 insertions(+), 123 deletions(-) diff --git a/core/src/avm1.rs b/core/src/avm1.rs index 318914e18..12b3e69c8 100644 --- a/core/src/avm1.rs +++ b/core/src/avm1.rs @@ -1,6 +1,7 @@ use crate::avm1::function::Avm1Function; use crate::avm1::globals::create_globals; use crate::avm1::object::Object; +use crate::avm1::return_value::ReturnValue; use crate::avm1::return_value::ReturnValue::*; use crate::backend::navigator::NavigationMethod; use crate::context::UpdateContext; @@ -108,10 +109,17 @@ impl<'gc> Avm1<'gc> { for k in keys { let v = locals.read().get(&k, self, context, locals); - //TODO: Support on-stack properties - if let Immediate(instant_value) = v { - form_values.insert(k, instant_value.clone().into_string()); - } + //TODO: What happens if an error occurs inside a virtual property? + form_values.insert( + k, + v.ok() + .unwrap_or(ReturnValue::Immediate(Value::Undefined)) + .resolve(self, context) + .ok() + .unwrap_or(Value::Undefined) + .clone() + .into_string(), + ); } form_values @@ -729,7 +737,7 @@ impl<'gc> Avm1<'gc> { .unwrap() .clone() .read() - .resolve(fn_name.as_string()?, self, context) + .resolve(fn_name.as_string()?, self, context)? .resolve(self, context)?; let this = context.active_clip.read().object().as_object()?.to_owned(); target_fn.call(self, context, this, &args)?.push(self); @@ -764,7 +772,7 @@ impl<'gc> Avm1<'gc> { let callable = object .as_object()? .read() - .get(&name, self, context, target) + .get(&name, self, context, target)? .resolve(self, context)?; if let Value::Undefined = callable { @@ -949,7 +957,7 @@ impl<'gc> Avm1<'gc> { .current_stack_frame() .unwrap() .read() - .resolve(name, self, context) + .resolve(name, self, context)? .resolve(self, context)?; match object { @@ -1016,7 +1024,7 @@ impl<'gc> Avm1<'gc> { let name = name_val.into_string(); let object = self.pop()?.as_object()?; let this = self.current_stack_frame().unwrap().read().this_cell(); - object.read().get(&name, self, context, this).push(self); + object.read().get(&name, self, context, this)?.push(self); Ok(()) } @@ -1105,7 +1113,7 @@ impl<'gc> Avm1<'gc> { if object.read().has_property(var_name) { object .read() - .get(var_name, self, context, object) + .get(var_name, self, context, object)? .push(self); } else { self.push(Value::Undefined); @@ -1118,7 +1126,7 @@ impl<'gc> Avm1<'gc> { self.current_stack_frame() .unwrap() .read() - .resolve(path, self, context) + .resolve(path, self, context)? .push(self); } else { self.push(Value::Undefined); @@ -1556,7 +1564,7 @@ impl<'gc> Avm1<'gc> { object .write(context.gc_context) - .set(name, value, self, context, this); + .set(&name, value, self, context, this)?; Ok(()) } @@ -1628,13 +1636,15 @@ impl<'gc> Avm1<'gc> { self, context, this, - ); + )?; } } } else { let this = self.current_stack_frame().unwrap().read().this_cell(); let scope = self.current_stack_frame().unwrap().read().scope_cell(); - let unused_value = scope.read().overwrite(var_path, value, self, context, this); + let unused_value = scope + .read() + .overwrite(var_path, value, self, context, this)?; if let Some(value) = unused_value { self.current_stack_frame().unwrap().read().define( var_path, diff --git a/core/src/avm1/activation.rs b/core/src/avm1/activation.rs index aae40f9b1..567219feb 100644 --- a/core/src/avm1/activation.rs +++ b/core/src/avm1/activation.rs @@ -4,7 +4,7 @@ use crate::avm1::object::Object; use crate::avm1::return_value::ReturnValue; use crate::avm1::scope::Scope; use crate::avm1::stack_continuation::StackContinuation; -use crate::avm1::{Avm1, Value}; +use crate::avm1::{Avm1, Error, Value}; use crate::context::UpdateContext; use crate::tag_utils::SwfSlice; use gc_arena::{GcCell, MutationContext}; @@ -273,20 +273,21 @@ impl<'gc> Activation<'gc> { /// Resolve a particular named local variable within this activation. /// /// Because scopes are object chains, the same rules for `Object::get` - /// still apply here. This function is allowed to yield `None` to indicate - /// that the result will be calculated on the AVM stack. + /// still apply here. pub fn resolve( &self, name: &str, avm: &mut Avm1<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> ReturnValue<'gc> { + ) -> Result, Error> { if name == "this" { - return ReturnValue::Immediate(Value::Object(self.this)); + return Ok(ReturnValue::Immediate(Value::Object(self.this))); } if name == "arguments" && self.arguments.is_some() { - return ReturnValue::Immediate(Value::Object(self.arguments.unwrap())); + return Ok(ReturnValue::Immediate(Value::Object( + self.arguments.unwrap(), + ))); } self.scope().resolve(name, avm, context, self.this) diff --git a/core/src/avm1/function.rs b/core/src/avm1/function.rs index 13ffe8619..b86168713 100644 --- a/core/src/avm1/function.rs +++ b/core/src/avm1/function.rs @@ -5,7 +5,7 @@ use crate::avm1::object::{Attribute::*, Object}; use crate::avm1::return_value::ReturnValue; use crate::avm1::scope::Scope; use crate::avm1::value::Value; -use crate::avm1::{Avm1, UpdateContext}; +use crate::avm1::{Avm1, Error, UpdateContext}; use crate::tag_utils::SwfSlice; use gc_arena::GcCell; use swf::avm1::types::FunctionParam; @@ -29,7 +29,7 @@ pub type NativeFunction<'gc> = fn( &mut UpdateContext<'_, 'gc, '_>, GcCell<'gc, Object<'gc>>, &[Value<'gc>], -) -> ReturnValue<'gc>; +) -> Result, Error>; /// Represents a function defined in the AVM1 runtime, either through /// `DefineFunction` or `DefineFunction2`. @@ -184,7 +184,7 @@ impl<'gc> Executable<'gc> { ac: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], - ) -> ReturnValue<'gc> { + ) -> Result, Error> { match self { Executable::Native(nf) => nf(avm, ac, this, args), Executable::Action(af) => { @@ -265,9 +265,8 @@ impl<'gc> Executable<'gc> { if af.preload_parent { let parent = child_scope .read() - .resolve("_parent", avm, ac, this) - .resolve(avm, ac) - .unwrap(); + .resolve("_parent", avm, ac, this)? + .resolve(avm, ac)?; frame_cell.write(ac.gc_context).set_local_register( preload_r, @@ -297,7 +296,7 @@ impl<'gc> Executable<'gc> { } avm.insert_stack_frame(frame_cell); - ReturnValue::ResultOf(frame_cell) + Ok(ReturnValue::ResultOf(frame_cell)) } } } diff --git a/core/src/avm1/globals.rs b/core/src/avm1/globals.rs index d189c4cd8..243332bc1 100644 --- a/core/src/avm1/globals.rs +++ b/core/src/avm1/globals.rs @@ -1,6 +1,6 @@ use crate::avm1::fscommand; use crate::avm1::return_value::ReturnValue; -use crate::avm1::{Avm1, Object, UpdateContext, Value}; +use crate::avm1::{Avm1, Error, Object, UpdateContext, Value}; use crate::backend::navigator::NavigationMethod; use enumset::EnumSet; use gc_arena::{GcCell, MutationContext}; @@ -14,13 +14,13 @@ pub fn getURL<'a, 'gc>( context: &mut UpdateContext<'a, 'gc, '_>, _this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], -) -> ReturnValue<'gc> { +) -> Result, Error> { //TODO: Error behavior if no arguments are present if let Some(url_val) = args.get(0) { let url = url_val.clone().into_string(); if let Some(fscommand) = fscommand::parse(&url) { fscommand::handle(fscommand, avm, context); - return ReturnValue::Immediate(Value::Undefined); + return Ok(ReturnValue::Immediate(Value::Undefined)); } let window = args.get(1).map(|v| v.clone().into_string()); @@ -34,7 +34,7 @@ pub fn getURL<'a, 'gc>( context.navigator.navigate_to_url(url, window, vars_method); } - ReturnValue::Immediate(Value::Undefined) + Ok(ReturnValue::Immediate(Value::Undefined)) } pub fn random<'gc>( @@ -42,12 +42,12 @@ pub fn random<'gc>( action_context: &mut UpdateContext<'_, 'gc, '_>, _this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], -) -> ReturnValue<'gc> { +) -> Result, Error> { match args.get(0) { - Some(Value::Number(max)) => ReturnValue::Immediate(Value::Number( + Some(Value::Number(max)) => Ok(ReturnValue::Immediate(Value::Number( action_context.rng.gen_range(0.0f64, max).floor(), - )), - _ => ReturnValue::Immediate(Value::Undefined), //TODO: Shouldn't this be an error condition? + ))), + _ => Ok(ReturnValue::Immediate(Value::Undefined)), //TODO: Shouldn't this be an error condition? } } @@ -56,11 +56,13 @@ pub fn boolean<'gc>( _action_context: &mut UpdateContext<'_, 'gc, '_>, _this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], -) -> ReturnValue<'gc> { +) -> Result, Error> { if let Some(val) = args.get(0) { - ReturnValue::Immediate(Value::Bool(val.as_bool(avm.current_swf_version()))) + Ok(ReturnValue::Immediate(Value::Bool( + val.as_bool(avm.current_swf_version()), + ))) } else { - ReturnValue::Immediate(Value::Bool(false)) + Ok(ReturnValue::Immediate(Value::Bool(false))) } } @@ -69,11 +71,11 @@ pub fn number<'gc>( _action_context: &mut UpdateContext<'_, 'gc, '_>, _this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], -) -> ReturnValue<'gc> { +) -> Result, Error> { if let Some(val) = args.get(0) { - ReturnValue::Immediate(Value::Number(val.as_number())) + Ok(ReturnValue::Immediate(Value::Number(val.as_number()))) } else { - ReturnValue::Immediate(Value::Number(0.0)) + Ok(ReturnValue::Immediate(Value::Number(0.0))) } } @@ -82,11 +84,13 @@ pub fn is_nan<'gc>( _action_context: &mut UpdateContext<'_, 'gc, '_>, _this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], -) -> ReturnValue<'gc> { +) -> Result, Error> { if let Some(val) = args.get(0) { - ReturnValue::Immediate(Value::Bool(val.as_number().is_nan())) + Ok(ReturnValue::Immediate(Value::Bool( + val.as_number().is_nan(), + ))) } else { - ReturnValue::Immediate(Value::Bool(true)) + Ok(ReturnValue::Immediate(Value::Bool(true))) } } @@ -129,7 +133,7 @@ mod tests { $( args.push($arg.into()); )* - assert_eq!($fun(avm, context, this, &args), ReturnValue::Immediate($out.into())); + assert_eq!($fun(avm, context, this, &args).unwrap(), ReturnValue::Immediate($out.into())); )* Ok(()) diff --git a/core/src/avm1/globals/math.rs b/core/src/avm1/globals/math.rs index b41788a9b..90df82f5b 100644 --- a/core/src/avm1/globals/math.rs +++ b/core/src/avm1/globals/math.rs @@ -1,6 +1,6 @@ use crate::avm1::object::Attribute::*; use crate::avm1::return_value::ReturnValue; -use crate::avm1::{Avm1, Object, UpdateContext, Value}; +use crate::avm1::{Avm1, Error, Object, UpdateContext, Value}; use gc_arena::{GcCell, MutationContext}; use rand::Rng; use std::f64::NAN; @@ -10,11 +10,11 @@ macro_rules! wrap_std { $( $object.force_set_function( $name, - |_avm, _context, _this, args| -> ReturnValue<'gc> { + |_avm, _context, _this, args| -> Result, Error> { if let Some(input) = args.get(0) { - ReturnValue::Immediate(Value::Number($std(input.as_number()))) + Ok(ReturnValue::Immediate(Value::Number($std(input.as_number())))) } else { - ReturnValue::Immediate(Value::Number(NAN)) + Ok(ReturnValue::Immediate(Value::Number(NAN))) } }, $gc_context, @@ -29,15 +29,19 @@ fn atan2<'gc>( _context: &mut UpdateContext<'_, 'gc, '_>, _this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], -) -> ReturnValue<'gc> { +) -> Result, Error> { if let Some(y) = args.get(0) { if let Some(x) = args.get(1) { - return ReturnValue::Immediate(Value::Number(y.as_number().atan2(x.as_number()))); + return Ok(ReturnValue::Immediate(Value::Number( + y.as_number().atan2(x.as_number()), + ))); } else { - return ReturnValue::Immediate(Value::Number(y.as_number().atan2(0.0))); + return Ok(ReturnValue::Immediate(Value::Number( + y.as_number().atan2(0.0), + ))); } } - ReturnValue::Immediate(Value::Number(NAN)) + Ok(ReturnValue::Immediate(Value::Number(NAN))) } pub fn random<'gc>( @@ -45,8 +49,10 @@ pub fn random<'gc>( action_context: &mut UpdateContext<'_, 'gc, '_>, _this: GcCell<'gc, Object<'gc>>, _args: &[Value<'gc>], -) -> ReturnValue<'gc> { - ReturnValue::Immediate(Value::Number(action_context.rng.gen_range(0.0f64, 1.0f64))) +) -> Result, Error> { + Ok(ReturnValue::Immediate(Value::Number( + action_context.rng.gen_range(0.0f64, 1.0f64), + ))) } pub fn create<'gc>(gc_context: MutationContext<'gc, '_>) -> GcCell<'gc, Object<'gc>> { @@ -131,7 +137,7 @@ mod tests { fn $test() -> Result<(), Error> { with_avm(19, |avm, context, _root| { let math = create(context.gc_context); - let function = math.read().get($name, avm, context, math).unwrap_immediate(); + let function = math.read().get($name, avm, context, math)?.unwrap_immediate(); $( #[allow(unused_mut)] @@ -238,7 +244,7 @@ mod tests { with_avm(19, |avm, context, _root| { let math = GcCell::allocate(context.gc_context, create(context.gc_context)); assert_eq!( - atan2(avm, context, *math.read(), &[]), + atan2(avm, context, *math.read(), &[]).unwrap(), ReturnValue::Immediate(Value::Number(NAN)) ); assert_eq!( @@ -247,7 +253,8 @@ mod tests { context, *math.read(), &[Value::Number(1.0), Value::Null] - ), + ) + .unwrap(), ReturnValue::Immediate(Value::Number(NAN)) ); assert_eq!( @@ -256,7 +263,8 @@ mod tests { context, *math.read(), &[Value::Number(1.0), Value::Undefined] - ), + ) + .unwrap(), ReturnValue::Immediate(Value::Number(NAN)) ); assert_eq!( @@ -265,7 +273,8 @@ mod tests { context, *math.read(), &[Value::Undefined, Value::Number(1.0)] - ), + ) + .unwrap(), ReturnValue::Immediate(Value::Number(NAN)) ); }); @@ -276,7 +285,7 @@ mod tests { with_avm(19, |avm, context, _root| { let math = GcCell::allocate(context.gc_context, create(context.gc_context)); assert_eq!( - atan2(avm, context, *math.read(), &[Value::Number(10.0)]), + atan2(avm, context, *math.read(), &[Value::Number(10.0)]).unwrap(), ReturnValue::Immediate(Value::Number(std::f64::consts::FRAC_PI_2)) ); assert_eq!( @@ -285,7 +294,8 @@ mod tests { context, *math.read(), &[Value::Number(1.0), Value::Number(2.0)] - ), + ) + .unwrap(), ReturnValue::Immediate(Value::Number(f64::atan2(1.0, 2.0))) ); }); diff --git a/core/src/avm1/movie_clip.rs b/core/src/avm1/movie_clip.rs index f7c61f0d4..6fcda578c 100644 --- a/core/src/avm1/movie_clip.rs +++ b/core/src/avm1/movie_clip.rs @@ -1,7 +1,7 @@ use crate::avm1::function::Executable; use crate::avm1::object::{Attribute::*, Object}; use crate::avm1::return_value::ReturnValue; -use crate::avm1::{Avm1, UpdateContext, Value}; +use crate::avm1::{Avm1, Error, UpdateContext, Value}; use crate::display_object::{DisplayNode, DisplayObject, MovieClip}; use enumset::EnumSet; use gc_arena::{GcCell, MutationContext}; @@ -11,13 +11,13 @@ macro_rules! with_movie_clip { $( $object.force_set_function( $name, - |_avm, _context, this, args| -> ReturnValue<'gc> { + |_avm, _context, this, args| -> Result, Error> { if let Some(display_object) = this.read().display_node() { if let Some(movie_clip) = display_object.read().as_movie_clip() { - return ReturnValue::Immediate($fn(movie_clip, args)); + return Ok(ReturnValue::Immediate($fn(movie_clip, args))); } } - ReturnValue::Immediate(Value::Undefined) + Ok(ReturnValue::Immediate(Value::Undefined)) }, $gc_context, DontDelete | ReadOnly | DontEnum, @@ -31,13 +31,13 @@ macro_rules! with_movie_clip_mut { $( $object.force_set_function( $name, - |_avm, context: &mut UpdateContext<'_, 'gc, '_>, this, args| -> ReturnValue<'gc> { + |_avm, context: &mut UpdateContext<'_, 'gc, '_>, this, args| -> Result, Error> { if let Some(display_object) = this.read().display_node() { if let Some(movie_clip) = display_object.write(context.gc_context).as_movie_clip_mut() { - return ReturnValue::Immediate($fn(movie_clip, context, display_object, args)); + return Ok(ReturnValue::Immediate($fn(movie_clip, context, display_object, args))); } } - ReturnValue::Immediate(Value::Undefined) + Ok(ReturnValue::Immediate(Value::Undefined)) } as crate::avm1::function::NativeFunction<'gc>, $gc_context, DontDelete | ReadOnly | DontEnum, @@ -51,7 +51,7 @@ pub fn overwrite_root<'gc>( ac: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], -) -> ReturnValue<'gc> { +) -> Result, Error> { let new_val = args .get(0) .map(|v| v.to_owned()) @@ -59,7 +59,7 @@ pub fn overwrite_root<'gc>( this.write(ac.gc_context) .force_set("_root", new_val, EnumSet::new()); - ReturnValue::Immediate(Value::Undefined) + Ok(ReturnValue::Immediate(Value::Undefined)) } pub fn overwrite_global<'gc>( @@ -67,7 +67,7 @@ pub fn overwrite_global<'gc>( ac: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], -) -> ReturnValue<'gc> { +) -> Result, Error> { let new_val = args .get(0) .map(|v| v.to_owned()) @@ -75,7 +75,7 @@ pub fn overwrite_global<'gc>( this.write(ac.gc_context) .force_set("_global", new_val, EnumSet::new()); - ReturnValue::Immediate(Value::Undefined) + Ok(ReturnValue::Immediate(Value::Undefined)) } pub fn create_movie_object<'gc>(gc_context: MutationContext<'gc, '_>) -> Object<'gc> { @@ -121,7 +121,7 @@ pub fn create_movie_object<'gc>(gc_context: MutationContext<'gc, '_>) -> Object< object.force_set_virtual( "_global", Executable::Native(|avm, context, _this, _args| { - ReturnValue::Immediate(avm.global_object(context)) + Ok(ReturnValue::Immediate(avm.global_object(context))) }), Some(Executable::Native(overwrite_global)), EnumSet::new(), @@ -130,7 +130,7 @@ pub fn create_movie_object<'gc>(gc_context: MutationContext<'gc, '_>) -> Object< object.force_set_virtual( "_root", Executable::Native(|avm, context, _this, _args| { - ReturnValue::Immediate(avm.root_object(context)) + Ok(ReturnValue::Immediate(avm.root_object(context))) }), Some(Executable::Native(overwrite_root)), EnumSet::new(), @@ -139,14 +139,14 @@ pub fn create_movie_object<'gc>(gc_context: MutationContext<'gc, '_>) -> Object< object.force_set_virtual( "_parent", Executable::Native(|_avm, _context, this, _args| { - ReturnValue::Immediate( + Ok(ReturnValue::Immediate( this.read() .display_node() .and_then(|mc| mc.read().parent()) .and_then(|dn| dn.read().object().as_object().ok()) .map(|o| Value::Object(o.to_owned())) .unwrap_or(Value::Undefined), - ) + )) }), None, EnumSet::new(), diff --git a/core/src/avm1/object.rs b/core/src/avm1/object.rs index ee181e438..da43f2bf8 100644 --- a/core/src/avm1/object.rs +++ b/core/src/avm1/object.rs @@ -1,7 +1,7 @@ use self::Attribute::*; use crate::avm1::function::{Avm1Function, Executable, NativeFunction}; use crate::avm1::return_value::ReturnValue; -use crate::avm1::{Avm1, UpdateContext, Value}; +use crate::avm1::{Avm1, Error, UpdateContext, Value}; use crate::display_object::DisplayNode; use core::fmt; use enumset::{EnumSet, EnumSetType}; @@ -19,8 +19,8 @@ fn default_to_string<'gc>( _: &mut UpdateContext<'_, 'gc, '_>, _: GcCell<'gc, Object<'gc>>, _: &[Value<'gc>], -) -> ReturnValue<'gc> { - ReturnValue::Immediate("[Object object]".into()) +) -> Result, Error> { + Ok(ReturnValue::Immediate("[Object object]".into())) } #[derive(EnumSetType, Debug)] @@ -54,10 +54,10 @@ impl<'gc> Property<'gc> { avm: &mut Avm1<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, - ) -> ReturnValue<'gc> { + ) -> Result, Error> { match self { Property::Virtual { get, .. } => get.exec(avm, context, this, &[]), - Property::Stored { value, .. } => ReturnValue::Immediate(value.to_owned()), + Property::Stored { value, .. } => Ok(ReturnValue::Immediate(value.to_owned())), } } @@ -73,14 +73,14 @@ impl<'gc> Property<'gc> { context: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, new_value: impl Into>, - ) -> bool { + ) -> Result { match self { Property::Virtual { set, .. } => { if let Some(function) = set { - let return_value = function.exec(avm, context, this, &[new_value.into()]); - return_value.is_immediate() + let return_value = function.exec(avm, context, this, &[new_value.into()])?; + Ok(return_value.is_immediate()) } else { - true + Ok(true) } } Property::Stored { @@ -90,7 +90,7 @@ impl<'gc> Property<'gc> { replace::>(value, new_value.into()); } - true + Ok(true) } } } @@ -235,16 +235,18 @@ impl<'gc> Object<'gc> { avm: &mut Avm1<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, - ) { + ) -> Result<(), Error> { match self.values.entry(name.to_owned()) { Entry::Occupied(mut entry) => { - entry.get_mut().set(avm, context, this, value); + entry.get_mut().set(avm, context, this, value)?; + Ok(()) } Entry::Vacant(entry) => { entry.insert(Property::Stored { value: value.into(), attributes: Default::default(), }); + Ok(()) } } } @@ -311,12 +313,12 @@ impl<'gc> Object<'gc> { avm: &mut Avm1<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, - ) -> ReturnValue<'gc> { + ) -> Result, Error> { if let Some(value) = self.values.get(name) { return value.get(avm, context, this); } - ReturnValue::Immediate(Value::Undefined) + Ok(ReturnValue::Immediate(Value::Undefined)) } /// Delete a given value off the object. @@ -358,11 +360,11 @@ impl<'gc> Object<'gc> { context: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, args: &[Value<'gc>], - ) -> ReturnValue<'gc> { + ) -> Result, Error> { if let Some(function) = &self.function { function.exec(avm, context, this, args) } else { - ReturnValue::Immediate(Value::Undefined) + Ok(ReturnValue::Immediate(Value::Undefined)) } } @@ -452,7 +454,10 @@ mod tests { fn test_get_undefined() { with_object(0, |avm, context, object| { assert_eq!( - object.read().get("not_defined", avm, context, object), + object + .read() + .get("not_defined", avm, context, object) + .unwrap(), ReturnValue::Immediate(Value::Undefined) ); }) @@ -466,14 +471,15 @@ mod tests { .force_set("forced", "forced", EnumSet::empty()); object .write(context.gc_context) - .set("natural", "natural", avm, context, object); + .set("natural", "natural", avm, context, object) + .unwrap(); assert_eq!( - object.read().get("forced", avm, context, object), + object.read().get("forced", avm, context, object).unwrap(), ReturnValue::Immediate("forced".into()) ); assert_eq!( - object.read().get("natural", avm, context, object), + object.read().get("natural", avm, context, object).unwrap(), ReturnValue::Immediate("natural".into()) ); }) @@ -491,17 +497,19 @@ mod tests { object .write(context.gc_context) - .set("normal", "replaced", avm, context, object); + .set("normal", "replaced", avm, context, object) + .unwrap(); object .write(context.gc_context) - .set("readonly", "replaced", avm, context, object); + .set("readonly", "replaced", avm, context, object) + .unwrap(); assert_eq!( - object.read().get("normal", avm, context, object), + object.read().get("normal", avm, context, object).unwrap(), ReturnValue::Immediate("replaced".into()) ); assert_eq!( - object.read().get("readonly", avm, context, object), + object.read().get("readonly", avm, context, object).unwrap(), ReturnValue::Immediate("initial".into()) ); }) @@ -516,17 +524,18 @@ mod tests { assert_eq!(object.write(context.gc_context).delete("test"), false); assert_eq!( - object.read().get("test", avm, context, object), + object.read().get("test", avm, context, object).unwrap(), ReturnValue::Immediate("initial".into()) ); object .write(context.gc_context) - .set("test", "replaced", avm, context, object); + .set("test", "replaced", avm, context, object) + .unwrap(); assert_eq!(object.write(context.gc_context).delete("test"), false); assert_eq!( - object.read().get("test", avm, context, object), + object.read().get("test", avm, context, object).unwrap(), ReturnValue::Immediate("replaced".into()) ); }) @@ -536,7 +545,7 @@ mod tests { fn test_virtual_get() { with_object(0, |avm, context, object| { let getter = Executable::Native(|_avm, _context, _this, _args| { - ReturnValue::Immediate("Virtual!".into()) + Ok(ReturnValue::Immediate("Virtual!".into())) }); object.write(context.gc_context).force_set_virtual( @@ -547,16 +556,17 @@ mod tests { ); assert_eq!( - object.read().get("test", avm, context, object), + object.read().get("test", avm, context, object).unwrap(), ReturnValue::Immediate("Virtual!".into()) ); // This set should do nothing object .write(context.gc_context) - .set("test", "Ignored!", avm, context, object); + .set("test", "Ignored!", avm, context, object) + .unwrap(); assert_eq!( - object.read().get("test", avm, context, object), + object.read().get("test", avm, context, object).unwrap(), ReturnValue::Immediate("Virtual!".into()) ); }) @@ -566,7 +576,7 @@ mod tests { fn test_delete() { with_object(0, |avm, context, object| { let getter = Executable::Native(|_avm, _context, _this, _args| { - ReturnValue::Immediate("Virtual!".into()) + Ok(ReturnValue::Immediate("Virtual!".into())) }); object.write(context.gc_context).force_set_virtual( @@ -598,19 +608,25 @@ mod tests { ); assert_eq!( - object.read().get("virtual", avm, context, object), + object.read().get("virtual", avm, context, object).unwrap(), ReturnValue::Immediate(Value::Undefined) ); assert_eq!( - object.read().get("virtual_un", avm, context, object), + object + .read() + .get("virtual_un", avm, context, object) + .unwrap(), ReturnValue::Immediate("Virtual!".into()) ); assert_eq!( - object.read().get("stored", avm, context, object), + object.read().get("stored", avm, context, object).unwrap(), ReturnValue::Immediate(Value::Undefined) ); assert_eq!( - object.read().get("stored_un", avm, context, object), + object + .read() + .get("stored_un", avm, context, object) + .unwrap(), ReturnValue::Immediate("Stored!".into()) ); }) @@ -620,7 +636,7 @@ mod tests { fn test_iter_values() { with_object(0, |_avm, context, object| { let getter = Executable::Native(|_avm, _context, _this, _args| { - ReturnValue::Immediate(Value::Null) + Ok(ReturnValue::Immediate(Value::Null)) }); object diff --git a/core/src/avm1/scope.rs b/core/src/avm1/scope.rs index 13aa5e2b1..f1e82762e 100644 --- a/core/src/avm1/scope.rs +++ b/core/src/avm1/scope.rs @@ -1,7 +1,7 @@ //! Represents AVM1 scope chain resolution. use crate::avm1::return_value::ReturnValue; -use crate::avm1::{Avm1, Object, UpdateContext, Value}; +use crate::avm1::{Avm1, Error, Object, UpdateContext, Value}; use enumset::EnumSet; use gc_arena::{GcCell, MutationContext}; use std::cell::{Ref, RefMut}; @@ -247,7 +247,7 @@ impl<'gc> Scope<'gc> { avm: &mut Avm1<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, - ) -> ReturnValue<'gc> { + ) -> Result, Error> { if self.locals().has_property(name) { return self.locals().get(name, avm, context, this); } @@ -255,7 +255,8 @@ impl<'gc> Scope<'gc> { return scope.resolve(name, avm, context, this); } - ReturnValue::Immediate(Value::Undefined) + //TODO: Should undefined variables halt execution? + Ok(ReturnValue::Immediate(Value::Undefined)) } /// Check if a particular property in the scope chain is defined. @@ -286,18 +287,18 @@ impl<'gc> Scope<'gc> { avm: &mut Avm1<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, this: GcCell<'gc, Object<'gc>>, - ) -> Option> { + ) -> Result>, Error> { if self.locals().has_property(name) { self.locals_mut(context.gc_context) - .set(name, value, avm, context, this); - return None; + .set(name, value, avm, context, this)?; + return Ok(None); } if let Some(scope) = self.parent() { return scope.overwrite(name, value, avm, context, this); } - Some(value) + Ok(Some(value)) } /// Set a particular value in the locals for this scope. diff --git a/core/src/avm1/tests.rs b/core/src/avm1/tests.rs index fd041f911..1dcb60e96 100644 --- a/core/src/avm1/tests.rs +++ b/core/src/avm1/tests.rs @@ -11,10 +11,12 @@ fn locals_into_form_values() { my_locals .write(context.gc_context) - .set("value1", "string", avm, context, my_locals); + .set("value1", "string", avm, context, my_locals) + .unwrap(); my_locals .write(context.gc_context) - .set("value2", 2.0, avm, context, my_locals); + .set("value2", 2.0, avm, context, my_locals) + .unwrap(); avm.insert_stack_frame(GcCell::allocate(context.gc_context, my_activation)); diff --git a/core/src/avm1/value.rs b/core/src/avm1/value.rs index 3a925f7f5..00a4a8949 100644 --- a/core/src/avm1/value.rs +++ b/core/src/avm1/value.rs @@ -260,7 +260,7 @@ impl<'gc> Value<'gc> { args: &[Value<'gc>], ) -> Result, Error> { if let Value::Object(object) = self { - Ok(object.read().call(avm, context, this, args)) + object.read().call(avm, context, this, args) } else { Err(format!("Expected function, found {:?}", self).into()) }