1113980 - crash in js::ScriptedIndirectProxyHandler::defineProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JSPropertyDescriptor>)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Alice0775 White]

This bug was filed from the Socorro interface and is 
report bp-956ca8b6-2208-49d3-81cb-19b882141220.
=============================================================

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6f1696df360b&tochange=7837628feeca

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]:

Comment 2

[Alice0775 White]

bp-be10f4d5-bbe4-4114-954c-ec4e42141220

Comment 3

[Alice0775 White]

In local build
Last Good: 38c02eda337d
First Bad: 6a71021584ff

Comment 4

[Tom S]

Test case:
var p = Proxy.create({
    getOwnPropertyDescriptor: function() {
        return {value: 1, configurable: true, writable: true};
    }
}, null);

var o = Object.create(p);
o.a = 1;

Comment 5

[Tom S]

Oh also interesting that apparently this is a version of Caja using the old proxies.

Comment 6

[Spencer Selander [greenknight]]

Tested that URL with e10s disabled, browser crashed and session restore failed - one time reopened to a blank browser, other times with "Trouble restoring..." message. With e10s enabled, just the one tab crashed.

Comment 7

[Tom S]

Attachment: set-property

Comment 8

[Jeff Walden [:Waldo]]

Comment on attachment 8540994 [details] [diff] [review]
set-property

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

Dunno if the exact semantics implemented here are correct, but this clearly removes the type error of passing an arbitrary receiver into a proxy handler trap where the proxy object was expected.  So let's do this.  Correct semantics should eventually fall out of jorendorff's ongoing refactoring of all this junk.

Comment 9

[Tom S]

Thanks. I agree this is probably better, but Jason should still take a look at this.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6767f9dce400
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fbd629599de

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7fbd629599de

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

This landed on m-c at the beginning of Jan. As this bug is tracked for 37, does it need to remain open? Has this issue been addressed?

Comment 12

[Tom S]

This is waiting on Jason to answer his needinfo request.

Comment 13

[Jason Orendorff [:jorendorff]]

Thanks for leaving it open. I'm worried about this and need to confer with bz.

Comment 14

[Jason Orendorff [:jorendorff]]

Talked with bz. Tom's fix is the right thing.

I've filed follow-up bug 1130537 to fix some underlying weirdness, and hope to get a patch in quickly...

Comment 15

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.