Closed Bug 1445973 Opened 2 years ago Closed 2 years ago

[wasm] Assertion failure: !frames->empty(), at js/src/vm/SavedStacks.cpp:143

Categories

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

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: decoder, Assigned: jimb)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(4 files, 3 obsolete files)

The following testcase crashes on mozilla-central revision fcb11e93adf5+ (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

var lfLogBuffer = `
  function testBoth(source, exportName, expectedValue) {
    WebAssembly.compileStreaming(code).then(m => { module = m });
    drainJobQueue();
  }
  var code = wasmTextToBinary('(module (func (export "run") (result i32) i32.const 42))');
  testBoth(code, 'run', 42);
`;
loadFile("");
loadFile(lfLogBuffer);
function loadFile(lfVarx) {
  try {
    oomTest(function() {
        let m = parseModule(lfVarx);
        m.declarationInstantiation();
        m.evaluation();
    });
  } catch (lfVare) {}
}


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000000000bf702c in js::LiveSavedFrameCache::find (this=this@entry=0x7fffffffb720, cx=0x7ffff5f16000, framePtr=..., pc=0x7ffff49fb6bf <incomplete sequence \347>, frame=...) at js/src/vm/SavedStacks.cpp:143
#0  0x0000000000bf702c in js::LiveSavedFrameCache::find (this=this@entry=0x7fffffffb720, cx=0x7ffff5f16000, framePtr=..., pc=0x7ffff49fb6bf <incomplete sequence \347>, frame=...) at js/src/vm/SavedStacks.cpp:143
#1  0x0000000000bfbb32 in js::SavedStacks::insertFrames(JSContext*, js::FrameIter&, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=this@entry=0x7ffff5f3e0f8, cx=cx@entry=0x7ffff5f16000, iter=..., frame=..., capture=capture@entry=...) at js/src/vm/SavedStacks.cpp:1370
#2  0x0000000000bfcc66 in js::SavedStacks::saveCurrentStack(JSContext*, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=0x7ffff5f3e0f8, cx=cx@entry=0x7ffff5f16000, frame=..., capture=capture@entry=...) at js/src/vm/SavedStacks.cpp:1225
#3  0x00000000009c630a in JS::CaptureCurrentStack(JSContext*, JS::MutableHandle<JSObject*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (cx=0x7ffff5f16000, stackp=..., capture=capture@entry=...) at js/src/jsapi.cpp:7749
#4  0x0000000000606ce2 in PromiseDebugInfo::setResolutionInfo (cx=cx@entry=0x7ffff5f16000, promise=promise@entry=...) at js/src/builtin/Promise.cpp:274
#5  0x00000000005ec868 in js::PromiseObject::onSettled (cx=0x7ffff5f16000, promise=...) at js/src/builtin/Promise.cpp:3474
#6  0x00000000005eca1c in ResolvePromise (cx=0x7ffff5f16000, promise=..., promise@entry=..., valueOrReason=valueOrReason@entry=..., state=state@entry=JS::PromiseState::Rejected) at js/src/builtin/Promise.cpp:804
#7  0x00000000005ece79 in RejectMaybeWrappedPromise (cx=0x7ffff5f16000, promiseObj=..., reason_=...) at js/src/builtin/Promise.cpp:1043
#8  0x00000000005edf67 in RunResolutionFunction (cx=0x7ffff5f16000, resolutionFun=..., result=result@entry=..., mode=mode@entry=RejectMode, promiseObj=...) at js/src/builtin/Promise.cpp:1929
#9  0x00000000005f4b5f in PromiseReactionJob (cx=0x7ffff5f16000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Promise.cpp:1250
#10 0x000000000057a81d in js::CallJSNative (cx=0x7ffff5f16000, native=0x5f4390 <PromiseReactionJob(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:290
[...]
#16 js::RunJobs (cx=cx@entry=0x7ffff5f16000) at js/src/vm/JSContext.cpp:1224
#17 0x0000000000450972 in DrainJobQueue (cx=0x7ffff5f16000, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:963
#18 0x000000000057a81d in js::CallJSNative (cx=0x7ffff5f16000, native=0x450930 <DrainJobQueue(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:290
[...]
#25 0x0000000000571fc1 in js::Execute (cx=cx@entry=0x7ffff5f16000, script=script@entry=..., envChainArg=..., rval=rval@entry=0x7fffffffbc08) at js/src/vm/Interpreter.cpp:733
#26 0x00000000005c9c1e in js::ModuleObject::execute (cx=cx@entry=0x7ffff5f16000, self=..., self@entry=..., rval=rval@entry=...) at js/src/builtin/ModuleObject.cpp:1111
#27 0x0000000000c0fc07 in intrinsic_ExecuteModule (cx=0x7ffff5f16000, argc=<optimized out>, vp=<optimized out>) at js/src/vm/SelfHosting.cpp:2103
#28 0x00003c09ff4721b1 in ?? ()
[...]
#32 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7fffffffb720	140737488336672
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffff9a60	140737488329312
rsp	0x7fffffff99f0	140737488329200
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7fffffff9ba0	140737488329632
r13	0x7ffff5f16000	140737319624704
r14	0x7ffff49fb6bf	140737297495743
r15	0x7fffffff9a10	140737488329232
rip	0xbf702c <js::LiveSavedFrameCache::find(JSContext*, js::LiveSavedFrameCache::FramePtr&, unsigned char const*, JS::MutableHandle<js::SavedFrame*>) const+508>
=> 0xbf702c <js::LiveSavedFrameCache::find(JSContext*, js::LiveSavedFrameCache::FramePtr&, unsigned char const*, JS::MutableHandle<js::SavedFrame*>) const+508>:	movl   $0x0,0x0
   0xbf7037 <js::LiveSavedFrameCache::find(JSContext*, js::LiveSavedFrameCache::FramePtr&, unsigned char const*, JS::MutableHandle<js::SavedFrame*>) const+519>:	ud2
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/b9053d53c1ca
user:        Luke Wagner
date:        Tue Oct 10 14:17:50 2017 -0500
summary:     Bug 1347644 - Baldr: shell WebAssembly.compileStreaming and instantiateStreaming (r=till)

This iteration took 278.148 seconds to run.
Luke, is bug 1347644 a likely regressor?
Blocks: 1347644
Flags: needinfo?(luke)
Bug 1347644 introduced a promise-returning function, but I *think* the problem is with the OOM-handling in JS::CaptureCurrentStack() that happens for *any* promise.  When I run with "--disable-ion --disable-baseline" I hit the failure really quickly, with 'allocation = 243' and 'thread = 1' in the OOMTest frame.  If I break in OOMTest at these numbers and then break in the OOM path of ShouldFailWithOOM(), this is the callstack:

#0  js::oom::ShouldFailWithOOM () at /moz/mi/js/obj/d/dist/include/js/Utility.h:167
#1  0x0000000000465d93 in js::SystemAllocPolicy::checkSimulatedOOM (this=0x7ffff5d3e180) at /moz/mi/js/obj/d/dist/include/js/AllocPolicy.h:46
#2  0x0000000000eedd69 in js::detail::HashTable<js::HashMapEntry<js::SavedStacks::PCKey, js::SavedStacks::LocationValue>, js::HashMap<js::SavedStacks::PCKey, js::SavedStacks::LocationValue, js::SavedStacks::PCLocationHasher, js::SystemAllocPolicy>::MapHashPolicy, js::SystemAllocPolicy>::add<js::SavedStacks::PCKey&, js::SavedStacks::LocationValue&> (this=0x7ffff5d3e180, p=...)
    at /moz/mi/js/obj/d/dist/include/js/HashTable.h:1817
#3  0x0000000000ee24d0 in js::HashMap<js::SavedStacks::PCKey, js::SavedStacks::LocationValue, js::SavedStacks::PCLocationHasher, js::SystemAllocPolicy>::add<js::SavedStacks::PCKey&, js::SavedStacks::LocationValue&> (this=0x7ffff5d3e180, p=..., k=..., v=...) at /moz/mi/js/obj/d/dist/include/js/HashTable.h:159
#4  0x0000000000ec6c80 in js::SavedStacks::getLocation (this=0x7ffff5d3e0f8, cx=0x7ffff5d16000, iter=..., locationp=...) at /moz/mi/js/src/vm/SavedStacks.cpp:1644
#5  0x0000000000ec57b5 in js::SavedStacks::insertFrames(JSContext*, js::FrameIter&, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (
    this=0x7ffff5d3e0f8, cx=0x7ffff5d16000, iter=..., frame=..., capture=<unknown type in /moz/mi/js/obj/d/js/src/shell/js, CU 0x4a10536, DIE 0x4bcb887>) at /moz/mi/js/src/vm/SavedStacks.cpp:1389
