AVM2 interpreter #404

kmeisthax · 2020-02-22T05:54:10Z

This implements an AVM2 interpreter. AVM2 is the virtual machine that ActionScript 3.0 programs compile into; it's completely separate from AVM1 and almost became part of the web platform itself in the form of ECMAScript 4 (no published standard exists) and Adobe Tamarin (FOSS but MPL trilicensed instead of Apache).

Notable features include packages, namespaces, access control, E4X, type assertions, a very explicit scope stack, no primitive values (if the one ES4 overview PDF I could find is accurate), and more. Coming from modern JavaScript, or even AS2; this feels like a completely different programming language and it took a while to understand very basic things about the VM. (This is probably why non-Flash web developers outright rejected ES4.) Nevertheless, it's part of Flash, so we need to implement it as it gates a whole bunch of interesting content.

In this PR

PR Roadmap

The following are features out of scope of this PR. Due to the fact that this PR's scope has more or less been completed, I'm working on them in separate branches. If they're relatively "complete" before this PR is merged in, they may be merged in.

Primitive type coercion, abstract comparisons, and abstract equality - avm2-primitives on my user's repo.
- Numerical calculations - some implementations exist on the above but will be given their own grandchild PR
- String manipulation - not yet started
- Primitive boxing - not yet started
Native builtins - avm2-nativeclasses on my user's repo. Covers defining ES4 classes in Rust.
- AVM2 StageObject - TObject implementation that holds stage objects, plus native implementations of the builtin display object classes.
  - SymbolClass tag - required for executing frame scripts
  - FileAttributes tag & VM isolation - Already implemented in core: Print swf version on startup, and warn when we run into avm2 #661, but for the purpose of warning users about our lack of AVM2 support. We will need to change this to configuring what VM a particular movie's display objects belong to.

kmeisthax · 2020-03-09T03:37:37Z

I am releasing the draft lock on this PR and requesting review and merge. Hopefully it passes CircleCI.

I'm deliberately walling off a whole bunch of future AVM2 features for future PRs, as mentioned in the PR description. This PR already has 143 commits and makes object_prototyping, a PR I had to cut up into four painful-to-rebase pieces, look minimal. I've specifically selected future work that isn't likely to have interdependencies between future PRs, and thus can be developed separately on top of the work I've done bootstrapping a basic AVM2 interpreter.

There are approximately 31 tests, a significant number of which challenged my notions about how this interpreter should work. I think I at least have the object model locked, though I wasn't able to write a test that uses slots. In fact, clippy seems to think Slot::Occupied is unreachable, even though there's a function constructing it. I disabled that warning for now.

Justin-CB · 2020-04-15T20:05:50Z

Is there any ETA on this pull request?

jfmherokiller · 2020-04-21T00:26:40Z

Is there any ETA on this pull request?

Out of my own curositiy i tried to merge this into master (current as of today) and it seems this branch is way behind. Just based off a cursory observation (im no rust master) we might need to create an avm super type that encases both avm1 and avm2 and cast based upon the information extracted from the swf.

kmeisthax · 2020-04-21T00:31:27Z

@jfmherokiller There's no need to do that, I just need to resolve the merge conflicts. AVM1 and AVM2 are supposed to be entirely separate; the conflicts are in other parts of the player where I added AVM2 stuff to.

kmeisthax · 2020-04-21T00:34:57Z

I should also point out that this does not contain the capability to play AS3 games. It can't even add numbers yet - it's purely the AVM2 interpreter loop, object model, and just enough builtins to get tests to run.

jfmherokiller · 2020-04-21T00:40:27Z

the question then i have is how will the player distinguish between an avm1 and avm2 swf?
Edit: just reread your discription and checked my own research yep its the fileattibs tag

kmeisthax · 2020-04-21T01:54:44Z

As you can tell by that massive list of new commits I have rebased the AVM2 PR and resolved the merge conflicts.

Given @Dinnerbone's recent work with init actions I'm wondering if ABC files should also execute immediately rather than resolving on the action queue.

jfmherokiller · 2020-04-21T03:01:13Z

I will say I did succed in adding a kind of standin for FileAttributes (I just started using rust today so code will be very basic) in movie_clip.rs.
mydiff.txt
file attached is the git diff showing the changes

