1335643 - Assertion failure: rt->gc.atomMarking.atomIsMarked(compartment->zone(), cell), at js/src/jscntxtinlines.h:92 with saveStack

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 9c06e744b1be (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off min.js):

low = high = newGlobal({})
high.low = low
high.eval("function a() { return saveStack(1, low) }")
set = eval("high.a()")
serialize(set)


Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000535be8 in js::CompartmentChecker::checkAtom (cell=<optimized out>, this=<optimized out>) at js/src/jscntxtinlines.h:92
#0  0x0000000000535be8 in js::CompartmentChecker::checkAtom (cell=<optimized out>, this=<optimized out>) at js/src/jscntxtinlines.h:92
#1  js::CompartmentChecker::check (symbol=<optimized out>, this=<optimized out>) at js/src/jscntxtinlines.h:107
#2  js::CompartmentChecker::check (this=0x7fffffffcdd0, v=...) at js/src/jscntxtinlines.h:116
#3  0x0000000000936abc in js::CompartmentChecker::check<JS::Value> (handle=..., this=0x7fffffffcdd0) at js/src/jscntxtinlines.h:83
#4  js::assertSameCompartment<JS::Handle<JS::Value> > (cx=0x7ffff6946000, t1=...) at js/src/jscntxtinlines.h:189
#5  0x0000000000bcfd5c in JSStructuredCloneWriter::startWrite (this=this@entry=0x7fffffffd1a0, v=...) at js/src/vm/StructuredClone.cpp:1378
#6  0x0000000000bd0e3d in JSStructuredCloneWriter::traverseSavedFrame (this=this@entry=0x7fffffffd1a0, obj=..., obj@entry=...) at js/src/vm/StructuredClone.cpp:1351
#7  0x0000000000bd035f in JSStructuredCloneWriter::startWrite (this=this@entry=0x7fffffffd1a0, v=...) at js/src/vm/StructuredClone.cpp:1448
#8  0x0000000000bd263d in JSStructuredCloneWriter::write (v=..., this=0x7fffffffd1a0) at js/src/vm/StructuredClone.cpp:1616
#9  WriteStructuredClone (cx=cx@entry=0x7ffff6946000, v=..., bufp=bufp@entry=0x7fffffffd660, scope=scope@entry=JS::StructuredCloneScope::SameProcessSameThread, cloneDataPolicy=..., cloneDataPolicy@entry=..., cb=cb@entry=0x0, cbClosure=0x0, transferable=...) at js/src/vm/StructuredClone.cpp:541
#10 0x0000000000bd275c in JS_WriteStructuredClone (cx=cx@entry=0x7ffff6946000, value=..., bufp=bufp@entry=0x7fffffffd660, scope=JS::StructuredCloneScope::SameProcessSameThread, cloneDataPolicy=..., cloneDataPolicy@entry=..., optionalCallbacks=optionalCallbacks@entry=0x0, closure=0x0, transferable=...) at js/src/vm/StructuredClone.cpp:2497
#11 0x0000000000bd27db in JSAutoStructuredCloneBuffer::write (this=this@entry=0x7fffffffd658, cx=cx@entry=0x7ffff6946000, value=..., transferable=..., cloneDataPolicy=..., cloneDataPolicy@entry=..., optionalCallbacks=optionalCallbacks@entry=0x0, closure=0x0) at js/src/vm/StructuredClone.cpp:2662
#12 0x000000000086a961 in Serialize (cx=cx@entry=0x7ffff6946000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:2449
#13 0x0000000000535d09 in js::CallJSNative (cx=cx@entry=0x7ffff6946000, native=0x86a5f0 <Serialize(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:262
[...]
#26 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:7975
rax	0x0	0
rbx	0x7ffff06aa128	140737226907944
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffcdb0	140737488342448
rsp	0x7fffffffcda0	140737488342432
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7fffffffcdd0	140737488342480
r13	0x7fffffffcf30	140737488342832
r14	0x7fffffffcf10	140737488342800
r15	0x7fffffffcf70	140737488342896
rip	0x535be8 <js::CompartmentChecker::check(JS::Value const&)+376>
=> 0x535be8 <js::CompartmentChecker::check(JS::Value const&)+376>:	movl   $0x0,0x0
   0x535bf3 <js::CompartmentChecker::check(JS::Value const&)+387>:	ud2    


Another GC assert, but I'm not sure if this one is shell-only or not (due to saveStack being involved). Marking s-s anyway until triaged.

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/7311c06a7271
user:        Brian Hackett
date:        Mon Jan 30 06:31:47 2017 -0700
summary:     Bug 1324002 - Mark atoms separately in each zone, r=jonco,mccr8,peterv.

This iteration took 0.437 seconds to run.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Brian, is bug 1324002 a likely regressor?

Comment 3

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]:

Comment 4

[Brian Hackett [Laid off!]]

Attachment: patch

This is a false positive, JSStructuredCloneWriter::startWrite always asserts the value is in the same compartment as the context, but for non-objects it just writes out the value without storing it anywhere.  This patch only checks when objects are being written out, though an alternative I guess would be to mark the atoms in the context when the caller is inspecting an unwrapped saved frame.

Comment 5

[Marcia Knous [:marcia]]

Tracking 54+ for this sec high issue.

Comment 6

[Jon Coppeard (:jonco)]

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

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

I think I'd like to keep the assertion as it is since it seems like an invariant that we should enforce.  Can you try your alternative approach?

If you really want to do this you could ask Steve what he thinks as he knows this code better then me.

Comment 7

[Brian Hackett [Laid off!]]

Attachment: patch

Sure, here's the alternative patch.

Comment 8

[Jon Coppeard (:jonco)]

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

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

Thanks.

Comment 9

[Andrew McCreight [:mccr8]]

Unhiding per comment 4.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5475dc2e1f4
Fix bogus assert by marking atoms when serializing cross-zone saved frames, r=jonco.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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