704340 - Assertion failure: cx->typeInferenceEnabled() && hasLazyType(), at jsinfer.cpp:5697 with OOM

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: Test case for shell (see README file inside).

The attached test asserts on mozilla-central revision 5ebeef1eabcb (see README for running instructions). The test might be 64 bit only.

Partial stack:

#0  0x00007ffff7bcdb3b in raise (sig=<value optimized out>) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:42
#1  0x00000000005c285e in CrashInJS () at /srv/repos/mozilla-central/js/src/jsutil.cpp:95
#2  0x00000000005c28b6 in JS_Assert (s=0x796528 "cx->typeInferenceEnabled() && hasLazyType()", file=0x794f88 "/srv/repos/mozilla-central/js/src/jsinfer.cpp", ln=5697)
    at /srv/repos/mozilla-central/js/src/jsutil.cpp:103
#3  0x00000000004d6ec3 in JSObject::makeLazyType (this=0x7ffff6002598, cx=0xb42ee0) at /srv/repos/mozilla-central/js/src/jsinfer.cpp:5697
#4  0x000000000045b545 in JSObject::getType (this=0x7ffff6002598, cx=0xb42ee0) at ../jsobjinlines.h:931
#5  0x00000000004d6c65 in JSObject::splicePrototype (this=0x7ffff6000128, cx=0xb42ee0, proto=0x7ffff6002598) at /srv/repos/mozilla-central/js/src/jsinfer.cpp:5659
#6  0x000000000060a1dd in js::GlobalObject::initFunctionAndObjectClasses (this=0x7ffff6000128, cx=0xb42ee0) at /srv/repos/mozilla-central/js/src/vm/GlobalObject.cpp:235
#7  0x00000000004a76f6 in js::GlobalObject::getOrCreateObjectPrototype (this=0x7ffff6000128, cx=0xb42ee0) at /srv/repos/mozilla-central/js/src/vm/GlobalObject.h:213
#8  0x00000000006097ef in js_InitObjectClass (cx=0xb42ee0, obj=0x7ffff6000128) at /srv/repos/mozilla-central/js/src/vm/GlobalObject.cpp:63
#9  0x000000000052e72b in js_GetClassObject (cx=0xb42ee0, obj=0x7ffff6000128, key=JSProto_Object, objp=0x7fffffffb308) at /srv/repos/mozilla-central/js/src/jsobj.cpp:4806
#10 0x000000000052e912 in js_FindClassObject (cx=0xb42ee0, start=0x0, protoKey=JSProto_Object, vp=0x7fffffffb370, clasp=0xb06040)
    at /srv/repos/mozilla-central/js/src/jsobj.cpp:4857
#11 0x0000000000533a8b in js::FindClassPrototype (cx=0xb42ee0, scopeobj=0x7ffff6000128, protoKey=JSProto_Object, protop=0x7fffffffb458, clasp=0xb06040)
    at /srv/repos/mozilla-central/js/src/jsobj.cpp:6726
#12 0x0000000000533c35 in js_GetClassPrototype (cx=0xb42ee0, scopeobj=0x7ffff6000128, protoKey=JSProto_Object, protop=0x7fffffffb458, clasp=0xb06040)
    at /srv/repos/mozilla-central/js/src/jsobj.cpp:6769
#13 0x0000000000574f72 in js::FindProto (cx=0xb42ee0, clasp=0xb06040, parent=0x7ffff6000128, proto=0x7fffffffb458) at ../jsobjinlines.h:1703
#14 0x000000000057fb45 in js::detail::NewObject<false, false> (cx=0xb42ee0, clasp=0xb06040, proto=0x0, parent=0x7ffff6000128, kind=js::gc::FINALIZE_OBJECT0)
    at ../jsobjinlines.h:1720
#15 0x000000000057f4b7 in js::NewNonFunction<(js::WithProto::e)0> (cx=0xb42ee0, clasp=0xb06040, proto=0x0, parent=0x7ffff6000128) at ../jsobjinlines.h:1801
#16 0x000000000057f39d in JS_InitReflect (cx=0xb42ee0, obj=0x7ffff6000128) at /srv/repos/mozilla-central/js/src/jsreflect.cpp:3262
#17 0x000000000040fd8e in NewGlobalObject (cx=0xb42ee0, compartment=SAME_COMPARTMENT) at /srv/repos/mozilla-central/js/src/shell/js.cpp:5001
#18 0x000000000040e17a in NewGlobal (cx=0xb42ee0, argc=1, vp=0x7ffff63fb1b0) at /srv/repos/mozilla-central/js/src/shell/js.cpp:3912
#19 0x000000000050a95d in js::CallJSNative (cx=0xb42ee0, native=0x40e02b <NewGlobal(JSContext*, uintN, jsval*)>, args=...) at ../jscntxtinlines.h:297
#20 0x00000000004e9fd9 in js::InvokeKernel (cx=0xb42ee0, args=..., construct=js::NO_CONSTRUCT) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:629
#21 0x00000000004fb1f5 in js::Interpret (cx=0xb42ee0, entryFrame=0x7ffff63fb140, interpMode=js::JSINTERP_NORMAL) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:3950

Comment 1

[Brian Hackett [Laid off!]]

Attachment: patch

Make handling after OOM more robust when dealing with objects with singleton types.  These are only created with inference enabled but if inference is disabled after an OOM some handling is needed to avoid botching asserts.

Comment 2

[Luke Wagner [:luke]]

I don't really understand this part of the code, but from general principals this sort of ad hoc oom handling smells wrong.  Why can't the OOM'ing location 'return false' and have that take you all the way out of TI code?  Furthermore, why is it necessary to disable TI instead of just having normal error handling run its course?

Comment 3

[Brian Hackett [Laid off!]]

(In reply to Luke Wagner [:luke] from comment #2)
> I don't really understand this part of the code, but from general principals
> this sort of ad hoc oom handling smells wrong.  Why can't the OOM'ing
> location 'return false' and have that take you all the way out of TI code? 
> Furthermore, why is it necessary to disable TI instead of just having normal
> error handling run its course?

OOMs can be triggered while we are in the middle of solving type constraints.  If this occurs, for correctness we would need to unwind all type sets to the state before we started solving the constraints --- the state of the type sets always needs to be at the least fixed point.  We can't do this unwinding because keeping track of the info needed to do this would have an unacceptable performance cost.

Comment 4

[Luke Wagner [:luke]]

(In reply to Brian Hackett (:bhackett) from comment #3)
> (In reply to Luke Wagner [:luke] from comment #2)

What about blasting away all type information with whatever is called from a purge GC?

Comment 5

[Brian Hackett [Laid off!]]

(In reply to Luke Wagner [:luke] from comment #4)
> (In reply to Brian Hackett (:bhackett) from comment #3)
> > (In reply to Luke Wagner [:luke] from comment #2)
> 
> What about blasting away all type information with whatever is called from a
> purge GC?

This will eventually happen (the next GC will throw away type information, and it won't be regenerated).  That isn't really the problem with this bug, though.  When type inference is disabled we don't mark objects as having singleton types, so some code didn't expect to see singleton objects in an inference-disabled context, leading to botched asserts (but not incorrect behavior).

Comment 6

[Christian Holler (:decoder)]

Mass-closing old JS OOM reports. I've confirmed that none of these signatures currently appear in FuzzManager, so we can safely assume that the code causing this is gone or has been fixed.