488203 - "Assertion failure: !(fp->flags & JSFRAME_COMPUTED_THIS) && !fp->thisp" on eBay Pulse

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

Loading http://pulse.ebay.com/ triggers:

Assertion failure: !(fp->flags & JSFRAME_COMPUTED_THIS) && !fp->thisp, at /Users/jruderman/tracemonkey/js/src/jstracer.cpp:4293

Happens on both mozilla-central and tracemonkey.  I'm about to start working on a reduced testcase.

Comment 1

[Andreas Gal :gal]

#0  JS_Assert (s=0x409018 "!(fp->flags & JSFRAME_COMPUTED_THIS) && !fp->thisp", file=0x407ce5 "../../../js/src/jstracer.cpp", ln=4294) at ../../../js/src/jsutil.cpp:69
#1  0x0038dfc3 in LeaveTree (state=@0xbfff5690, lr=0x18bc3820) at ../../../js/src/jstracer.cpp:4293
#2  0x0038e722 in js_ExecuteTree (cx=0xfda000, f=0x184d6f10, inlineCallCount=@0xbfff8cc0, innermostNestedGuardp=0xbfff88c4) at ../../../js/src/jstracer.cpp:4078
#3  0x003a7c96 in js_MonitorLoopEdge (cx=0xfda000, inlineCallCount=@0xbfff8cc0) at ../../../js/src/jstracer.cpp:4393
#4  0x002c24d7 in js_Interpret (cx=0xfda000) at ../../../js/src/jsinterp.cpp:3196
#5  0x002eac81 in js_Execute (cx=0xfda000, chain=0x16c0e040, script=0x184d6e20, down=0x1eb94c4c, flags=18, result=0xbfff921c) at jsinterp.cpp:1614
#6  0x00307443 in obj_eval (cx=0xfda000, obj=0x1dce2140, argc=1, argv=0x1eb94ccc, rval=0xbfff921c) at ../../../js/src/jsobj.cpp:1479
#7  0x002ec2fd in js_Invoke (cx=0xfda000, argc=1, vp=0x1eb94cc4, flags=2) at jsinterp.cpp:1380
#8  0x002d7d49 in js_Interpret (cx=0xfda000) at ../../../js/src/jsinterp.cpp:5110
#9  0x002eac81 in js_Execute (cx=0xfda000, chain=0x16c0fee0, script=0x184698a0, down=0x1eb94b9c, flags=18, result=0xbfff9bcc) at jsinterp.cpp:1614
#10 0x00307443 in obj_eval (cx=0xfda000, obj=0x1dce2140, argc=1, argv=0x1eb94c20, rval=0xbfff9bcc) at ../../../js/src/jsobj.cpp:1479
#11 0x002ec2fd in js_Invoke (cx=0xfda000, argc=1, vp=0x1eb94c18, flags=2) at jsinterp.cpp:1380
#12 0x002d7d49 in js_Interpret (cx=0xfda000) at ../../../js/src/jsinterp.cpp:5110
#13 0x002eac81 in js_Execute (cx=0xfda000, chain=0x16c0fe80, script=0x184d6600, down=0x1eb94ae8, flags=16, result=0xbfffa57c) at jsinterp.cpp:1614
#14 0x00307443 in obj_eval (cx=0xfda000, obj=0x1dce2140, argc=1, argv=0x1eb94b74, rval=0xbfffa57c) at ../../../js/src/jsobj.cpp:1479
#15 0x002ec2fd in js_Invoke (cx=0xfda000, argc=1, vp=0x1eb94b6c, flags=2) at jsinterp.cpp:1380
#16 0x002d7d49 in js_Interpret (cx=0xfda000) at ../../../js/src/jsinterp.cpp:5110
#17 0x002eac81 in js_Execute (cx=0xfda000, chain=0x16c0fd60, script=0x184d6500, down=0x1eb949a8, flags=18, result=0xbfffaf2c) at jsinterp.cpp:1614
#18 0x00307443 in obj_eval (cx=0xfda000, obj=0x1dce2140, argc=1, argv=0x1eb94a2c, rval=0xbfffaf2c) at ../../../js/src/jsobj.cpp:1479
#19 0x002ec2fd in js_Invoke (cx=0xfda000, argc=1, vp=0x1eb94a24, flags=2) at jsinterp.cpp:1380
#20 0x002d7d49 in js_Interpret (cx=0xfda000) at ../../../js/src/jsinterp.cpp:5110
#21 0x002eac81 in js_Execute (cx=0xfda000, chain=0x16c0fd00, script=0x184d5a40, down=0x1eb948f4, flags=16, result=0xbfffb8dc) at jsinterp.cpp:1614
#22 0x00307443 in obj_eval (cx=0xfda000, obj=0x1dce2140, argc=1, argv=0x1eb94980, rval=0xbfffb8dc) at ../../../js/src/jsobj.cpp:1479
#23 0x002ec2fd in js_Invoke (cx=0xfda000, argc=1, vp=0x1eb94978, flags=2) at jsinterp.cpp:1380
#24 0x002d7d49 in js_Interpret (cx=0xfda000) at ../../../js/src/jsinterp.cpp:5110
#25 0x002eac81 in js_Execute (cx=0xfda000, chain=0x16c0fda0, script=0x18467f80, down=0x1eb94844, flags=16, result=0xbfffc28c) at jsinterp.cpp:1614
#26 0x00307443 in obj_eval (cx=0xfda000, obj=0x1dce2140, argc=1, argv=0x1eb948d4, rval=0xbfffc28c) at ../../../js/src/jsobj.cpp:1479
#27 0x002ec2fd in js_Invoke (cx=0xfda000, argc=1, vp=0x1eb948cc, flags=2) at jsinterp.cpp:1380
#28 0x002d7d49 in js_Interpret (cx=0xfda000) at ../../../js/src/jsinterp.cpp:5110
#29 0x002ec346 in js_Invoke (cx=0xfda000, argc=1, vp=0x1eb94824, flags=0) at jsinterp.cpp:1388
#30 0x002ec5f1 in js_InternalInvoke (cx=0xfda000, obj=0x1ae38140, fval=382563872, flags=0, argc=1, argv=0x1eb94820, rval=0xbfffcc1c) at jsinterp.cpp:1441
#31 0x0026f905 in JS_CallFunctionValue (cx=0xfda000, obj=0x1ae38140, fval=382563872, argc=1, argv=0x1eb94820, rval=0xbfffcc1c) at ../../../js/src/jsapi.cpp:5224
#32 0x1334eaf1 in nsJSContext::CallEventHandler (this=0x1e025d90, aTarget=0x1f2dbb50, aScope=0x1ae38140, aHandler=0x16cd7620, aargv=0x184d5860, arv=0xbfffcd94) at ../../../dom/base/nsJSEnvironment.cpp:2007
#33 0x133c15e6 in nsJSEventListener::HandleEvent (this=0x19a0a230, aEvent=0x1e071630) at ../../../../dom/src/events/nsJSEventListener.cpp:247
#34 0x1315480b in nsEventListenerManager::HandleEventSubType (this=0x18f55140, aListenerStruct=0x19a0abc0, aListener=0x19a0a230, aDOMEvent=0x1e071630, aCurrentTarget=0x1e025c10, aPhaseFlags=6) at ../../../../content/events/src/nsEventListenerManager.cpp:1090
#35 0x13154c09 in nsEventListenerManager::HandleEvent (this=0x18f55140, aPresContext=0x1eb7b400, aEvent=0xbfffd248, aDOMEvent=0xbfffd1b0, aCurrentTarget=0x1e025c10, aFlags=6, aEventStatus=0xbfffd1b4) at ../../../../content/events/src/nsEventListenerManager.cpp:1187
#36 0x131893ce in nsEventTargetChainItem::HandleEvent (this=0x1e947820, aVisitor=@0xbfffd1a8, aFlags=6, aMayHaveNewListenerManagers=1) at ../../../../content/events/src/nsEventDispatcher.cpp:227
#37 0x1318960e in nsEventTargetChainItem::HandleEventTargetChain (this=0x1e947940, aVisitor=@0xbfffd1a8, aFlags=6, aCallback=0x0, aMayHaveNewListenerManagers=1) at ../../../../content/events/src/nsEventDispatcher.cpp:291
#38 0x13189edf in nsEventDispatcher::Dispatch (aTarget=0x1e025bd0, aPresContext=0x1eb7b400, aEvent=0xbfffd248, aDOMEvent=0x0, aEventStatus=0xbfffd274, aCallback=0x0) at ../../../../content/events/src/nsEventDispatcher.cpp:508
#39 0x12dd3483 in DocumentViewerImpl::LoadComplete (this=0x1f2d9fa0, aStatus=0) at ../../../layout/base/nsDocumentViewer.cpp:1001
#40 0x146a971a in nsDocShell::EndPageLoad (this=0x1e025500, aProgress=0x1e025514, aChannel=0x1e070700, aStatus=0) at ../../../docshell/base/nsDocShell.cpp:5263
#41 0x146c81e9 in nsWebShell::EndPageLoad (this=0x1e025500, aProgress=0x1e025514, channel=0x1e070700, aStatus=0) at ../../../docshell/base/nsWebShell.cpp:1013
#42 0x14696b72 in nsDocShell::OnStateChange (this=0x1e025500, aProgress=0x1e025514, aRequest=0x1e070700, aStateFlags=131088, aStatus=0) at ../../../docshell/base/nsDocShell.cpp:5159
#43 0x146defc7 in nsDocLoader::FireOnStateChange (this=0x1e025500, aProgress=0x1e025514, aRequest=0x1e070700, aStateFlags=131088, aStatus=0) at ../../../uriloader/base/nsDocLoader.cpp:1254
#44 0x146df1ff in nsDocLoader::doStopDocumentLoad (this=0x1e025500, request=0x1e070700, aStatus=0) at ../../../uriloader/base/nsDocLoader.cpp:877
#45 0x146df515 in nsDocLoader::DocLoaderIsEmpty (this=0x1e025500, aFlushLayout=1) at ../../../uriloader/base/nsDocLoader.cpp:782
#46 0x146e1c90 in nsDocLoader::ChildDoneWithOnload (this=0x1e025500, aChild=0x19a2f5e0) at nsDocLoader.h:205
#47 0x146df543 in nsDocLoader::DocLoaderIsEmpty (this=0x19a2f5e0, aFlushLayout=1) at ../../../uriloader/base/nsDocLoader.cpp:785
#48 0x146e0007 in nsDocLoader::OnStopRequest (this=0x19a2f5e0, aRequest=0x1f2df4d0, aCtxt=0x0, aStatus=0) at ../../../uriloader/base/nsDocLoader.cpp:678
#49 0x11432ee3 in nsLoadGroup::RemoveRequest (this=0x18407030, request=0x1f2df4d0, ctxt=0x0, aStatus=0) at ../../../../netwerk/base/src/nsLoadGroup.cpp:680
#50 0x130a6e37 in nsDocument::DoUnblockOnload (this=0x20975e00) at ../../../../content/base/src/nsDocument.cpp:7046
#51 0x130b3bb5 in nsDocument::UnblockOnload (this=0x20975e00, aFireSync=1) at ../../../../content/base/src/nsDocument.cpp:6993
#52 0x130f171b in nsImageLoadingContent::Event::~Event (this=0x18458f80) at ../../../../content/base/src/nsImageLoadingContent.cpp:807
#53 0x004c5fbd in nsRunnable::Release (this=0x18458f80) at nsThreadUtils.cpp:51
#54 0x0053e424 in nsCOMPtr<nsIRunnable>::~nsCOMPtr (this=0xbfffdb5c) at nsCOMPtr.h:510
#55 0x0053d870 in nsThread::ProcessNextEvent (this=0x715a90, mayWait=0, result=0xbfffdbb4) at ../../../xpcom/threads/nsThread.cpp:523
#56 0x004c6288 in NS_ProcessPendingEvents_P (thread=0x715a90, timeout=20) at nsThreadUtils.cpp:180
#57 0x1186450f in nsBaseAppShell::NativeEventCallback (this=0x736200) at ../../../../widget/src/xpwidgets/nsBaseAppShell.cpp:121
#58 0x1181a1ee in nsAppShell::ProcessGeckoEvents (aInfo=0x736200) at ../../../../widget/src/cocoa/nsAppShell.mm:412
#59 0x918af5f5 in CFRunLoopRunSpecific ()
#60 0x918afcd8 in CFRunLoopRunInMode ()
#61 0x93d4e2c0 in RunCurrentEventLoopInMode ()
#62 0x93d4e012 in ReceiveNextEventCommon ()
#63 0x93d4df4d in BlockUntilNextEventMatchingListInMode ()
#64 0x95b89d7d in _DPSNextEvent ()
#65 0x95b89630 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#66 0x95b8266b in -[NSApplication run] ()
#67 0x118182bc in nsAppShell::Run (this=0x736200) at ../../../../widget/src/cocoa/nsAppShell.mm:723
#68 0x1253df72 in nsAppStartup::Run (this=0x74fd70) at ../../../../../toolkit/components/startup/src/nsAppStartup.cpp:192
#69 0x000a8ba6 in XRE_main (argc=1, argv=0xbffff20c, aAppData=0x70edd0) at ../../../toolkit/xre/nsAppRunner.cpp:3232
#70 0x000027cb in main (argc=1, argv=0xbffff20c) at ../../../browser/app/nsBrowserApp.cpp:156

