1148921 - Assertion failure: !IsInsideNursery(t), at js/src/jsgc.h:1262 with Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 385840329d91 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2):

gczeal(13);
var g = newGlobal();
var dbg = Debugger(g);
dbg.onDebuggerStatement = function (frame) {
    var env = frame.environment.find("x");
};
g.eval("with (Object.create({})) { debugger; }");



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x000000000046bfd6 in js::gc::CheckGCThingAfterMovingGC<JSObject> (t=<optimized out>) at js/src/jsgc.h:1262
#0  0x000000000046bfd6 in js::gc::CheckGCThingAfterMovingGC<JSObject> (t=<optimized out>) at js/src/jsgc.h:1262
#1  0x0000000000b4a833 in CheckGCThingAfterMovingGC<JSObject> (t=<optimized out>) at js/src/jsweakmap.cpp:736
#2  js::ObjectWeakMap::checkAfterMovingGC (this=this@entry=0x7ffff696d240) at js/src/jsweakmap.cpp:733
#3  0x00000000006bc5b7 in js::DebugScopes::checkHashTablesAfterMovingGC (this=0x7ffff696d240, runtime=runtime@entry=0x7ffff693c000) at js/src/vm/ScopeObject.cpp:1940
#4  0x0000000000ab2e32 in js::gc::CheckHashTablesAfterMovingGC (rt=rt@entry=0x7ffff693c000) at js/src/jsgc.cpp:6902
#5  0x00000000005ee050 in js::Nursery::collect (this=this@entry=0x7ffff693c3a0, rt=0x7ffff693c000, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT, pretenureGroups=pretenureGroups@entry=0x0) at js/src/gc/Nursery.cpp:855
#6  0x0000000000a965f7 in js::gc::GCRuntime::minorGCImpl (this=this@entry=0x7ffff693c348, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT, pretenureGroups=pretenureGroups@entry=0x0) at js/src/jsgc.cpp:6299
#7  0x00000000006008d4 in js::gc::GCRuntime::evictNursery (this=0x7ffff693c348, reason=JS::gcreason::DESTROY_CONTEXT) at js/src/gc/GCRuntime.h:618
#8  0x0000000000afb848 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff693c348, incremental=incremental@entry=false, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:5915
#9  0x0000000000afbd3d in js::gc::GCRuntime::collect (this=this@entry=0x7ffff693c348, incremental=incremental@entry=false, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6088
#10 0x0000000000afc025 in js::gc::GCRuntime::gc (this=this@entry=0x7ffff693c348, gckind=gckind@entry=GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6149
#11 0x0000000000a51407 in js::DestroyContext (cx=0x7ffff691b4e0, mode=js::DCM_FORCE_GC) at js/src/jscntxt.cpp:186
#12 0x0000000000a5177e in JS_DestroyContext (cx=<optimized out>) at js/src/jsapi.cpp:681
#13 0x0000000000471a31 in DestroyContext (withGC=true, cx=0x7ffff691b4e0) at js/src/shell/js.cpp:5603
#14 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6394
rax	0x0	0
rbx	0x7fffffffcf40	140737488342848
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffcf30	140737488342832
rsp	0x7fffffffcf30	140737488342832
r8	0x7ffff7fe0780	140737354008448
r9	0x697a6f6d2f73656d	7600509835879343469
r10	0x7fffffffccf0	140737488342256
r11	0x7ffff6c27960	140737333328224
r12	0xbad0bad1	3134241489
r13	0x7ffff696d240	140737330467392
r14	0x7ffff693c000	140737330266112
r15	0x7ffff693c4a0	140737330267296
rip	0x46bfd6 <js::gc::CheckGCThingAfterMovingGC<JSObject>(JSObject*)+28>
=> 0x46bfd6 <js::gc::CheckGCThingAfterMovingGC<JSObject>(JSObject*)+28>:	movl   $0x4ee,0x0
   0x46bfe1 <js::gc::CheckGCThingAfterMovingGC<JSObject>(JSObject*)+39>:	callq  0x423140 <abort@plt>

Comment 1

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20150325083837" and the hash "e7688fa5ba48".
The "bad" changeset has the timestamp "20150325090835" and the hash "6465d9a4d0dc".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e7688fa5ba48&tochange=6465d9a4d0dc

Comment 2

[Christian Holler (:decoder)]

Bisection points to bug 1143256, needinfo from bhackett based on that.

Comment 3

[Boris Zbarsky [:bzbarsky]]

[Tracking Requested - why for this release]: Seems like this could lead to bad things, like most invariant violations.

Comment 4

[Liz Henry (:lizzard)]

Tracking for 39 and 40 since this is a regression.

Comment 5

[Brian Hackett [Laid off!]]

Attachment: patch

Bug 1143256 improved the checking we do for correct minor GC behavior with object weak maps.  However, we are doing this checking near the beginning of the minor GC, not at the end as seems to be more appropriate.  I haven't tested this thoroughly but this patch moves the hashtable checking to a more sensible place, which fixes this assertion.

Comment 6

[Terrence Cole [:terrence]]

Comment on attachment 8588666 [details] [diff] [review]
patch

Review of attachment 8588666 [details] [diff] [review]:
-----------------------------------------------------------------

Forwarding review to Jon. Maybe he remembers why this is here and not at the end of the minor GC.

Comment 7

[Jon Coppeard (:jonco)]

Comment on attachment 8588666 [details] [diff] [review]
patch

Review of attachment 8588666 [details] [diff] [review]:
-----------------------------------------------------------------

CheckHashTablesAfterMovingGC() was originally written to check that the postbarriers were correctly updating our internal hashtables, so it went after the store buffer marking.

It looks like the issue here is that the DebugScopes::proxiedScopes weakmap is updated by tracing when we mark the runtime, which happens after marking the store buffers.  

Anyway, doing the checking at the end seems like the right place for it.

Brian, thanks for adding the extra weak map checks.

Comment 8

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d36bebd329a

Comment 9

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/mozilla-central/rev/7d36bebd329a

Comment 10

[Liz Henry (:lizzard)]

Brian, should this uplift to Aurora since it seems to be an issue in 39 as well?

Comment 11

[Brian Hackett [Laid off!]]

This change only affects debugging code.

Comment 12

[Liz Henry (:lizzard)]

OK, I'll take that as a "no".