#6  0x0000000000ec4eab in js::SavedStacks::saveCurrentStack(JSContext*, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=0x7ffff5d3e0f8, 
    cx=0x7ffff5d16000, frame=..., capture=<unknown type in /moz/mi/js/obj/d/js/src/shell/js, CU 0x4a10536, DIE 0x4bcb1be>) at /moz/mi/js/src/vm/SavedStacks.cpp:1225
#7  0x0000000000bffc60 in JS::CaptureCurrentStack(JSContext*, JS::MutableHandle<JSObject*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (cx=0x7ffff5d16000, stackp=..., 
    capture=<unknown type in /moz/mi/js/obj/d/js/src/shell/js, CU 0x34985cf, DIE 0x367320a>) at /moz/mi/js/src/jsapi.cpp:7749
#8  0x00000000006a5bf5 in PromiseDebugInfo::setResolutionInfo (cx=0x7ffff5d16000, promise=...) at /moz/mi/js/src/builtin/Promise.cpp:274
#9  0x0000000000679582 in js::PromiseObject::onSettled (cx=0x7ffff5d16000, promise=...) at /moz/mi/js/src/builtin/Promise.cpp:3474
#10 0x000000000066bf4b in ResolvePromise (cx=0x7ffff5d16000, promise=..., valueOrReason=..., state=JS::PromiseState::Fulfilled) at /moz/mi/js/src/builtin/Promise.cpp:804
#11 0x000000000066c250 in FulfillMaybeWrappedPromise (cx=0x7ffff5d16000, promiseObj=..., value_=...) at /moz/mi/js/src/builtin/Promise.cpp:837
#12 0x000000000066b071 in ResolvePromiseInternal (cx=0x7ffff5d16000, promise=..., resolutionVal=...) at /moz/mi/js/src/builtin/Promise.cpp:597
#13 0x00000000006790a0 in js::PromiseObject::resolve (cx=0x7ffff5d16000, promise=..., resolutionValue=...) at /moz/mi/js/src/builtin/Promise.cpp:3433
#14 0x000000000115fe50 in Resolve (cx=0x7ffff5d16000, module=..., promise=..., instantiate=false, importObj=...) at /moz/mi/js/src/wasm/WasmJS.cpp:2283
#15 0x0000000001183f39 in CompileStreamTask::resolve (this=0x7ffff474bc00, cx=0x7ffff5d16000, promise=...) at /moz/mi/js/src/wasm/WasmJS.cpp:2712
#16 0x0000000000679c7d in js::OffThreadPromiseTask::run (this=0x7ffff474bc00, cx=0x7ffff5d16000, maybeShuttingDown=JS::Dispatchable::NotShuttingDown) at /moz/mi/js/src/builtin/Promise.cpp:3538
#17 0x000000000067a615 in js::OffThreadPromiseRuntimeState::internalDrain (this=0x7ffff5d1a0f0, cx=0x7ffff5d16000) at /moz/mi/js/src/builtin/Promise.cpp:3667
#18 0x0000000000dd4f2f in js::RunJobs (cx=0x7ffff5d16000) at /moz/mi/js/src/vm/JSContext.cpp:1191
#19 0x000000000043df09 in DrainJobQueue (cx=0x7ffff5d16000, argc=0, vp=0x7ffff475b440) at /moz/mi/js/src/shell/js.cpp:963
#20 0x00000000005e735e in js::CallJSNative (cx=0x7ffff5d16000, native=0x43de86 <DrainJobQueue(JSContext*, unsigned int, JS::Value*)>, args=...) at /moz/mi/js/src/vm/JSContext-inl.h:290
#21 0x00000000005c33f0 in js::InternalCallOrConstruct (cx=0x7ffff5d16000, args=..., construct=js::NO_CONSTRUCT) at /moz/mi/js/src/vm/Interpreter.cpp:467
#22 0x00000000005c37b7 in InternalCall (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:516
#23 0x00000000005c37f5 in js::CallFromStack (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:522
#24 0x00000000005d1aa7 in Interpret (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:3085
#25 0x00000000005c2f79 in js::RunScript (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:417
#26 0x00000000005c45fe in js::ExecuteKernel (cx=0x7ffff5d16000, script=..., envChainArg=..., newTargetValue=..., evalInFrame=..., result=0x7ffff475b300) at /moz/mi/js/src/vm/Interpreter.cpp:700
#27 0x00000000005c48e7 in js::Execute (cx=0x7ffff5d16000, script=..., envChainArg=..., rval=0x7ffff475b300) at /moz/mi/js/src/vm/Interpreter.cpp:733
#28 0x000000000065ea57 in js::ModuleObject::execute (cx=0x7ffff5d16000, self=..., rval=...) at /moz/mi/js/src/builtin/ModuleObject.cpp:1111
#29 0x0000000000f0eabc in intrinsic_ExecuteModule (cx=0x7ffff5d16000, argc=1, vp=0x7ffff475b300) at /moz/mi/js/src/vm/SelfHosting.cpp:2103
#30 0x00000000005e735e in js::CallJSNative (cx=0x7ffff5d16000, native=0xf0e9b2 <intrinsic_ExecuteModule(JSContext*, unsigned int, JS::Value*)>, args=...) at /moz/mi/js/src/vm/JSContext-inl.h:290
#31 0x00000000005c33f0 in js::InternalCallOrConstruct (cx=0x7ffff5d16000, args=..., construct=js::NO_CONSTRUCT) at /moz/mi/js/src/vm/Interpreter.cpp:467
#32 0x00000000005c37b7 in InternalCall (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:516
#33 0x00000000005c37f5 in js::CallFromStack (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:522
#34 0x00000000005d1aa7 in Interpret (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:3085
#35 0x00000000005c2f79 in js::RunScript (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:417
#36 0x00000000005c3529 in js::InternalCallOrConstruct (cx=0x7ffff5d16000, args=..., construct=js::NO_CONSTRUCT) at /moz/mi/js/src/vm/Interpreter.cpp:489
#37 0x00000000005c37b7 in InternalCall (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:516
#38 0x00000000005c37f5 in js::CallFromStack (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:522
#39 0x00000000005d1aa7 in Interpret (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:3085
#40 0x00000000005c2f79 in js::RunScript (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:417
#41 0x00000000005c3529 in js::InternalCallOrConstruct (cx=0x7ffff5d16000, args=..., construct=js::NO_CONSTRUCT) at /moz/mi/js/src/vm/Interpreter.cpp:489
#42 0x00000000005c37b7 in InternalCall (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:516
#43 0x00000000005c37f5 in js::CallFromStack (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:522
#44 0x00000000005d1aa7 in Interpret (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:3085
#45 0x00000000005c2f79 in js::RunScript (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:417
#46 0x00000000005c3529 in js::InternalCallOrConstruct (cx=0x7ffff5d16000, args=..., construct=js::NO_CONSTRUCT) at /moz/mi/js/src/vm/Interpreter.cpp:489
#47 0x00000000005c37b7 in InternalCall (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:516
#48 0x00000000005c3871 in js::Call (cx=0x7ffff5d16000, fval=..., thisv=..., args=..., rval=...) at /moz/mi/js/src/vm/Interpreter.cpp:535
#49 0x0000000000bebac6 in JS_CallFunction (cx=0x7ffff5d16000, obj=..., fun=..., args=..., rval=...) at /moz/mi/js/src/jsapi.cpp:2970
#50 0x0000000000a4406b in OOMTest (cx=0x7ffff5d16000, argc=1, vp=0x7ffff475b118) at /moz/mi/js/src/builtin/TestingFunctions.cpp:1692
#51 0x00000000005e735e in js::CallJSNative (cx=0x7ffff5d16000, native=0xa43a2b <OOMTest(JSContext*, unsigned int, JS::Value*)>, args=...) at /moz/mi/js/src/vm/JSContext-inl.h:290
#52 0x00000000005c33f0 in js::InternalCallOrConstruct (cx=0x7ffff5d16000, args=..., construct=js::NO_CONSTRUCT) at /moz/mi/js/src/vm/Interpreter.cpp:467
#53 0x00000000005c37b7 in InternalCall (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:516
#54 0x00000000005c37f5 in js::CallFromStack (cx=0x7ffff5d16000, args=...) at /moz/mi/js/src/vm/Interpreter.cpp:522
#55 0x00000000005d1aa7 in Interpret (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:3085
#56 0x00000000005c2f79 in js::RunScript (cx=0x7ffff5d16000, state=...) at /moz/mi/js/src/vm/Interpreter.cpp:417
#57 0x00000000005c45fe in js::ExecuteKernel (cx=0x7ffff5d16000, script=..., envChainArg=..., newTargetValue=..., evalInFrame=..., result=0x0) at /moz/mi/js/src/vm/Interpreter.cpp:700
#58 0x00000000005c48e7 in js::Execute (cx=0x7ffff5d16000, script=..., envChainArg=..., rval=0x0) at /moz/mi/js/src/vm/Interpreter.cpp:733
#59 0x0000000000bf3803 in ExecuteScript (cx=0x7ffff5d16000, scope=..., script=..., rval=0x0) at /moz/mi/js/src/jsapi.cpp:4712
---Type <return> to continue, or q <return> to quit---
#60 0x0000000000bf3bed in JS_ExecuteScript (cx=0x7ffff5d16000, scriptArg=...) at /moz/mi/js/src/jsapi.cpp:4745
#61 0x000000000043d386 in RunFile (cx=0x7ffff5d16000, filename=0x7fffffffe7cd "test.js", file=0x7ffff6917c00, compileOnly=false) at /moz/mi/js/src/shell/js.cpp:833
#62 0x000000000043f57c in Process (cx=0x7ffff5d16000, filename=0x7fffffffe7cd "test.js", forceTTY=false, kind=FileScript) at /moz/mi/js/src/shell/js.cpp:1277
#63 0x000000000045bc6b in ProcessArgs (cx=0x7ffff5d16000, op=0x7fffffffe1c0) at /moz/mi/js/src/shell/js.cpp:8547
#64 0x000000000045d61a in Shell (cx=0x7ffff5d16000, op=0x7fffffffe1c0, envp=0x7fffffffe420) at /moz/mi/js/src/shell/js.cpp:8939
#65 0x000000000045f28c in main (argc=4, argv=0x7fffffffe3f8, envp=0x7fffffffe420) at /moz/mi/js/src/shell/js.cpp:9410

