Created attachment 602412 [details] Test case for shell (see README file inside). The attached test asserts/crashes on mozilla-central revision 343ec916dfd5 (see README for running instructions). Crash backtrace (from opt build): ==38069== Process terminating with default action of signal 11 (SIGSEGV) ==38069== Access not within mapped region at address 0xBAD ==38069== at 0x8184DF3: CrashIfInvalidSlot(js::StackFrame*, JS::Value*) (Stack.cpp:1065) ==38069== by 0x8186D2E: js::StackIter::settleOnNewState() (Stack.cpp:1153) ==38069== by 0x80AFFBA: js::types::TypeObject::clearNewScript(JSContext*) (Stack.h:1860) ==38069== by 0x80B3247: js::types::TypeObject::markUnknown(JSContext*) (jsinferinlines.h:1099) ==38069== by 0x80BEFA8: js::types::TypeObject::getProperty(JSContext*, int, bool) (jsinferinlines.h:1209) ==38069== by 0x80B457C: js::types::TypeObject::getFromPrototypes(JSContext*, int, js::types::TypeSet*, bool) (jsinfer.cpp:2774) ==38069== by 0x80B580B: PropertyAccess(JSContext*, JSScript*, unsigned char*, js::types::TypeObject*, bool, js::types::TypeSet*, int) (jsinfer.cpp:1040) ==38069== by 0x80BCF2F: TypeConstraintProp::newType(JSContext*, js::types::TypeSet*, js::types::Type) (jsinfer.cpp:1098) ==38069== by 0x80763A9: js::types::TypeCompartment::resolvePending(JSContext*) (jsinferinlines.h:795) ==38069== by 0x80B2B9F: js::types::TypeMonitorResult(JSContext*, JSScript*, unsigned char*, JS::Value const&) (jsinfer.cpp:5160) ==38069== by 0x80C9DCD: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinferinlines.h:574) ==38069== by 0x81C875C: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1079)
The fix is pretty simple: diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp @@ -2694,18 +2694,19 @@ BEGIN_CASE(JSOP_CALL) - regs.sp = args.spAfterCall(); - TypeScript::Monitor(cx, script, regs.pc, regs.sp[-1]); + Value *newsp = args.spAfterCall(); + TypeScript::Monitor(cx, script, regs.pc, newsp[-1]); + regs.sp = newsp; Brian, do you know how to generate a short test-case for this? To error condition is to get this TypeScript::Monitor call to inspect the stack with FrameIter/StackIter (in this case a call to clearNewScript).
*** Bug 733141 has been marked as a duplicate of this bug. ***
(In reply to Luke Wagner [:luke] from comment #1) > The fix is pretty simple: Tested this fix and it worked for me (solved also other OOM situations with the same assert). Can we get this landed or is it blocked by a better testcase? > Brian, do you know how to generate a short test-case for this? To error > condition is to get this TypeScript::Monitor call to inspect the stack with > FrameIter/StackIter (in this case a call to clearNewScript). Would it be sufficient if this was covered by the OOM testing mechanism or do we need to cover more situations?
Created attachment 604562 [details] [diff] [review] fix Righto
Comment on attachment 604562 [details] [diff] [review] fix Oh, this would be kind of tricky to write a small testcase for. The new type needs to trigger type changes that invalidate a type's newScript info, which can only happen in narrow circumstances, e.g. calling foo.call in the constructor and either foo or foo.call is changed by the new type.
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd2a4596f15c
https://hg.mozilla.org/mozilla-central/rev/dd2a4596f15c
or