Comment 2

[Andreas Gal :gal]

Looks like a pretty bad bug. Bisecting and reducing would be good.

Comment 3

[Jesse Ruderman]

Attachment: partially reduced, about 200 lines

Comment 4

[Andreas Gal :gal]

Attachment: patch (untested)

Comment 5

[Jesse Ruderman]

Attachment: shell testcase, 8 lines

Comment 6

[Andreas Gal :gal]

Attachment: v2

Comment 7

[Andreas Gal :gal]

Attachment: v3

Comment 8

[Andreas Gal :gal]

Attachment: v3, fixed comment

Comment 9

[Andreas Gal :gal]

Several bad correctness bugs and bad asserts. Definitively b+.

Comment 10

[Andreas Gal :gal]

The current test case is a bit weak and mostly exposes a bad assert which this patch removes. More comprehensive test coverage would be good.

http://hg.mozilla.org/tracemonkey/rev/bf37cd061e3c

Comment 11

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/99143ce38e68

http://hg.mozilla.org/tracemonkey/rev/1ad60736a3c8

Comment 12

[Andreas Gal :gal]

ComputeThis_tn can re-enter, so this patch has to be backed out and it needs a bit more work.

Thread 0 (crashed)
 0  libmozjs.so!LeaveTree(InterpState&, VMSideExit*) [jstracer.cpp:99143ce38e68 : 4163 + 0x0]
    eip = 0xb718a098   esp = 0xbf8ed2d0   ebp = 0xbf8ed318   ebx = 0xb71af3f4
    esi = 0xbf8eda18   edi = 0xb2c2fc00   eax = 0xbf8ed560   ecx = 0x00000000
    edx = 0xbf8efa18   efl = 0x00010292
 1  libmozjs.so!js_DeepBail [jstracer.cpp:99143ce38e68 : 4971 + 0xa]
    eip = 0xb718a40c   esp = 0xbf8ed320   ebp = 0xbf8ed328
 2  libmozjs.so!JS_GetScopeChain [jscntxt.h:99143ce38e68 : 1400 + 0x8]
    eip = 0xb70fe107   esp = 0xbf8ed330   ebp = 0xbf8ed358
 3  libxul.so!XPC_WN_JSOp_ThisObject(JSContext*, JSObject*) [xpcwrappednativejsops.cpp:99143ce38e68 : 1325 + 0x8]
    eip = 0xb7393001   esp = 0xbf8ed360   ebp = 0xbf8ed3b8
 4  libmozjs.so!js_ComputeThis [jsinterp.cpp:99143ce38e68 : 922 + 0x11]
    eip = 0xb7132be7   esp = 0xbf8ed3c0   ebp = 0xbf8ed3e8
 5  libmozjs.so!js_ComputeThisForFrame [jsinterp.h:99143ce38e68 : 474 + 0xb]
    eip = 0xb7183fbe   esp = 0xbf8ed3f0   ebp = 0xbf8ed408
 6  0xb3b3405f
    eip = 0xb3b34060   esp = 0xbf8ed410   ebp = 0xbf8ed478
 7  0xbf8ed497
    eip = 0xbf8ed498   esp = 0xbf8ed480   ebp = 0xbf8ed498
 8  0xb0b4c52c
    eip = 0xb0b4c52d   esp = 0xbf8ed4a0   ebp = 0xbf8ed538
 9  0xbf8efb27

