Run all string constant retrieval through the `TranslationUnit`, preventing us from making multiple copies of the same string.

For good measure, most of the other methods in `value` for retrieving pool primitives now also use `TranslationUnit` instead of `AbcFile`. This is the result of a handful of cascading changes throughout the project, and itself caused a few more.
This commit is contained in:
David Wendt 2020-07-14 20:05:25 -04:00
parent 70e9e7e9e9
commit f4c5075086
6 changed files with 90 additions and 105 deletions

View File

@ -301,7 +301,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
method: Gc<'gc, BytecodeMethod<'gc>>,
index: Index<i32>,
) -> Result<i32, Error> {
value::abc_int(&method.abc(), index)
value::abc_int(method.translation_unit(), index)
}
/// Retrieve a int from the current constant pool.
@ -310,7 +310,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
method: Gc<'gc, BytecodeMethod<'gc>>,
index: Index<u32>,
) -> Result<u32, Error> {
value::abc_uint(&method.abc(), index)
value::abc_uint(method.translation_unit(), index)
}
/// Retrieve a double from the current constant pool.
@ -319,7 +319,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
method: Gc<'gc, BytecodeMethod<'gc>>,
index: Index<f64>,
) -> Result<f64, Error> {
value::abc_double(&method.abc(), index)
value::abc_double(method.translation_unit(), index)
}
/// Retrieve a string from the current constant pool.
@ -329,7 +329,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
index: Index<String>,
mc: MutationContext<'gc, '_>,
) -> Result<AvmString<'gc>, Error> {
value::abc_string(&method.abc_ref(), index, mc)
method.translation_unit().pool_string(index.0, mc)
}
/// Retrieve a namespace from the current constant pool.
@ -339,7 +339,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
index: Index<AbcNamespace>,
mc: MutationContext<'gc, '_>,
) -> Result<Namespace<'gc>, Error> {
Namespace::from_abc_namespace(&method.abc(), index, mc)
Namespace::from_abc_namespace(method.translation_unit(), index, mc)
}
/// Retrieve a multiname from the current constant pool.
@ -349,7 +349,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
index: Index<AbcMultiname>,
mc: MutationContext<'gc, '_>,
) -> Result<Multiname<'gc>, Error> {
Multiname::from_abc_multiname(&method.abc(), index, self.avm2, mc)
Multiname::from_abc_multiname(method.translation_unit(), index, self.avm2, mc)
}
/// Retrieve a static, or non-runtime, multiname from the current constant
@ -360,7 +360,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
index: Index<AbcMultiname>,
mc: MutationContext<'gc, '_>,
) -> Result<Multiname<'gc>, Error> {
Multiname::from_abc_multiname_static(&method.abc(), index, mc)
Multiname::from_abc_multiname_static(method.translation_unit(), index, mc)
}
/// Retrieve a method entry from the current ABC file's method table.

View File

