608959 - Object.create misbehave across js sandboxes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Please note that this happens on linux but not on mac (not sure about windows).
I guess it's a bug related to compartments. Here is the test case: 

js> var sandbox = Components.utils.Sandbox('http://mozilla.com')
js> Components.utils.evalInSandbox('function boom(proto, map)
Object.create(proto, map)', sandbox)
js> Object.prototype.constructor
function Object() {
    [native code]...}
js> sandbox.boom(Object.prototype, {}).constructor
js> typeof sandbox.boom(Object.prototype, {}).constructor
"undefined"
js> Object.prototype.constructor
js> typeof Object.prototype.constructor
"undefined"

Important detail is that Object.create not just returned object with a missing `constructor` property, it actually removed property from Object.prototype, which may break lot's of completely unrelated code.

Comment 1

[Jeff Walden [:Waldo]]

So at some point we start going astray when we try to js_NativeGetInline the constructor property off of Object.prototype.  The problem is that the constructor property doesn't have a stubbed getter/setter -- it uses CheckCtorGetAccess/CheckCtorSetAccess, and the value returned through those hooks is undefined.  I'm not sure why this is the case yet, seems quite wrong.

I'd include a stack trace here except that I think Fedora's gdb Python pretty-printers have a bug wherein printing the representation of an object whose first field is the same type of object results in infinite Python recursion which, tho caught, breaks backtrace printing.  I'm trying a hackaround in our source to see if I can kill that, then I'll post a full backtrace.

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Stack trace (hackaround worked!)

Comment 3

[Jeff Walden [:Waldo]]

(The stack trace is captured at the moment that slot 0 of Object.prototype, which ordinarily should contain the constructor property, is overwritten with undefined.  To be precise, the property isn't removed -- its value is incorrectly modified, but it's still there.)

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

With today's nightly bug is reproducible also on mac.

Comment 5

[Jeff Walden [:Waldo]]

I don't see any indications that this should be platform-specific, for what it's worth.

Comment 6

[Jeff Walden [:Waldo]]

So, aided by the greater lucidity of a fuller night's sleep, I think I see the problem.  Looking at attachment 487252 [details] [diff] [review], it changed the call in the attached stack to js_NativeGetInline from this:

    if (!js_NativeGetInline(cx, obj, obj2, shape, getHow, vp))

to this:

    if (!js_NativeGetInline(cx, receiver, obj2, shape, getHow, vp))

In the first case obj is the object where the property's being looked up, obj2 is the object along the prototype chain where it's actually found.  Here, obj == obj2 should be the Object.prototype in question, unless I'm misreading somehow.

In the second case, we pass in receiver, which is the proxy for the result of the boom() call.  obj2 is the Object.prototype in question.  Of course the proxy object isn't going to make any sense to CheckCtorGetAccess, so you get back undefined as the property value.

Under this analysis I could believe maybe the bug lies with CheckCtorGetAccess (and maybe CheckCtorSetAccess?  not sure).  If it unwrapped, and then found the actual object to apply against, maybe that would be adequate.  But that seems like an awful lot of work for one C-style accessor to do, and if this is the correct diagnosis the problem is endemic.

Proxy and JSAPI experts should speak up here -- I've about reached the limit of my knowledge of both, and I'm really not certain of a fix here.

Comment 7

[Jeff Walden [:Waldo]]

Attachment: You can never pass too many objects to an API concerning properties of an object

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 488041 [details] [diff] [review]
You can never pass too many objects to an API concerning properties of an object

Thanks for fixing this.

Comment 9

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/c400bbbd97f3

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/c400bbbd97f3