Implement register underflow behavior.

This has the side effect of letting us remove the `Option` on register_count since setting this to `0` is equivalent now. Furthermore, we can skip an allocation if a function requests no registers.
This commit is contained in:
David Wendt 2019-10-13 18:41:07 -04:00
parent 911cf64cb0
commit 7e2cf03789
6 changed files with 76 additions and 60 deletions

View File

@ -489,13 +489,14 @@ impl<'gc> Avm1<'gc> {
pub fn current_register(&self, id: u8) -> Value<'gc> { pub fn current_register(&self, id: u8) -> Value<'gc> {
if self if self
.current_stack_frame() .current_stack_frame()
.map(|sf| sf.read().has_local_registers()) .map(|sf| sf.read().has_local_register(id))
.unwrap_or(false) .unwrap_or(false)
{ {
self.current_stack_frame() self.current_stack_frame()
.unwrap() .unwrap()
.read() .read()
.local_register(id) .local_register(id)
.unwrap_or(Value::Undefined)
} else { } else {
self.registers self.registers
.get(id as usize) .get(id as usize)
@ -515,7 +516,7 @@ impl<'gc> Avm1<'gc> {
) { ) {
if self if self
.current_stack_frame() .current_stack_frame()
.map(|sf| sf.read().has_local_registers()) .map(|sf| sf.read().has_local_register(id))
.unwrap_or(false) .unwrap_or(false)
{ {
self.current_stack_frame() self.current_stack_frame()

View File

@ -41,6 +41,10 @@ impl<'gc> RegisterSet<'gc> {
pub fn get_mut(&mut self, num: u8) -> Option<&mut Value<'gc>> { pub fn get_mut(&mut self, num: u8) -> Option<&mut Value<'gc>> {
self.0.get_mut(num as usize) self.0.get_mut(num as usize)
} }
pub fn len(&self) -> u8 {
self.0.len() as u8
}
} }
/// Represents a single activation of a given AVM1 function or keyframe. /// Represents a single activation of a given AVM1 function or keyframe.
@ -275,25 +279,27 @@ impl<'gc> Activation<'gc> {
pub fn this_cell(&self) -> GcCell<'gc, Object<'gc>> { pub fn this_cell(&self) -> GcCell<'gc, Object<'gc>> {
self.this self.this
} }
/// Returns true if this function was called with a local register set.
pub fn has_local_registers(&self) -> bool { /// Returns true if this activation has a given local register ID.
self.local_registers.is_some() pub fn has_local_register(&self, id: u8) -> bool {
self.local_registers
.map(|rs| id < rs.read().len() - 1)
.unwrap_or(false)
} }
pub fn allocate_local_registers(&mut self, num: u8, mc: MutationContext<'gc, '_>) { pub fn allocate_local_registers(&mut self, num: u8, mc: MutationContext<'gc, '_>) {
self.local_registers = Some(GcCell::allocate(mc, RegisterSet::new(num))); self.local_registers = match num {
0 => None,
num => Some(GcCell::allocate(mc, RegisterSet::new(num))),
};
} }
/// Retrieve a local register. /// Retrieve a local register.
pub fn local_register(&self, id: u8) -> Value<'gc> { pub fn local_register(&self, id: u8) -> Option<Value<'gc>> {
if let Some(local_registers) = self.local_registers { if let Some(local_registers) = self.local_registers {
local_registers local_registers.read().get(id).cloned()
.read()
.get(id)
.cloned()
.unwrap_or(Value::Undefined)
} else { } else {
Value::Undefined None
} }
} }

View File

@ -29,11 +29,8 @@ pub struct Avm1Function<'gc> {
name: Option<String>, name: Option<String>,
/// The number of registers to allocate for this function's private register /// The number of registers to allocate for this function's private register
/// set. /// set. Any register beyond this ID will be served from the global one.
/// register_count: u8,
/// If None, then no register set will be allocated and the preload options
/// have no effect.
register_count: Option<u8>,
preload_parent: bool, preload_parent: bool,
preload_root: bool, preload_root: bool,
@ -75,7 +72,7 @@ impl<'gc> Avm1Function<'gc> {
swf_version, swf_version,
data: actions, data: actions,
name, name,
register_count: None, register_count: 0,
preload_parent: false, preload_parent: false,
preload_root: false, preload_root: false,
suppress_super: false, suppress_super: false,
@ -115,7 +112,7 @@ impl<'gc> Avm1Function<'gc> {
swf_version, swf_version,
data: actions, data: actions,
name, name,
register_count: Some(swf_function.params.capacity() as u8), register_count: swf_function.params.capacity() as u8,
preload_parent: swf_function.preload_parent, preload_parent: swf_function.preload_parent,
preload_root: swf_function.preload_root, preload_root: swf_function.preload_root,
suppress_super: swf_function.suppress_super, suppress_super: swf_function.suppress_super,
@ -142,7 +139,7 @@ impl<'gc> Avm1Function<'gc> {
self.scope self.scope
} }
pub fn register_count(&self) -> Option<u8> { pub fn register_count(&self) -> u8 {
self.register_count self.register_count
} }
} }
@ -215,52 +212,48 @@ impl<'gc> Executable<'gc> {
Some(argcell), Some(argcell),
); );
if let Some(register_count) = af.register_count() { frame.allocate_local_registers(af.register_count(), ac.gc_context);
frame.allocate_local_registers(register_count, ac.gc_context);
let mut preload_r = 1; let mut preload_r = 1;
if af.preload_this { if af.preload_this {
//TODO: What happens if you specify both suppress and //TODO: What happens if you specify both suppress and
//preload for this? //preload for this?
frame.set_local_register(preload_r, Value::Object(this), ac.gc_context); frame.set_local_register(preload_r, Value::Object(this), ac.gc_context);
preload_r += 1; preload_r += 1;
} }
if af.preload_arguments { if af.preload_arguments {
//TODO: What happens if you specify both suppress and //TODO: What happens if you specify both suppress and
//preload for arguments? //preload for arguments?
frame.set_local_register(preload_r, Value::Object(argcell), ac.gc_context); frame.set_local_register(preload_r, Value::Object(argcell), ac.gc_context);
preload_r += 1; preload_r += 1;
} }
if af.preload_super { if af.preload_super {
//TODO: super not implemented //TODO: super not implemented
log::warn!( log::warn!("Cannot preload super into register because it's not implemented");
"Cannot preload super into register because it's not implemented" //TODO: What happens if you specify both suppress and
); //preload for super?
//TODO: What happens if you specify both suppress and preload_r += 1;
//preload for super? }
preload_r += 1;
}
if af.preload_root { if af.preload_root {
frame.set_local_register(preload_r, avm.root_object(ac), ac.gc_context); frame.set_local_register(preload_r, avm.root_object(ac), ac.gc_context);
preload_r += 1; preload_r += 1;
} }
if af.preload_parent { if af.preload_parent {
frame.set_local_register( frame.set_local_register(
preload_r, preload_r,
child_scope.read().resolve("_parent", avm, ac, this), child_scope.read().resolve("_parent", avm, ac, this),
ac.gc_context, ac.gc_context,
); );
preload_r += 1; preload_r += 1;
} }
if af.preload_global { if af.preload_global {
frame.set_local_register(preload_r, avm.global_object(ac), ac.gc_context); frame.set_local_register(preload_r, avm.global_object(ac), ac.gc_context);
}
} }
//TODO: What happens if the argument registers clash with the //TODO: What happens if the argument registers clash with the

View File

@ -53,6 +53,7 @@ swf_tests! {
(delete, "avm1/delete", 3), (delete, "avm1/delete", 3),
(timeline_function_def, "avm1/timeline_function_def", 3), (timeline_function_def, "avm1/timeline_function_def", 3),
(root_global_parent, "avm1/root_global_parent", 3), (root_global_parent, "avm1/root_global_parent", 3),
(register_underflow, "avm1/register_underflow", 1),
} }
/// Loads an SWF and runs it through the Ruffle core for a number of frames. /// Loads an SWF and runs it through the Ruffle core for a number of frames.

View File

@ -0,0 +1,15 @@
global reg 0: 0
global reg 1: 1
global reg 2: 2
global reg 3: 3
Function start f(a), RegisterCount = 3, a => register2
Function reg 0: undefined
Function reg 1: undefined
Function reg 2: 66
Function reg 3: 3
Changing registers to 9.
Function end
global reg 0: 0
global reg 1: 1
global reg 2: 2
global reg 3: 9

Binary file not shown.