Closed Bug 490741 Opened 15 years ago Closed 15 years ago

Crash [@ js_GetUpvar] on datepick

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: jruderman, Assigned: brendan)

References

()

Details

(Keywords: crash, testcase, verified1.9.1, Whiteboard: fixed-in-tracemonkey)

Crash Data

Attachments

(3 files, 5 obsolete files)

mozilla-central, debug:

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

Result: Crash [@ js_GetUpvar]
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090428 Minefield/3.6a1pre
OS: Mac OS X → All
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 :(
Assignee: general → brendan
Status: NEW → ASSIGNED
Flags: blocking1.9.1?
Priority: -- → P1
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.1
Flags: blocking1.9.1? → blocking1.9.1+
Attached patch one-line fixSplinter Review
So obvious in hindsight.

Working on a reduced testcase.

/be
Attachment #375413 - Flags: review?(mrbkap)
Attachment #375413 - Flags: review?(mrbkap) → review+
Fixed in tm:

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

/be
Whiteboard: fixed-in-tracemonkey
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
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
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?
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
I should have written this a long time ago.  Thanks, Brendan ;)
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.
Attached file 40% smaller testcase (obsolete) —
Chili itself is now 50-odd lines (the rest aren't fully reduced yet).
Attachment #375574 - Attachment is obsolete: true
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.
Status: RESOLVED → VERIFIED
Attached file 185-line testcase (obsolete) —
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.
Attachment #375605 - Attachment is obsolete: true
Attached file 115-line testcase (obsolete) —
Down to 115 lines, and that's about it for now, really.
Attachment #375721 - Attachment is obsolete: true
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
(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. :(
Attached file mostly reduced shell testcase (obsolete) —
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
I suspect further reduction is possible but I have no time for it. If someone else does, I'd be interested to watch.

/be
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).
Blocks: 488015
(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
Attached file 15-line shell testcase
Attachment #375772 - Attachment is obsolete: true
Attachment #375819 - Attachment is obsolete: true
(In reply to comment #21)
> Created an attachment (id=375930) [details]
> 15-line shell testcase

Huzzah!

/be
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
Target Milestone: mozilla1.9.1 → mozilla1.9.2a1
Crash Signature: [@ js_GetUpvar]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: