823715 - "Assertion failure: JSVAL_IS_DOUBLE_IMPL(data),"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

for (var i = 0; i < 9; ++i) {
    var g = this[Object.getOwnPropertyNames(this)[i]];
    for (var j = 0; j < 9; ++j) {
        try {
            g.prototype[Object.getOwnPropertyNames(g.prototype)[j]]
        } catch (e) {}
    }
}

asserts js debug shell on m-c changeset 868b21bed3eb without any CLI arguments at Assertion failure: JSVAL_IS_DOUBLE_IMPL(data),

This testcase requires --enable-more-deterministic but I think I can get a testcase without --enable-more-deterministic.

jsfunfuzz hits this very very early on prior to generation of any random code, so having this bug renders jsfunfuzz almost inoperable.

Comment 1

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

for (var i = 0; i < 99; ++i) {
    var g = this[Object.getOwnPropertyNames(this)[i]];
    for (var j = 0; j < 9; ++j) {
        try {
            g.prototype[Object.getOwnPropertyNames(g.prototype)[j]];
        } catch (e) {}
    }
}

This testcase does not require --enable-more-deterministic - I think the only difference is the first for-loop needing to loop 99 times here instead of only 9 times in comment 0.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   116633:91dae8287643
user:        Jan de Mooij
date:        Thu Dec 20 15:27:54 2012 +0100
summary:     Bug 822385 - Add getter, setter and method with jitinfo to the shell. r=bz

jandem, this is pretty bad, do you mind looking at this asap?

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Error: Failed to isolate test from comment

Comment 4

[Boris Zbarsky [:bzbarsky]]

Oops.  This is definitely a bug in the patch for bug 822385.  This part:

>+    JS::Value val = js::GetReservedSlot(obj, DOM_OBJECT_SLOT);

Is wrong because the shell object setup differs from actual DOM objects in a crucial way.  In particular, the "DOM proto" is set up with the same JSClass as the "DOM objects" in the shell, which is NOT how actual DOM objects work.  But the proto of course does not have a useful private set.  So we pass the class check in dom_generic*, but then try to val.toPrivate() on the reserved slot, and that fails.

We should probably just use ObjectClass for the proto.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch

I'm not sure how to use the Object proto as prototype and still be able to use InitClass. Furthermore, I'm not sure if the IonMonkey paths still work with that. So this patch just initializes FakeDOMObject.prototype.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 694741 [details] [diff] [review]
Patch

The Ion paths should work fine with that, but yeah, you can't JS_InitClass, I guess.

r=me; at some point if it becomes needed we can make this look more like actual DOM objects.

Comment 7

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a431a8e935bf

Thanks for the quick review!

Comment 8

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision ea373e534245).

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/a431a8e935bf

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

A variant of the test landed -> in-testsuite+ and VERIFIED on behalf of jsbugmon.