From 889a9bd00990abee72995c0e5f5238eb76fdc734 Mon Sep 17 00:00:00 2001 From: Moulins Date: Thu, 16 Mar 2023 17:53:06 +0100 Subject: [PATCH] avm2: Intern strings in constant pools Strings belonging to multinames are interned too, but the multinames themselves aren't. --- core/src/avm2/activation.rs | 28 ++++++++++++-------- core/src/avm2/class.rs | 12 ++++----- core/src/avm2/method.rs | 6 ++--- core/src/avm2/multiname.rs | 28 +++++++++++--------- core/src/avm2/namespace.rs | 22 +++++++-------- core/src/avm2/qname.rs | 7 ++--- core/src/avm2/script.rs | 53 ++++++++++++++++++++----------------- core/src/avm2/traits.rs | 7 +++-- core/src/avm2/value.rs | 13 +++++---- 9 files changed, 93 insertions(+), 83 deletions(-) diff --git a/core/src/avm2/activation.rs b/core/src/avm2/activation.rs index ec9662909..a43eec186 100644 --- a/core/src/avm2/activation.rs +++ b/core/src/avm2/activation.rs @@ -20,7 +20,7 @@ use crate::avm2::Multiname; use crate::avm2::Namespace; use crate::avm2::QName; use crate::avm2::{value, Avm2, Error}; -use crate::context::UpdateContext; +use crate::context::{GcContext, UpdateContext}; use crate::string::AvmString; use crate::swf::extensions::ReadSwfExt; use gc_arena::{Gc, GcCell}; @@ -701,6 +701,11 @@ impl<'a, 'gc> Activation<'a, 'gc> { self.context.avm2 } + #[inline] + pub fn borrow_gc(&mut self) -> GcContext<'_, 'gc> { + self.context.borrow_gc() + } + /// Get the class that defined the currently-executing method, if it /// exists. /// @@ -815,24 +820,24 @@ impl<'a, 'gc> Activation<'a, 'gc> { /// Retrieve a string from the current constant pool. fn pool_string<'b>( - &self, + &mut self, method: &'b BytecodeMethod<'gc>, index: Index, ) -> Result, Error<'gc>> { method .translation_unit() - .pool_string(index.0, self.context.gc_context) + .pool_string(index.0, &mut self.borrow_gc()) } /// Retrieve a namespace from the current constant pool. fn pool_namespace( - &self, + &mut self, method: Gc<'gc, BytecodeMethod<'gc>>, index: Index, ) -> Result, Error<'gc>> { method .translation_unit() - .pool_namespace(index, self.context.gc_context) + .pool_namespace(index, &mut self.borrow_gc()) } /// Retrieve a multiname from the current constant pool. @@ -843,7 +848,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { ) -> Result>, Error<'gc>> { method .translation_unit() - .pool_maybe_uninitialized_multiname(index, self.context.gc_context) + .pool_maybe_uninitialized_multiname(index, &mut self.borrow_gc()) } /// Retrieve a multiname from the current constant pool. @@ -855,7 +860,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { ) -> Result>, Error<'gc>> { let name = method .translation_unit() - .pool_maybe_uninitialized_multiname(index, self.context.gc_context)?; + .pool_maybe_uninitialized_multiname(index, &mut self.borrow_gc())?; if name.has_lazy_component() { let name = name.fill_with_runtime_params(self)?; Ok(Gc::allocate(self.context.gc_context, name)) @@ -875,7 +880,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { ) -> Result>, Error<'gc>> { method .translation_unit() - .pool_multiname_static(index, self.context.gc_context) + .pool_multiname_static(index, &mut self.borrow_gc()) } /// Retrieve a static, or non-runtime, multiname from the current constant @@ -889,7 +894,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { ) -> Result>, Error<'gc>> { method .translation_unit() - .pool_multiname_static_any(index, self.context.gc_context) + .pool_multiname_static_any(index, &mut self.borrow_gc()) } /// Retrieve a method entry from the current ABC file's method table. @@ -1275,7 +1280,8 @@ impl<'a, 'gc> Activation<'a, 'gc> { method: Gc<'gc, BytecodeMethod<'gc>>, value: Index, ) -> Result, Error<'gc>> { - self.push_stack(self.pool_string(&method, value)?); + let s = self.pool_string(&method, value)?; + self.push_stack(s); Ok(FrameControl::Continue) } @@ -1739,7 +1745,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let qname = QName::from_abc_multiname( method.translation_unit(), vname, - self.context.gc_context, + &mut self.borrow_gc(), )?; ScriptObject::catch_scope(self.context.gc_context, &qname) }; diff --git a/core/src/avm2/class.rs b/core/src/avm2/class.rs index e639842d2..859e2d284 100644 --- a/core/src/avm2/class.rs +++ b/core/src/avm2/class.rs @@ -272,20 +272,20 @@ 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.gc_context)?; + let mut context = activation.borrow_gc(); + let name = QName::from_abc_multiname(unit, abc_instance.name, &mut context)?; let super_class = if abc_instance.super_name.0 == 0 { None } else { Some( - unit.pool_multiname_static(abc_instance.super_name, activation.context.gc_context)? + unit.pool_multiname_static(abc_instance.super_name, &mut context)? .deref() .clone(), ) }; let protected_namespace = if let Some(ns) = &abc_instance.protected_namespace { - Some(unit.pool_namespace(*ns, activation.context.gc_context)?) + Some(unit.pool_namespace(*ns, &mut context)?) } else { None }; @@ -293,7 +293,7 @@ impl<'gc> Class<'gc> { let mut interfaces = Vec::with_capacity(abc_instance.interfaces.len()); for interface_name in &abc_instance.interfaces { interfaces.push( - unit.pool_multiname_static(*interface_name, activation.context.gc_context)? + unit.pool_multiname_static(*interface_name, &mut context)? .deref() .clone(), ); @@ -495,7 +495,7 @@ impl<'gc> Class<'gc> { body: &AbcMethodBody, ) -> Result, Error<'gc>> { let name = - translation_unit.pool_string(method.name.as_u30(), activation.context.gc_context)?; + translation_unit.pool_string(method.name.as_u30(), &mut activation.borrow_gc())?; let mut traits = Vec::with_capacity(body.traits.len()); for trait_entry in body.traits.iter() { diff --git a/core/src/avm2/method.rs b/core/src/avm2/method.rs index 85ba3532e..a1330f9dc 100644 --- a/core/src/avm2/method.rs +++ b/core/src/avm2/method.rs @@ -57,12 +57,12 @@ impl<'gc> ParamConfig<'gc> { activation: &mut Activation<'_, 'gc>, ) -> Result> { let param_name = if let Some(name) = &config.name { - txunit.pool_string(name.0, activation.context.gc_context)? + txunit.pool_string(name.0, &mut activation.borrow_gc())? } else { "".into() }; let param_type_name = txunit - .pool_multiname_static_any(config.kind, activation.context.gc_context)? + .pool_multiname_static_any(config.kind, &mut activation.borrow_gc())? .deref() .clone(); @@ -151,7 +151,7 @@ impl<'gc> BytecodeMethod<'gc> { } let return_type = txunit - .pool_multiname_static_any(method.return_type, activation.context.gc_context)? + .pool_multiname_static_any(method.return_type, &mut activation.borrow_gc())? .deref() .clone(); diff --git a/core/src/avm2/multiname.rs b/core/src/avm2/multiname.rs index cea33a1c6..6976f5255 100644 --- a/core/src/avm2/multiname.rs +++ b/core/src/avm2/multiname.rs @@ -4,6 +4,7 @@ use crate::avm2::Error; use crate::avm2::Namespace; use crate::avm2::QName; use crate::avm2::{Object, Value}; +use crate::context::GcContext; use crate::string::{AvmString, WStr, WString}; use bitflags::bitflags; use gc_arena::Gc; @@ -113,7 +114,7 @@ impl<'gc> Multiname<'gc> { fn abc_namespace_set( translation_unit: TranslationUnit<'gc>, namespace_set_index: Index, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result, Error<'gc>> { if namespace_set_index.0 == 0 { return Err(Error::RustError( @@ -134,37 +135,38 @@ impl<'gc> Multiname<'gc> { if ns_set.len() == 1 { Ok(NamespaceSet::single( - translation_unit.pool_namespace(ns_set[0], mc)?, + translation_unit.pool_namespace(ns_set[0], context)?, )) } else { let mut result = Vec::with_capacity(ns_set.len()); for ns in ns_set { - result.push(translation_unit.pool_namespace(*ns, mc)?) + result.push(translation_unit.pool_namespace(*ns, context)?) } - Ok(NamespaceSet::multiple(result, mc)) + Ok(NamespaceSet::multiple(result, context.gc_context)) } } pub fn from_abc_index( translation_unit: TranslationUnit<'gc>, multiname_index: Index, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result> { + let mc = context.gc_context; let abc = translation_unit.abc(); let abc_multiname = Self::resolve_multiname_index(&abc, multiname_index)?; let mut multiname = match abc_multiname { AbcMultiname::QName { namespace, name } | AbcMultiname::QNameA { namespace, name } => { Self { - ns: NamespaceSet::single(translation_unit.pool_namespace(*namespace, mc)?), - name: translation_unit.pool_string_option(name.0, mc)?, + ns: NamespaceSet::single(translation_unit.pool_namespace(*namespace, context)?), + name: translation_unit.pool_string_option(name.0, context)?, params: Vec::new(), flags: Default::default(), } } AbcMultiname::RTQName { name } | AbcMultiname::RTQNameA { name } => Self { ns: NamespaceSet::multiple(vec![], mc), - name: translation_unit.pool_string_option(name.0, mc)?, + name: translation_unit.pool_string_option(name.0, context)?, params: Vec::new(), flags: MultinameFlags::HAS_LAZY_NS, }, @@ -182,14 +184,14 @@ impl<'gc> Multiname<'gc> { namespace_set, name, } => Self { - ns: Self::abc_namespace_set(translation_unit, *namespace_set, mc)?, - name: translation_unit.pool_string_option(name.0, mc)?, + ns: Self::abc_namespace_set(translation_unit, *namespace_set, context)?, + name: translation_unit.pool_string_option(name.0, context)?, params: Vec::new(), flags: Default::default(), }, AbcMultiname::MultinameL { namespace_set } | AbcMultiname::MultinameLA { namespace_set } => Self { - ns: Self::abc_namespace_set(translation_unit, *namespace_set, mc)?, + ns: Self::abc_namespace_set(translation_unit, *namespace_set, context)?, name: None, params: Vec::new(), flags: MultinameFlags::HAS_LAZY_NAME, @@ -199,7 +201,7 @@ impl<'gc> Multiname<'gc> { parameters, } => { let mut base = translation_unit - .pool_multiname_static(*base_type, mc)? + .pool_multiname_static(*base_type, context)? .deref() .clone(); @@ -213,7 +215,7 @@ impl<'gc> Multiname<'gc> { for param_type in parameters { let param_multiname = - translation_unit.pool_multiname_static_any(*param_type, mc)?; + translation_unit.pool_multiname_static_any(*param_type, context)?; base.params.push(param_multiname); } diff --git a/core/src/avm2/namespace.rs b/core/src/avm2/namespace.rs index bf7034318..db2dcc9be 100644 --- a/core/src/avm2/namespace.rs +++ b/core/src/avm2/namespace.rs @@ -1,6 +1,6 @@ -use crate::avm2::script::TranslationUnit; use crate::avm2::Error; use crate::string::AvmString; +use crate::{avm2::script::TranslationUnit, context::GcContext}; use gc_arena::{Collect, Gc, MutationContext}; use std::fmt::Debug; use swf::avm2::types::{Index, Namespace as AbcNamespace}; @@ -46,10 +46,10 @@ impl<'gc> Namespace<'gc> { pub fn from_abc_namespace( translation_unit: TranslationUnit<'gc>, namespace_index: Index, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result> { if namespace_index.0 == 0 { - return Ok(Self::any(mc)); + return Ok(Self::any(context.gc_context)); } let actual_index = namespace_index.0 as usize - 1; @@ -62,28 +62,28 @@ impl<'gc> Namespace<'gc> { let ns = match abc_namespace? { AbcNamespace::Namespace(idx) => { - NamespaceData::Namespace(translation_unit.pool_string(idx.0, mc)?) + NamespaceData::Namespace(translation_unit.pool_string(idx.0, context)?) } AbcNamespace::Package(idx) => { - NamespaceData::Namespace(translation_unit.pool_string(idx.0, mc)?) + NamespaceData::Namespace(translation_unit.pool_string(idx.0, context)?) } AbcNamespace::PackageInternal(idx) => { - NamespaceData::PackageInternal(translation_unit.pool_string(idx.0, mc)?) + NamespaceData::PackageInternal(translation_unit.pool_string(idx.0, context)?) } AbcNamespace::Protected(idx) => { - NamespaceData::Protected(translation_unit.pool_string(idx.0, mc)?) + NamespaceData::Protected(translation_unit.pool_string(idx.0, context)?) } AbcNamespace::Explicit(idx) => { - NamespaceData::Explicit(translation_unit.pool_string(idx.0, mc)?) + NamespaceData::Explicit(translation_unit.pool_string(idx.0, context)?) } AbcNamespace::StaticProtected(idx) => { - NamespaceData::StaticProtected(translation_unit.pool_string(idx.0, mc)?) + NamespaceData::StaticProtected(translation_unit.pool_string(idx.0, context)?) } AbcNamespace::Private(idx) => { - NamespaceData::Private(translation_unit.pool_string(idx.0, mc)?) + NamespaceData::Private(translation_unit.pool_string(idx.0, context)?) } }; - Ok(Self(Gc::allocate(mc, ns))) + Ok(Self(Gc::allocate(context.gc_context, ns))) } pub fn any(mc: MutationContext<'gc, '_>) -> Self { diff --git a/core/src/avm2/qname.rs b/core/src/avm2/qname.rs index e76cbd1a9..a8fc9ebcb 100644 --- a/core/src/avm2/qname.rs +++ b/core/src/avm2/qname.rs @@ -2,6 +2,7 @@ use crate::avm2::script::TranslationUnit; use crate::avm2::Activation; use crate::avm2::Error; use crate::avm2::{Namespace, NamespaceData}; +use crate::context::GcContext; use crate::either::Either; use crate::string::{AvmString, WStr, WString}; use gc_arena::{Collect, MutationContext}; @@ -48,7 +49,7 @@ impl<'gc> QName<'gc> { pub fn from_abc_multiname( translation_unit: TranslationUnit<'gc>, multiname_index: Index, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result> { if multiname_index.0 == 0 { return Err("Attempted to load a trait name of index zero".into()); @@ -64,8 +65,8 @@ impl<'gc> QName<'gc> { Ok(match abc_multiname? { AbcMultiname::QName { namespace, name } => Self { - ns: translation_unit.pool_namespace(*namespace, mc)?, - name: translation_unit.pool_string(name.0, mc)?, + ns: translation_unit.pool_namespace(*namespace, context)?, + name: translation_unit.pool_string(name.0, context)?, }, _ => return Err("Attempted to pull QName from non-QName multiname".into()), }) diff --git a/core/src/avm2/script.rs b/core/src/avm2/script.rs index 05fbef441..d20599f1a 100644 --- a/core/src/avm2/script.rs +++ b/core/src/avm2/script.rs @@ -12,7 +12,7 @@ use crate::avm2::value::Value; use crate::avm2::Multiname; use crate::avm2::Namespace; use crate::avm2::{Avm2, Error}; -use crate::context::UpdateContext; +use crate::context::{GcContext, UpdateContext}; use crate::string::AvmString; use gc_arena::{Collect, Gc, GcCell, MutationContext}; use std::cell::Ref; @@ -231,9 +231,9 @@ impl<'gc> TranslationUnit<'gc> { pub fn pool_string_option( self, string_index: u32, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result>, Error<'gc>> { - let mut write = self.0.write(mc); + let mut write = self.0.write(context.gc_context); if let Some(Some(string)) = write.strings.get(string_index as usize) { return Ok(Some(*string)); } @@ -242,17 +242,18 @@ impl<'gc> TranslationUnit<'gc> { return Ok(None); } - let avm_string = AvmString::new_utf8( - mc, - write - .abc - .constant_pool - .strings - .get(string_index as usize - 1) - .ok_or_else(|| format!("Unknown string constant {string_index}"))?, - ); - write.strings[string_index as usize] = Some(avm_string); + let raw = write + .abc + .constant_pool + .strings + .get(string_index as usize - 1) + .ok_or_else(|| format!("Unknown string constant {string_index}"))?; + let avm_string = context + .interner + .intern_wstr(context.gc_context, ruffle_wstr::from_utf8(raw)); + + write.strings[string_index as usize] = Some(avm_string); Ok(Some(avm_string)) } @@ -265,11 +266,11 @@ impl<'gc> TranslationUnit<'gc> { pub fn pool_string( self, string_index: u32, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result, Error<'gc>> { Ok(self - .pool_string_option(string_index, mc)? - .unwrap_or_default()) + .pool_string_option(string_index, context)? + .unwrap_or_else(AvmString::default)) } /// Retrieve a static, or non-runtime, multiname from the current constant @@ -279,7 +280,7 @@ impl<'gc> TranslationUnit<'gc> { pub fn pool_namespace( self, ns_index: Index, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result, Error<'gc>> { let read = self.0.read(); if let Some(Some(namespace)) = read.namespaces.get(ns_index.0 as usize) { @@ -288,8 +289,8 @@ impl<'gc> TranslationUnit<'gc> { drop(read); - let namespace = Namespace::from_abc_namespace(self, ns_index, mc)?; - self.0.write(mc).namespaces[ns_index.0 as usize] = Some(namespace); + let namespace = Namespace::from_abc_namespace(self, ns_index, context)?; + self.0.write(context.gc_context).namespaces[ns_index.0 as usize] = Some(namespace); Ok(namespace) } @@ -299,8 +300,9 @@ impl<'gc> TranslationUnit<'gc> { pub fn pool_maybe_uninitialized_multiname( self, multiname_index: Index, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result>, Error<'gc>> { + let mc = context.gc_context; let read = self.0.read(); if let Some(Some(multiname)) = read.multinames.get(multiname_index.0 as usize) { return Ok(*multiname); @@ -308,7 +310,7 @@ impl<'gc> TranslationUnit<'gc> { drop(read); - let multiname = Multiname::from_abc_index(self, multiname_index, mc)?; + let multiname = Multiname::from_abc_index(self, multiname_index, context)?; let multiname = Gc::allocate(mc, multiname); self.0.write(mc).multinames[multiname_index.0 as usize] = Some(multiname); @@ -322,9 +324,9 @@ impl<'gc> TranslationUnit<'gc> { pub fn pool_multiname_static( self, multiname_index: Index, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result>, Error<'gc>> { - let multiname = self.pool_maybe_uninitialized_multiname(multiname_index, mc)?; + let multiname = self.pool_maybe_uninitialized_multiname(multiname_index, context)?; if multiname.has_lazy_component() { return Err(format!("Multiname {} is not static", multiname_index.0).into()); } @@ -339,12 +341,13 @@ impl<'gc> TranslationUnit<'gc> { pub fn pool_multiname_static_any( self, multiname_index: Index, - mc: MutationContext<'gc, '_>, + context: &mut GcContext<'_, 'gc>, ) -> Result>, Error<'gc>> { if multiname_index.0 == 0 { + let mc = context.gc_context; Ok(Gc::allocate(mc, Multiname::any(mc))) } else { - self.pool_multiname_static(multiname_index, mc) + self.pool_multiname_static(multiname_index, context) } } } diff --git a/core/src/avm2/traits.rs b/core/src/avm2/traits.rs index 3afc8e932..fb5f77b26 100644 --- a/core/src/avm2/traits.rs +++ b/core/src/avm2/traits.rs @@ -191,8 +191,7 @@ impl<'gc> Trait<'gc> { abc_trait: &AbcTrait, activation: &mut Activation<'_, 'gc>, ) -> Result> { - let mc = activation.context.gc_context; - let name = QName::from_abc_multiname(unit, abc_trait.name, mc)?; + let name = QName::from_abc_multiname(unit, abc_trait.name, &mut activation.borrow_gc())?; Ok(match &abc_trait.kind { AbcTraitKind::Slot { @@ -201,7 +200,7 @@ impl<'gc> Trait<'gc> { value, } => { let type_name = unit - .pool_multiname_static_any(*type_name, mc)? + .pool_multiname_static_any(*type_name, &mut activation.borrow_gc())? .deref() .clone(); let default_value = slot_default_value(unit, value, &type_name, activation)?; @@ -262,7 +261,7 @@ impl<'gc> Trait<'gc> { value, } => { let type_name = unit - .pool_multiname_static_any(*type_name, mc)? + .pool_multiname_static_any(*type_name, &mut activation.borrow_gc())? .deref() .clone(); let default_value = slot_default_value(unit, value, &type_name, activation)?; diff --git a/core/src/avm2/value.rs b/core/src/avm2/value.rs index a0231aeee..0b7f5b60c 100644 --- a/core/src/avm2/value.rs +++ b/core/src/avm2/value.rs @@ -516,8 +516,8 @@ pub fn abc_default_value<'gc>( AbcDefaultValue::Uint(u) => abc_uint(translation_unit, *u).map(|v| v.into()), AbcDefaultValue::Double(d) => abc_double(translation_unit, *d).map(|v| v.into()), AbcDefaultValue::String(s) => translation_unit - .pool_string(s.0, activation.context.gc_context) - .map(|v| v.into()), + .pool_string(s.0, &mut activation.borrow_gc()) + .map(Into::into), AbcDefaultValue::True => Ok(true.into()), AbcDefaultValue::False => Ok(false.into()), AbcDefaultValue::Null => Ok(Value::Null), @@ -528,11 +528,10 @@ pub fn abc_default_value<'gc>( | AbcDefaultValue::Protected(ns) | AbcDefaultValue::Explicit(ns) | AbcDefaultValue::StaticProtected(ns) - | AbcDefaultValue::Private(ns) => Ok(NamespaceObject::from_namespace( - activation, - translation_unit.pool_namespace(*ns, activation.context.gc_context)?, - )? - .into()), + | AbcDefaultValue::Private(ns) => { + let ns = translation_unit.pool_namespace(*ns, &mut activation.borrow_gc())?; + NamespaceObject::from_namespace(activation, ns).map(Into::into) + } } }