Closed Bug 1246607 Opened 8 years ago Closed 8 years ago

Assertion failure: !cx->isExceptionPending(), at js/src/builtin/TestingFunctions.cpp:1213 with OOM

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: decoder, Assigned: bbouvier)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update,ignore])

Attachments

(2 files, 1 obsolete file)

The following testcase crashes on mozilla-central revision a0d0344ed47a (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --baseline-eager):

oomTest(() => {
    try { 
        new Intl.DateTimeFormat;  
        x1 = 0; 
    }  catch (e) {
        switch (1) {
            case 0:
                let s;
            case 1:
                x;
        }
    }
})



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000a27c27 in OOMTest (cx=0x7ffff6907800, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:1213
#0  0x0000000000a27c27 in OOMTest (cx=0x7ffff6907800, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:1213
#1  0x0000000000aa1e02 in js::CallJSNative (cx=0x7ffff6907800, native=0xa27600 <OOMTest(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
#2  0x0000000000a9b351 in js::Invoke (cx=cx@entry=0x7ffff6907800, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:475
#3  0x0000000000a9be7c in js::Invoke (cx=cx@entry=0x7ffff6907800, thisv=..., fval=..., argc=argc@entry=1, argv=argv@entry=0x7fffffffcb88, rval=..., rval@entry=...) at js/src/vm/Interpreter.cpp:527
#4  0x0000000000602636 in js::jit::DoCallFallback (cx=0x7ffff6907800, frame=0x7fffffffcbc8, stub_=<optimized out>, argc=<optimized out>, vp=0x7fffffffcb78, res=...) at js/src/jit/BaselineIC.cpp:6136
#5  0x00007ffff7ff1abf in ?? ()
[...]
#32 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x16	22
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffc4d0	140737488340176
rsp	0x7fffffffc3f0	140737488339952
r8	0x7ffff7fe07c0	140737354008512
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffffc1b0	140737488339376
r11	0x7ffff6c27960	140737333328224
r12	0x7fffffffc480	140737488340096
r13	0x5c80	23680
r14	0x7ffff6907800	140737330051072
r15	0x1ba469c	28984988
rip	0xa27c27 <OOMTest(JSContext*, unsigned int, JS::Value*)+1575>
=> 0xa27c27 <OOMTest(JSContext*, unsigned int, JS::Value*)+1575>:	movl   $0x4bd,0x0
   0xa27c32 <OOMTest(JSContext*, unsigned int, JS::Value*)+1586>:	callq  0x4a4690 <abort()>
Attached patch Propagate OOM error reporting (obsolete) — Splinter Review
AddClearDefinite... can throw out of memory, but this isn't checked on the caller's side. Alternative would be to have the function return true/false only to indicate VM's status (oom, e.g.) and add an outparam to indicate success/failure of the analysis, but that's much bigger.

Review was intended for bhackett, but he blocked reviews :-} Bouncing to you Jan, but feel free to bounce to somebody else!
Assignee: nobody → bbouvier
Status: NEW → ASSIGNED
Attachment #8716957 - Flags: review?(jdemooij)
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9c365490d4ce
user:        Jon Coppeard
date:        Tue Oct 13 13:37:07 2015 +0100
summary:     Bug 1212469 - Make oomTest() into a shell function r=nbp

This iteration took 275.799 seconds to run.
Comment on attachment 8716957 [details] [diff] [review]
Propagate OOM error reporting

Review of attachment 8716957 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for fixing this!

It's a bit confusing that AddClearDefiniteGetterSetterForPrototypeChain returning false means either "we reported OOM" or "we have imprecise type information". I think only the proto->getGroup(cx) call there can report OOM and it might be a bit simpler to add a cx->recoverFromOutOfMemory() call there. That way, AddClearDefiniteGetterSetterForPrototypeChain returning false means "unable to do that" and the callers don't have to care about the exact reason.
Attachment #8716957 - Flags: review?(jdemooij)
Attached patch recover.patchSplinter Review
Indeed, this fixes the issue. Error reporting is hard.
Attachment #8716957 - Attachment is obsolete: true
Attachment #8718271 - Flags: review?(jdemooij)
Comment on attachment 8718271 [details] [diff] [review]
recover.patch

Review of attachment 8718271 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8718271 - Flags: review?(jdemooij) → review+
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision e355cacefc88).
https://hg.mozilla.org/mozilla-central/rev/48c5159ddfbc
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: