Closed Bug 507424 Opened 10 years ago Closed 10 years ago

"Assertion failure: fun->u.i.script->regexpsOffset == 0, at ../jsfun.cpp"

Categories

(Core :: JavaScript Engine, defect, critical)

x86
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: gkw, Assigned: dmandelin)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

(new Function("'a'.replace(/a/,function(x){return(/x/ for each(y in[x]))})"))()

asserts js dbg shell without -j at Assertion failure: fun->u.i.script->regexpsOffset == 0, at ../jsfun.cpp:2526

The first bad revision is:
changeset:   30634:ada08e63ab62
user:        Igor Bukanov
date:        Fri Jul 24 12:01:37 2009 +0200
summary:     bug 505460 - preallocating reserved slots. r=brendan
Flags: blocking1.9.2?
Summary: Assertion failure: fun->u.i.script->regexpsOffset == 0, at ../jsfun.cpp → "Assertion failure: fun->u.i.script->regexpsOffset == 0, at ../jsfun.cpp"
Flags: blocking1.9.2? → blocking1.9.2+
Assignee: general → igor
This seems like it might be interacting badly with the patch for bug 471214, so I'm gonna take a look at it. Let me know if I'll be conflicting with anything.
The assert is in here:

JSObject * JS_FASTCALL
js_AllocFlatClosure(JSContext *cx, JSFunction *fun, JSObject *scopeChain)
{
    [snip unrelated precondition asserts]
    /*
     * Assert that fun->countInterpretedReservedSlots returns 0 when
     * fun->u.i.nupvars is zero.
     */
    JS_ASSERT(fun->u.i.script->regexpsOffset == 0);

    JSObject *closure = js_CloneFunctionObject(cx, fun, scopeChain);
    if (!closure || fun->u.i.nupvars == 0)
        return closure;
    if (!js_EnsureReservedSlots(cx, closure,
                                fun->countInterpretedReservedSlots())) {

IIUC, the assert ensures that the early exit when |fun->u.i.nupvars == 0| is valid. If the asserted condition is false, then we do need to do js_EnsureReservedSlots. So apparently the fix just needs to handle this case. 

I'm gonna assume that's correct and make a patch. You can correct me later if I've assumed horribly wrong.
Attached patch PatchSplinter Review
Assignee: igor → dmandelin
Status: NEW → ASSIGNED
Attachment #391741 - Flags: review?(igor)
Comment on attachment 391741 [details] [diff] [review]
Patch

countInterpretedReservedSlots is defined in the same file so the compiler presumably can optimize a call to the function. If not, we can consider making it inline - it is sufficiently simple to deserve so. But that can wait another bug.
Attachment #391741 - Flags: review?(igor) → review+
(In reply to comment #4)
> (From update of attachment 391741 [details] [diff] [review])
> countInterpretedReservedSlots is defined in the same file so the compiler
> presumably can optimize a call to the function. If not, we can consider making
> it inline - it is sufficiently simple to deserve so. But that can wait another
> bug.

I figured your original design was trying to be efficient about slot counting. Not having a clear handle on that issue, I just plowed ahead with the easy way. Hopefully soon we will have microbenchmarks for this sort of thing and we can figure these things out more easily.

Pushed to TM as 46ed2bc15e56.
Whiteboard: fixed-in-tracemonkey
If this is fixed in TM, could someone please merge to trunk?
This prevents running mochitest on linux (debug build).
I second smaug's complaint (on irc) that we should have a tinderbox running debug mochitests. This certainly isn't the first time I've hit a js assert running mochitests, and as we start moving to fatal assertions in more modules (imagelib is doing that soon, for one), this will become more and more of a problem.
Blocks: 372581
i continuously crash on this while running Places xpcshell tests
http://hg.mozilla.org/mozilla-central/rev/46ed2bc15e56
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to comment #7)
> I second smaug's complaint (on irc) that we should have a tinderbox running
> debug mochitests. This certainly isn't the first time I've hit a js assert
> running mochitests, and as we start moving to fatal assertions in more modules
> (imagelib is doing that soon, for one), this will become more and more of a
> problem.

Blame us for the assert-fail, to be sure, but I assure you in the strongest terms possible that a large number of people have wanted and have complained about the absence of debug tinderboxen for *years*.  See the bug this was recent marked as blocking for considerable griping over the matter.  Unfortunately, I know no way to expedite getting them, so I try not to dwell on an intransigent problem as it isn't going to improve morale to do so.
Flags: in-testsuite?
Mass change: adding fixed1.9.2 keyword

(This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)
Keywords: fixed1.9.2
verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.2a2pre) Gecko/20090911 Namoroka/3.6a2pre
Keywords: verified1.9.2
js/tests/js1_8_1/regress/regress-507424.js
Flags: in-testsuite? → in-testsuite+
v 1.9.3, 1.9.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.