Stepping through, the failure is propagated but eventually cleared in frame #8, PromiseDebugInfo::setResolutionInfo().  I don't understand the SavedStacks invariant to understand what's fundamentally being broken here, but I'm guessing there's some assumption that a whole stack is captured at once and the fact that we're failing half-way through is causing a seeming impossibility.  Nick, do you know?
Flags: needinfo?(luke) → needinfo?(nfitzgerald)
No longer blocks: 1347644
See also bug 1449589. Adding an additional needinfo because Jim has been working on the SavedStacks code recently.
Flags: needinfo?(jimb)
Duplicate of this bug: 1449589
I think Jim is the best person to handle this right now.
Flags: needinfo?(nfitzgerald)
I can reproduce this, and it's almost certainly related to my recent changes. Taking.
Assignee: nobody → jimb
Flags: needinfo?(jimb)
Attachment #8963851 - Flags: review?(jorendorff)
Comment on attachment 8963851 [details] [diff] [review]
Add FramePtr::clearHasCachedSavedFrame method.

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

I think this is the wrong patch...
Attachment #8963851 - Attachment is obsolete: true
Attachment #8963851 - Flags: review?(jorendorff)
Attachment #8963997 - Flags: review?(jorendorff)
Comment on attachment 8963852 [details] [diff] [review]
Clear the hasCachedSavedFrame bit on a frame when we miss for a pc mismatch.

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

::: js/src/vm/SavedStacks.cpp
@@ +148,5 @@
>      // older frame has executed any code; it would have been necessary to pop
>      // this frame for that to happen, but this frame's bit is set.
>      if (pc != frames->back().pc) {
>          frames->popBack();
> +        // Since we've removed this entry from the cache, clear the bit on its

Style nit: blank line before this comment, please.
Attachment #8963852 - Flags: review?(jorendorff) → review+
Attachment #8963997 - Flags: review?(jorendorff) → review+
Priority: -- → P1
The test case here takes 94s to run, longer than anything else in the saved-stacks directory. I feel guilty landing such an expensive test, and I think I can construct an equivalent that is much faster.
Hmm. oomTest isn't working the way I expected. When the allocation count is 38, we do 847 allocations. The selected thread is correct. We are not in an OOM unsafe region.
Okay, this is a known wrinkle, not a bug in oomTest:

The oomTest function takes some function f, and calls f repeatedly, making the
first allocation fail, then the second, and so on, checking that each call
correctly fails and reports OOM. When the count increases to the point that the
call to f doesn't actually perform that many allocations, and thus no failure is
ever injected, the oomTest is complete.

Unfortunately, the JITs and other influences can change the number of
allocations that occur during a call to f drastically from one run to the next.
In my particular test case, the calls perform up to 990 allocations (injecting a
failure at the 989'th) without reaching the call to SavedStacks::insertFrames,
and then the next call reaches that point after only 618. The rest of the call
completes in the remaining ~400 allocations, and the call whose failure I want
to exercise never fails.

The workaround is to find ways to avoid variations in allocation behavior:
disabling jits, make sure lazy functions are warm, etc.
Attachment #8965799 - Flags: review?(jorendorff)
carrying over r=jorendorff
Attachment #8963997 - Attachment is obsolete: true
Attachment #8965804 - Flags: review+
Fix is unchanged; test is new.
Attachment #8963852 - Attachment is obsolete: true
Attachment #8965805 - Flags: review?(jorendorff)
I'm not including the fuzz test from comment 0 as a regression test, because it alone takes several times longer to run than the entire rest of js/src/jit-test/tests/saved-stacks. Part 4 includes a regression test that runs quickly.
Attachment #8965799 - Flags: review?(jorendorff) → review+
Attachment #8965802 - Flags: review?(jorendorff) → review+
Attachment #8965805 - Flags: review?(jorendorff) → review+
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e427340ec249
Part 1: Comment typo. r=jorendorff
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a11067ab1e6
Part 2: Add 'clearSavedFrames' testing function to JS shell. r=jorendorff
https://hg.mozilla.org/integration/mozilla-inbound/rev/014fdec944a9
Part 3: Add FramePtr::clearHasCachedSavedFrame method. r=jorendorff
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ed6d9d910b0
Part 4: Clear the hasCachedSavedFrame bit on a frame when we miss for a pc mismatch. r=jorendorff
Backed out 4 changesets (bug 1445973) for failing on tests/saved-stacks/bug-1445973-quick.js on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/40bf894c8c67bcbf3f1b622c03be9679530d1595

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=2ed6d9d910b0f3dd24e163f17e3a70d327286582

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=172514536&repo=mozilla-inbound&lineNumber=81042

Log snippet: 
17:37:39     INFO -  TEST-PASS | tests/jit-test/jit-test/tests/saved-stacks/bug-1289073.js | Success (code 0, args "--ion-eager --ion-offthread-compile=off --non-writable-jitcode --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [0.1 s]
17:37:39     INFO -  {"action": "test_start", "pid": 738, "source": "jittests", "test": "saved-stacks/bug-1289073.js", "thread": "main", "time": 1523147859.1607678}
17:37:39     INFO -  {"action": "test_end", "extra": {"jitflags": "--ion-eager --ion-offthread-compile=off --non-writable-jitcode --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads"}, "message": "Success", "pid": 738, "source": "jittests", "status": "PASS", "test": "saved-stacks/bug-1289073.js", "thread": "main", "time": 1523147859.230322}
17:37:39     INFO -  /Users/cltbld/tasks/task_1523143644/build/tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js:53:1 ReferenceError: oomTest is not defined
17:37:39     INFO -  Stack:
17:37:39     INFO -    @/Users/cltbld/tasks/task_1523143644/build/tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js:53:1
17:37:39     INFO -  Exit code: 3
17:37:39     INFO -  FAIL - saved-stacks/bug-1445973-quick.js
17:37:39  WARNING -  TEST-UNEXPECTED-FAIL | tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js | /Users/cltbld/tasks/task_1523143644/build/tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js:53:1 ReferenceError: oomTest is not defined (code 3, args "--no-baseline --ion-eager --ion-offthread-compile=off") [0.1 s]
17:37:39     INFO -  {"action": "test_start", "pid": 738, "source": "jittests", "test": "saved-stacks/bug-1445973-quick.js", "thread": "main", "time": 1523147859.229737}
17:37:39     INFO -  {"action": "test_end", "extra": {"jitflags": "--no-baseline --ion-eager --ion-offthread-compile=off"}, "message": "/Users/cltbld/tasks/task_1523143644/build/tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js:53:1 ReferenceError: oomTest is not defined", "pid": 738, "source": "jittests", "status": "FAIL", "test": "saved-stacks/bug-1445973-quick.js", "thread": "main", "time": 1523147859.282814}
17:37:39     INFO -  INFO exit-status     : 3
17:37:39     INFO -  INFO timed-out       : False
17:37:39     INFO -  INFO stderr         2> /Users/cltbld/tasks/task_1523143644/build/tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js:53:1 ReferenceError: oomTest is not defined
17:37:39     INFO -  INFO stderr         2> Stack:
17:37:39     INFO -  INFO stderr         2> @/Users/cltbld/tasks/task_1523143644/build/tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js:53:1
17:37:39     INFO -  /Users/cltbld/tasks/task_1523143644/build/tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js:53:1 ReferenceError: oomTest is not defined
17:37:39     INFO -  Stack:
17:37:39     INFO -    @/Users/cltbld/tasks/task_1523143644/build/tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js:53:1
17:37:39     INFO -  Exit code: 3
17:37:39     INFO -  FAIL - saved-stacks/bug-1445973-quick.js
17:37:39  WARNING -  TEST-UNEXPECTED-FAIL | tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js | /Users/cltbld/tasks/task_1523143644/build/tests/jit-test/jit-test/tests/saved-stacks/bug-1445973-quick.js:53:1 ReferenceError: oomTest is not defined (code 3, args "--no-baseline --baseline-eager") [0.0 s]
Flags: needinfo?(jimb)
I forgot to check whether oomTest was present in the jit-test.
Flags: needinfo?(jimb)
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/804f13c4f178
Part 1: Comment typo. r=jorendorff
https://hg.mozilla.org/integration/mozilla-inbound/rev/5dd12cdf566e
Part 2: Add 'clearSavedFrames' testing function to JS shell. r=jorendorff
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e181d2357a1
Part 3: Add FramePtr::clearHasCachedSavedFrame method. r=jorendorff
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bf4c9308221
Part 4: Clear the hasCachedSavedFrame bit on a frame when we miss for a pc mismatch. r=jorendorff
Is there a user impact which justifies uplift here or can it just ride the trains?
Flags: needinfo?(jimb)
Flags: in-testsuite+
(In reply to Ryan VanderMeulen [:RyanVM] from comment #26)
> Is there a user impact which justifies uplift here or can it just ride the
> trains?

This was hit somewhat often on mozilla-beta too. It would be nice! (But I'll let :jimb decide)
A variant bisects to the fix here in comment 25 on mozilla-beta, fwiw:

Assertion failure: false (!frames->empty()), at vm/SavedStacks.cpp
I think this OOM is going to be very rare in practice. I think it can ride the trains.
Flags: needinfo?(jimb)
You need to log in before you can comment on or make changes to this bug.