Closed Bug 470300 Opened 16 years ago Closed 16 years ago

"Assertion failure: StackBase(fp) + blockDepth == regs.sp" with |let|

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1b3

People

(Reporter: jruderman, Assigned: brendan)

References

Details

(Keywords: assertion, testcase, verified1.9.1, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 1 obsolete file)

js> for (let a = 0; a < 3; ++a) { let b = '' + []; } Assertion failure: StackBase(fp) + blockDepth == regs.sp, at ../jsinterp.cpp:6853 js> for (let a = 0; a < 7; ++a) { let e = 8; if (a > 3) { let x; } } Assertion failure: !fp->blockChain || OBJ_GET_PARENT(cx, obj) == fp->blockChain, at ../jsinterp.cpp:6814 I'm hitting these assertions frequently, and they seem to be related to each other. Could this be a regression from bug 469927?
Assignee: general → brendan
Blocks: 469927
Status: NEW → ASSIGNED
Flags: blocking1.9.1?
OS: Mac OS X → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1b3
We are failing to update fp->blockChain on side-exit from trace. I will fix. /be
Blocks: 470223
Flags: blocking1.9.1? → blocking1.9.1+
Attached patch fix, may need optimization obfuscation (obsolete) (deleted) — Splinter Review
I can squeeze the block's object table index into existing FrameInfo space by shrinking argc, but this is the simplest patch. If it doesn't noticeably slow anything down, I'll stop here. /be
Attachment #353759 - Flags: review?(gal)
I'm probably going to check this in tonight, anticipating Andreas's r+ (he has been in on the design of the patch). /be
Attachment #353759 - Flags: review?(gal) → review+
Comment on attachment 353759 [details] [diff] [review] fix, may need optimization obfuscation >diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp >--- a/js/src/jstracer.cpp >+++ b/js/src/jstracer.cpp >@@ -1949,16 +1949,17 @@ TraceRecorder::snapshot(ExitType exitTyp > exit->calldepth = callDepth; > exit->numGlobalSlots = ngslots; > exit->numStackSlots = stackSlots; > exit->numStackSlotsBelowCurrentFrame = cx->fp->callee > ? nativeStackOffset(&cx->fp->argv[-2])/sizeof(double) > : 0; > exit->exitType = exitType; > exit->addGuard(rec); >+ exit->block = fp->blockChain; > exit->ip_adj = ip_adj; > exit->sp_adj = (stackSlots * sizeof(double)) - treeInfo->nativeStackBase; > exit->rp_adj = exit->calldepth * sizeof(FrameInfo); > memcpy(getTypeMap(exit), typemap, typemap_size); > > /* BIG FAT WARNING: If compilation fails, we currently don't reset the lirbuf so its safe > to keep references to the side exits here. If we ever start rewinding those lirbufs, > we have to make sure we purge the side exits that then no longer will be in valid >@@ -2854,16 +2855,25 @@ js_SynthesizeFrame(JSContext* cx, const > newifp->frame.argc = argc; > > jsbytecode* imacro_pc = FI_IMACRO_PC(fi, cx->fp); > jsbytecode* script_pc = FI_SCRIPT_PC(fi, cx->fp); > newifp->callerRegs.pc = imacro_pc ? imacro_pc : script_pc; > newifp->callerRegs.sp = cx->fp->slots + fi.s.spdist; > cx->fp->imacpc = imacro_pc ? script_pc : NULL; > >+ JS_ASSERT(!(cx->fp->flags & JSFRAME_POP_BLOCKS)); >+ if (fi.block != cx->fp->blockChain) { >+#ifdef DEBUG >+ for (JSObject* obj = fi.block; obj != cx->fp->blockChain; obj = STOBJ_GET_PARENT(obj)) >+ JS_ASSERT(obj); >+#endif >+ cx->fp->blockChain = fi.block; What is this piece of code trying to achieve here? Should this be just cx->fp->blockChain = fi.block? The debug code can have the if it needs it. Also, I suggest extracting fp here to save the indirection. This path is very hot. Also further up where we keep reading cx->fp. >+ } >+ > newifp->frame.argv = newifp->callerRegs.sp - argc; > JS_ASSERT(newifp->frame.argv); > #ifdef DEBUG > // Initialize argv[-1] to a known-bogus value so we'll catch it if > // someone forgets to initialize it later. > newifp->frame.argv[-1] = JSVAL_HOLE; > #endif > JS_ASSERT(newifp->frame.argv >= StackBase(cx->fp) + 2); >@@ -3648,27 +3658,30 @@ js_ExecuteTree(JSContext* cx, Fragment* > debug_only_v(printf("synthesized shallow frame for %s:%u@%u\n", > fp->script->filename, js_FramePCToLineNumber(cx, fp), > FramePCOffset(fp));) > #endif > } > > /* Adjust sp and pc relative to the tree we exited from (not the tree we entered into). > These are our final values for sp and pc since js_SynthesizeFrame has already taken >- care of all frames in between. */ >- JSStackFrame* fp = cx->fp; >+ care of all frames in between. But first we recover fp->blockChain, which comes from >+ the side exit struct. */ >+ JSStackFrame* fp = cx->fp; >+ >+ JS_ASSERT(!(fp->flags & JSFRAME_POP_BLOCKS)); >+ fp->blockChain = innermost->block; > > /* If we are not exiting from an inlined frame the state->sp is spbase, otherwise spbase > is whatever slots frames around us consume. */ > DECODE_IP_ADJ(innermost->ip_adj, fp); > fp->regs->sp = StackBase(fp) + (innermost->sp_adj / sizeof(double)) - calldepth_slots; > JS_ASSERT_IF(!fp->imacpc, > fp->slots + fp->script->nfixed + > js_ReconstructStackDepth(cx, fp->script, fp->regs->pc) == fp->regs->sp); >- > > #if defined(JS_JIT_SPEW) && (defined(NANOJIT_IA32) || (defined(NANOJIT_AMD64) && defined(__GNUC__))) > uint64 cycles = rdtsc() - start; > #elif defined(JS_JIT_SPEW) > uint64 cycles = 0; > #endif > > debug_only_v(printf("leaving trace at %s:%u@%u, op=%s, lr=%p, exitType=%d, sp=%d, " >@@ -6580,33 +6593,37 @@ TraceRecorder::interpretedFunctionCall(j > *m++ = determineSlotType(vp); > ); > > if (argc >= 0x8000) > ABORT_TRACE("too many arguments"); > > FrameInfo fi = { > JSVAL_TO_OBJECT(fval), >+ fp->blockChain, > ENCODE_IP_ADJ(fp, fp->regs->pc), > typemap, > { { fp->regs->sp - fp->slots, argc | (constructing ? 0x8000 : 0) } } > }; > > unsigned callDepth = getCallDepth(); > if (callDepth >= treeInfo->maxCallDepth) > treeInfo->maxCallDepth = callDepth + 1; > >- lir->insStorei(INS_CONSTPTR(fi.callee), lirbuf->rp, >- callDepth * sizeof(FrameInfo) + offsetof(FrameInfo, callee)); >- lir->insStorei(INS_CONSTPTR(fi.ip_adj), lirbuf->rp, >- callDepth * sizeof(FrameInfo) + offsetof(FrameInfo, ip_adj)); >- lir->insStorei(INS_CONSTPTR(fi.typemap), lirbuf->rp, >- callDepth * sizeof(FrameInfo) + offsetof(FrameInfo, typemap)); >- lir->insStorei(INS_CONST(fi.word), lirbuf->rp, >- callDepth * sizeof(FrameInfo) + offsetof(FrameInfo, word)); >+#define STORE_AT_RP(name) \ >+ lir->insStorei(INS_CONSTPTR(fi.name), lirbuf->rp, \ >+ callDepth * sizeof(FrameInfo) + offsetof(FrameInfo, name)) >+ >+ STORE_AT_RP(callee); >+ STORE_AT_RP(block); >+ STORE_AT_RP(ip_adj); >+ STORE_AT_RP(typemap); >+ STORE_AT_RP(word); >+ >+#undef STORE_AT_RP > > atoms = fun->u.i.script->atomMap.vector; > return true; > } > > JS_REQUIRES_STACK bool > TraceRecorder::record_JSOP_CALL() > { >diff --git a/js/src/jstracer.h b/js/src/jstracer.h >--- a/js/src/jstracer.h >+++ b/js/src/jstracer.h >@@ -186,16 +186,17 @@ enum ExitType { > OOM_EXIT, > OVERFLOW_EXIT, > UNSTABLE_LOOP_EXIT, > TIMEOUT_EXIT > }; > > struct VMSideExit : public nanojit::SideExit > { >+ JSObject* block; > intptr_t ip_adj; > intptr_t sp_adj; > intptr_t rp_adj; > int32_t calldepth; > uint32 numGlobalSlots; > uint32 numStackSlots; > uint32 numStackSlotsBelowCurrentFrame; > ExitType exitType; >@@ -243,17 +244,18 @@ public: > TreeInfo(nanojit::Fragment* _fragment) : unstableExits(NULL) { > fragment = _fragment; > } > ~TreeInfo(); > }; > > struct FrameInfo { > JSObject* callee; // callee function object >- intptr_t ip_adj; // callee script-based pc index and imacro pc >+ JSObject* block; // caller block chain head >+ intptr_t ip_adj; // caller script-based pc index and imacro pc > uint8* typemap; // typemap for the stack frame > union { > struct { > uint16 spdist; // distance from fp->slots to fp->regs->sp at JSOP_CALL > uint16 argc; // actual argument count, may be < fun->nargs > } s; > uint32 word; // for spdist/argc LIR store in record_JSOP_CALL > }; Looks good. We should measure perf impact (if any).
Attachment #353759 - Attachment is obsolete: true
Attachment #353777 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-tracemonkey
Depends on: 470375
Checking in js1_7/extensions/regress-470300-01.js Checking in js1_7/extensions/regress-470300-02.js http://hg.mozilla.org/mozilla-central/rev/aa3026fa8b4f
Flags: in-testsuite+
Flags: in-litmus-
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: