Closed Bug 361273 Opened 13 years ago Closed 13 years ago

Assert fail: cg->stackDepth >= 0, at jsemit.c:164

Categories

(Core :: JavaScript Engine, defect, P1, major)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: gavin.reaney, Assigned: brendan)

Details

(Keywords: fixed1.8.0.9, fixed1.8.1.1)

Attachments

(4 files, 2 obsolete files)

In low memory conditions it's possible to get the following assertion failure reported (stack trace will follow shortly):

Assertion failure: cg->stackDepth >= 0, at jsemit.c:164

The root cause is this error handling in jsemit:

#define EMIT_ATOM_INDEX_OP(op, atomIndex)                                     \
    JS_BEGIN_MACRO                                                            \
        if (!EmitAtomIndexOp(cx, op, atomIndex, cg) < 0)                      \
            return JS_FALSE;                                                  \
    JS_END_MACRO

EmitAtomIndexOp returns a JSBool so any error returned will actually be ignored by mistake, leading to the assert fail later.

In a release build we get stack underflow reported when the compiled script is run, and JS_ArenaAllocate can get itself into an infinite loop that only exits when malloc fails (more details on this soon).
To reproduce the assert fail:

- apply the patch
- run the JS shell
- enter the following script:

obj = new Object()
The attachment shows what would happen in a release build of the JS shell with the single malloc failure simulated by attachment 246017 [details] [diff] [review].

JS_ArenaAllocate is being called with a request for a block of 4294967288 bytes which it attempts to honour:

    gross = hdrsz + JS_MAX(nb, pool->arenasize);
    b = (JSArena *) malloc(gross);

The calculation of 'gross' overflows and we end up requesting 15 bytes instead. Since this is less than 'nb' we remain stuck in a near infinite loop that will exit only when malloc fails (in low memory situations this should happen quite quickly, but it still seems like something we should guard against).

I'll upload a patch soon with the simple fix to jsemit, and some detection code for the wrap-around described above. This may be of interest for some of the other bugs that report that 'firefox uses all CPU and memory', although it's just a random guess that the two might be related.
Ready for review.
Attachment #246021 - Flags: review?(brendan)
Assignee: gavin → brendan
Attachment #246021 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #246050 - Flags: review+
Attachment #246021 - Flags: review?(brendan)
Flags: blocking1.8.1.1?
Priority: -- → P1
Target Milestone: --- → mozilla1.9alpha
Gavin, thanks for another great find and fix.  Landed on trunk:

Checking in jsarena.c;
/cvsroot/mozilla/js/src/jsarena.c,v  <--  jsarena.c
new revision: 3.31; previous revision: 3.30
done
Checking in jsemit.c;
/cvsroot/mozilla/js/src/jsemit.c,v  <--  jsemit.c
new revision: 3.228; previous revision: 3.227
done

/be
Attachment #246050 - Attachment is obsolete: true
Attachment #246051 - Flags: review+
Attachment #246050 - Flags: review+
Comment on attachment 246051 [details] [diff] [review]
better version, just landed

Affects the 1.8.0 branch too.

/be
Attachment #246051 - Flags: approval1.8.1.1?
Attachment #246051 - Flags: approval1.8.0.9?
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: blocking1.8.0.9?
Resolution: --- → FIXED
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1+
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.9+
Comment on attachment 246051 [details] [diff] [review]
better version, just landed

Approved for both branches.  a=jay for drivers.  Please land ASAP on 1.8.1 and 1.8.0 branches.
Attachment #246051 - Flags: approval1.8.1.1?
Attachment #246051 - Flags: approval1.8.1.1+
Attachment #246051 - Flags: approval1.8.0.9?
Attachment #246051 - Flags: approval1.8.0.9+
Fixed on the 1.8 branch:

Checking in jsarena.c;
/cvsroot/mozilla/js/src/jsarena.c,v  <--  jsarena.c
new revision: 3.28.8.3; previous revision: 3.28.8.2
done
Checking in jsemit.c;
/cvsroot/mozilla/js/src/jsemit.c,v  <--  jsemit.c
new revision: 3.128.2.58; previous revision: 3.128.2.57
done

and (with merging) on the 1.8.0 branch:

Checking in jsarena.c;
/cvsroot/mozilla/js/src/jsarena.c,v  <--  jsarena.c
new revision: 3.28.16.2; previous revision: 3.28.16.1
done
Checking in jsemit.c;
/cvsroot/mozilla/js/src/jsemit.c,v  <--  jsemit.c
new revision: 3.128.2.3.2.12; previous revision: 3.128.2.3.2.11
done

/be
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.