690376 - Crash [@ JSObject::nonNativeSetProperty]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

(function() {
    function b(c) {
        if (c) u = function() {};
        this.x = /x/;
        switch (c) {
        case 4:
        case 6:
        }
        this.r = {}
    }
    for each(let a in [
        new Boolean(true),
        0,
        new Boolean(true),
        function() {},
        new Boolean(true),
        new Boolean(true),
        function() {},
        function() {},
        function() {}
        ]) {
        try {
            new b(a)();
        } catch (e) {}
    }
})()

crashes js opt shell 64-bit on Mac 10.6 on JM changeset 44ef245b8706 with -m, -a and -j at JSObject::nonNativeSetProperty. Testcase should be passed in as a CLI argument to reproduce.

The $pc value seems to be accessing register r11 at a scary address 0xf1c6d7e000000001 so locking s-s till shown otherwise.

Comment 1

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

Crashes m-c changeset dbb129f069b1 64-bit js shell too, with possibly-related-but-not-really-identical stack.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: patch

Codegen bug in JM+TM interaction, introduced in the TI merge I think.  JM+TM can enter jitcode at any opcode marked as a safe point, but JM did not guarantee this for safe points that are not the targets of a jump.

Comment 3

[Brian Hackett [Laid off!]]

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

Comment 4

[Daniel Veditz [:dveditz]]

Gary: does this crash affect Beta (Firefox 8)? Given the patch (if .. !typeInferenceEnabled()) maybe it does, then again this could have been completely different code prior to the TI landing.

Comment 5

[Christian Holler (:decoder)]

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

Comment 6

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

(In reply to Daniel Veditz from comment #4)
> Gary: does this crash affect Beta (Firefox 8)? Given the patch (if ..
> !typeInferenceEnabled()) maybe it does, then again this could have been
> completely different code prior to the TI landing.

I have to hold off testing on this, as the minis are in boxes and soon-to-be shipping..

Comment 7

[Daniel Veditz [:dveditz]]

bhackett: do you know the answer to comment 4?

Comment 8

[Brian Hackett [Laid off!]]

(In reply to Daniel Veditz from comment #7)
> bhackett: do you know the answer to comment 4?

This should not affect Firefox 8 or earlier.  The bug is caused by doing cross-branch register allocation when we shouldn't (when JM+TM is enabled), this kind of register allocation was introduced with the TI merge.

Comment 9

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c0c7322521b7

Comment 10

[u279076]

Can someone who is already set up to reproduce this bug please verify the fix?

Comment 11

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

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #10)
> Can someone who is already set up to reproduce this bug please verify the
> fix?

Verified fixed on 64-bit debug and opt builds on Mac 10.6 on m-c changeset bc48009a6bbb.

Comment 12

[Christian Holler (:decoder)]

Bug involving TM, but TM is already removed, in-testsuite-.