kmeisthax · 2020-04-25T18:58:43Z

So, that's great, but kinda out of scope for this PR. I specifically excluded anything to do with either FileAttributes, or tying display objects into the AVM2 interpreter, because this PR is already fairly sprawling and I want @Herschel's eyes on it and this merged before I start doing other related work.

Whenever this actually gets merged, and we start work on an "AVM2 DisplayObjects" PR, then we'd want to start parsing FileAttributes so we'd know how to instantiate a given movie. The relevant flags would both have to set a bit on the containing SwfMovie (so that contained movieclips can't mix VMs) and instantiate the entire movie hierarchy as either AVM1 or AVM2. Furthermore, I'm a little wary of parsing FileAttributes in preload because there are script actions and instantiations that can happen before a FileAttributes tag has been encountered which do need to know what VM is allowed to run for a given movie. Ideally, we'd scan the SWF's root movie clip before preload (or even before making the initial MovieClip) so we could get at those attributes and use that to instantiate the correct VM's Object.

kmeisthax · 2020-06-09T03:35:09Z

Turns out this thing had been sitting unmergeable for a good week or two 'cause I didn't notice the fact that beta/nightly clippy tests are now being run.

Also, given that #675 landed I'm now considering adjusting the AVM2 Value and Object to match...

This is primarily used for interfaces, in case you can't guess by the previous commit.

This test will fail if the AVM2 implementation does not support bodyless methods or bare classes properly.

… stick them in the prototype for later use.

…rything.

…ild.

…the AVM2 runtime.

…hat pulls strings from the ABC file now uses `AvmString`s.

…enting 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.

Holding a `Ref` on a garbage-collected object inherently extends any borrow locks on that object. Since ABC files are references already, taking a `Ref` to them only helps to skip the refcount update. This is less useful than expected: in most situations, using `abc_ref` causes double-borrow panics. The few methods that can use it are going to be fragile in the face of future refactors, so I'm nipping the problem in the bud now.

Herschel

Thanks for your work on this! I think we should have avm2::AvmString and avm2::PropertyMap (at least aliased or copy+pasted from the Avm1 versions for now), as these will almost certainly have differences than the avm1 versions, and we should at least have types in place to make it easier for us to modify these later.

There are also some unimportant naming questions that I ~~feel strongly~~ don't feel strongly about (how closely should we try to mirror the avmplus naming conventions?)

Otherwise I think this is ready to merge, and I will merge this whenever you're ready.

swf/src/avm2/read.rs

Herschel · 2020-07-17T18:43:05Z

core/src/avm2/value.rs

+pub enum Value<'gc> {
+    Undefined,
+    Null,
+    Bool(bool),
+    Number(f64),
+    String(AvmString<'gc>),
+    Namespace(Namespace<'gc>),
+    Object(Object<'gc>),
+}


Bikeshed: Should we call this Atom (since this is what avmplus calls it?)

I have no strong opinion either way.

I did use Value specifically because it matches the AVM1 Value. I doubt we're going to have people used to hacking on avmplus wondering where Atom is here, as much as we're going to have people used to hacking on Ruffle's AVM1 interpreter wondering where Value is.

core/src/avm1/string.rs

core/src/avm2/script_object.rs

core/src/avm2/names.rs

core/src/avm2/method.rs

Herschel · 2020-07-17T21:41:23Z

core/src/avm2/script.rs

+
+#[derive(Copy, Clone, Debug, Collect)]
+#[collect(no_drop)]
+pub struct TranslationUnit<'gc>(GcCell<'gc, TranslationUnitData<'gc>>);


TranslationUnit feels like a weird name to me. avmplus calls this PoolObject which I don't think is much better.

So, the name came about while I was switching the AVM2 interpreter away from pulling ABC traits and data directly to storing them in other structures that could be dynamically created (which we need for avm2-nativeclasses.) I can't call it AbcFile because that's already in use by swf, so TranslationUnit (a term used by C/C++ compiler specifications to refer to a single source file) seemed like a good option, since, as far as I could tell I was getting one DoABC per source file in my tests...

If you have a better option I'll definitely change the name.

core/src/avm2/script.rs

Herschel · 2020-07-17T22:09:13Z

core/src/avm2/script.rs

+        if let Some(method) = write.methods.get(&method_index) {
+            return Ok(method.clone());
+        }
+
+        drop(write);
+
+        let method: Result<Gc<'gc, BytecodeMethod<'gc>>, Error> =
+            BytecodeMethod::from_method_index(self, Index::new(method_index), mc)
+                .ok_or_else(|| "Method index does not exist".into());
+        let method: Method<'gc> = method?.into();
+
+        self.0
+            .write(mc)
+            .methods
+            .insert(method_index, method.clone());


Feels like we should be able to use the entry API with or_insert_with, although I guess the issue is that from_method_index does some txunit.read().

The timing of when GcCell locks are held and released is important here (which is why I used the "wrapped GcCell pattern" here). Due to the way traits work, all four of these methods can recurse into one another on the same stack at any time, so I'm being highly conservative with how long locks are actually being held.

BytecodeMethod never needs a write lock on the translation unit, and it only ever reads it to get an ABC. In fact, it reads it three times too many because this used to be abc_ref (which caused other double-borrow problems). I'm going to change this particular constructor into taking both a TranslationUnit and an AbcFile, so it doesn't need to hold either read or write locks and we can use entries.

Turns out that won't work, but not for the reason you'd think: or_insert_with is infallible. We need to generate a verification error if the method index doesn't exist, which we can't return inside of the or_insert_with callable. Ergo, I'll be leaving this as-is.

(This seems to be a problem with a lot of Rust methods that take closures...)

core/src/avm2/script.rs

…to diverge.

…rom_abc_multiname`.

…mplify a struct declaration)

Herschel · 2020-07-20T02:58:11Z

Ok, let's get this in! Thanks for the patience while this PR sat around for a long time. I'm excited to make progress here.

(BTW, I meant to say that I don't feel strongly about the name stuff in my earlier post, sorry if that came off the wrong way).

kmeisthax force-pushed the avm2-interpreter branch 2 times, most recently from 6079a48 to b71bdf6 Compare February 24, 2020 23:34

kmeisthax changed the title ~~[DRAFT] AVM2 interpreter~~ AVM2 interpreter Mar 9, 2020

kmeisthax force-pushed the avm2-interpreter branch from 664bf4b to ce6f282 Compare March 9, 2020 03:35

kmeisthax marked this pull request as ready for review March 9, 2020 03:37

kmeisthax requested a review from Herschel March 9, 2020 03:37

kmeisthax mentioned this pull request Mar 15, 2020

Run app in Flex #441

Open

This was referenced Apr 12, 2020

avm1 super doesn't correctly resolve this properties #496

Closed

Allow supercalled methods to access properties of this correctly #499

Merged

kmeisthax force-pushed the avm2-interpreter branch from ce6f282 to 8950903 Compare April 21, 2020 01:50

kmeisthax force-pushed the avm2-interpreter branch from 43152b2 to 3eb14bb Compare April 25, 2020 19:13

Justin-CB mentioned this pull request May 13, 2020

Prepare for first alpha release #575

Open

kmeisthax force-pushed the avm2-interpreter branch from 3eb14bb to c891b90 Compare May 16, 2020 23:29

Dinnerbone mentioned this pull request May 29, 2020

core: Print swf version on startup, and warn when we run into avm2 #661

Merged

kmeisthax force-pushed the avm2-interpreter branch 4 times, most recently from 30f52a6 to eeff5f1 Compare June 9, 2020 02:51

kmeisthax force-pushed the avm2-interpreter branch 2 times, most recently from 45bbf03 to c7cd509 Compare June 25, 2020 21:49

kmeisthax added 12 commits July 13, 2020 17:45

Allow the construction of classes with no base class.

8f2d331

This is primarily used for interfaces, in case you can't guess by the previous commit.

Add a test for loading interfaces into the AVM2.

563a515

This test will fail if the AVM2 implementation does not support bodyless methods or bare classes properly.

Add trait methods for getting and setting the interfaces list.

cce68db

Read the interface list when instantiating classes, resolve them, and…

3fae8b3

… stick them in the prototype for later use.

Add is_instance_of trait method to TObject

a0cb052

Implement istype, istypelate, and instanceof.

c917b3d

Instances should be listed as their prototypes (empty as they are)

39a4566

Add tests for instanceof and is operators.

64e5b46

Allow borrowing string values from an ABC file instead of cloning eve…

f0c633f

…rything.

coerce_string may return a static string, which we shouldn't clone.

ae26615

Don't attempt to log debugging information if we're not in a debug bu…

a406bda

…ild.

pool_string should return a Ref<str> just like value::abc_string.

508fcd6

kmeisthax force-pushed the avm2-interpreter branch from ef5cba2 to 508fcd6 Compare July 13, 2020 21:59

kmeisthax added 6 commits July 13, 2020 22:09

Impl Copy, Eq, PartialOrd, Ord, and Hash for AvmString.

2021cec

Replace String or &str references with AvmString everywhere in …

61a3ff8

…the AVM2 runtime.

Remove abc_string and replace it with abc_string_copy. All code t…

e1b9b82

…hat pulls strings from the ABC file now uses `AvmString`s.

Add support for cached/interned pool strings in the TranslationUnit.

70e9e7e

Herschel requested changes Jul 17, 2020

View reviewed changes

kmeisthax added 8 commits July 18, 2020 16:12

Remove dead code on all now-in-use structs and methods.

ccacc54

Add a stub AVM2 string representation to allow for both VMs' strings …

37b6b89

…to diverge.

Add AVM2 equivalent of PropertyMap for further expansion.

dc962f2

Zero-index multinames should generate a validation error in `QName::f…

c415190

…rom_abc_multiname`.

Rename a2me to entry (or method in one case where it lets me si…

262bb14

…mplify a struct declaration)

Use FnvHashMap for the translation unit pools.

575a9b0

Script should not hold write locks when probing it's internal caches.

44b924d

Use unwrap_or_default

7adabc8

Herschel merged commit 1a5d7fe into ruffle-rs:master Jul 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AVM2 interpreter #404

AVM2 interpreter #404

kmeisthax commented Feb 22, 2020 •

edited

Loading

kmeisthax commented Mar 9, 2020

Justin-CB commented Apr 15, 2020

jfmherokiller commented Apr 21, 2020

kmeisthax commented Apr 21, 2020

kmeisthax commented Apr 21, 2020

jfmherokiller commented Apr 21, 2020 •

edited

Loading

kmeisthax commented Apr 21, 2020

jfmherokiller commented Apr 21, 2020 •

edited

Loading

kmeisthax commented Apr 25, 2020

kmeisthax commented Jun 9, 2020

Herschel left a comment •

edited

Loading

Herschel Jul 17, 2020

kmeisthax Jul 18, 2020

Herschel Jul 17, 2020

kmeisthax Jul 18, 2020

Herschel Jul 17, 2020

kmeisthax Jul 18, 2020

kmeisthax Jul 18, 2020

Herschel commented Jul 20, 2020

AVM2 interpreter #404

AVM2 interpreter #404

Conversation

kmeisthax commented Feb 22, 2020 • edited Loading

In this PR

PR Roadmap

kmeisthax commented Mar 9, 2020

Justin-CB commented Apr 15, 2020

jfmherokiller commented Apr 21, 2020

kmeisthax commented Apr 21, 2020

kmeisthax commented Apr 21, 2020

jfmherokiller commented Apr 21, 2020 • edited Loading

kmeisthax commented Apr 21, 2020

jfmherokiller commented Apr 21, 2020 • edited Loading

kmeisthax commented Apr 25, 2020

kmeisthax commented Jun 9, 2020

Herschel left a comment • edited Loading

Choose a reason for hiding this comment

Herschel Jul 17, 2020

Choose a reason for hiding this comment

kmeisthax Jul 18, 2020

Choose a reason for hiding this comment

Herschel Jul 17, 2020

Choose a reason for hiding this comment

kmeisthax Jul 18, 2020

Choose a reason for hiding this comment

Herschel Jul 17, 2020

Choose a reason for hiding this comment

kmeisthax Jul 18, 2020

Choose a reason for hiding this comment

kmeisthax Jul 18, 2020

Choose a reason for hiding this comment

Herschel commented Jul 20, 2020

kmeisthax commented Feb 22, 2020 •

edited

Loading

jfmherokiller commented Apr 21, 2020 •

edited

Loading

jfmherokiller commented Apr 21, 2020 •

edited

Loading

Herschel left a comment •

edited

Loading