1148383 - Crash [@ zone] or Crash [@ CheckMarkedThing<JSObject>] with setObjectMetadataCallback

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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

function TestCase() {}
function reportCompare () {
  var output = "";
  var testcase = new TestCase();
  testcase.reason = output;
}
reportCompare();
gczeal(4);
setObjectMetadataCallback(true);
for (var i = 0; i < 1000000; ++i)
  reportCompare();



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
zone (this=0x0) at js/src/gc/Heap.h:1355
#0  zone (this=0x0) at js/src/gc/Heap.h:1355
#1  zone (this=<optimized out>) at js/src/jsobj.h:275
#2  MarkInternal<JSObject> (trc=0x7ffff6943fb8, thingp=<optimized out>) at js/src/gc/Marking.cpp:292
#3  0x0000000000529839 in js::gc::MarkKind (trc=<optimized out>, thingp=thingp@entry=0x7fffffffd0c0, kind=<optimized out>) at js/src/gc/Marking.cpp:647
#4  0x00000000005a9df5 in writeBarrierPre (thing=<optimized out>) at js/src/gc/Heap.h:1403
#5  writeBarrierPre (obj=<optimized out>) at js/src/jsobj.h:602
#6  js::InternalGCMethods<JSObject*>::preBarrier (v=<optimized out>) at js/src/gc/Barrier.h:297
#7  0x0000000000949861 in pre (this=0x7fffffffd0f0) at js/src/gc/Barrier.h:452
#8  ~BarrieredBase (this=0x7fffffffd0f0, __in_chrg=<optimized out>) at js/src/gc/Barrier.h:426
#9  ~PreBarriered (this=0x7fffffffd0f0, __in_chrg=<optimized out>) at js/src/gc/Barrier.h:472
#10 ~HashMapEntry (this=0x7fffffffd0f0, __in_chrg=<optimized out>) at ../../dist/include/js/HashTable.h:627
#11 add<JSObject*&, JS::Value> (k=<optimized out>, v=<unknown type in /home/decoder/LangFuzz/work/remote/builds/opt64/dist/bin/js, CU 0x24acefe, DIE 0x25b8dba>, p=<synthetic pointer>, this=<optimized out>) at ../../dist/include/js/HashTable.h:140
#12 put<JSObject*&, JS::Value> (v=<unknown type in /home/decoder/LangFuzz/work/remote/builds/opt64/dist/bin/js, CU 0x24acefe, DIE 0x267c7d1>, k=<synthetic pointer>, this=<optimized out>) at ../../dist/include/js/HashTable.h:224
#13 js::ObjectWeakMap::add (this=0x7ffff51fe0b0, cx=cx@entry=0x7ffff69820f0, obj=obj@entry=0x7ffff5277c70, target=target@entry=0x7ffff5278820) at js/src/jsweakmap.cpp:702
#14 0x000000000086ed5c in JSCompartment::setNewObjectMetadata (this=0x7ffff6914400, cx=cx@entry=0x7ffff69820f0, obj=obj@entry=0x7ffff5277c70) at js/src/jscompartment.cpp:721
#15 0x00000000008283b6 in SetNewObjectMetadata (obj=0x7ffff5277c70, cxArg=0x7ffff69820f0) at js/src/jsobjinlines.h:249
#16 js::jit::NewGCObject (cx=0x7ffff69820f0, allocKind=<optimized out>, initialHeap=<optimized out>, ndynamic=<optimized out>, clasp=<optimized out>) at js/src/jit/VMFunctions.cpp:97
#17 0x00007ffff7febdd2 in ?? ()
#18 0x00007ffff51fe100 in ?? ()
#19 0x00007fffffffd1f8 in ?? ()
#20 0x00007ffff6995bb0 in ?? ()
#21 0x00000000017266e0 in js::jit::NewIonArgumentsObjectInfo ()
#22 0x00007ffff5250fd0 in ?? ()
#23 0x00007ffff7ff5ac8 in ?? ()
#24 0x0000000000000380 in ?? ()
#25 0x0000000000000003 in ?? ()
#26 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff6943fb8	140737330298808
rcx	0xf104	61700
rdx	0xffffffffff9ea288	-6380920
rsi	0x0	0
rdi	0x7ffff6943fb8	140737330298808
rbp	0x7ffff5277c70	140737306393712
rsp	0x7fffffffd0a0	140737488343200
r8	0x1	1
r9	0x1	1
r10	0x0	0
r11	0x0	0
r12	0x7ffff51fe0b0	140737305895088
r13	0x7ffff69820f0	140737330553072
r14	0x7fffffffd0f0	140737488343280
r15	0x20	32
rip	0x518dc5 <MarkInternal<JSObject>(JSTracer*, JSObject**)+69>
=> 0x518dc5 <MarkInternal<JSObject>(JSTracer*, JSObject**)+69>:	mov    (%rax),%rax
   0x518dc8 <MarkInternal<JSObject>(JSTracer*, JSObject**)+72>:	mov    (%rax),%rdx


