Closed Bug 1368735 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: JavaScript Engine, defect, critical)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- wontfix
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 2 open bugs)

Details

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

Attachments

(1 file)

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
Flags: needinfo?(jdemooij)
Attached patch PatchSplinter Review
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.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8873392 - Flags: review?(jcoppeard)
Oh I didn't add the test because it's unreliable and it's pretty slow.
Attachment #8873392 - Flags: review?(jcoppeard) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d375de81480f
Fix GeneratorObject::suspend to allocate the array before changing generator state. r=jonco
https://hg.mozilla.org/mozilla-central/rev/d375de81480f
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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?
Flags: needinfo?(jdemooij)
This can just ride the trains I think.
Flags: needinfo?(jdemooij)
You need to log in before you can comment on or make changes to this bug.