490741 - Crash [@ js_GetUpvar] on datepick

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

mozilla-central, debug:

1. Load http://keith-wood.name/datepick.html

Result: Crash [@ js_GetUpvar]

Comment 1

[Nochum Sossonko [:Natch]]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090428 Minefield/3.6a1pre

Comment 2

[Jesse Ruderman]

http://www.wil-linssen.com/extending-the-jquery-sortable-with-ajax-mysql/ too.

But I can't reproduce with a local copy of either page :(

Comment 3

[Brendan Eich [:brendan]]

Attachment: one-line fix

So obvious in hindsight.

Working on a reduced testcase.

/be

Comment 4

[Brendan Eich [:brendan]]

Fixed in tm:

http://hg.mozilla.org/tracemonkey/rev/3bdc174e297b

/be

Comment 5

[Brendan Eich [:brendan]]

I'm having a hard time reducing. I don't see why local copies would fail to repro, however. Jesse, any further insights?

Fixed on m-c:

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

/be

Comment 6

[Jesse Ruderman]

I tried my usual local-reproduction tricks a few days ago and didn't get anywhere.  I guess you can't make a testcase from scratch either, despite understanding the bug?

Comment 7

[Brendan Eich [:brendan]]

I can, it's just going to take more time.

I don't see how local vs. remote can matter, though. Is there an XHR involved in loading code?

/be

Comment 8

[Jesse Ruderman]

Attachment: XMLHttpRequest interceptor

I should have written this a long time ago.  Thanks, Brendan ;)

Comment 9

[Jesse Ruderman]

Attachment: self-contained testcase (crashes Firefox when loaded)

Comment 10

[Jesse Ruderman]

That one is based on http://noteslog.com/chili/.  All three URLs use the Chili syntax highlighter from that page.

Chili itself is pretty small, so this should be fairly easy to reduce now, assuming the bug isn't all tangled up in jQuery.  I'll try running Lithium first.

Comment 11

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

Attachment: 40% smaller testcase

Chili itself is now 50-odd lines (the rest aren't fully reduced yet).

Comment 12

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090504 Minefield/3.6a1pre.  Test URLS from comment 0 and 2 are no longer crashing on trunk.

Comment 13

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

Attachment: 185-line testcase

This crashes at least on Ubuntu 9.04 - I haven't fully reduced it yet, won't have time to work on this for the next few hours.

Comment 14

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

Attachment: 115-line testcase

Down to 115 lines, and that's about it for now, really.

Comment 15

[Brendan Eich [:brendan]]

Gary: great work on the reduction. Did you try removing the document-using functions too? Maybe they just need to be turned into empty stubs (scrubbing all uses of document).

Bob: this can be a shell testcase, I think, if you fake up setTimeout like so:

var timeouts = [];
function setTimeout(f, t) {
    var a = Array.prototype.slice.call(arguments, 2);
    t += Date.now();
    for (var i = 0; i < timeouts.length; i++) {
        if (timeouts[i].deadline >= t)
            break;
    }
    timeouts.splice(i, 0, {deadline: t, handler: f, args: a});
}

and then run timeouts after everything else has run as if "on load":

for (var i = 0; i < timeouts.length; i++)
    timeouts[i].handler.apply(null, timeouts[i].args);

I hope the document uses can be eliminated or stubbed.

/be

Comment 16

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

(In reply to comment #15)
> Gary: great work on the reduction. Did you try removing the document-using
> functions too? Maybe they just need to be turned into empty stubs (scrubbing
> all uses of document).

There's 3 uses of document - line 9 "return jQuery(document).ready(selector);" - line 44 "document.addEventListener("DOMContentLoaded"," - line 58 "this.call(document, jQuery);", there's also the use of window, along with setTimeout as previously mentioned.

I couldn't find a way to eradicate all 3 uses of document at one go. :(

Comment 17

[Brendan Eich [:brendan]]

Attachment: mostly reduced shell testcase

Note the prolog and epilog:

var window = this;
var eventListeners = [];
var document = {
    addEventListener: function (name, func, flag) {
        eventListeners.push(func);
    }
};

// html testcase goes here

var timeouts = [];
function setTimeout(f, t) {
    var a = Array.prototype.slice.call(arguments, 2);
    t += Date.now();
    for (var i = 0; i < timeouts.length; i++) {
        if (timeouts[i].deadline >= t)
            break;
    }
    timeouts.splice(i, 0, {deadline: t, handler: f, args: a});
}
for each (var el in eventListeners)
    el();
for (var i = 0; i < timeouts.length; i++)
    timeouts[i].handler.apply(null, timeouts[i].args);

These techniques are applicable to other html-only reductions that could be turned into js shell testcases.

/be

Comment 18

[Brendan Eich [:brendan]]

I suspect further reduction is possible but I have no time for it. If someone else does, I'd be interested to watch.

/be

Comment 19

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

autoBisect shows this is probably related to bug 488015 :

The first bad revision is:
changeset:   27205:78a21b8efe1b
user:        Brendan Eich
date:        Wed Apr 15 01:57:13 2009 -0700
summary:     Bug 488015 - Crash [@ js_GetUpvar ] (also bogus JS errors, also probably Crash [@js_Interpret]) (future r=mrbkap, see bug).

Comment 20

[Brendan Eich [:brendan]]

(In reply to comment #18)
> I suspect further reduction is possible but I have no time for it. If someone
> else does, I'd be interested to watch.

Obviously there's only one document.addEventListener call, so the prolog and epilog code could be specialized at the price of reusability in other tests.

(In reply to comment #19)
> autoBisect shows this is probably related to bug 488015 :
> 
> The first bad revision is:
> changeset:   27205:78a21b8efe1b
> user:        Brendan Eich
> date:        Wed Apr 15 01:57:13 2009 -0700
> summary:     Bug 488015 - Crash [@ js_GetUpvar ] (also bogus JS errors, also
> probably Crash [@js_Interpret]) (future r=mrbkap, see bug).

The bug was latent all along since upvar2 was patched, but sure -- this was the exposing patch.

/be

Comment 21

[Jesse Ruderman]

Attachment: 15-line shell testcase

Comment 22

[Brendan Eich [:brendan]]

(In reply to comment #21)
> Created an attachment (id=375930) [details]
> 15-line shell testcase

Huzzah!

/be

Comment 23

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/aa65e95a9b4b

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with given url in comment 0 and the following debug builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090522 Minefield/3.6a1pre ID:20090522133810

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre)
Gecko/20090522 Shiretoko/3.5pre ID:20090522153422