779210 - Sampler thunk for builtins changes the default xml namespace

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Profiler, defect)

Description

[George Comninos]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_0) AppleWebKit/537.1 (KHTML, like Gecko) Chrome/21.0.1180.57 Safari/537.1

Steps to reproduce:

The sampler thunks for builtin native calls generate a MethodFrame entry for the native call, but that entry does not inherit the current calling scope's default xml namespace. This causes code to return erroneous results when you have something like:

var myXml:XML = <extension xmlns="http://ns.adobe.com/air/extension/3.4"> <myXmlNode> <childNode> </childNode> </myXmlNode> </extension>
default xml namespace = myXml.namespace();
var result:Boolean = myXml.myXmlNode.hasOwnProperty("childNode");

result will be FALSE, which is obviously incorrect.

Comment 1

[George Comninos]

Attachment: Set sampler thunk method frame dxns to current method dxns

Comment 2

[George Comninos]

Note: I only included the diff for one method in the generated builtin.cpp to show what the change in the script does, obviously it will change all methods in builtin.cpp and shell_toplevel.cpp, as well as the generated avmglue.cpp while building.

Comment 3

[Paul Hohensee]

Comment on attachment 647599 [details] [diff] [review]
Set sampler thunk method frame dxns to current method dxns

Review of attachment 647599 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

Comment 4

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to George Comninos from comment #2)
> Note: I only included the diff for one method in the generated builtin.cpp
> to show what the change in the script does, obviously it will change all
> methods in builtin.cpp and shell_toplevel.cpp, as well as the generated
> avmglue.cpp while building.

I don't understand: Shouldn't there be changes to the nativegen script up here for review?  I assume that's the important thing to fix (though seeing concretely what the corresponding change in output will be is of use), and your note above implies that there is "the change in the script", but it was not included as an attachment on this ticket?

Or were you just showing us a concrete illustration of the core idea before you spent time making some hypothetical changes to nativegen.py ?

Comment 5

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to Felix S Klock II from comment #4)
> (In reply to George Comninos from comment #2)
> > Note: I only included the diff for one method in the generated builtin.cpp
> > to show what the change in the script does, obviously it will change all
> > methods in builtin.cpp and shell_toplevel.cpp, as well as the generated
> > avmglue.cpp while building.
> 
> I don't understand: Shouldn't there be changes to the nativegen script up
> here for review?  I assume that's the important thing to fix (though seeing
> concretely what the corresponding change in output will be is of use), and
> your note above implies that there is "the change in the script", but it was
> not included as an attachment on this ticket?
> 
> Or were you just showing us a concrete illustration of the core idea before
> you spent time making some hypothetical changes to nativegen.py ?

Oh, I am sorry, I got confused by the Bugzilla splinter presentation.  Ignore the above note.

Comment 6

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 647599 [details] [diff] [review]
Set sampler thunk method frame dxns to current method dxns

SR+.  (mostly rubber stamping; so much of our semantics for E4X and Namespaces remains a mystery to me.)

Comment 7

[Tamarin Bot]

changeset: 7521:20e1b78f1d00
user:      Dan Schaffer <[e-mail]>
summary:   bug 779210: Set sampler thunk method frame dxns to current method dxn (p=gcomnino,r=fklockii,r=phohense)

http://hg.mozilla.org/tamarin-redux/rev/20e1b78f1d00