Skip to content

Commit 1530a74

Browse files
committed
avm1: More accurate semantics for instanceof and CastOp
- only coerce RHS when the LHS is object-like - `prototype` access should ignore __proto__, properties, and version attributes This exposes some Flash weirdness; in FP, the following code traces `false` if placed on frame 1, but hangs if placed on any other frame. ``` a = {}; b = {}; a.__proto__ = b; b.__proto__ = a; trace(a instanceof b); ``` Prior to this PR, Ruffle raised an `Error::PrototypeRecursionLimit`, for slightly wrong reasons (we tried to crawl b's `__proto__` chain to find a `prototype` property); now it returns `false`. This affects Gnash's `Inheritance-*` tests, as they run the above code on frame 2, and so hang in FP.
1 parent 2a96e72 commit 1530a74

File tree

21 files changed

+258
-25
lines changed

21 files changed

+258
-25
lines changed

core/src/avm1/activation.rs

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -811,20 +811,15 @@ impl<'a, 'gc> Activation<'a, 'gc> {
811811

812812
fn action_cast_op(&mut self) -> Result<FrameControl<'gc>, Error<'gc>> {
813813
let obj = self.context.avm1.pop();
814-
let constr = self.context.avm1.pop().coerce_to_object_or_bare(self)?;
815-
816-
let is_instance_of = if let Some(obj) = obj.as_object(self) {
817-
let prototype = constr
818-
.get(istr!(self, "prototype"), self)?
819-
.coerce_to_object_or_bare(self)?;
820-
obj.is_instance_of(self, constr, prototype)?
821-
} else {
822-
false
823-
};
814+
let constr = self.context.avm1.pop();
815+
// For some reason, FP does this useless extra coercion.
816+
if obj.is_primitive() {
817+
let _ = obj.coerce_to_object(self)?;
818+
}
824819

820+
let is_instance_of = obj.instance_of(constr, self)?;
825821
let result = if is_instance_of { obj } else { Value::Null };
826822
self.context.avm1.push(result);
827-
828823
Ok(FrameControl::Continue)
829824
}
830825

@@ -1458,6 +1453,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
14581453
}
14591454

14601455
let prototype = constructor
1456+
// TODO(moulins): should this use `Object::prototype`?
14611457
.get(istr!(self, "prototype"), self)?
14621458
.coerce_to_object_or_bare(self)?;
14631459
prototype.set_interfaces(self.gc(), interfaces);
@@ -1467,18 +1463,9 @@ impl<'a, 'gc> Activation<'a, 'gc> {
14671463
}
14681464

14691465
fn action_instance_of(&mut self) -> Result<FrameControl<'gc>, Error<'gc>> {
1470-
let constr = self.context.avm1.pop().coerce_to_object_or_bare(self)?;
1466+
let constr = self.context.avm1.pop();
14711467
let obj = self.context.avm1.pop();
1472-
1473-
let result = if let Some(obj) = obj.as_object(self) {
1474-
let prototype = constr
1475-
.get(istr!(self, "prototype"), self)?
1476-
.coerce_to_object_or_bare(self)?;
1477-
obj.is_instance_of(self, constr, prototype)?
1478-
} else {
1479-
false
1480-
};
1481-
1468+
let result = obj.instance_of(constr, self)?;
14821469
self.context.avm1.push(result.into());
14831470
Ok(FrameControl::Continue)
14841471
}

core/src/avm1/object.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,8 @@ impl<'gc> Object<'gc> {
325325
proto_stack.push(p);
326326
}
327327

328+
// TODO(moulins): should we guard against infinite loops here?
329+
// A recursive prototype chain will hang Flash Player.
328330
while let Some(this_proto) = proto_stack.pop() {
329331
if Object::ptr_eq(this_proto, prototype) {
330332
return Ok(true);
@@ -340,6 +342,7 @@ impl<'gc> Object<'gc> {
340342
return Ok(true);
341343
}
342344

345+
// TODO(moulins): should this use `Object::prototype`?
343346
if let Value::Object(o) = interface.get(istr!("prototype"), activation)? {
344347
proto_stack.push(o);
345348
}
@@ -361,6 +364,9 @@ impl<'gc> Object<'gc> {
361364

362365
/// Check if this object is in the prototype chain of the specified test object.
363366
pub fn is_prototype_of(self, activation: &mut Activation<'_, 'gc>, other: Object<'gc>) -> bool {
367+
// TODO(moulins): should we guard against infinite loops here?
368+
// A recursive prototype chain will hang Flash Player.
369+
364370
let mut proto = other.proto(activation);
365371

366372
while let Value::Object(proto_ob) = proto {

core/src/avm1/object/script_object.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ impl<'gc> Object<'gc> {
161161
))
162162
}
163163

164-
/// Gets the value of a data property on this object.
164+
/// Gets the value of a data property on this object, ignoring attributes.
165165
///
166166
/// Doesn't look up the prototype chain and ignores virtual properties, thus cannot cause
167167
/// any side-effects.
@@ -177,7 +177,7 @@ impl<'gc> Object<'gc> {
177177
.map_or(Value::Undefined, |property| property.data())
178178
}
179179

180-
/// Sets a data property on this object.
180+
/// Sets a data property on this object, ignoring attributes.
181181
///
182182
/// Doesn't look up the prototype chain and ignores virtual properties, but still might
183183
/// call to watchers.
@@ -216,7 +216,8 @@ impl<'gc> Object<'gc> {
216216
.collect()
217217
}
218218

219-
/// Retrieve a named, non-virtual property from this object exclusively.
219+
/// Retrieve a named, non-virtual property from this object exclusively, taking
220+
/// attributes into account.
220221
///
221222
/// This function should not inspect prototype chains. Instead, use
222223
/// `get_stored` to do ordinary property look-up and resolution.
@@ -649,6 +650,7 @@ impl<'gc> Object<'gc> {
649650
}
650651

651652
/// Retrieve the `__proto__` of a given object.
653+
/// (don't confuse this with `self.prototype()`!)
652654
///
653655
/// The proto is another object used to resolve methods across a class of
654656
/// multiple objects. It should also be accessible as `__proto__` from
@@ -661,6 +663,13 @@ impl<'gc> Object<'gc> {
661663
self.get_data(istr!("__proto__"), activation)
662664
}
663665

666+
/// Retrieve the `prototype` of this object, as if it was a function.
667+
/// (don't confuse this with `self.proto()`!)
668+
pub fn prototype(self, activation: &mut Activation<'_, 'gc>) -> Value<'gc> {
669+
// Ignore getters, __proto__, and SWF version attributes.
670+
self.get_data(istr!("prototype"), activation)
671+
}
672+
664673
/// Checks if the object has a given named property.
665674
pub fn has_property(self, activation: &mut Activation<'_, 'gc>, name: AvmString<'gc>) -> bool {
666675
let dobj = match self.native_no_super() {

core/src/avm1/value.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,26 @@ impl<'gc> Value<'gc> {
552552
};
553553
Ok(result)
554554
}
555+
556+
/// ActionScript 2's `self instanceof Class`.
557+
pub fn instance_of(
558+
self,
559+
class: Value<'gc>,
560+
activation: &mut Activation<'_, 'gc>,
561+
) -> Result<bool, Error<'gc>> {
562+
if !self.is_primitive() {
563+
// These coercions happen even if `obj` is a dead `MovieClipReference`.
564+
if let Some(class) = class.coerce_to_object(activation)? {
565+
if let Some(p) = class.prototype(activation).coerce_to_object(activation)? {
566+
if let Some(obj) = self.as_object(activation) {
567+
return obj.is_instance_of(activation, class, p);
568+
}
569+
}
570+
}
571+
}
572+
573+
Ok(false)
574+
}
555575
}
556576

557577
/// Calculate `value * 10^exp` through repeated multiplication or division.

core/src/avm1/xml/tree.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,7 @@ impl<'gc> XmlNode<'gc> {
359359
Some(object) => object,
360360
None => {
361361
let xml_node = activation.prototypes().xml_node_constructor;
362+
// TODO(moulins): should this use `Object::prototype`?
362363
let prototype = xml_node.get(istr!("prototype"), activation).ok();
363364
let object = Object::new(&activation.context.strings, prototype);
364365
self.introduce_script_object(activation.gc(), object);

core/src/display_object/movie_clip.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,6 +1903,7 @@ impl<'gc> MovieClip<'gc> {
19031903
);
19041904

19051905
if let Ok(prototype) = constructor
1906+
// TODO(moulins): should this use `Object::prototype`?
19061907
.get(istr!("prototype"), &mut activation)
19071908
.and_then(|v| v.coerce_to_object_or_bare(&mut activation))
19081909
{

core/src/player.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2175,6 +2175,7 @@ impl Player {
21752175
ActivationIdentifier::root("[Construct]"),
21762176
action.clip,
21772177
);
2178+
// TODO(moulins): should this use `Object::prototype`?
21782179
if let Ok(prototype) = constructor.get(istr!("prototype"), &mut activation) {
21792180
if let Some(object) = action.clip.object1() {
21802181
object.define_value(
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
class NoisyString extends String {
2+
function NoisyString(val) {
3+
super(val);
4+
trace('"' + val + '" coerced to NoisyString!');
5+
}
6+
}
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// Compile with:
2+
// mtasc -main -version 8 Test.as -swf shim.swf -keep -out test.swf
3+
class Test {
4+
5+
// shim.swf provides this function, which wraps the `CastOp` AVM1 action.
6+
// arguments: (object, constructor)
7+
// It is normally available through `catch(e: Class)`, but direct access
8+
// makes for simpler test code, and allows inspecting the actual return value.
9+
static var opcode__cast = _root.opcode__cast;
10+
11+
static function main(current) {
12+
_global.String = NoisyString;
13+
14+
trace('// obj: "left", class: "right"');
15+
check("left", "right");
16+
trace('// obj: "left", class: Object');
17+
check("left", Object);
18+
trace('// obj: {}, class: "right"');
19+
check({}, "right");
20+
trace('');
21+
22+
trace('// obj: {}, class: { prototype: Object.prototype }');
23+
check({}, { prototype: Object.prototype });
24+
25+
trace('// obj: { __proto__: "left" }, class: { prototype: "right" }');
26+
check({ __proto__: "left" }, { prototype: "right" });
27+
28+
trace('// obj: { __proto__: "proto" }, class: { prototype: "proto" }');
29+
check({ __proto__: "proto" }, { prototype: "proto" });
30+
31+
trace('// obj: { __proto__: null }, class: { prototype: null }');
32+
check({ __proto__: null }, { prototype: null });
33+
34+
// Prototype properties don't work...
35+
var fakeClass = {};
36+
fakeClass.addProperty("prototype", function() {
37+
trace("prototype property called!");
38+
return "property proto";
39+
}, null);
40+
trace('// obj: {}, class: { get prototype(): ... }');
41+
trace('.prototype is: ' + fakeClass.prototype);
42+
check({}, fakeClass);
43+
44+
// ...and neither do 'indirect' prototypes...
45+
fakeClass = { __proto__: { prototype: {}}};
46+
trace('// obj: { __proto__: X }, class: { __proto__: { prototype: X }}');
47+
check({ __proto__: fakeClass.prototype }, fakeClass);
48+
49+
// ...nor prototypes behind 'super'.
50+
var FakeClassSuper = function() { this.zuper = super; };
51+
FakeClassSuper.prototype = { __proto__: { prototype: "super proto" }};
52+
fakeClass = (new FakeClassSuper()).zuper;
53+
trace('// obj: {}, class: super { prototype: ... }');
54+
trace('.prototype is: ' + fakeClass.prototype);
55+
check({}, fakeClass);
56+
57+
// SWF version flags are ignored.
58+
fakeClass = { prototype: "SWFv9 proto" };
59+
_global.ASSetPropFlags(fakeClass, "prototype", 0x2000 /* version 9 */);
60+
trace('// obj: {}, class: { prototype(SWFv9): ... }');
61+
trace('.prototype is: ' + fakeClass.prototype);
62+
check({}, fakeClass);
63+
64+
trace('');
65+
66+
var movieclip = current.createEmptyMovieClip("mc", 1);
67+
movieclip.prototype = Object.prototype;
68+
testsForMovieClip(movieclip);
69+
trace('// kill movieclip');
70+
movieclip.removeMovieClip();
71+
testsForMovieClip(movieclip);
72+
73+
fscommand('quit');
74+
}
75+
76+
static function testsForMovieClip(movieclip) {
77+
trace('// obj: movieclip, class: "right"');
78+
check(movieclip, "right");
79+
trace('// obj: movieclip, class: Object');
80+
check(movieclip, Object);
81+
trace('// obj: movieclip, class: { prototype: "proto" }');
82+
check(movieclip, { prototype: "proto" });
83+
trace('// obj: {}, class: MovieClip { prototype: Object.prototype }');
84+
check({}, movieclip);
85+
trace('// obj: { __proto__: movieclip }, class: { prototype: movieclip }');
86+
check({ __proto__: movieclip }, { prototype: movieclip });
87+
}
88+
89+
static function check(obj, klass) {
90+
trace("instanceof: " + (obj instanceof klass));
91+
trace("typeof cast: " + typeof opcode__cast(obj, klass));
92+
}
93+
}
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// obj: "left", class: "right"
2+
instanceof: false
3+
"left" coerced to NoisyString!
4+
typeof cast: null
5+
// obj: "left", class: Object
6+
instanceof: false
7+
"left" coerced to NoisyString!
8+
typeof cast: null
9+
// obj: {}, class: "right"
10+
"right" coerced to NoisyString!
11+
instanceof: false
12+
"right" coerced to NoisyString!
13+
typeof cast: null
14+
15+
// obj: {}, class: { prototype: Object.prototype }
16+
instanceof: true
17+
typeof cast: object
18+
// obj: { __proto__: "left" }, class: { prototype: "right" }
19+
"right" coerced to NoisyString!
20+
instanceof: false
21+
"right" coerced to NoisyString!
22+
typeof cast: null
23+
// obj: { __proto__: "proto" }, class: { prototype: "proto" }
24+
"proto" coerced to NoisyString!
25+
instanceof: false
26+
"proto" coerced to NoisyString!
27+
typeof cast: null
28+
// obj: { __proto__: null }, class: { prototype: null }
29+
instanceof: false
30+
typeof cast: null
31+
// obj: {}, class: { get prototype(): ... }
32+
prototype property called!
33+
.prototype is: property proto
34+
instanceof: false
35+
typeof cast: null
36+
// obj: { __proto__: X }, class: { __proto__: { prototype: X }}
37+
instanceof: false
38+
typeof cast: null
39+
// obj: {}, class: super { prototype: ... }
40+
.prototype is: super proto
41+
instanceof: false
42+
typeof cast: null
43+
// obj: {}, class: { prototype(SWFv9): ... }
44+
.prototype is: undefined
45+
"SWFv9 proto" coerced to NoisyString!
46+
instanceof: false
47+
"SWFv9 proto" coerced to NoisyString!
48+
typeof cast: null
49+
50+
// obj: movieclip, class: "right"
51+
"right" coerced to NoisyString!
52+
instanceof: false
53+
"right" coerced to NoisyString!
54+
typeof cast: null
55+
// obj: movieclip, class: Object
56+
instanceof: true
57+
typeof cast: movieclip
58+
// obj: movieclip, class: { prototype: "proto" }
59+
"proto" coerced to NoisyString!
60+
instanceof: false
61+
"proto" coerced to NoisyString!
62+
typeof cast: null
63+
// obj: {}, class: MovieClip { prototype: Object.prototype }
64+
instanceof: true
65+
typeof cast: object
66+
// obj: { __proto__: movieclip }, class: { prototype: movieclip }
67+
instanceof: false
68+
typeof cast: null
69+
// kill movieclip
70+
// obj: movieclip, class: "right"
71+
"right" coerced to NoisyString!
72+
instanceof: false
73+
"right" coerced to NoisyString!
74+
typeof cast: null
75+
// obj: movieclip, class: Object
76+
instanceof: false
77+
typeof cast: null
78+
// obj: movieclip, class: { prototype: "proto" }
79+
"proto" coerced to NoisyString!
80+
instanceof: false
81+
"proto" coerced to NoisyString!
82+
typeof cast: null
83+
// obj: {}, class: MovieClip { prototype: Object.prototype }
84+
instanceof: false
85+
typeof cast: null
86+
// obj: { __proto__: movieclip }, class: { prototype: movieclip }
87+
instanceof: false
88+
typeof cast: null

0 commit comments

Comments
 (0)