445402 - VM is not rejecting classes with slots that override slots in the base class

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Chris Peyer]

Attachment: swf that demonstrates bug

Bug transferred from player bugbase: 233068

Reproduction:
1. open attached bug file "SWF10_loadSWF9.swf"

Actual Results:
loader.content.z: 0;
MovieClip (loader.content.z): [object Object]

Expected Results:
loader.content.z: 0;
MovieClip (loader.content.z): 0

Comment 1

[Chris Peyer]

Attachment: swf that gets loaded by SWF10_loadSWF9

Comment 2

[Chris Peyer]

Attachment: source fla

Comment 3

[Chris Peyer]

Attachment: source fla

Comment 4

[Chris Peyer]

Transferred Comments from bugbase:
[dlollar 6/30/08] Entered Bug. Issue from Trevor. With the MovieClip cast, or assigning to an untyped variable, you can get whatever value for MovieClip.z was set in a SWF 9 movie. Without it, the Player sees MovieClip.z as a number per FP 10 API.
Per email, it looks like this is in discussion -- whether this should be allowed or blocked. Filing bug for record of decision.
[wsharp 7/2/08] Also, what about "loader.content["z"]" with a string property access.
[cmckeon 7/2/08] assigned

[tierney 7/2/08] It looks like the VM is not rejecting classes with slots that override slots in the base class. It checks methods, and should be checking slots, but it isn't for some reason. I can take this one.


[tierney 7/3/08] Fixed in changelist 429588 :
http://bugtracker.macromedia.com/perforce/describe.jsp?depot=flashfarm&change=429588

We weren't checking to make sure that slot traits didn't override traits in the base class. Now we do. Thankfully, ASC prevents anybody from compiling code like that, so it shouldn't be a backwards compatible issue. This is also why we didn't notice this bug until testing the versioning stuff.

[dlollar 7/7/08] regressed - confirm/closing.

[tierney 7/10/08] So this has caused some existing content to break, namely the bombaysapphire.com website (bug 233715). I don't know how they got content that did this - either they built against stale versions of code, or there is a Flex/ASC bug where this situation is not always caught.

So, what do we do for Flash10? Do we accept the 9 behavior where overriding a slot is not an error, or do we just version check, and report errors in 10 swfs, but let the 9 ones through?

We certainly have some strange behavior when we don't error, but I suppose that the 9 swfs are depending on it (probably inadvertently).

Consider this code:

class B extends A
{
public var z = "B.z";
}

function f(x : A)
{
trace(x.z);
}
function f2(x)
{
trace(x.z);
}
function f3( x:B )
{
trace(x.z);
}

If A has a z property as well (i.e., recompile the base class but not the derived class), and we don't detect the error, then the output is:

f -> A.z
f2 -> B.z
f3 -> B.z

If we turn off the JIT all three produce B.z.

What is happening is that A has a slot for z. B also has a slot for z, but it is not the same as the slot in A. Effectively the slot in the derived class has hidden the slot from the base class.

However, the JIT early binds, so when it sees something typed as A, it will early bind to the slot in A. The interpreter never early binds, so when it looks up the property it finds the most derived version and uses that.

It seems like we should version check, and make it an error in 10 swfs or greater. The behavior certainly isn't great when we let it through, as what happens depends on if the code is JIT'ed or not.


[tierney 7/11/08] Well, it looks like flex might be taking advantage of this bug. If so, then fixing this would mean flex would have to change their stuff to work with player 10. I have no idea how big of an undertaking that would be.

I've sent some questions to the flex guys, but it looks like the code they introduce for [Bindable] properties may define the same var (mx_internal::_bindingsByDestination) in multiple classes up and down the inheritance chain, which is where the BombaySapphire problem came from. And we'd probably see that in other apps built with flex as well.
[dansmith 7/11/08] This fix breaks existing Flex content so priority raised to A, Re-opening this bug to Erik.

[tierney 7/11/08] Flex is indeed generating the code that causes these conflicts. So, I don't think we can fix the bug without also changing flex (we probably don't want flex unable to generate FP10 content).

To be clear, the bug doesn't break existing content, the fix to the bug breaks existing content. I've already reverted the fix, so existing content is working once again. The only bug now is that we don't detect when one slot overrides another, and we need to decide what, if anything, we do about that bug.

[edwong 7/11/08] was in new - moving to qrb to sort out
[ecostell 7/11/08] Open to Lee
[lthomaso 7/11/08] Given the time remaining in FP10, backing out the "fix" is clearly the correct thing to do. (And that has already happend.) The bug in FP10 regarding variable conflicts between versions is very minor, and easily deferred/retired. The AVM team should look at this as a VM issue.

Recommend xfer to AVM team. Not critical for the player 10 release.
[mccauley 7/11/08] marking with needs tech note
[ecostell 7/14/08] Marked UTR - Chris, can you transfer this to Jira/VM and recommend deferral?  

Comment 5

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.