From f50b68abfd4ff4bebf65cc9bd9125a976ca203e8 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sat, 27 Apr 2024 14:59:49 -0700 Subject: [PATCH] avm2: Split `GetLex` into `FindPropStrict` and `GetProperty` --- core/src/avm2/activation.rs | 39 ----------------------------- core/src/avm2/op.rs | 3 --- core/src/avm2/optimize.rs | 3 --- core/src/avm2/verify.rs | 49 ++++++++++++++++++++++++++++++------- 4 files changed, 40 insertions(+), 54 deletions(-) diff --git a/core/src/avm2/activation.rs b/core/src/avm2/activation.rs index 0018de5c7..75d304860 100644 --- a/core/src/avm2/activation.rs +++ b/core/src/avm2/activation.rs @@ -305,27 +305,6 @@ impl<'a, 'gc> Activation<'a, 'gc> { } } - /// Resolves a definition using either the current or outer scope of this activation. - pub fn resolve_definition( - &mut self, - name: &Multiname<'gc>, - ) -> Result>, Error<'gc>> { - let outer_scope = self.outer; - - if let Some(obj) = search_scope_stack(self.scope_frame(), name, outer_scope.is_empty())? { - Ok(Some(obj.get_property(name, self)?)) - } else if let Some(result) = outer_scope.resolve(name, self)? { - Ok(Some(result)) - } else if let Some(global) = self.global_scope() { - if !global.base().has_own_property(name) { - return Ok(None); - } - return Ok(Some(global.base().get_property_local(name, self)?)); - } else { - Ok(None) - } - } - /// Resolve a single parameter value. /// /// Given an individual parameter value and the associated parameter's @@ -943,7 +922,6 @@ impl<'a, 'gc> Activation<'a, 'gc> { Op::FindDef { multiname } => self.op_find_def(*multiname), Op::FindProperty { multiname } => self.op_find_property(*multiname), Op::FindPropStrict { multiname } => self.op_find_prop_strict(*multiname), - Op::GetLex { multiname } => self.op_get_lex(*multiname), Op::GetDescendants { multiname } => self.op_get_descendants(*multiname), Op::GetSlot { index } => self.op_get_slot(*index), Op::SetSlot { index } => self.op_set_slot(*index), @@ -1760,23 +1738,6 @@ impl<'a, 'gc> Activation<'a, 'gc> { Ok(FrameControl::Continue) } - fn op_get_lex( - &mut self, - multiname: Gc<'gc, Multiname<'gc>>, - ) -> Result, Error<'gc>> { - // Verifier ensures that multiname is non-lazy - - avm_debug!(self.avm2(), "Resolving {:?}", *multiname); - let found: Result, Error<'gc>> = - self.resolve_definition(&multiname)?.ok_or_else(|| { - make_reference_error(self, ReferenceErrorCode::InvalidLookup, &multiname, None) - }); - - self.push_stack(found?); - - Ok(FrameControl::Continue) - } - fn op_get_slot(&mut self, index: u32) -> Result, Error<'gc>> { let object = self.pop_stack().coerce_to_object_or_typeerror(self, None)?; let value = object.get_slot(index)?; diff --git a/core/src/avm2/op.rs b/core/src/avm2/op.rs index 5ad40b861..e5e9021a1 100644 --- a/core/src/avm2/op.rs +++ b/core/src/avm2/op.rs @@ -134,9 +134,6 @@ pub enum Op<'gc> { GetGlobalSlot { index: u32, }, - GetLex { - multiname: Gc<'gc, Multiname<'gc>>, - }, GetLocal { index: u32, }, diff --git a/core/src/avm2/optimize.rs b/core/src/avm2/optimize.rs index 68013cd6a..a8f62cf4d 100644 --- a/core/src/avm2/optimize.rs +++ b/core/src/avm2/optimize.rs @@ -730,9 +730,6 @@ pub fn optimize<'gc>( let local_type = local_types.at(*index as usize); stack.push(local_type); } - Op::GetLex { .. } => { - stack.push_any(); - } Op::FindPropStrict { multiname } => { stack.pop_for_multiname(*multiname); diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index 913d8ee8d..56ba3a93e 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -45,9 +45,20 @@ enum ByteInfo { OpStart(AbcOp), OpContinue, + OpStartNonJumpable(AbcOp), + NotYetReached, } +impl ByteInfo { + fn get_op(&self) -> Option<&AbcOp> { + match self { + ByteInfo::OpStart(op) | ByteInfo::OpStartNonJumpable(op) => Some(op), + _ => None, + } + } +} + pub enum JumpSources { Known(Vec), Unknown, @@ -152,7 +163,10 @@ pub fn verify_method<'gc>( let lookedup_target_info = byte_info.get(target_position as usize); - if matches!(lookedup_target_info, Some(ByteInfo::OpContinue)) { + if matches!( + lookedup_target_info, + Some(ByteInfo::OpContinue | ByteInfo::OpStartNonJumpable(_)) + ) { return Err(make_error_1021(activation)); } @@ -290,7 +304,7 @@ pub fn verify_method<'gc>( } } - AbcOp::GetLex { index } | AbcOp::FindDef { index } => { + AbcOp::FindDef { index } => { let multiname = method .translation_unit() .pool_maybe_uninitialized_multiname(index, &mut activation.context)?; @@ -304,6 +318,26 @@ pub fn verify_method<'gc>( } } + AbcOp::GetLex { index } => { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(index, &mut activation.context)?; + + if multiname.has_lazy_component() { + return Err(Error::AvmError(verify_error( + activation, + "Error #1078: Illegal opcode/multiname combination.", + 1078, + )?)); + } + + assert!(bytes_read > 1); + byte_info[previous_position as usize] = + ByteInfo::OpStart(AbcOp::FindPropStrict { index }); + byte_info[(previous_position + 1) as usize] = + ByteInfo::OpStartNonJumpable(AbcOp::GetProperty { index }); + } + AbcOp::GetOuterScope { index } => { if activation.outer().get(index as usize).is_none() { return Err(Error::AvmError(verify_error( @@ -349,9 +383,9 @@ pub fn verify_method<'gc>( let mut new_code = Vec::new(); for (i, info) in byte_info.iter().enumerate() { - if let ByteInfo::OpStart(c) = info { + if let Some(op) = info.get_op() { byte_offset_to_idx.insert(i, new_code.len() as i32); - new_code.push(c.clone()); + new_code.push(op.clone()); idx_to_byte_offset.push(i); } } @@ -922,11 +956,8 @@ fn resolve_op<'gc>( Op::FindPropStrict { multiname } } - AbcOp::GetLex { index } => { - let multiname = pool_multiname(activation, translation_unit, index)?; - // Verifier guarantees that multiname was non-lazy - - Op::GetLex { multiname } + AbcOp::GetLex { .. } => { + unreachable!("Verifier emits FindPropStrict and GetProperty instead of GetLex") } AbcOp::GetDescendants { index } => { let multiname = pool_multiname(activation, translation_unit, index)?;