Comment 13

[Andreas Gal :gal]

Backed out.

http://hg.mozilla.org/tracemonkey/rev/aa49eee7617c

Comment 14

[Bob Clary [:bc] (inactive)]

fyi, this caused

js1_8/extensions/regress-452476.js 
Assertion failure: initializing || known(p), at ../jstracer.cpp:1954

Comment 15

[Bob Clary [:bc] (inactive)]

ditto js1_5/Regress/regress-452495.js

Comment 16

[Andreas Gal :gal]

Attachment: v4

Mismatch exit and let the interpreter wrap if needed.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 372965 [details] [diff] [review]
v4

>diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp
>--- a/js/src/jsinterp.cpp
>+++ b/js/src/jsinterp.cpp
>@@ -4359,25 +4359,18 @@ js_Interpret(JSContext *cx)
>             OBJ_SET_SLOT(cx, fp->varobj, slot, rval);
>             len = JSOP_INCGVAR_LENGTH;  /* all gvar incops are same length */
>             JS_ASSERT(len == js_CodeSpec[op].length);
>             DO_NEXT_OP(len);
>           }
> 
> #define COMPUTE_THIS(cx, fp, obj)                                             \
>     JS_BEGIN_MACRO                                                            \
>-        if (fp->flags & JSFRAME_COMPUTED_THIS) {                              \
>-            obj = fp->thisp;                                                  \
>-        } else {                                                              \
>-            obj = js_ComputeThis(cx, JS_TRUE, fp->argv);                      \
>-            if (!obj)                                                         \
>-                goto error;                                                   \
>-            fp->thisp = obj;                                                  \
>-            fp->flags |= JSFRAME_COMPUTED_THIS;                               \
>-        }                                                                     \
>+        if (!(obj = js_ComputeThisForFrame(cx, fp)))                          \
>+            goto error;                                                       \
>     JS_END_MACRO
> 
>           BEGIN_CASE(JSOP_THIS)
>             COMPUTE_THIS(cx, fp, obj);
>             PUSH_OPND(OBJECT_TO_JSVAL(obj));
>           END_CASE(JSOP_THIS)
> 
>           BEGIN_CASE(JSOP_GETTHISPROP)
>diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h
>--- a/js/src/jsinterp.h
>+++ b/js/src/jsinterp.h
>@@ -461,16 +461,29 @@ js_ComputeThis(JSContext *cx, JSBool laz
> 
> extern const uint16 js_PrimitiveTestFlags[];
> 
> #define PRIMITIVE_THIS_TEST(fun,thisv)                                        \
>     (JS_ASSERT(!JSVAL_IS_VOID(thisv)),                                        \
>      JSFUN_THISP_TEST(JSFUN_THISP_FLAGS((fun)->flags),                        \
>                       js_PrimitiveTestFlags[JSVAL_TAG(thisv) - 1]))
> 
>+static inline JSObject *
>+js_ComputeThisForFrame(JSContext *cx, JSStackFrame *fp)
>+{
>+    if (fp->flags & JSFRAME_COMPUTED_THIS)
>+        return fp->thisp;
>+    JSObject* obj = js_ComputeThis(cx, JS_TRUE, fp->argv);
>+    if (!obj)
>+        return NULL;
>+    fp->thisp = obj;
>+    fp->flags |= JSFRAME_COMPUTED_THIS;
>+    return obj;
>+}
>+
> /*
>  * NB: js_Invoke requires that cx is currently running JS (i.e., that cx->fp
>  * is non-null), and that vp points to the callee, |this| parameter, and
>  * actual arguments of the call. [vp .. vp + 2 + argc) must belong to the last
>  * JS stack segment that js_AllocStack allocated. The function may use the
>  * space available after vp + 2 + argc in the stack segment for temporaries,
>  * so the caller should not use that space for values that must be preserved
>  * across the call.
>diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp
>--- a/js/src/jstracer.cpp
>+++ b/js/src/jstracer.cpp
>@@ -1738,19 +1738,21 @@ skip:
>             }
>             // Skip over stopFrame itself.
>             JS_ASSERT(n != 0);
>             --n;
>             fp = fp->down;
>         }
>         for (; n != 0; fp = fp->down) {
>             --n;
>-            if (fp->callee) { // might not have it if the entry frame is global
>+            if (fp->callee) {
>                 JS_ASSERT(JSVAL_IS_OBJECT(fp->argv[-1]));
>                 fp->thisp = JSVAL_TO_OBJECT(fp->argv[-1]);
>+                if (fp->flags & JSFRAME_CONSTRUCTING) // constructors always compute 'this'
>+                    fp->flags |= JSFRAME_COMPUTED_THIS;
>             }
>         }
>     }
>     debug_only_v(printf("\n");)
>     return mp - mp_base;
> }
> 
> /* Emit load instructions onto the trace that read the initial stack state. */
>@@ -3338,17 +3340,17 @@ js_SynthesizeFrame(JSContext* cx, const 
>     newifp->frame.scopeChain = OBJ_GET_PARENT(cx, fi.callee);
>     newifp->frame.sharpDepth = 0;
>     newifp->frame.sharpArray = NULL;
>     newifp->frame.flags = constructing ? JSFRAME_CONSTRUCTING : 0;
>     newifp->frame.dormantNext = NULL;
>     newifp->frame.xmlNamespace = NULL;
>     newifp->frame.blockChain = NULL;
>     newifp->mark = newmark;
>-    newifp->frame.thisp = NULL; // will be set by js_ExecuteTree -> FlushNativeStackFrame
>+    newifp->frame.thisp = NULL; // will be updated in FlushNativeStackFrame
> 
>     newifp->frame.regs = fp->regs;
>     newifp->frame.regs->pc = script->code;
>     newifp->frame.regs->sp = newsp + script->nfixed;
>     newifp->frame.imacpc = NULL;
>     newifp->frame.slots = newsp;
>     if (script->staticLevel < JS_DISPLAY_SIZE) {
>         JSStackFrame **disp = &cx->display[script->staticLevel];
>@@ -4284,18 +4286,16 @@ LeaveTree(InterpState& state, VMSideExit
>                               getStackTypeMap(innermost),
>                               stack, NULL);
>     JS_ASSERT(unsigned(slots) == innermost->numStackSlots);
> 
> #ifdef DEBUG
>     // Verify that our state restoration worked.
>     for (JSStackFrame* fp = cx->fp; fp; fp = fp->down) {
>         JS_ASSERT_IF(fp->callee, JSVAL_IS_OBJECT(fp->argv[-1]));
>-        JS_ASSERT_IF(fp->callee && fp->thisp != JSVAL_TO_OBJECT(fp->argv[-1]),
>-                     !(fp->flags & JSFRAME_COMPUTED_THIS) && !fp->thisp);
>     }
> #endif
> #ifdef JS_JIT_SPEW
>     if (innermost->exitType != TIMEOUT_EXIT)
>         AUDIT(sideExitIntoInterpreter);
>     else
>         AUDIT(timeoutIntoInterpreter);
> #endif
>@@ -6213,23 +6213,41 @@ TraceRecorder::unbox_jsval(jsval v, LIns
>         v_ins = lir->ins2(LIR_piand, v_ins, INS_CONST(~JSVAL_TAGMASK));
>         return;
>     }
> }
> 
> JS_REQUIRES_STACK bool
> TraceRecorder::getThis(LIns*& this_ins)
> {
>-    if (cx->fp->callee) { /* in a function */
>-        if (JSVAL_IS_NULL(cx->fp->argv[-1]))
>-            return false;
>-        this_ins = get(&cx->fp->argv[-1]);
>-        guard(false, lir->ins_eq0(this_ins), MISMATCH_EXIT);
>-    } else { /* in global code */
>-        this_ins = scopeChain();
>+    JSObject* thisObj = js_ComputeThisForFrame(cx, cx->fp);
>+    if (!thisObj)
>+        ABORT_TRACE("js_ComputeThis failed");
>+    if (!cx->fp->callee || JSVAL_IS_NULL(cx->fp->argv[-1])) {
>+        JS_ASSERT(callDepth == 0);
>+        /*
>+         * In global code, or if this is NULL, wrap the global object and bake it directly
>+         * into the trace.
>+         */
>+        this_ins = INS_CONSTPTR(thisObj);
>+        set(&cx->fp->argv[-1], this_ins);
>+        return true;
>+    }
>+    this_ins = get(&cx->fp->argv[-1]);
>+    /*
>+     * When we inline through scripted functions, we have already previously touched the 'this'
>+     * object and hence it is already guaranteed to be wrapped. Otherwise we have to explicitly
>+     * check that the object has been wrapped. If not, we side exit and let the interpreter
>+     * wrap it.
>+     */
>+    if (callDepth == 0) {
>+        LIns* map_ins = lir->insLoad(LIR_ldp, this_ins, (int)offsetof(JSObject, map));
>+        LIns* ops_ins = lir->insLoad(LIR_ldp, map_ins, (int)offsetof(JSObjectMap, ops));
>+        LIns* op_ins = lir->insLoad(LIR_ldp, ops_ins, (int)offsetof(JSObjectOps, thisObject));
>+        guard(true, lir->ins_eq0(op_ins), MISMATCH_EXIT);
>     }
>     return true;
> }
> 
> JS_REQUIRES_STACK bool
> TraceRecorder::guardClass(JSObject* obj, LIns* obj_ins, JSClass* clasp, LIns* exit)
> {
>     bool cond = STOBJ_GET_CLASS(obj) == clasp;

Comment 18

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/e8c23c42db7f

Comment 19

[Andreas Gal :gal]

Backing out the patch to see whether it causes the leak.

http://hg.mozilla.org/tracemonkey/rev/64d7df1fe160

Comment 20

[Bob Clary [:bc] (inactive)]

(In reply to comment #18)
> http://hg.mozilla.org/tracemonkey/rev/e8c23c42db7f

I still see this assert

Assertion failure: initializing || known(p), at jstracer.cpp:1954

js1_5/Regress/regress-210682.js browser
js1_5/Regress/regress-452495.js browser
js1_5/Regress/regress-452495.js browser
js1_5/Regress/regress-452495.js shell
js1_5/Regress/regress-452495.js shell
js1_8/extensions/regress-452476.js shell
js1_8/extensions/regress-452476.js shell

Comment 21

[Andreas Gal :gal]

Debug only. I just pushed the patch in to see if we cycle green or not and I will let it ride. I know how to fix it but I have to reduce a testcase to figure out why this happens.

back-in to get tinderbox data: http://hg.mozilla.org/tracemonkey/rev/062ea62f9bda

Comment 22

[Andreas Gal :gal]

Attachment: patch for bc

bc, could you give this a try?

Comment 23

[Bob Clary [:bc] (inactive)]

that fixes my asserts for the listed tests. i'll do a quick full js shell run.

Comment 24

[Andreas Gal :gal]

re-landed with fix

http://hg.mozilla.org/tracemonkey/rev/5e56592a8d89

Comment 25

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/5e56592a8d89

Comment 26

[Brendan Eich [:brendan]]

Comment on attachment 372965 [details] [diff] [review]
v4

>+++ b/js/src/jsinterp.h
>@@ -461,16 +461,29 @@ js_ComputeThis(JSContext *cx, JSBool laz
> 
> extern const uint16 js_PrimitiveTestFlags[];
> 
> #define PRIMITIVE_THIS_TEST(fun,thisv)                                        \
>     (JS_ASSERT(!JSVAL_IS_VOID(thisv)),                                        \
>      JSFUN_THISP_TEST(JSFUN_THISP_FLAGS((fun)->flags),                        \
>                       js_PrimitiveTestFlags[JSVAL_TAG(thisv) - 1]))
> 
>+static inline JSObject *

Why didn't this break LiveConnect? We have __cplusplus ifdefs above, also JS_INLINE. If we no longer need those, great -- but this would be news, *news* I say!

/be

Comment 27

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ab091d238f88

Comment 28

[Robert Sayre]

wrong changeset

Comment 29

[Jeff Walden [:Waldo]]

(In reply to comment #26)
> >+static inline JSObject *
> 
> Why didn't this break LiveConnect?

The inline keyword is C99, and practically most compilers have added it as an extension anyway even if they don't target C99.  It's the pure C++-isms that snag on LiveConnect.

Comment 30

[Brendan Eich [:brendan]]

Then why JS_INLINE? I guess from when SpiderMonkey was C.

MSVC fails C99, presumably gets inline right. Whoop-de-doo.

/be

Comment 31

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f981b36489c9

Comment 32

[Boris Zbarsky [:bzbarsky]]

> Why didn't this break LiveConnect?

I have no idea, for m-c.  It did on 1.9.1, sending all the Windows stuff red; I pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0a7ecbe4553b before I looked up this bug.

Presumably we want that on t-m and m-c as well.

I do wonder why it didn't break on t-m or m-c.

Comment 33

[Boris Zbarsky [:bzbarsky]]

That said, using JS_INLINE there fixed the bustage we've been getting:

e:\builds\moz2_slave\mozilla-1.9.1-win32\build\obj-firefox\dist\include\js\jsinterp.h(469) : error C2054: expected '(' to follow 'inline'
e:\builds\moz2_slave\mozilla-1.9.1-win32\build\obj-firefox\dist\include\js\jsinterp.h(471) : error C2085: 'js_ComputeThisForFrame' : not in formal parameter list
e:\builds\moz2_slave\mozilla-1.9.1-win32\build\obj-firefox\dist\include\js\jsinterp.h(471) : error C2143: syntax error : missing ';' before '{'

and now we have new bustage:

e:\builds\moz2_slave\mozilla-1.9.1-win32-unittest\build\objdir\dist\include\js\jsinterp.h(474) : error C2275: 'JSObject' : illegal use of this type as an expression
        e:\builds\moz2_slave\mozilla-1.9.1-win32-unittest\build\objdir\dist\include\js\jspubtd.h(144) : see declaration of 'JSObject'
e:\builds\moz2_slave\mozilla-1.9.1-win32-unittest\build\objdir\dist\include\js\jsinterp.h(474) : error C2065: 'obj' : undeclared identifier

This is while compiling jsj_JSObject.c

Comment 34

[Boris Zbarsky [:bzbarsky]]

Jonathan Kew points out that if this is going to be compiled as C then the variable declaration needs to be at the top of the function.

Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e351dc56a970 with that change.  Hopefully it'll send the tree green.

Comment 35

[Boris Zbarsky [:bzbarsky]]

> Why didn't this break LiveConnect?

And now we have an answer.  We don't compile liveconnect on m-c (and presumably t-m) ever since bug 485984 happened to remove the 

 tier_gecko_dirs        += js/src/liveconnect

line.

Comment 36

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Verified fixed with xpshell testcase and the following debug builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090522 Minefield/3.6a1pre ID:20090522133810

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre)
Gecko/20090522 Shiretoko/3.5pre ID:20090522153422

Comment 37

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929