Closed
Bug 491013
Opened 16 years ago
Closed 16 years ago
"Assertion failure: OBJ_SCOPE(obj)->object == pobj, at ../jsobj.cpp" or "Assertion failure: OBJ_IS_CLONED_BLOCK(obj), at ../jsobj.cpp"
Categories
(Core :: JavaScript Engine, defect, P2)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: gkw, Assigned: igor)
References
Details
(4 keywords, Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 3 obsolete files)
28.74 KB,
patch
|
igor
:
review+
|
Details | Diff | Splinter Review |
let(x) ((function(){for( y in [0, function(){}, 0, 0]) x = x})())
asserts debug js shell without -j at Assertion failure: OBJ_SCOPE(obj)->object == pobj, at ../jsobj.cpp:4069
let(x) ((function(){for( y in [0, function(){}]) x = x})())
asserts debug js shell without -j at Assertion failure: OBJ_IS_CLONED_BLOCK(obj), at ../jsobj.cpp:2445
Both seem related, as autoBisect shows they are probably related to bug 490666 :
The first bad revision is:
changeset: 27505:3519ddacbe2e
user: Andreas Gal
date: Thu Apr 30 15:52:13 2009 -0700
summary: We don't cache access to shared properties in the property cache (490666, r=igor,brendan).
Flags: blocking1.9.1?
Assignee | ||
Updated•16 years ago
|
Assignee: general → igor
Comment 1•16 years ago
|
||
No JIT involvement.
#0 JS_Assert (s=0x1ad3f8 "OBJ_IS_CLONED_BLOCK(obj)", file=0x1ac55a "../jsobj.cpp", ln=2445) at ../jsutil.cpp:69
#1 0x000adf29 in js_PutBlockObject (cx=0x30bc80, normalUnwind=1) at ../jsobj.cpp:2445
#2 0x00092a78 in js_Interpret (cx=0x30bc80) at ../jsinterp.cpp:6955
#3 0x00096a29 in js_Execute (cx=0x30bc80, chain=0x2ac000, script=0x30d8d0, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1633
#4 0x00010d3a in JS_ExecuteScript (cx=0x30bc80, obj=0x2ac000, script=0x30d8d0, rval=0x0) at ../jsapi.cpp:5056
#5 0x0000a206 in Process (cx=0x30bc80, obj=0x2ac000, filename=0xbffff991 "x.js", forceTTY=0) at ../../shell/js.cpp:412
#6 0x0000af1a in ProcessArgs (cx=0x30bc80, obj=0x2ac000, argv=0xbffff898, argc=1) at ../../shell/js.cpp:806
#7 0x0000b2e7 in main (argc=1, argv=0xbffff898, envp=0xbffff8a0) at ../../shell/js.cpp:4728
(gdb)
No idea what happens here. Jim?
Assignee: igor → general
Assignee | ||
Comment 2•16 years ago
|
||
The reason for the bug is js_GetMutableScope call is JSOP_SETPROP/JSOP_SETNAME. Now, since we cache the shared properties, it is wrong to do that unconditionally.
Assignee: general → igor
Assignee | ||
Comment 3•16 years ago
|
||
The essence of the patch is to make sure that the property cache code for SETPROP/SETNAME does not create scopes for shared properties.
Another set of changes is to remove useless property-less prototypes for anonymous objects consistently using JS_NewObjectWithGivenProto to create prototype-less Block, NoSuchMethod and XMLFilter objects. That allowed to simplifu code and provide better assert coverage to ensure that no scope is ever created for cloned block objects.
Comment 4•16 years ago
|
||
Separately from any cache issues, a block object created by js_GetBlockObject
should always share its scope with its prototype --- the immutable object built
by the compiler. So I don't think we should ever be applying
js_GetMutableScope to a block... unless perhaps the compiler needs to do so.
Assignee | ||
Comment 5•16 years ago
|
||
The testing with the try server revealed that the Block object prototype was used to xdr scripts. The new version fixes that using direct calls to Block, Function or RegExp functions in js_XDRScript. It generates strictly less code than keeping the Block prototype initialization code.
Attachment #375391 -
Attachment is obsolete: true
Assignee | ||
Comment 6•16 years ago
|
||
(In reply to comment #4)
> So I don't think we should ever be applying
> js_GetMutableScope to a block... unless perhaps the compiler needs to do so.
This is what the patch asserts - it insists that js_GetMutableScope is not feed with a block object.
Assignee | ||
Comment 7•16 years ago
|
||
Comment on attachment 375439 [details] [diff] [review]
v2
Asking for a review as the patch has passed the try server. For details, see the comments above.
Attachment #375439 -
Flags: review?(brendan)
Comment 8•16 years ago
|
||
Comment on attachment 375439 [details] [diff] [review]
v2
>+ /*
>+ * Fastest path: check if the cached sprop is
>+ * already in scope to call NATIVE_GET and break
s/to call/and call/
>+ * to get out of the do-while(0). But we only can
>+ * call NATIVE_GET if obj owns scope or sprop is
s/But we only can call NATIVE_GET if/But we can call NATIVE_GET only if/
> js_NewBlockObject(JSContext *cx)
> {
> /*
> * Null obj's proto slot so that Object.prototype.* does not pollute block
>+ * scopes and to give own scope to the block object.
s/own scope to the block object/the block object its own scope/
>@@ -65,18 +65,24 @@ js_GetMutableScope(JSContext *cx, JSObje
> {
> JSScope *scope, *newscope;
> JSClass *clasp;
> uint32 freeslot;
>
> scope = OBJ_SCOPE(obj);
> JS_ASSERT(JS_IS_SCOPE_LOCKED(cx, scope));
> if (scope->object == obj)
> return scope;
>+
>+ /*
>+ * Only a compile-time block object has own scope which is created
>+ * together with the object.
How about "Compile-time block objects each have their own scope, created at birth, and runtime clone of a block objects are never mutated" instead?
>+ uint32 kind;
>+ if (xdr->mode == JSXDR_ENCODE) {
>+ JSClass *clasp = STOBJ_GET_CLASS(*objp);
>+ if (clasp == &js_FunctionClass) {
>+ kind = 0;
>+ } else {
>+ JS_ASSERT(clasp == &js_BlockClass);
>+ kind = 1;
Use an enum for 0 and 1?
/be
Assignee | ||
Comment 9•16 years ago
|
||
The new version of the patch addresses the nits above.
In js_XDRScript I replaced kind with the isBlock pseudo-boolean. The reason I do not want to have an enumeration is that it suggests that more objects, besides Block and Function, may come in future. But I suspect those new objects, if any, will endup using separated layout and there would be no need to use any enums to distinguish them. Case in point is the regular expressions that are stored and serialized separately.
Attachment #375439 -
Attachment is obsolete: true
Attachment #375478 -
Flags: review?(brendan)
Attachment #375439 -
Flags: review?(brendan)
Comment 10•16 years ago
|
||
Comment on attachment 375478 [details] [diff] [review]
v3
>+ /*
..12345678901234567890123456789012345678901234567890123456789012345678901234567890
>+ * Fastest path: check if the cached sprop is
Sorry, final nit: s/if/whether/ -- should fit.
>+ * already in scope and call NATIVE_GET and break
>+ * to get out of the do-while(0). But we can call
>+ * NATIVE_GET only if obj owns scope or sprop is
>+ * shared.
>+ */
r=me with that, thanks.
/be
Attachment #375478 -
Flags: review?(brendan) → review+
Comment 11•16 years ago
|
||
I was experiencing this bug reproducibly when I opened an IMAP folder in my custom debug build of TB on 1.9.1. I applied the patch v2 to mozilla 1.9.1 (with a small manual correction due to an unsuccessful hunk) and the crash went away.
It would be useful if you would post the 1.9.1 version of the new patch as well, since those of us doing TB development are typically using 1.9.1, may be halted by this bug, and need to apply it locally even if you decide for some reason to delay the application to 1.9.1
![]() |
Reporter | |
Comment 12•16 years ago
|
||
(In reply to comment #11)
> I was experiencing this bug reproducibly when I opened an IMAP folder in my
> custom debug build of TB on 1.9.1. I applied the patch v2 to mozilla 1.9.1
> (with a small manual correction due to an unsuccessful hunk) and the crash went
> away.
>
> It would be useful if you would post the 1.9.1 version of the new patch as
> well, since those of us doing TB development are typically using 1.9.1, may be
> halted by this bug, and need to apply it locally even if you decide for some
> reason to delay the application to 1.9.1
See bug 490128 which is already marked as a bug that this bug blocks.
Assignee | ||
Comment 13•16 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/6534f8b9aa74 - landed with the nit from the comment 10 addressed
Whiteboard: fixed-in-tracemonkey
Comment 14•16 years ago
|
||
I spoke too soon in comment 11. Today, in the same circumstance of comment 11 (after applying v2 of the patch to 1.9.1) I am getting this error reproducibly followed by a crash:
Assertion failure: STOBJ_GET_CLASS(obj) != &js_BlockClass, at c:/tb/test/src/moz
illa/js/src/jsscope.cpp:79
Assignee | ||
Comment 15•16 years ago
|
||
(In reply to comment #14)
> I spoke too soon in comment 11. Today, in the same circumstance of comment 11
> (after applying v2 of the patch to 1.9.1) I am getting this error reproducibly
> followed by a crash:
>
> Assertion failure: STOBJ_GET_CLASS(obj) != &js_BlockClass, at
> c:/tb/test/src/moz
> illa/js/src/jsscope.cpp:79
What is the stack trace for the error?
![]() |
Reporter | |
Comment 16•16 years ago
|
||
(In reply to comment #15)
> (In reply to comment #14)
> > I spoke too soon in comment 11. Today, in the same circumstance of comment 11
> > (after applying v2 of the patch to 1.9.1) I am getting this error reproducibly
> > followed by a crash:
> >
> > Assertion failure: STOBJ_GET_CLASS(obj) != &js_BlockClass, at
> > c:/tb/test/src/moz
> > illa/js/src/jsscope.cpp:79
>
> What is the stack trace for the error?
Kent, besides backtrace (for js assertions),
(gdb) bt
one can also do:
(gdb) frame 1
(gdb) call JS_GetFramePC(cx, cx->fp)
(gdb) call JS_PCToLineNumber(cx, cx->fp->script, $1)
(gdb) p cx->fp->script->filename
for more useful information (see bug 490128 comment #0 near the bottom) if you are using gdb.
Comment 17•16 years ago
|
||
Assertion failure: isBlock == 1, at ../../../js/src/jsscript.cpp:614
Program received signal SIGTRAP, Trace/breakpoint trap.
JS_Assert (s=0x45 <Address 0x45 out of bounds>, file=0x45 <Address 0x45 out of bounds>, ln=69) at ../../../js/src/jsutil.cpp:69
69 abort();
(gdb) bt
#0 JS_Assert (s=0x45 <Address 0x45 out of bounds>, file=0x45 <Address 0x45 out of bounds>, ln=69) at ../../../js/src/jsutil.cpp:69
#1 0x0028456f in js_XDRScript (xdr=0x65b310, scriptp=0xbfffe724, hasMagic=0x0) at ../../../js/src/jsscript.cpp:614
#2 0x0028e004 in JS_XDRScript (xdr=0x65b310, scriptp=0xbfffe724) at ../../../js/src/jsxdrapi.cpp:690
#3 0x111d45b8 in ReadScriptFromStream [inlined] () at ../../../../../js/src/xpconnect/loader/mozJSComponentLoader.cpp:443
#4 0x111d45b8 in mozJSComponentLoader::ReadScript (this=0x622350, flSvc=0x655700, nativePath=0x657428 "file:///Users/gal/workspace/tracemonkey-repository/debug-build/dist/MinefieldDebug.app/Contents/MacOS/components/fuelApplication.js", uri=0x65d6d0, cx=0x862600, script=0xbfffe724) at ../../../../../js/src/xpconnect/loader/mozJSComponentLoader.cpp:1069
Comment 18•16 years ago
|
||
Backed out due to assert during startup in debug builds.
http://hg.mozilla.org/tracemonkey/rev/38512deaca7e
Priority: -- → P2
Whiteboard: fixed-in-tracemonkey
Updated•16 years ago
|
Flags: blocking1.9.1? → blocking1.9.1+
Assignee | ||
Comment 19•16 years ago
|
||
(In reply to comment #17)
> Assertion failure: isBlock == 1, at ../../../js/src/jsscript.cpp:614
Hm, it seems as if the system has not picked up XDR version change. Have you tried an optimized build?
Assignee | ||
Comment 20•16 years ago
|
||
Given that the backed out version of the patch has passed all tinderbox tests and my local debug builds of the browser on Linux runs just fine it is unclear what caused the assert on Mac. The fact that the code around the asserts is heavily executed on the startup adds even more fog.
So to make sure that this is not caused by some wired implementation of boolean-to-unsigned cast I updates the patch to use an explicit check, not implicit conversion in js_XDRScript:
if (xdr->mode == JSXDR_ENCODE) {
JSClass *clasp = STOBJ_GET_CLASS(*objp);
JS_ASSERT(clasp == &js_FunctionClass ||
clasp == &js_BlockClass);
- isBlock = (clasp == &js_BlockClass);
+ isBlock = (clasp == &js_BlockClass) ? 1 : 0;
}
if (!JS_XDRUint32(xdr, &isBlock))
goto error;
if (isBlock == 0) {
if (!js_XDRFunctionObject(xdr, objp))
goto error;
} else {
JS_ASSERT(isBlock == 1);
I plan to try to land the new patch on Tuesday.
Attachment #375478 -
Attachment is obsolete: true
Attachment #375755 -
Flags: review+
Comment 21•16 years ago
|
||
Here's my stack trace. I am running on Windows XP (Visual Studio, not gdb).:
js3250.dll!JS_Assert(const char * s=0x00691734, const char * file=0x00691708, int ln=0x0000004f) Line 65 C++
js3250.dll!js_GetMutableScope(JSContext * cx=0x02ff1600, JSObject * obj=0x02463740) Line 79 + 0x26 bytes C++
js3250.dll!js_SetPropertyHelper(JSContext * cx=0x02ff1600, JSObject * obj=0x02463740, int id=0x047438a4, int cacheResult=0x00000001, int * vp=0x0012ec58) Line 4594 + 0xd bytes C++
js3250.dll!js_Interpret(JSContext * cx=0x02ff1600) Line 4729 + 0x1a bytes C++
js3250.dll!js_Invoke(JSContext * cx=0x02ff1600, unsigned int argc=0x00000003, int * vp=0x05a45100, unsigned int flags=0x00000000) Line 1352 + 0x9 bytes C++
js3250.dll!array_extra(JSContext * cx=0x02ff1600, ArrayExtraMode mode=FOREACH, unsigned int argc=0x00000003, int * vp=0x05a450dc) Line 3051 + 0x13 bytes C++
js3250.dll!array_forEach(JSContext * cx=0x02ff1600, unsigned int argc=0x00000001, int * vp=0x05a450dc) Line 3107 + 0x13 bytes C++
js3250.dll!js_Interpret(JSContext * cx=0x02ff1600) Line 5088 + 0x17 bytes C++
js3250.dll!js_Execute(JSContext * cx=0x02ff1600, JSObject * chain=0x04540be0, JSScript * script=0x0889e1f0, JSStackFrame * down=0x00000000, unsigned int flags=0x00000000, int * result=0x00000000) Line 1578 + 0x9 bytes C++
js3250.dll!JS_EvaluateUCScriptForPrincipals(JSContext * cx=0x02ff1600, JSObject * obj=0x04540be0, JSPrincipals * principals=0x01ef9fbc, const unsigned short * chars=0x0313cf00, unsigned int length=0x00000029, const char * filename=0x08a38b00, unsigned int lineno=0x000000d1, int * rval=0x00000000) Line 5143 + 0x19 bytes C++
gklayout.dll!nsJSContext::EvaluateString(const nsAString_internal & aScript={...}, void * aScopeObject=0x04540be0, nsIPrincipal * aPrincipal=0x01ef9fb8, const char * aURL=0x08a38b00, unsigned int aLineNo=0x000000d1, unsigned int aVersion=0x00000000, nsAString_internal * aRetValue=0x00000000, int * aIsUndefined=0x0012f6f8) Line 1603 + 0x42 bytes C++
gklayout.dll!nsGlobalWindow::RunTimeout(nsTimeout * aTimeout=0x08a38b58) Line 7763 + 0xb3 bytes C++
gklayout.dll!nsGlobalWindow::TimerCallback(nsITimer * aTimer=0x08a38bc8, void * aClosure=0x08a38b58) Line 8119 C++
xpcom_core.dll!nsTimerImpl::Fire() Line 420 + 0xe bytes C++
xpcom_core.dll!nsTimerEvent::Run() Line 514 C++
xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0012f850) Line 511 C++
xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00cc22b8, int mayWait=0x00000001) Line 227 + 0x16 bytes C++
gkwidget.dll!nsBaseAppShell::Run() Line 170 + 0xc bytes C++
tkitcmps.dll!nsAppStartup::Run() Line 193 + 0x1c bytes C++
xul.dll!XRE_main(int argc=0x00000002, char * * argv=0x00c9cac0, const nsXREAppData * aAppData=0x00c9cfc8) Line 3298 + 0x25 bytes C++
thunderbird.exe!NS_internal_main(int argc=0x00000002, char * * argv=0x00c9cac0) Line 103 + 0x12 bytes C++
thunderbird.exe!wmain(int argc=0x00000002, unsigned short * * argv=0x00c9c9f8) Line 110 + 0xd bytes C++
thunderbird.exe!__tmainCRTStartup() Line 583 + 0x19 bytes C
thunderbird.exe!wmainCRTStartup() Line 403 C
kernel32.dll!7c817077()
![]() |
Reporter | |
Comment 22•16 years ago
|
||
(In reply to comment #20)
> Created an attachment (id=375755) [details]
> v4
I couldn't apply this patch to mozilla-1.9.1 to test rkent's assertion:
$ patch -p1 --dry-run < ~/Desktop/attachment.cgi
patching file js/src/jsapi.cpp
patching file js/src/jsfun.cpp
patching file js/src/jsfun.h
patching file js/src/jsinterp.cpp
Hunk #1 succeeded at 922 (offset -34 lines).
Hunk #2 succeeded at 973 (offset -34 lines).
Hunk #3 FAILED at 4571.
1 out of 3 hunks FAILED -- saving rejects to file js/src/jsinterp.cpp.rej
patching file js/src/jsobj.cpp
Hunk #1 succeeded at 2390 (offset -16 lines).
Hunk #2 succeeded at 2415 (offset -16 lines).
Hunk #3 succeeded at 2543 (offset -16 lines).
Hunk #4 succeeded at 2657 (offset -16 lines).
Hunk #5 succeeded at 4236 (offset -10 lines).
Hunk #6 succeeded at 4270 (offset -10 lines).
patching file js/src/jsobj.h
Hunk #1 succeeded at 423 (offset -6 lines).
Hunk #2 succeeded at 467 (offset -6 lines).
patching file js/src/jsproto.tbl
patching file js/src/jsregexp.cpp
Hunk #1 succeeded at 4533 (offset -21 lines).
Hunk #2 succeeded at 4573 (offset -21 lines).
Hunk #3 succeeded at 4597 (offset -21 lines).
patching file js/src/jsregexp.h
Hunk #1 succeeded at 167 (offset -8 lines).
patching file js/src/jsscope.cpp
patching file js/src/jsscript.cpp
patching file js/src/jsxdrapi.h
patching file js/src/jsxml.cpp
patching file js/src/jsxml.h
Hunk #1 succeeded at 260 (offset 1 line).
Assignee | ||
Comment 23•16 years ago
|
||
Whiteboard: fixed-in-tracemonkey
Assignee | ||
Comment 24•16 years ago
|
||
(In reply to comment #22)
> (In reply to comment #20)
> > Created an attachment (id=375755) [details] [details]
> > v4
>
> I couldn't apply this patch to mozilla-1.9.1 to test rkent's assertion:
I presume this comes from the absence of the bug 490666 on 1.9.1.
Comment 25•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 26•16 years ago
|
||
Keywords: fixed1.9.1
![]() |
Reporter | |
Updated•16 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•