Closed Bug 609244 Opened 9 years ago Closed 9 years ago

crash [@ PopActiveVMFrame ]

Categories

(Core :: JavaScript Engine, defect, critical)

x86
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta8+

People

(Reporter: scoobidiver, Assigned: dmandelin)

Details

(Keywords: crash, regression, topcrash, Whiteboard: fixed-in-tracemonkey)

Crash Data

Attachments

(1 file)

It is a new crash signature.
As it is discontinious, the regression range can not be determined.
It is #95 top crasher in 4.0b8pre for the last week.

Signature	PopActiveVMFrame
UUID	4045b063-d25d-4364-83e1-420972101102
Time 	2010-11-02 22:18:50.623763
Uptime	20663
Last Crash	189886 seconds (2.2 days) before submission
Install Age	20663 seconds (5.7 hours) since version was first installed.
Product	Firefox
Version	4.0b8pre
Build ID	20101102042148
Branch	2.0
OS	Windows NT
OS Version	6.1.7600
CPU	x86
CPU Info	GenuineIntel family 6 model 30 stepping 5
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0xc
App Notes 	AdapterVendorID: 10de, AdapterDeviceID: 05e2

Frame 	Module 	Signature [Expand] 	Source
0 	mozjs.dll 	PopActiveVMFrame 	js/src/methodjit/MethodJIT.cpp:126
1 	mozjs.dll 	JaegerThrowpoline 	js/src/methodjit/MethodJIT.cpp:650
2 	mozjs.dll 	js::mjit::EnterMethodJIT 	js/src/methodjit/MethodJIT.cpp:739
3 	mozjs.dll 	CheckStackAndEnterMethodJIT 	js/src/methodjit/MethodJIT.cpp:764
4 	mozjs.dll 	js::mjit::JaegerShot 	js/src/methodjit/MethodJIT.cpp:781
5 	mozjs.dll 	js::RunScript 	js/src/jsinterp.cpp:662
6 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:768
7 	mozjs.dll 	js::ExternalInvoke 	js/src/jsinterp.cpp:881
8 	mozjs.dll 	JS_CallFunctionValue 	js/src/jsapi.cpp:4895
9 	xul.dll 	nsJSContext::CallEventHandler 	dom/base/nsJSEnvironment.cpp:2171
10 	xul.dll 	nsGlobalWindow::RunTimeout 	dom/base/nsGlobalWindow.cpp:8916
11 	xul.dll 	nsGlobalWindow::TimerCallback 	dom/base/nsGlobalWindow.cpp:9261
12 	xul.dll 	nsTimerImpl::Fire 	xpcom/threads/nsTimerImpl.cpp:425
13 	xul.dll 	nsTimerEvent::Run 	xpcom/threads/nsTimerImpl.cpp:517
14 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:609
15 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:134
16 	xul.dll 	xul.dll@0xb034db 	
17 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:202
18 	xul.dll 	_SEH_epilog4 	
19 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:176
20 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:181
21 	xul.dll 	xul.dll@0xb034db 	
22 	xul.dll 	nsAppShell::Run 	widget/src/windows/nsAppShell.cpp:243
23 		@0x74e8ffff 	
24 	mozcrt19.dll 	mozcrt19.dll@0xaffff 	

More reports at:
http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=PopActiveVMFrame
This appears in the top 10 list in early beta7 stats and on the trunk. Nominating or 2.0.
blocking2.0: --- → ?
I added the topcrash keyword since it's now in the top 10.
Keywords: topcrash
blocking2.0: ? → beta8+
I think this started when I moved activeFrame to the compartment (from thread data). It's certainly the right time frame, and the fact that I modified this particular function is a big hint. The surprise is that that change was supposed to *reduce* this sort of crash.

I talked about it with dvander and we decided the likeliest problem is that some call within a JM activation switches compartments, and then as we come back out (usually with an exception, in this crash), the compartment doesn't get reset. We're now on a new compartment, where nothing was pushed, so we crash.

JSScript::compartment isn't supposed to change, so changing (Push|Pop)ActiveVMFrame to get its compartment through f.fp->script() instead of f.cx should stop the crash if the above is the issue.
Assignee: general → dmandelin
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
Attachment #489315 - Flags: review?(dvander)
Comment on attachment 489315 [details] [diff] [review]
Patch