@ -120,19 +120,19 @@ 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(), abc_instance.name.clone(), mc)?;
let name = QName::from_abc_multiname(unit, abc_instance.name.clone(), mc)?;
let super_class = if abc_instance.super_name.0 == 0 {
None
} else {
Some(Multiname::from_abc_multiname_static(
&unit.abc(),
unit,
abc_instance.super_name.clone(),
mc,
)?)
};
let protected_namespace = if let Some(ns) = &abc_instance.protected_namespace {
Some(Namespace::from_abc_namespace(&unit.abc(), ns.clone(), mc)?)
Some(Namespace::from_abc_namespace(unit, ns.clone(), mc)?)
} else {
None
};
@ -140,7 +140,7 @@ impl<'gc> Class<'gc> {
let mut interfaces = Vec::new();
for interface_name in abc_instance.interfaces.iter() {
interfaces.push(Multiname::from_abc_multiname_static(
&unit.abc(),
unit,
interface_name.clone(),
mc,
)?);

View File

@ -1,12 +1,11 @@
//! AVM2 names & namespacing
use crate::avm1::AvmString;
use crate::avm2::value::{abc_string, abc_string_option};
use crate::avm2::script::TranslationUnit;
use crate::avm2::{Avm2, Error};
use gc_arena::{Collect, MutationContext};
use swf::avm2::types::{
AbcFile, Index, Multiname as AbcMultiname, Namespace as AbcNamespace,
NamespaceSet as AbcNamespaceSet,
Index, Multiname as AbcMultiname, Namespace as AbcNamespace, NamespaceSet as AbcNamespaceSet,
};
/// Represents the name of a namespace.
@ -27,7 +26,7 @@ impl<'gc> Namespace<'gc> {
/// Read a namespace declaration from the ABC constant pool and copy it to
/// a namespace value.
pub fn from_abc_namespace(
file: &AbcFile,
translation_unit: TranslationUnit<'gc>,
namespace_index: Index<AbcNamespace>,
mc: MutationContext<'gc, '_>,
) -> Result<Self, Error> {
@ -36,24 +35,29 @@ impl<'gc> Namespace<'gc> {
}
let actual_index = namespace_index.0 as usize - 1;
let abc_namespace: Result<&AbcNamespace, Error> = file
let abc = translation_unit.abc();
let abc_namespace: Result<_, Error> = abc
.constant_pool
.namespaces
.get(actual_index)
.ok_or_else(|| format!("Unknown namespace constant {}", namespace_index.0).into());
Ok(match abc_namespace? {
AbcNamespace::Namespace(idx) => Self::Namespace(abc_string(file, idx.clone(), mc)?),
AbcNamespace::Package(idx) => Self::Package(abc_string(file, idx.clone(), mc)?),
AbcNamespace::Namespace(idx) => {
Self::Namespace(translation_unit.pool_string(idx.0, mc)?)
}
AbcNamespace::Package(idx) => Self::Package(translation_unit.pool_string(idx.0, mc)?),
AbcNamespace::PackageInternal(idx) => {
Self::PackageInternal(abc_string(file, idx.clone(), mc)?)
Self::PackageInternal(translation_unit.pool_string(idx.0, mc)?)
}
AbcNamespace::Protected(idx) => Self::Protected(abc_string(file, idx.clone(), mc)?),
AbcNamespace::Explicit(idx) => Self::Explicit(abc_string(file, idx.clone(), mc)?),
AbcNamespace::Protected(idx) => {
Self::Protected(translation_unit.pool_string(idx.0, mc)?)
}
AbcNamespace::Explicit(idx) => Self::Explicit(translation_unit.pool_string(idx.0, mc)?),
AbcNamespace::StaticProtected(idx) => {
Self::StaticProtected(abc_string(file, idx.clone(), mc)?)
Self::StaticProtected(translation_unit.pool_string(idx.0, mc)?)
}
AbcNamespace::Private(idx) => Self::Private(abc_string(file, idx.clone(), mc)?),
AbcNamespace::Private(idx) => Self::Private(translation_unit.pool_string(idx.0, mc)?),
})
}
@ -120,12 +124,13 @@ impl<'gc> QName<'gc> {
/// This function returns an Err if the multiname does not exist or is not
/// a `QName`.
pub fn from_abc_multiname(
file: &AbcFile,
translation_unit: TranslationUnit<'gc>,
multiname_index: Index<AbcMultiname>,
mc: MutationContext<'gc, '_>,
) -> Result<Self, Error> {
let actual_index = multiname_index.0 as usize - 1;
let abc_multiname: Result<&AbcMultiname, Error> = file
let abc = translation_unit.abc();
let abc_multiname: Result<_, Error> = abc
.constant_pool
.multinames
.get(actual_index)
@ -133,8 +138,8 @@ impl<'gc> QName<'gc> {
Ok(match abc_multiname? {
AbcMultiname::QName { namespace, name } => Self {
ns: Namespace::from_abc_namespace(file, namespace.clone(), mc)?,
name: abc_string(file, name.clone(), mc)?,
ns: Namespace::from_abc_namespace(translation_unit, namespace.clone(), mc)?,
name: translation_unit.pool_string(name.0, mc)?,
},
_ => return Err("Attempted to pull QName from non-QName multiname".into()),
})
@ -167,7 +172,7 @@ impl<'gc> Multiname<'gc> {
/// Read a namespace set from the ABC constant pool, and return a list of
/// copied namespaces.
fn abc_namespace_set(
file: &AbcFile,
translation_unit: TranslationUnit<'gc>,
namespace_set_index: Index<AbcNamespaceSet>,
mc: MutationContext<'gc, '_>,
) -> Result<Vec<Namespace<'gc>>, Error> {
@ -177,7 +182,8 @@ impl<'gc> Multiname<'gc> {
}
let actual_index = namespace_set_index.0 as usize - 1;
let ns_set: Result<&AbcNamespaceSet, Error> = file
let abc = translation_unit.abc();
let ns_set: Result<_, Error> = abc
.constant_pool
.namespace_sets
.get(actual_index)
@ -187,7 +193,11 @@ impl<'gc> Multiname<'gc> {
let mut result = vec![];
for ns in ns_set? {
result.push(Namespace::from_abc_namespace(file, ns.clone(), mc)?)
result.push(Namespace::from_abc_namespace(
translation_unit,
ns.clone(),
mc,
)?)
}
Ok(result)
@ -196,7 +206,7 @@ impl<'gc> Multiname<'gc> {
/// Read a multiname from the ABC constant pool, copying it into the most
/// general form of multiname.
pub fn from_abc_multiname(
file: &AbcFile,
translation_unit: TranslationUnit<'gc>,
multiname_index: Index<AbcMultiname>,
avm: &mut Avm2<'gc>,
mc: MutationContext<'gc, '_>,
@ -205,7 +215,8 @@ impl<'gc> Multiname<'gc> {
.checked_sub(1)
.ok_or_else(|| "Attempted to resolve a multiname at index zero. This is a bug.".into());
let actual_index = actual_index?;
let abc_multiname: Result<&AbcMultiname, Error> = file
let abc = translation_unit.abc();
let abc_multiname: Result<_, Error> = abc
.constant_pool
.multinames
.get(actual_index)
@ -214,15 +225,19 @@ impl<'gc> Multiname<'gc> {
Ok(match abc_multiname? {
AbcMultiname::QName { namespace, name } | AbcMultiname::QNameA { namespace, name } => {
Self {
ns: vec![Namespace::from_abc_namespace(file, namespace.clone(), mc)?],
name: abc_string_option(file, name.clone(), mc)?,
ns: vec![Namespace::from_abc_namespace(
translation_unit,
namespace.clone(),
mc,
)?],
name: translation_unit.pool_string_option(name.0, mc)?,
}
}
AbcMultiname::RTQName { name } | AbcMultiname::RTQNameA { name } => {
let ns = avm.pop().as_namespace()?.clone();
Self {
ns: vec![ns],
name: abc_string_option(file, name.clone(), mc)?,
name: translation_unit.pool_string_option(name.0, mc)?,
}
}
AbcMultiname::RTQNameL | AbcMultiname::RTQNameLA => {
@ -241,14 +256,14 @@ impl<'gc> Multiname<'gc> {
namespace_set,
name,
} => Self {
ns: Self::abc_namespace_set(file, namespace_set.clone(), mc)?,
name: abc_string_option(file, name.clone(), mc)?,
ns: Self::abc_namespace_set(translation_unit, namespace_set.clone(), mc)?,
name: translation_unit.pool_string_option(name.0, mc)?,
},
AbcMultiname::MultinameL { namespace_set }
| AbcMultiname::MultinameLA { namespace_set } => {
let name = avm.pop().as_string()?;
Self {
ns: Self::abc_namespace_set(file, namespace_set.clone(), mc)?,
ns: Self::abc_namespace_set(translation_unit, namespace_set.clone(), mc)?,
name: Some(name),
}
}
@ -260,7 +275,7 @@ impl<'gc> Multiname<'gc> {
/// This function prohibits the use of runtime-qualified and late-bound
/// names. Runtime multinames will instead result in an error.
pub fn from_abc_multiname_static(
file: &AbcFile,
translation_unit: TranslationUnit<'gc>,
multiname_index: Index<AbcMultiname>,
mc: MutationContext<'gc, '_>,
) -> Result<Self, Error> {
@ -269,7 +284,8 @@ impl<'gc> Multiname<'gc> {
"Attempted to resolve a (static) multiname at index zero. This is a bug.".into()
});
let actual_index = actual_index?;
let abc_multiname: Result<&AbcMultiname, Error> = file
let abc = translation_unit.abc();
let abc_multiname: Result<_, Error> = abc
.constant_pool
.multinames
.get(actual_index)
@ -278,8 +294,12 @@ impl<'gc> Multiname<'gc> {
Ok(match abc_multiname? {
AbcMultiname::QName { namespace, name } | AbcMultiname::QNameA { namespace, name } => {
Self {
ns: vec![Namespace::from_abc_namespace(file, namespace.clone(), mc)?],
name: abc_string_option(file, name.clone(), mc)?,
ns: vec![Namespace::from_abc_namespace(
translation_unit,
namespace.clone(),
mc,
)?],
name: translation_unit.pool_string_option(name.0, mc)?,
}
}
AbcMultiname::Multiname {
@ -290,8 +310,8 @@ impl<'gc> Multiname<'gc> {
namespace_set,
name,
} => Self {
ns: Self::abc_namespace_set(file, namespace_set.clone(), mc)?,
name: abc_string_option(file, name.clone(), mc)?,
ns: Self::abc_namespace_set(translation_unit, namespace_set.clone(), mc)?,
name: translation_unit.pool_string_option(name.0, mc)?,
},
_ => return Err(format!("Multiname {} is not static", multiname_index.0).into()),
})

View File

@ -260,7 +260,7 @@ impl<'gc> Script<'gc> {
self.traits_loaded = true;
let abc = unit.abc();
let script: Result<&AbcScript, Error> = abc
let script: Result<_, Error> = abc
.scripts
.get(script_index as usize)
.ok_or_else(|| "LoadError: Script index not valid".into());

View File

@ -87,7 +87,7 @@ impl<'gc> Trait<'gc> {
abc_trait: &AbcTrait,
mc: MutationContext<'gc, '_>,
) -> Result<Self, Error> {
let name = QName::from_abc_multiname(&unit.abc(), abc_trait.name.clone(), mc)?;
let name = QName::from_abc_multiname(unit, abc_trait.name.clone(), mc)?;
Ok(match &abc_trait.kind {
AbcTraitKind::Slot {
@ -103,10 +103,10 @@ impl<'gc> Trait<'gc> {
type_name: if type_name.0 == 0 {
Multiname::any()
} else {
Multiname::from_abc_multiname_static(&unit.abc(), type_name.clone(), mc)?
Multiname::from_abc_multiname_static(unit, type_name.clone(), mc)?
},
default_value: if let Some(dv) = value {
Some(abc_default_value(&unit.abc(), &dv, mc)?)
Some(abc_default_value(unit, &dv, mc)?)
} else {
None
},
@ -170,10 +170,10 @@ impl<'gc> Trait<'gc> {
type_name: if type_name.0 == 0 {
Multiname::any()
} else {
Multiname::from_abc_multiname_static(&unit.abc(), type_name.clone(), mc)?
Multiname::from_abc_multiname_static(unit, type_name.clone(), mc)?
},
default_value: if let Some(dv) = value {
Some(abc_default_value(&unit.abc(), &dv, mc)?)
Some(abc_default_value(unit, &dv, mc)?)
} else {
None
},

View File

@ -3,10 +3,11 @@
use crate::avm1::AvmString;
use crate::avm2::names::Namespace;
use crate::avm2::object::Object;
use crate::avm2::script::TranslationUnit;
use crate::avm2::Error;
use gc_arena::{Collect, MutationContext};
use std::f64::NAN;
use swf::avm2::types::{AbcFile, DefaultValue as AbcDefaultValue, Index};
use swf::avm2::types::{DefaultValue as AbcDefaultValue, Index};
/// An AVM2 value.
///
@ -139,95 +140,59 @@ impl PartialEq for Value<'_> {
}
}
pub fn abc_int(file: &AbcFile, index: Index<i32>) -> Result<i32, Error> {
pub fn abc_int(translation_unit: TranslationUnit<'_>, index: Index<i32>) -> Result<i32, Error> {
if index.0 == 0 {
return Ok(0);
}
file.constant_pool
translation_unit
.abc_ref()
.constant_pool
.ints
.get(index.0 as usize - 1)
.cloned()
.ok_or_else(|| format!("Unknown int constant {}", index.0).into())
}
pub fn abc_uint(file: &AbcFile, index: Index<u32>) -> Result<u32, Error> {
pub fn abc_uint(translation_unit: TranslationUnit<'_>, index: Index<u32>) -> Result<u32, Error> {
if index.0 == 0 {
return Ok(0);
}
file.constant_pool
translation_unit
.abc_ref()
.constant_pool
.uints
.get(index.0 as usize - 1)
.cloned()
.ok_or_else(|| format!("Unknown uint constant {}", index.0).into())
}
pub fn abc_double(file: &AbcFile, index: Index<f64>) -> Result<f64, Error> {
pub fn abc_double(translation_unit: TranslationUnit<'_>, index: Index<f64>) -> Result<f64, Error> {
if index.0 == 0 {
return Ok(NAN);
}
file.constant_pool
translation_unit
.abc_ref()
.constant_pool
.doubles
.get(index.0 as usize - 1)
.cloned()
.ok_or_else(|| format!("Unknown double constant {}", index.0).into())
}
/// Copy a string from an ABC constant pool, yielding `""` if the string is the
/// zero string.
pub fn abc_string<'gc>(
file: &AbcFile,
index: Index<String>,
mc: MutationContext<'gc, '_>,
) -> Result<AvmString<'gc>, Error> {
if index.0 == 0 {
return Ok("".into());
}
file.constant_pool
.strings
.get(index.0 as usize - 1)
.map(|s| AvmString::new(mc, s.as_str()))
.ok_or_else(|| format!("Unknown string constant {}", index.0).into())
}
/// Retrieve a string from an ABC constant pool, yielding `None` if the string
/// is the zero string.
///
/// This function still yields `Err` for out-of-bounds string constants, which
/// should cause the runtime to halt. `None` indicates that the zero string is
/// in use, which callers are free to interpret as necessary (although this
/// usually means "any name").
pub fn abc_string_option<'gc>(
file: &AbcFile,
index: Index<String>,
mc: MutationContext<'gc, '_>,
) -> Result<Option<AvmString<'gc>>, Error> {
if index.0 == 0 {
return Ok(None);
}
file.constant_pool
.strings
.get(index.0 as usize - 1)
.map(|s| AvmString::new(mc, s.as_str()))
.map(Some)
.ok_or_else(|| format!("Unknown string constant {}", index.0).into())
}
/// Retrieve a default value as an AVM2 `Value`.
pub fn abc_default_value<'gc>(
file: &AbcFile,
translation_unit: TranslationUnit<'gc>,
default: &AbcDefaultValue,
mc: MutationContext<'gc, '_>,
) -> Result<Value<'gc>, Error> {
match default {
AbcDefaultValue::Int(i) => abc_int(file, *i).map(|v| v.into()),
AbcDefaultValue::Uint(u) => abc_uint(file, *u).map(|v| v.into()),
AbcDefaultValue::Double(d) => abc_double(file, *d).map(|v| v.into()),
AbcDefaultValue::String(s) => abc_string(file, s.clone(), mc).map(|v| v.into()),
AbcDefaultValue::Int(i) => abc_int(translation_unit, *i).map(|v| v.into()),
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, mc).map(|v| v.into()),
AbcDefaultValue::True => Ok(true.into()),
AbcDefaultValue::False => Ok(false.into()),
AbcDefaultValue::Null => Ok(Value::Null),
@ -239,7 +204,7 @@ pub fn abc_default_value<'gc>(
| AbcDefaultValue::Explicit(ns)
| AbcDefaultValue::StaticProtected(ns)
| AbcDefaultValue::Private(ns) => {
Namespace::from_abc_namespace(file, ns.clone(), mc).map(|v| v.into())
Namespace::from_abc_namespace(translation_unit, ns.clone(), mc).map(|v| v.into())
}
}
}