Skip to content

Commit

Permalink
avm2: Return actual VerifyErrors for constant pool errors
Browse files Browse the repository at this point in the history
This requires passing around an Activation
  • Loading branch information
Lord-McSweeney authored and Lord-McSweeney committed Sep 12, 2024
1 parent 5760aa7 commit f1da145
Show file tree
Hide file tree
Showing 12 changed files with 162 additions and 153 deletions.
4 changes: 1 addition & 3 deletions core/src/avm2/activation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,9 +732,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
method: Gc<'gc, BytecodeMethod<'gc>>,
index: Index<AbcNamespace>,
) -> Result<Namespace<'gc>, 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.
Expand Down
20 changes: 7 additions & 13 deletions core/src/avm2/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
})?,
);
}
Expand Down
52 changes: 38 additions & 14 deletions core/src/avm2/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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> {
Expand Down
5 changes: 2 additions & 3 deletions core/src/avm2/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl<'gc> ParamConfig<'gc> {
} else {
AvmString::from("<Unnamed Parameter>")
};
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)?)
Expand Down Expand Up @@ -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);
Expand Down
90 changes: 48 additions & 42 deletions core/src/avm2/multiname.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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<AbcNamespaceSet>,
context: &mut UpdateContext<'gc>,
) -> Result<NamespaceSet<'gc>, 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;
Expand All @@ -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<AbcMultiname>,
context: &mut UpdateContext<'gc>,
) -> Result<Self, Error<'gc>> {
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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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();

Expand All @@ -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
}
};
Expand Down Expand Up @@ -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<AbcMultiname>,
) -> Result<&AbcMultiname, Error<'gc>> {
let actual_index: Result<usize, Error<'gc>> = (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 {
Expand Down Expand Up @@ -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,
}
}

Expand Down
Loading

0 comments on commit f1da145

Please sign in to comment.