From f1da1457ebf86ddad4c27ddd20621b0162a2fa6e Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Wed, 11 Sep 2024 15:43:51 -0700 Subject: [PATCH] avm2: Return actual VerifyErrors for constant pool errors This requires passing around an Activation --- core/src/avm2/activation.rs | 4 +- core/src/avm2/class.rs | 20 +++------ core/src/avm2/error.rs | 52 +++++++++++++++------ core/src/avm2/method.rs | 5 +-- core/src/avm2/multiname.rs | 90 ++++++++++++++++++++----------------- core/src/avm2/namespace.rs | 25 ++++++----- core/src/avm2/property.rs | 4 +- core/src/avm2/qname.rs | 64 +++++++++++--------------- core/src/avm2/script.rs | 23 +++++----- core/src/avm2/traits.rs | 6 +-- core/src/avm2/value.rs | 2 +- core/src/avm2/verify.rs | 18 ++++---- 12 files changed, 161 insertions(+), 152 deletions(-) diff --git a/core/src/avm2/activation.rs b/core/src/avm2/activation.rs index c69f02fa8..60a1c0cf4 100644 --- a/core/src/avm2/activation.rs +++ b/core/src/avm2/activation.rs @@ -732,9 +732,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { method: Gc<'gc, BytecodeMethod<'gc>>, index: Index, ) -> Result, Error<'gc>> { - method - .translation_unit() - .pool_namespace(index, self.context) + method.translation_unit().pool_namespace(self, index) } /// Retrieve a method entry from the current ABC file's method table. diff --git a/core/src/avm2/class.rs b/core/src/avm2/class.rs index 9d6c809ce..46c58ec1b 100644 --- a/core/src/avm2/class.rs +++ b/core/src/avm2/class.rs @@ -439,45 +439,39 @@ impl<'gc> Class<'gc> { .ok_or_else(|| "LoadError: Instance index not valid".into()); let abc_instance = abc_instance?; - let name = QName::from_abc_multiname(unit, abc_instance.name, activation.context)?; + let name = QName::from_abc_multiname(activation, unit, abc_instance.name)?; + let super_class = if abc_instance.super_name.0 == 0 { None } else { - let multiname = - unit.pool_multiname_static(abc_instance.super_name, activation.context)?; + let multiname = unit.pool_multiname_static(activation, abc_instance.super_name)?; Some( activation .domain() .get_class(activation.context, &multiname) .ok_or_else(|| { - make_error_1014( - activation, - multiname.to_qualified_name(activation.context.gc_context), - ) + make_error_1014(activation, multiname.to_qualified_name(activation.gc())) })?, ) }; let protected_namespace = if let Some(ns) = &abc_instance.protected_namespace { - Some(unit.pool_namespace(*ns, activation.context)?) + Some(unit.pool_namespace(activation, *ns)?) } else { None }; let mut interfaces = Vec::with_capacity(abc_instance.interfaces.len()); for interface_name in &abc_instance.interfaces { - let multiname = unit.pool_multiname_static(*interface_name, activation.context)?; + let multiname = unit.pool_multiname_static(activation, *interface_name)?; interfaces.push( activation .domain() .get_class(activation.context, &multiname) .ok_or_else(|| { - make_error_1014( - activation, - multiname.to_qualified_name(activation.context.gc_context), - ) + make_error_1014(activation, multiname.to_qualified_name(activation.gc())) })?, ); } diff --git a/core/src/avm2/error.rs b/core/src/avm2/error.rs index ed50a03af..1b293c362 100644 --- a/core/src/avm2/error.rs +++ b/core/src/avm2/error.rs @@ -253,6 +253,16 @@ pub fn make_error_1032<'gc>(activation: &mut Activation<'_, 'gc>, index: u32) -> } } +#[inline(never)] +#[cold] +pub fn make_error_1033<'gc>(activation: &mut Activation<'_, 'gc>) -> Error<'gc> { + let err = verify_error(activation, "Error #1033: Cpool entry is wrong type.", 1033); + match err { + Ok(err) => Error::AvmError(err), + Err(err) => err, + } +} + #[inline(never)] #[cold] pub fn make_error_1054<'gc>(activation: &mut Activation<'_, 'gc>) -> Error<'gc> { @@ -286,6 +296,20 @@ pub fn make_error_1065<'gc>( } } +#[inline(never)] +#[cold] +pub fn make_error_1080<'gc>(activation: &mut Activation<'_, 'gc>) -> Error<'gc> { + let err = type_error( + activation, + "Error #1080: Illegal value for namespace.", + 1080, + ); + match err { + Ok(err) => Error::AvmError(err), + Err(err) => err, + } +} + #[inline(never)] #[cold] pub fn make_error_1085<'gc>(activation: &mut Activation<'_, 'gc>, tag: &str) -> Error<'gc> { @@ -538,6 +562,20 @@ pub fn make_error_2008<'gc>(activation: &mut Activation<'_, 'gc>, param_name: &s } } +#[inline(never)] +#[cold] +pub fn make_error_2025<'gc>(activation: &mut Activation<'_, 'gc>) -> Error<'gc> { + let err = argument_error( + activation, + "Error #2025: The supplied DisplayObject must be a child of the caller.", + 2025, + ); + match err { + Ok(err) => Error::AvmError(err), + Err(err) => err, + } +} + #[inline(never)] #[cold] pub fn make_error_2027<'gc>(activation: &mut Activation<'_, 'gc>, value: i32) -> Error<'gc> { @@ -600,20 +638,6 @@ pub fn make_error_2097<'gc>(activation: &mut Activation<'_, 'gc>) -> Error<'gc> } } -#[inline(never)] -#[cold] -pub fn make_error_2025<'gc>(activation: &mut Activation<'_, 'gc>) -> Error<'gc> { - let err = argument_error( - activation, - "Error #2025: The supplied DisplayObject must be a child of the caller.", - 2025, - ); - match err { - Ok(err) => Error::AvmError(err), - Err(err) => err, - } -} - #[inline(never)] #[cold] pub fn make_error_2126<'gc>(activation: &mut Activation<'_, 'gc>) -> Error<'gc> { diff --git a/core/src/avm2/method.rs b/core/src/avm2/method.rs index daeb2070d..757df830d 100644 --- a/core/src/avm2/method.rs +++ b/core/src/avm2/method.rs @@ -79,7 +79,7 @@ impl<'gc> ParamConfig<'gc> { } else { AvmString::from("") }; - let param_type_name = txunit.pool_multiname_static_any(config.kind, activation.context)?; + let param_type_name = txunit.pool_multiname_static_any(activation, config.kind)?; let default_value = if let Some(dv) = &config.default_value { Some(abc_default_value(txunit, dv, activation)?) @@ -175,8 +175,7 @@ impl<'gc> BytecodeMethod<'gc> { signature.push(ParamConfig::from_abc_param(param, txunit, activation)?); } - return_type = - txunit.pool_multiname_static_any(method.return_type, activation.context)?; + return_type = txunit.pool_multiname_static_any(activation, method.return_type)?; if let Some(body) = method.body { abc_method_body = Some(body.0); diff --git a/core/src/avm2/multiname.rs b/core/src/avm2/multiname.rs index 08a07318c..3acfeb9ec 100644 --- a/core/src/avm2/multiname.rs +++ b/core/src/avm2/multiname.rs @@ -1,19 +1,18 @@ use crate::avm2::activation::Activation; +use crate::avm2::error::{make_error_1032, make_error_1080}; use crate::avm2::script::TranslationUnit; use crate::avm2::Error; use crate::avm2::Namespace; use crate::avm2::QName; use crate::avm2::{Object, Value}; -use crate::context::{GcContext, UpdateContext}; +use crate::context::GcContext; use crate::string::{AvmString, WStr, WString}; use bitflags::bitflags; use gc_arena::Gc; use gc_arena::{Collect, Mutation}; use std::fmt::Debug; use std::ops::Deref; -use swf::avm2::types::{ - AbcFile, Index, Multiname as AbcMultiname, NamespaceSet as AbcNamespaceSet, -}; +use swf::avm2::types::{Index, Multiname as AbcMultiname, NamespaceSet as AbcNamespaceSet}; #[derive(Clone, Copy, Debug, Collect)] #[collect(no_drop)] @@ -141,14 +140,12 @@ impl<'gc> Multiname<'gc> { /// Read a namespace set from the ABC constant pool, and return a list of /// copied namespaces. pub fn abc_namespace_set( + activation: &mut Activation<'_, 'gc>, translation_unit: TranslationUnit<'gc>, namespace_set_index: Index, - context: &mut UpdateContext<'gc>, ) -> Result, Error<'gc>> { if namespace_set_index.0 == 0 { - return Err(Error::RustError( - "Multiname namespace set must not be null".into(), - )); + return Err(make_error_1032(activation, 0)); } let actual_index = namespace_set_index.0 as usize - 1; @@ -163,33 +160,57 @@ impl<'gc> Multiname<'gc> { let ns_set = ns_set?; if ns_set.len() == 1 { - Ok(NamespaceSet::single( - translation_unit.pool_namespace(ns_set[0], context)?, - )) + let namespace = translation_unit.pool_namespace(activation, ns_set[0])?; + + if namespace.is_any() { + return Err(make_error_1080(activation)); + } + + Ok(NamespaceSet::single(namespace)) } else { let mut result = Vec::with_capacity(ns_set.len()); for ns in ns_set { - result.push(translation_unit.pool_namespace(*ns, context)?) + let namespace = translation_unit.pool_namespace(activation, *ns)?; + + // Namespace sets must not have Any namespaces in them + if namespace.is_any() { + return Err(make_error_1080(activation)); + } + + result.push(namespace) } - Ok(NamespaceSet::multiple(result, context.gc_context)) + + Ok(NamespaceSet::multiple(result, activation.gc())) } } pub fn from_abc_index( + activation: &mut Activation<'_, 'gc>, translation_unit: TranslationUnit<'gc>, multiname_index: Index, - context: &mut UpdateContext<'gc>, ) -> Result> { - let mc = context.gc_context; + let mc = activation.gc(); + + if multiname_index.0 == 0 { + return Err(make_error_1032(activation, 0)); + } + let abc = translation_unit.abc(); - let abc_multiname = Self::resolve_multiname_index(&abc, multiname_index)?; + + let abc_multiname = abc + .constant_pool + .multinames + .get(multiname_index.0 as usize - 1) + .ok_or_else(|| format!("Unknown multiname constant {}", multiname_index.0))?; let mut multiname = match abc_multiname { AbcMultiname::QName { namespace, name } | AbcMultiname::QNameA { namespace, name } => { Self { - ns: NamespaceSet::single(translation_unit.pool_namespace(*namespace, context)?), + ns: NamespaceSet::single( + translation_unit.pool_namespace(activation, *namespace)?, + ), name: translation_unit - .pool_string_option(name.0, &mut context.borrow_gc())? + .pool_string_option(name.0, &mut activation.borrow_gc())? .map(|v| v.into()), param: None, flags: MultinameFlags::IS_QNAME, @@ -198,7 +219,7 @@ impl<'gc> Multiname<'gc> { AbcMultiname::RTQName { name } | AbcMultiname::RTQNameA { name } => Self { ns: NamespaceSet::multiple(vec![], mc), name: translation_unit - .pool_string_option(name.0, &mut context.borrow_gc())? + .pool_string_option(name.0, &mut activation.borrow_gc())? .map(|v| v.into()), param: None, flags: MultinameFlags::HAS_LAZY_NS | MultinameFlags::IS_QNAME, @@ -219,16 +240,16 @@ impl<'gc> Multiname<'gc> { namespace_set, name, } => Self { - ns: Self::abc_namespace_set(translation_unit, *namespace_set, context)?, + ns: Self::abc_namespace_set(activation, translation_unit, *namespace_set)?, name: translation_unit - .pool_string_option(name.0, &mut context.borrow_gc())? + .pool_string_option(name.0, &mut activation.borrow_gc())? .map(|v| v.into()), param: None, flags: Default::default(), }, AbcMultiname::MultinameL { namespace_set } | AbcMultiname::MultinameLA { namespace_set } => Self { - ns: Self::abc_namespace_set(translation_unit, *namespace_set, context)?, + ns: Self::abc_namespace_set(activation, translation_unit, *namespace_set)?, name: None, param: None, flags: MultinameFlags::HAS_LAZY_NAME, @@ -238,7 +259,7 @@ impl<'gc> Multiname<'gc> { parameters, } => { let mut base = translation_unit - .pool_multiname_static(*base_type, context)? + .pool_multiname_static(activation, *base_type)? .deref() .clone(); @@ -251,7 +272,7 @@ impl<'gc> Multiname<'gc> { } base.param = - Some(translation_unit.pool_multiname_static_any(parameters[0], context)?); + Some(translation_unit.pool_multiname_static_any(activation, parameters[0])?); base } }; @@ -310,23 +331,6 @@ impl<'gc> Multiname<'gc> { }) } - /// Retrieve a given multiname index from the ABC file, yielding an error - /// if the multiname index is zero. - pub fn resolve_multiname_index( - abc: &AbcFile, - multiname_index: Index, - ) -> Result<&AbcMultiname, Error<'gc>> { - let actual_index: Result> = (multiname_index.0 as usize) - .checked_sub(1) - .ok_or_else(|| "Attempted to resolve a multiname at index zero. This is a bug.".into()); - - let actual_index = actual_index?; - abc.constant_pool - .multinames - .get(actual_index) - .ok_or_else(|| format!("Unknown multiname constant {}", multiname_index.0).into()) - } - /// Indicates the any type (any name in any namespace). pub fn any() -> Self { Self { @@ -414,7 +418,9 @@ impl<'gc> Multiname<'gc> { pub fn is_any_namespace(&self) -> bool { match self.ns { NamespaceSet::Single(ns) => ns.is_any(), - NamespaceSet::Multiple(ns) => ns.iter().any(|ns| ns.is_any()), + + // NamespaceSet::Multiple should not have any Any namespaces in it + NamespaceSet::Multiple(_) => false, } } diff --git a/core/src/avm2/namespace.rs b/core/src/avm2/namespace.rs index 94a0b307d..a8695b90b 100644 --- a/core/src/avm2/namespace.rs +++ b/core/src/avm2/namespace.rs @@ -1,5 +1,5 @@ +use crate::avm2::activation::Activation; use crate::avm2::Error; -use crate::context::UpdateContext; use crate::string::{AvmAtom, AvmString}; use crate::{avm2::script::TranslationUnit, context::GcContext}; use gc_arena::{Collect, Gc}; @@ -82,14 +82,16 @@ impl<'gc> Namespace<'gc> { /// otherwise you run a risk of creating a duplicate of private ns singleton. /// Based on https://github.com/adobe/avmplus/blob/858d034a3bd3a54d9b70909386435cf4aec81d21/core/AbcParser.cpp#L1459 pub fn from_abc_namespace( + activation: &mut Activation<'_, 'gc>, translation_unit: TranslationUnit<'gc>, namespace_index: Index, - context: &mut UpdateContext<'gc>, ) -> Result> { if namespace_index.0 == 0 { return Ok(Self::any()); } + let mc = activation.gc(); + let actual_index = namespace_index.0 as usize - 1; let abc = translation_unit.abc(); let abc_namespace: Result<_, Error<'gc>> = abc @@ -109,12 +111,13 @@ impl<'gc> Namespace<'gc> { | AbcNamespace::Private(idx) => idx, }; - let mut namespace_name = translation_unit.pool_string(index.0, &mut context.borrow_gc())?; + let mut namespace_name = + translation_unit.pool_string(index.0, &mut activation.borrow_gc())?; // Private namespaces don't get any of the namespace version checks if let AbcNamespace::Private(_) = abc_namespace { return Ok(Self(Some(Gc::new( - context.gc_context, + mc, NamespaceData::Private(namespace_name), )))); } @@ -159,14 +162,14 @@ impl<'gc> Namespace<'gc> { let api_version = if index.0 != 0 { let is_playerglobals = translation_unit .domain() - .is_playerglobals_domain(context.avm2); + .is_playerglobals_domain(activation.avm2()); let mut api_version = ApiVersion::AllVersions; let stripped = strip_version_mark(namespace_name.as_wstr(), is_playerglobals); let has_version_mark = stripped.is_some(); if let Some((stripped, version)) = stripped { - let stripped_string = AvmString::new(context.gc_context, stripped); - namespace_name = context.interner.intern(context.gc_context, stripped_string); + let stripped_string = AvmString::new(mc, stripped); + namespace_name = activation.context.interner.intern(mc, stripped_string); api_version = version; } @@ -191,9 +194,9 @@ impl<'gc> Namespace<'gc> { // However, there's no reason to hold on to invalid API versions for the // current active series (player runtime), so let's just do the conversion immediately. api_version = - api_version.to_valid_playerglobals_version(context.avm2.player_runtime); + api_version.to_valid_playerglobals_version(activation.avm2().player_runtime); } else if is_public { - api_version = translation_unit.api_version(context.avm2); + api_version = translation_unit.api_version(activation.avm2()); }; api_version } else { @@ -201,7 +204,7 @@ impl<'gc> Namespace<'gc> { // However, Flash Player appears to always use the root SWF api version // for all swfs (e.g. those loaded through `Loader`). We can simply our code // by skipping walking the stack, and just using the API version of our root SWF. - context.avm2.root_api_version + activation.avm2().root_api_version }; let ns = match abc_namespace { @@ -214,7 +217,7 @@ impl<'gc> Namespace<'gc> { AbcNamespace::StaticProtected(_) => NamespaceData::StaticProtected(namespace_name), AbcNamespace::Private(_) => unreachable!(), }; - Ok(Self(Some(Gc::new(context.gc_context, ns)))) + Ok(Self(Some(Gc::new(mc, ns)))) } pub fn any() -> Self { diff --git a/core/src/avm2/property.rs b/core/src/avm2/property.rs index b7fa54378..6d84d6ded 100644 --- a/core/src/avm2/property.rs +++ b/core/src/avm2/property.rs @@ -65,7 +65,7 @@ impl<'gc> PropertyClass<'gc> { PropertyClass::Class(class) => (Some(*class), false), PropertyClass::Name(gc) => { let (name, unit) = &**gc; - if name.is_any_name() { + if name.is_any_namespace() && name.is_any_name() { *self = PropertyClass::Any; (None, true) } else { @@ -108,7 +108,7 @@ impl<'gc> PropertyClass<'gc> { PropertyClass::Class(class) => Ok(Some(*class)), PropertyClass::Name(gc) => { let (name, unit) = &**gc; - if name.is_any_name() { + if name.is_any_namespace() && name.is_any_name() { *self = PropertyClass::Any; Ok(None) } else { diff --git a/core/src/avm2/qname.rs b/core/src/avm2/qname.rs index 0231d1370..f4d455704 100644 --- a/core/src/avm2/qname.rs +++ b/core/src/avm2/qname.rs @@ -1,3 +1,5 @@ +use crate::avm2::activation::Activation; +use crate::avm2::error::make_error_1033; use crate::avm2::script::TranslationUnit; use crate::avm2::{Error, Namespace}; use crate::context::UpdateContext; @@ -40,52 +42,36 @@ impl<'gc> QName<'gc> { /// Pull a `QName` from the multiname pool. /// - /// This function returns an Err if the multiname does not exist or is not - /// a `QName`. + /// This function returns an Err if the multiname is not a `QName`. pub fn from_abc_multiname( + activation: &mut Activation<'_, 'gc>, translation_unit: TranslationUnit<'gc>, multiname_index: Index, - context: &mut UpdateContext<'gc>, ) -> Result> { - if multiname_index.0 == 0 { - return Err("Attempted to load a trait name of index zero".into()); + let name = Multiname::from_abc_index(activation, translation_unit, multiname_index)?; + + if name.is_any_namespace() + || name.is_any_name() + || name.has_lazy_component() + || name.is_attribute() + { + return Err(make_error_1033(activation)); } - let actual_index = multiname_index.0 as usize - 1; - let abc = translation_unit.abc(); - let abc_multiname: Result<_, Error<'gc>> = abc - .constant_pool - .multinames - .get(actual_index) - .ok_or_else(|| format!("Unknown multiname constant {}", multiname_index.0).into()); + // For any non-QName not in the playerglobals domain, return an error. + if !name.is_qname() + && !translation_unit + .domain() + .is_playerglobals_domain(activation.avm2()) + { + return Err(make_error_1033(activation)); + } - Ok(match abc_multiname? { - AbcMultiname::QName { namespace, name } => Self { - ns: translation_unit.pool_namespace(*namespace, context)?, - name: translation_unit - .pool_string(name.0, &mut context.borrow_gc())? - .into(), - }, - AbcMultiname::Multiname { - namespace_set, - name, - } => { - let ns_set = - Multiname::abc_namespace_set(translation_unit, *namespace_set, context)?; - if ns_set.len() == 1 { - Self { - ns: ns_set.get(0).unwrap(), - name: translation_unit - .pool_string(name.0, &mut context.borrow_gc())? - .into(), - } - } else { - return Err( - "Attempted to pull QName from multiname with multiple namespaces".into(), - ); - } - } - _ => return Err("Attempted to pull QName from non-QName multiname".into()), + // Now we know that the Multiname must have an explicit namespace and a local name. + + Ok(Self { + ns: name.namespace_set()[0], + name: name.local_name().unwrap(), }) } diff --git a/core/src/avm2/script.rs b/core/src/avm2/script.rs index c2c08558c..5f4eeb3ce 100644 --- a/core/src/avm2/script.rs +++ b/core/src/avm2/script.rs @@ -327,9 +327,10 @@ impl<'gc> TranslationUnit<'gc> { /// This version of the function treats index 0 as an error condition. pub fn pool_namespace( self, + activation: &mut Activation<'_, 'gc>, ns_index: Index, - context: &mut UpdateContext<'gc>, ) -> Result, Error<'gc>> { + let mc = activation.gc(); let read = self.0.read(); if let Some(Some(namespace)) = read.namespaces.get(ns_index.0 as usize) { return Ok(*namespace); @@ -337,8 +338,8 @@ impl<'gc> TranslationUnit<'gc> { drop(read); - let namespace = Namespace::from_abc_namespace(self, ns_index, context)?; - self.0.write(context.gc_context).namespaces[ns_index.0 as usize] = Some(namespace); + let namespace = Namespace::from_abc_namespace(activation, self, ns_index)?; + self.0.write(mc).namespaces[ns_index.0 as usize] = Some(namespace); Ok(namespace) } @@ -347,10 +348,10 @@ impl<'gc> TranslationUnit<'gc> { /// The name can have a lazy component, do not pass it anywhere. pub fn pool_maybe_uninitialized_multiname( self, + activation: &mut Activation<'_, 'gc>, multiname_index: Index, - context: &mut UpdateContext<'gc>, ) -> Result>, Error<'gc>> { - let mc = context.gc_context; + let mc = activation.gc(); let read = self.0.read(); if let Some(Some(multiname)) = read.multinames.get(multiname_index.0 as usize) { return Ok(*multiname); @@ -358,7 +359,7 @@ impl<'gc> TranslationUnit<'gc> { drop(read); - let multiname = Multiname::from_abc_index(self, multiname_index, context)?; + let multiname = Multiname::from_abc_index(activation, self, multiname_index)?; let multiname = Gc::new(mc, multiname); self.0.write(mc).multinames[multiname_index.0 as usize] = Some(multiname); @@ -371,10 +372,10 @@ impl<'gc> TranslationUnit<'gc> { /// This version of the function treats index 0 as an error condition. pub fn pool_multiname_static( self, + activation: &mut Activation<'_, 'gc>, multiname_index: Index, - context: &mut UpdateContext<'gc>, ) -> Result>, Error<'gc>> { - let multiname = self.pool_maybe_uninitialized_multiname(multiname_index, context)?; + let multiname = self.pool_maybe_uninitialized_multiname(activation, multiname_index)?; if multiname.has_lazy_component() { return Err(format!("Multiname {} is not static", multiname_index.0).into()); } @@ -388,13 +389,13 @@ impl<'gc> TranslationUnit<'gc> { /// This version of the function treats index 0 as the any-type `*`. pub fn pool_multiname_static_any( self, + activation: &mut Activation<'_, 'gc>, multiname_index: Index, - context: &mut UpdateContext<'gc>, ) -> Result>, Error<'gc>> { if multiname_index.0 == 0 { - Ok(context.avm2.multinames.any) + Ok(activation.avm2().multinames.any) } else { - self.pool_multiname_static(multiname_index, context) + self.pool_multiname_static(activation, multiname_index) } } } diff --git a/core/src/avm2/traits.rs b/core/src/avm2/traits.rs index 7a5a6f3d3..9b75abae1 100644 --- a/core/src/avm2/traits.rs +++ b/core/src/avm2/traits.rs @@ -196,7 +196,7 @@ impl<'gc> Trait<'gc> { abc_trait: &AbcTrait, activation: &mut Activation<'_, 'gc>, ) -> Result> { - let name = QName::from_abc_multiname(unit, abc_trait.name, activation.context)?; + let name = QName::from_abc_multiname(activation, unit, abc_trait.name)?; Ok(match &abc_trait.kind { AbcTraitKind::Slot { @@ -204,7 +204,7 @@ impl<'gc> Trait<'gc> { type_name, value, } => { - let type_name = unit.pool_multiname_static_any(*type_name, activation.context)?; + let type_name = unit.pool_multiname_static_any(activation, *type_name)?; let default_value = slot_default_value(unit, value, &type_name, activation)?; Trait { name, @@ -268,7 +268,7 @@ impl<'gc> Trait<'gc> { type_name, value, } => { - let type_name = unit.pool_multiname_static_any(*type_name, activation.context)?; + let type_name = unit.pool_multiname_static_any(activation, *type_name)?; let default_value = slot_default_value(unit, value, &type_name, activation)?; Trait { name, diff --git a/core/src/avm2/value.rs b/core/src/avm2/value.rs index 728dc6abf..c679b6c53 100644 --- a/core/src/avm2/value.rs +++ b/core/src/avm2/value.rs @@ -534,7 +534,7 @@ pub fn abc_default_value<'gc>( | AbcDefaultValue::Explicit(ns) | AbcDefaultValue::StaticProtected(ns) | AbcDefaultValue::Private(ns) => { - let ns = translation_unit.pool_namespace(*ns, activation.context)?; + let ns = translation_unit.pool_namespace(activation, *ns)?; NamespaceObject::from_namespace(activation, ns).map(Into::into) } } diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index ee3bc60bf..099f2ad1a 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -314,7 +314,7 @@ pub fn verify_method<'gc>( AbcOp::FindDef { index } => { let multiname = method .translation_unit() - .pool_maybe_uninitialized_multiname(index, activation.context)?; + .pool_maybe_uninitialized_multiname(activation, index)?; if multiname.has_lazy_component() { return Err(Error::AvmError(verify_error( @@ -328,7 +328,7 @@ pub fn verify_method<'gc>( AbcOp::GetLex { index } => { let multiname = method .translation_unit() - .pool_maybe_uninitialized_multiname(index, activation.context)?; + .pool_maybe_uninitialized_multiname(activation, index)?; if multiname.has_lazy_component() { return Err(Error::AvmError(verify_error( @@ -370,7 +370,7 @@ pub fn verify_method<'gc>( | AbcOp::Coerce { index: name_index } => { let multiname = method .translation_unit() - .pool_maybe_uninitialized_multiname(name_index, activation.context)?; + .pool_maybe_uninitialized_multiname(activation, name_index)?; if multiname.has_lazy_component() { // This matches FP's error message @@ -419,7 +419,7 @@ pub fn verify_method<'gc>( } else { let pooled_type_name = method .translation_unit() - .pool_maybe_uninitialized_multiname(exception.type_name, activation.context)?; + .pool_maybe_uninitialized_multiname(activation, exception.type_name)?; if pooled_type_name.has_lazy_component() { // This matches FP's error message @@ -444,7 +444,7 @@ pub fn verify_method<'gc>( } else { let pooled_variable_name = method .translation_unit() - .pool_maybe_uninitialized_multiname(exception.variable_name, activation.context)?; + .pool_maybe_uninitialized_multiname(activation, exception.variable_name)?; // FIXME: avmplus also seems to check the namespace(s)? if pooled_variable_name.has_lazy_component() @@ -831,11 +831,9 @@ fn pool_multiname<'gc>( translation_unit: TranslationUnit<'gc>, index: Index, ) -> Result>, Error<'gc>> { - if index.0 == 0 { - return Err(make_error_1032(activation, 0)); - } - - translation_unit.pool_maybe_uninitialized_multiname(index, activation.context) + // `Multiname::from_abc_index` will do constant pool range checks anyway, so + // don't perform an extra one here + translation_unit.pool_maybe_uninitialized_multiname(activation, index) } fn pool_string<'gc>(