1368735 - Assertion failure: isRunning() || isClosing(), at js/src/vm/GeneratorObject.h:161 with OOM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision ebad93e11770 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu, run with --fuzzing-safe --thread-count=2 --ion-offthread-compile=off):

lfLogBuffer = `
  evaluate(\`   
    assertThrowsInstanceOf = function (f, ctor, msg) {
    var a = Function,
      Array_isArray = Array,
      Map_ = Map,
      Error_ = Error,
      Symbol_ = Symbol,
      bind,
      Map_get,
      Map_set,
      Object_toString,
      Function_toString,
      getOwnPropertyDescriptor,
      isExtensible,
      Object_getOwnPropertyNames = Object,
      uneval_ = uneval;
      function isPrimitive() {}
    }();
  \`);
assertIteratorResult = function () expectedProps;
function assertIteratorNext(iter)
  assertIteratorResult(iter.next());
function *f3() {
    try {
        try {} finally {
            yield;
        }
    } finally {
        yield;
    }
}
it = f3();
assertIteratorNext(it);
`;
oomTest(Function(lfLogBuffer))



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0877056d in js::GeneratorObject::setYieldAndAwaitIndex (yieldAndAwaitIndex=2, this=<optimized out>) at js/src/vm/GeneratorObject.h:161
#0  0x0877056d in js::GeneratorObject::setYieldAndAwaitIndex (yieldAndAwaitIndex=2, this=<optimized out>) at js/src/vm/GeneratorObject.h:161
#1  js::GeneratorObject::suspend (cx=0xf791d000, obj=..., frame=..., pc=0xf5e0763c <incomplete sequence \313>, vp=0xffffb930, nvalues=3) at js/src/vm/GeneratorObject.cpp:83
#2  0x08452be3 in js::GeneratorObject::normalSuspend (nvalues=3, vp=<optimized out>, pc=0xf5e0763c <incomplete sequence \313>, frame=..., obj=..., cx=0xf791d000) at js/src/vm/GeneratorObject.h:71
#3  js::jit::NormalSuspend (cx=0xf791d000, obj=..., frame=0xffffba08, pc=0xf5e0763c <incomplete sequence \313>, stackDepth=4) at js/src/jit/VMFunctions.cpp:890
#4  0x32e88370 in ?? ()
[..]
#11 0x32e87c66 in ?? ()
#12 0x0820c41e in EnterBaseline (cx=0x32e913ab, cx@entry=0xf791d000, data=...) at js/src/jit/BaselineJIT.cpp:162
#13 0x0821e8cd in js::jit::EnterBaselineMethod (cx=0xf791d000, state=...) at js/src/jit/BaselineJIT.cpp:200
#14 0x081686e2 in js::RunScript (cx=0xf791d000, state=...) at js/src/vm/Interpreter.cpp:400
#15 0x08168a08 in js::InternalCallOrConstruct (cx=0xf791d000, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:488
#16 0x08168cef in InternalCall (cx=cx@entry=0xf791d000, args=...) at js/src/vm/Interpreter.cpp:515
#17 0x08168e8a in js::Call (cx=0xf791d000, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:534
#18 0x08562d76 in JS_CallFunction (cx=0xf791d000, obj=..., fun=..., args=..., rval=...) at js/src/jsapi.cpp:2850
#19 0x08487c89 in OOMTest (cx=0xf791d000, argc=1, vp=0xf5e52058) at js/src/builtin/TestingFunctions.cpp:1541
#20 0x08172ed6 in js::CallJSNative (cx=0xf791d000, native=0x84878c0 <OOMTest(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:293
[...]
#34 main (argc=5, argv=0xffffcdc4, envp=0xffffcddc) at js/src/shell/js.cpp:8464
eax	0x0	0
ebx	0xf791d000	-141438976
ecx	0xf7da4864	-136689564
edx	0x0	0
esi	0x2	2
edi	0xf62042d8	-165657896
ebp	0xffffb838	4294948920
esp	0xffffb7e0	4294948832
eip	0x877056d <js::GeneratorObject::suspend(JSContext*, JS::Handle<JSObject*>, js::AbstractFramePtr, unsigned char*, JS::Value*, unsigned int)+941>
=> 0x877056d <js::GeneratorObject::suspend(JSContext*, JS::Handle<JSObject*>, js::AbstractFramePtr, unsigned char*, JS::Value*, unsigned int)+941>:	movl   $0x0,0x0
   0x8770577 <js::GeneratorObject::suspend(JSContext*, JS::Handle<JSObject*>, js::AbstractFramePtr, unsigned char*, JS::Value*, unsigned int)+951>:	ud2

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

In GeneratorObject::suspend we should allocate the stack array before changing the generator state or we get confused if we OOM and execute try-finally blocks.

Comment 2

[Jan de Mooij [:jandem]]

Oh I didn't add the test because it's unreliable and it's pretty slow.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d375de81480f
Fix GeneratorObject::suspend to allocate the array before changing generator state. r=jonco

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d375de81480f

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Looks like this code goes back a long ways. Worth considering for backport to ESR52 (since we've already missed the boat for Fx54) or can this ride the trains?

Comment 6

[Jan de Mooij [:jandem]]

This can just ride the trains I think.