S-s because this is a GC crash and I'm also seeing crashes with uaf crash pattern.

Comment 1

[Christian Holler (:decoder)]

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/d3c9b899f7d2
user:        Brian Hackett
date:        Fri Mar 20 06:33:33 2015 -0700
summary:     Bug 1143256 - Store object metadata using a weak map, r=luke.

This iteration took 151.476 seconds to run.

Comment 2

[Jon Coppeard (:jonco)]

This seems to be fallout from bug 1143256 which landed yesterday.

What's happening is that we are setting the metadata for an object before that object has been fully initialized when allocating in the JIT via NewGCObject().  If incremental barriers are active on the zone we try and mark it, which causes us to crash when we try to call zone() on it.

Comment 3

[Andrew McCreight [:mccr8]]

Setting to sec-moderate because it is a use-after-free related to setObjectMetadata, which is not going to be used outside of a debugger scenario.

Comment 4

[Christian Holler (:decoder)]

I'm also hitting the same signature with enableTrackAllocations(), is this likely the same bug?

NI from jonco for that question.

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Christian Holler (:decoder) from comment #4)

Yes, I expect it's the same crash just with a different way of turning on object metadata.

Comment 6

[Jon Coppeard (:jonco)]

There are a few possible approaches to fixing this but the simplest bug 1149135, which means that the barrier that is causing the problem is not fired at this point.

Comment 7

[Jon Coppeard (:jonco)]

Attachment: bug1148383-metadata-test

This bug is fixed by the change in bug 1149135.  This patch just adds the testcase (tweaked to take less time).

Comment 8

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 944fb6e682b8).

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Christian, can you verify this on aurora as well after the patch for bug 1149135 lands? Thanks!

Comment 10

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/66a81343c150

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/66a81343c150

Comment 12

[Christian Holler (:decoder)]

JSBugMon should automatically attempt to verify this once the bug is marked as FIXED (and the branch is marked as fixed as well).

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

OK, should it be marked as fixed here then, since it is fixed on 39 in bug 1149135?   
Or, do you intend to uplift the testcase?  
Either way; I'd like to make sure things are resolved before I untrack this. Thanks!

Comment 14

[Christian Holler (:decoder)]

Forwarding the needinfo to the developer that fixed the issue.

Comment 15

[Jon Coppeard (:jonco)]

Comment on attachment 8587364 [details] [diff] [review]
bug1148383-metadata-test

Might as well uplift the test to make sure this is fixed.

Approval Request Comment
[Feature/regressing bug #]: Bug 1143256.
[User impact if declined]: None, this is test code only - the issue was fixed by bug 1149135.
[Describe test coverage new/current, TreeHerder]: On central for 6 days.
[Risks and why]: Low.
[String/UUID change made/needed]: None.

Comment 16

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8587364 [details] [diff] [review]
bug1148383-metadata-test

Approved for uplift to aurora since the fix in bug 1149135 landed and this is test-only anyway.

Comment 18

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3bbd67c48d8d