>diff --git a/js/src/methodjit/InvokeHelpers.cpp b/js/src/methodjit/InvokeHelpers.cpp
>--- a/js/src/methodjit/InvokeHelpers.cpp
>+++ b/js/src/methodjit/InvokeHelpers.cpp
>@@ -1,9 +1,9 @@
>-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
>+/* -*- mOde: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
>  * vim: set ts=4 sw=4 et tw=99:
>  *
>  * ***** BEGIN LICENSE BLOCK *****
>  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
>  *
>  * The contents of this file are subject to the Mozilla Public License Version
>  * 1.1 (the "License"); you may not use this file except in compliance with
>  * the License. You may obtain a copy of the License at
>@@ -176,17 +176,17 @@ top:
>  * throwing an exception.
>  */
> static void
> InlineReturn(VMFrame &f)
> {
>     JSContext *cx = f.cx;
>     JSStackFrame *fp = f.regs.fp;
> 
>-    JS_ASSERT(f.fp() != f.entryFp);
>+    JS_ASSERT(f.fp() != f.entryfp);
> 
>     JS_ASSERT(!js_IsActiveWithOrBlock(cx, &fp->scopeChain(), 0));
> 
>     Value *newsp = fp->actualArgs() - 1;
>     newsp[-1] = fp->returnValue();
>     cx->stack().popInlineFrame(cx, fp->prev(), newsp);
> }
> 
>@@ -226,17 +226,17 @@ RemovePartialFrame(JSContext *cx, JSStac
>  * overflow f.stackLimit.
>  */
> void JS_FASTCALL
> stubs::HitStackQuota(VMFrame &f)
> {
>     /* Include space to push another frame. */
>     uintN nvals = f.fp()->script()->nslots + VALUES_PER_STACK_FRAME;
>     JS_ASSERT(f.regs.sp == f.fp()->base());
>-    if (f.cx->stack().bumpCommitAndLimit(f.entryFp, f.regs.sp, nvals, &f.stackLimit))
>+    if (f.cx->stack().bumpCommitAndLimit(f.entryfp, f.regs.sp, nvals, &f.stackLimit))
>         return;
> 
>     /* Remove the current partially-constructed frame before throwing. */
>     RemovePartialFrame(f.cx, f.fp());
>     js_ReportOverRecursed(f.cx);
>     THROW();
> }
> 
>@@ -264,17 +264,17 @@ stubs::FixupArity(VMFrame &f, uint32 nac
> 
>     /* Pop the inline frame. */
>     f.fp() = oldfp->prev();
>     f.regs.sp = (Value*) oldfp;
> 
>     /* Reserve enough space for a callee frame. */
>     JSStackFrame *newfp = cx->stack().getInlineFrameWithinLimit(cx, (Value*) oldfp, nactual,
>                                                                 fun, fun->script(), &flags,
>-                                                                f.entryFp, &f.stackLimit);
>+                                                                f.entryfp, &f.stackLimit);
>     if (!newfp)
>         THROWV(NULL);
> 
>     /* Reset the part of the stack frame set by the caller. */
>     newfp->initCallFrameCallerHalf(cx, nactual, flags);
> 
>     /* Reset the part of the stack frame set by the prologue up to now. */
>     newfp->initCallFrameEarlyPrologue(fun, ncode);
>@@ -359,17 +359,17 @@ UncachedInlineCall(VMFrame &f, uint32 fl
>     JSObject &callee = vp->toObject();
>     JSFunction *newfun = callee.getFunctionPrivate();
>     JSScript *newscript = newfun->script();
> 
>     /* Get pointer to new frame/slots, prepare arguments. */
>     StackSpace &stack = cx->stack();
>     JSStackFrame *newfp = stack.getInlineFrameWithinLimit(cx, f.regs.sp, argc,
>                                                           newfun, newscript, &flags,
>-                                                          f.entryFp, &f.stackLimit);
>+                                                          f.entryfp, &f.stackLimit);
>     if (JS_UNLIKELY(!newfp))
>         return false;
>     JS_ASSERT_IF(!vp[1].isPrimitive() && !(flags & JSFRAME_CONSTRUCTING),
>                  IsSaneThisObject(vp[1].toObject()));
> 
>     /* Initialize frame, locals. */
>     newfp->initCallFrame(cx, callee, newfun, argc, flags);
>     SetValueRangeToUndefined(newfp->slots(), newscript->nfixed);
>@@ -555,17 +555,17 @@ js_InternalThrow(VMFrame &f)
>         pc = FindExceptionHandler(cx);
>         if (pc)
>             break;
> 
>         // If on the 'topmost' frame (where topmost means the first frame
>         // called into through js_Interpret). In this case, we still unwind,
>         // but we shouldn't return from a JS function, because we're not in a
>         // JS function.
>-        bool lastFrame = (f.entryFp == f.fp());
>+        bool lastFrame = (f.entryfp == f.fp());
>         js_UnwindScope(cx, 0, cx->throwing);
> 
>         // For consistency with Interpret(), always run the script epilogue.
>         // This simplifies interactions with RunTracer(), since it can assume
>         // no matter how a function exited (error or not), that the epilogue
>         // does not need to be run.
>         ScriptEpilogue(f.cx, f.fp(), false);
> 
>diff --git a/js/src/methodjit/MethodJIT.cpp b/js/src/methodjit/MethodJIT.cpp
>--- a/js/src/methodjit/MethodJIT.cpp
>+++ b/js/src/methodjit/MethodJIT.cpp
>@@ -111,24 +111,24 @@ static const size_t STUB_CALLS_FOR_OP_CO
> static uint32 StubCallsForOp[STUB_CALLS_FOR_OP_COUNT];
> #endif
> 
> extern "C" void JaegerTrampolineReturn();
> 
> extern "C" void JS_FASTCALL
> PushActiveVMFrame(VMFrame &f)
> {
>-    f.cx->jaegerCompartment()->pushActiveFrame(&f);
>+    f.entryfp->script()->compartment->jaegerCompartment->pushActiveFrame(&f);
>     f.regs.fp->setNativeReturnAddress(JS_FUNC_TO_DATA_PTR(void*, JaegerTrampolineReturn));
> }
> 
> extern "C" void JS_FASTCALL
> PopActiveVMFrame(VMFrame &f)
> {
>-    f.cx->jaegerCompartment()->popActiveFrame();
>+    f.entryfp->script()->compartment->jaegerCompartment->popActiveFrame();
> }
> 
> extern "C" void JS_FASTCALL
> SetVMFrameRegs(VMFrame &f)
> {
>     f.cx->setCurrentRegs(&f.regs);
> }
> 
>@@ -201,17 +201,17 @@ SYMBOL_STRING(JaegerTrampoline) ":"     
>     "movq $0x00007FFFFFFFFFFF, %r14"     "\n"
> 
>     /* Build the JIT frame.
>      * rdi = cx
>      * rsi = fp
>      * rcx = inlineCallCount
>      * fp must go into rbx
>      */
>-    "pushq %rsi"                         "\n" /* entryFp */
>+    "pushq %rsi"                         "\n" /* entryfp */
>     "pushq %rcx"                         "\n" /* inlineCallCount */
>     "pushq %rdi"                         "\n" /* cx */
>     "pushq %rsi"                         "\n" /* fp */
>     "movq  %rsi, %rbx"                   "\n"
> 
>     /* Space for the rest of the VMFrame. */
>     "subq  $0x28, %rsp"                  "\n"
> 
>@@ -312,17 +312,17 @@ SYMBOL_STRING(JaegerTrampoline) ":"     
>     /* Save non-volatile registers. */
>     "pushl %esi"                         "\n"
>     "pushl %edi"                         "\n"
>     "pushl %ebx"                         "\n"
> 
>     /* Build the JIT frame. Push fields in order, 
>      * then align the stack to form esp == VMFrame. */
>     "movl  12(%ebp), %ebx"               "\n"   /* load fp */
>-    "pushl %ebx"                         "\n"   /* entryFp */
>+    "pushl %ebx"                         "\n"   /* entryfp */
>     "pushl 20(%ebp)"                     "\n"   /* stackLimit */
>     "pushl 8(%ebp)"                      "\n"   /* cx */
>     "pushl %ebx"                         "\n"   /* fp */
>     "subl $0x1C, %esp"                   "\n"
> 
>     /* Jump into the JIT'd code. */
>     "movl  %esp, %ecx"                   "\n"
>     "call " SYMBOL_STRING_VMFRAME(SetVMFrameRegs) "\n"
>@@ -391,17 +391,17 @@ SYMBOL_STRING(InjectJaegerReturn) ":"   
>     "movl 0x1C(%esp), %ebx"                 "\n" /* f.fp */
>     "jmp *%eax"                             "\n"
> );
> 
> # elif defined(JS_CPU_ARM)
> 
> JS_STATIC_ASSERT(sizeof(VMFrame) == 80);
> JS_STATIC_ASSERT(offsetof(VMFrame, savedLR) ==          (4*19));
>-JS_STATIC_ASSERT(offsetof(VMFrame, entryFp) ==          (4*10));
>+JS_STATIC_ASSERT(offsetof(VMFrame, entryfp) ==          (4*10));
> JS_STATIC_ASSERT(offsetof(VMFrame, stackLimit) ==       (4*9));
> JS_STATIC_ASSERT(offsetof(VMFrame, cx) ==               (4*8));
> JS_STATIC_ASSERT(offsetof(VMFrame, regs.fp) ==          (4*7));
> JS_STATIC_ASSERT(offsetof(VMFrame, unused) ==           (4*4));
> JS_STATIC_ASSERT(offsetof(VMFrame, previous) ==         (4*3));
> 
> JS_STATIC_ASSERT(JSFrameReg == JSC::ARMRegisters::r11);
> JS_STATIC_ASSERT(JSReturnReg_Data == JSC::ARMRegisters::r1);
>@@ -446,33 +446,33 @@ SYMBOL_STRING(JaegerTrampoline) ":"     
>      *  [ r11       ]   |
>      *  [ r10       ]   |
>      *  [ r9        ]   | Callee-saved registers.                             
>      *  [ r8        ]   | VFP registers d8-d15 may be required here too, but  
>      *  [ r7        ]   | unconditionally preserving them might be expensive
>      *  [ r6        ]   | considering that we might not use them anyway.
>      *  [ r5        ]   |
>      *  [ r4        ]   /
>-     *  [ entryFp   ]
>+     *  [ entryfp   ]
>      *  [ stkLimit  ]
>      *  [ cx        ]
>      *  [ regs.fp   ]
>      *  [ regs.pc   ]
>      *  [ regs.sp   ]
>      *  [ unused    ]
>      *  [ previous  ]
>      *  [ args.ptr3 ]
>      *  [ args.ptr2 ]
>      *  [ args.ptr  ]
>      */
>     
>     /* Push callee-saved registers. */
> "   push    {r4-r11,lr}"                        "\n"
>     /* Push interesting VMFrame content. */
>-"   push    {r1}"                               "\n"    /* entryFp */
>+"   push    {r1}"                               "\n"    /* entryfp */
> "   push    {r3}"                               "\n"    /* stackLimit */
> "   push    {r0}"                               "\n"    /* cx */
> "   push    {r1}"                               "\n"    /* regs.fp */
>     /* Remaining fields are set elsewhere, but we need to leave space for them. */
> "   sub     sp, sp, #(4*7)"                     "\n"
> 
>     /* Preserve 'code' (r2) in an arbitrary callee-saved register. */
> "   mov     r4, r2"                             "\n"
>diff --git a/js/src/methodjit/MethodJIT.h b/js/src/methodjit/MethodJIT.h
>--- a/js/src/methodjit/MethodJIT.h
>+++ b/js/src/methodjit/MethodJIT.h
>@@ -65,17 +65,17 @@ struct VMFrame
>         } x;
>     } u;
> 
>     VMFrame      *previous;
>     void         *unused;
>     JSFrameRegs  regs;
>     JSContext    *cx;
>     Value        *stackLimit;
>-    JSStackFrame *entryFp;
>+    JSStackFrame *entryfp;
> 
> #if defined(JS_CPU_X86)
>     void *savedEBX;
>     void *savedEDI;
>     void *savedESI;
>     void *savedEBP;
>     void *savedEIP;
> 
>diff --git a/js/src/methodjit/Retcon.cpp b/js/src/methodjit/Retcon.cpp
>--- a/js/src/methodjit/Retcon.cpp
>+++ b/js/src/methodjit/Retcon.cpp
>@@ -127,17 +127,17 @@ Recompiler::recompile()
> 
>     // Find all JIT'd stack frames to account for return addresses that will
>     // need to be patched after recompilation.
>     for (VMFrame *f = script->compartment->jaegerCompartment->activeFrame();
>          f != NULL;
>          f = f->previous) {
> 
>         // Scan all frames owned by this VMFrame.
>-        JSStackFrame *end = f->entryFp->prev();
>+        JSStackFrame *end = f->entryfp->prev();
>         for (JSStackFrame *fp = f->fp(); fp != end; fp = fp->prev()) {
>             // Remember the latest frame for each type of JIT'd code, so the
>             // compiler will have a frame to re-JIT from.
>             if (!firstCtorFrame && fp->script() == script && fp->isConstructing())
>                 firstCtorFrame = fp;
>             else if (!firstNormalFrame && fp->script() == script && !fp->isConstructing())
>                 firstNormalFrame = fp;
>
Attachment #489315 - Flags: review?(dvander) → review+
It's gone. Please reopen if it recurs in builds after Nov 10.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-tracemonkey
Crash Signature: [@ PopActiveVMFrame ]
You need to log in before you can comment on or make changes to this bug.