The following testcase crashes on mozilla-central revision 66fb852962c0 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --no-threads --disable-oom-functions main.js): See attachment. Backtrace: Program received signal SIGSEGV, Segmentation fault. JSObject::allocKindForTenure (this=0x7ffff54fff90, nursery=...) at ../../dist/include/js/Proxy.h:434 #0 JSObject::allocKindForTenure (this=0x7ffff54fff90, nursery=...) at ../../dist/include/js/Proxy.h:434 #1 0x0000000000bdccc4 in js::TenuringTracer::moveToTenured (this=0x7fffffec7b50, src=0x7ffff54fff90) at js/src/gc/Marking.cpp:2070 #2 0x0000000000bdd0ab in js::TenuringTracer::traverse<JSObject> (this=<optimized out>, objp=0x7fffffec78d0) at js/src/gc/Marking.cpp:1940 #3 0x0000000000bf15fd in operator()<JSObject> (this=<synthetic pointer>, trc=<optimized out>, t=0x7ffff54fff90) at js/src/gc/Marking.cpp:1946 #4 js::DispatchTyped<js::TenuringTraversalFunctor<JS::Value>, js::TenuringTracer* const>(js::TenuringTraversalFunctor<JS::Value>, JS::Value const&, (decltype ({parm#1}((JSObject*)((decltype(nullptr))0), (Forward<js::TenuringTracer* const>)({parm#3})))&&)...) (f=f@entry=..., val=...) at ../../dist/include/js/Value.h:1885 #5 0x0000000000bdf777 in traverse<JS::Value> (thingp=0x7ffff506e130, this=0x7fffffec7b50) at js/src/gc/Marking.cpp:1955 #6 traceSlots (end=0x7ffff506e138, vp=0x7ffff506e130, this=0x7fffffec7b50) at js/src/gc/Marking.cpp:2163 #7 js::TenuringTracer::traceObject (this=this@entry=0x7fffffec7b50, obj=obj@entry=0x7ffff506e100) at js/src/gc/Marking.cpp:2139 #8 0x0000000000bdfa28 in js::Nursery::collectToFixedPoint (this=this@entry=0x7ffff695d468, mover=..., tenureCounts=...) at js/src/gc/Marking.cpp:2103 #9 0x0000000000be0b14 in js::Nursery::collect (this=this@entry=0x7ffff695d468, rt=0x7ffff695d000, reason=reason@entry=JS::gcreason::OUT_OF_NURSERY, pretenureGroups=pretenureGroups@entry=0x7fffffec7ea0) at js/src/gc/Nursery.cpp:484 #10 0x00000000008e8c65 in js::gc::GCRuntime::minorGCImpl (this=this@entry=0x7ffff695d410, reason=reason@entry=JS::gcreason::OUT_OF_NURSERY, pretenureGroups=pretenureGroups@entry=0x7fffffec7ea0) at js/src/jsgc.cpp:6619 #11 0x00000000008f2709 in js::gc::GCRuntime::minorGC (this=this@entry=0x7ffff695d410, cx=cx@entry=0x7ffff6907400, reason=reason@entry=JS::gcreason::OUT_OF_NURSERY) at js/src/jsgc.cpp:6631 #12 0x0000000000bcc27a in tryNewNurseryObject<(js::AllowGC)1> (clasp=0x1b67900 <js::ArrayObject::class_>, nDynamicSlots=0, thingSize=64, cx=0x7ffff6907400, this=0x7ffff695d410) at js/src/gc/Allocator.cpp:159 #13 js::Allocate<JSObject, (js::AllowGC)1> (cx=cx@entry=0x7ffff6907400, kind=kind@entry=js::gc::OBJECT4_BACKGROUND, nDynamicSlots=0, heap=heap@entry=js::gc::DefaultHeap, clasp=0x1b67900 <js::ArrayObject::class_>) at js/src/gc/Allocator.cpp:125 #14 0x000000000052fc0d in js::ArrayObject::createArrayInternal (cx=0x7ffff6907400, kind=js::gc::OBJECT4_BACKGROUND, heap=js::gc::DefaultHeap, shape=..., group=...) at js/src/vm/ArrayObject-inl.h:54 #15 0x000000000052fe10 in js::ArrayObject::createArray (cx=cx@entry=0x7ffff6907400, kind=kind@entry=js::gc::OBJECT4_BACKGROUND, heap=<optimized out>, shape=..., shape@entry=..., group=..., group@entry=..., length=length@entry=1, metadata=...) at js/src/vm/ArrayObject-inl.h:82 #16 0x0000000000519727 in NewArray<4294967295u> (cxArg=0x7ffff6907400, length=1, protoArg=..., newKind=js::GenericObject) at js/src/jsarray.cpp:3374 #17 0x000000000051c19e in js::NewDenseCopiedArray (cx=<optimized out>, length=length@entry=1, values=values@entry=0x7fffffec82f8, proto=..., proto@entry=..., newKind=<optimized out>) at js/src/jsarray.cpp:3439 #18 0x00000000007fdff5 in js::jit::InitRestParameter (cx=<optimized out>, length=1, rest=0x7fffffec82f8, templateObj=..., objRes=...) at js/src/jit/VMFunctions.cpp:931 #19 0x00007ffff7fed8dd in ?? () [...] #29 0x0000000000000000 in ?? () rax 0x7ffff4f74af0 140737303235312 rbx 0x7ffff54fff90 140737309048720 rcx 0x1ba9c00 29006848 rdx 0xfffc7ffff4f724c0 -985162603617088 rsi 0x7ffff695d468 140737330402408 rdi 0x7ffff54fff90 140737309048720 rbp 0x7fffffec7820 140737487075360 rsp 0x7fffffec7800 140737487075328 r8 0x0 0 r9 0x7ffff5107150 140737304883536 r10 0x7ffff5107000 140737304883200 r11 0x1f 31 r12 0x1bb4800 29050880 r13 0x1bb5268 29053544 r14 0x7fffffec7900 140737487075584 r15 0x7ffff506e100 140737304256768 rip 0x92b8a4 <JSObject::allocKindForTenure(js::Nursery const&) const+68> => 0x92b8a4 <JSObject::allocKindForTenure(js::Nursery const&) const+68>: mov 0x8(%rdx),%ecx 0x92b8a7 <JSObject::allocKindForTenure(js::Nursery const&) const+71>: test $0x100000,%ecx Marking as s-s because this is a fragile GC-related crash with weird memory address. I was not able to merge the files into a single-file testcase. Bonus points if you can show me how to do that (in particular, replacing load with evaluate totally did not work in this testcase, not even with all the options I tried for evaluate).

This is an automated crash issue comment: Summary: Crash [@ JSObject::allocKindForTenure] Build version: mozilla-central revision 388bdc46ba51 Build flags: --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug Runtime options: --fuzzing-safe --thread-count=2 --ion-eager --ion-extra-checks --ion-offthread-compile=off Testcase: function runTestCase() testcase(); setJitCompilerOption("ion.warmup.trigger", 40); gczeal(2); function testcase(... of) { function foo() testcase(foo); this; foo(); } runTestCase(); Backtrace: Program received signal SIGSEGV, Segmentation fault. JSObject::allocKindForTenure (this=0x7ffff4803ae0, nursery=...) at js/src/jsobj.h:122 #0 JSObject::allocKindForTenure (this=0x7ffff4803ae0, nursery=...) at js/src/jsobj.h:122 #1 0x0000000000bdcb64 in js::TenuringTracer::moveToTenured (this=0x7ffffffee2d0, src=0x7ffff4803ae0) at js/src/gc/Marking.cpp:2070 #2 0x0000000000bdcf4b in js::TenuringTracer::traverse<JSObject> (this=<optimized out>, objp=0x7ffffffee050) at js/src/gc/Marking.cpp:1940 #3 0x0000000000bf149d in operator()<JSObject> (this=<synthetic pointer>, trc=<optimized out>, t=0x7ffff4803ae0) at js/src/gc/Marking.cpp:1946 #4 js::DispatchTyped<js::TenuringTraversalFunctor<JS::Value>, js::TenuringTracer* const>(js::TenuringTraversalFunctor<JS::Value>, JS::Value const&, (decltype ({parm#1}((JSObject*)((decltype(nullptr))0), (Forward<js::TenuringTracer* const>)({parm#3})))&&)...) (f=f@entry=..., val=...) at ../../dist/include/js/Value.h:1885 #5 0x0000000000bdf617 in traverse<JS::Value> (thingp=0x7ffff7e88770, this=0x7ffffffee2d0) at js/src/gc/Marking.cpp:1955 #6 traceSlots (end=0x7ffff7e88778, vp=0x7ffff7e88770, this=0x7ffffffee2d0) at js/src/gc/Marking.cpp:2163 #7 js::TenuringTracer::traceObject (this=this@entry=0x7ffffffee2d0, obj=obj@entry=0x7ffff7e88740) at js/src/gc/Marking.cpp:2139 #8 0x0000000000bdf8c8 in js::Nursery::collectToFixedPoint (this=this@entry=0x7ffff695d468, mover=..., tenureCounts=...) at js/src/gc/Marking.cpp:2103 #9 0x0000000000be09b4 in js::Nursery::collect (this=this@entry=0x7ffff695d468, rt=0x7ffff695d000, reason=reason@entry=JS::gcreason::DEBUG_GC, pretenureGroups=pretenureGroups@entry=0x0) at js/src/gc/Nursery.cpp:484 #10 0x00000000008e8b85 in js::gc::GCRuntime::minorGCImpl (this=this@entry=0x7ffff695d410, reason=reason@entry=JS::gcreason::DEBUG_GC, pretenureGroups=pretenureGroups@entry=0x0) at js/src/jsgc.cpp:6619 #11 0x0000000000915c13 in js::gc::GCRuntime::evictNursery (this=0x7ffff695d410, reason=JS::gcreason::DEBUG_GC) at js/src/gc/GCRuntime.h:611 #12 0x0000000000909e95 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff695d410, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6227 #13 0x000000000090a541 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff695d410, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6388 #14 0x000000000090a773 in js::gc::GCRuntime::gc (this=0x7ffff695d410, gckind=<optimized out>, reason=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6446 #15 0x000000000090bcc5 in js::gc::GCRuntime::runDebugGC (this=this@entry=0x7ffff695d410) at js/src/jsgc.cpp:6931 #16 0x0000000000bbd987 in js::gc::GCRuntime::gcIfNeededPerAllocation (this=this@entry=0x7ffff695d410, cx=cx@entry=0x7ffff6907400) at js/src/gc/Allocator.cpp:28 #17 0x0000000000bc6b6f in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=0x7ffff695d410, cx=0x7ffff6907400, kind=js::gc::OBJECT4_BACKGROUND) at js/src/gc/Allocator.cpp:55 #18 0x0000000000bcc057 in js::Allocate<JSObject, (js::AllowGC)1> (cx=cx@entry=0x7ffff6907400, kind=kind@entry=js::gc::OBJECT4_BACKGROUND, nDynamicSlots=0, heap=heap@entry=js::gc::DefaultHeap, clasp=0x1b67920 <js::ArrayObject::class_>) at js/src/gc/Allocator.cpp:121 #19 0x000000000052fbcd in js::ArrayObject::createArrayInternal (cx=0x7ffff6907400, kind=js::gc::OBJECT4_BACKGROUND, heap=js::gc::DefaultHeap, shape=..., group=...) at js/src/vm/ArrayObject-inl.h:54 #20 0x000000000052fdd0 in js::ArrayObject::createArray (cx=cx@entry=0x7ffff6907400, kind=kind@entry=js::gc::OBJECT4_BACKGROUND, heap=<optimized out>, shape=..., shape@entry=..., group=..., group@entry=..., length=length@entry=1, metadata=...) at js/src/vm/ArrayObject-inl.h:82 #21 0x0000000000519717 in NewArray<4294967295u> (cxArg=0x7ffff6907400, length=1, protoArg=..., newKind=js::GenericObject) at js/src/jsarray.cpp:3374 #22 0x000000000051c18e in js::NewDenseCopiedArray (cx=<optimized out>, length=length@entry=1, values=values@entry=0x7ffffffeed78, proto=..., proto@entry=..., newKind=<optimized out>) at js/src/jsarray.cpp:3439 #23 0x00000000007fdf95 in js::jit::InitRestParameter (cx=<optimized out>, length=1, rest=0x7ffffffeed78, templateObj=..., objRes=...) at js/src/jit/VMFunctions.cpp:925 #24 0x00007ffff7fed8dd in ?? () #25 0x00007ffff694e800 in ?? () #26 0x00007ffffffeecd0 in ?? () #27 0x0000000001bc43c0 in js::jit::GetPropertyInfo () #28 0x00007ffff7e5a580 in ?? () #29 0x00007ffff7fd4225 in ?? () #30 0x0000000000000c00 in ?? () #31 0x0000000000000001 in ?? () #32 0x00007ffffffeed78 in ?? () #33 0x00007ffff7e84040 in ?? () #34 0x0000000000000000 in ?? () rax 0x130000 1245184 rbx 0x7ffff4803ae0 140737295432416 rcx 0x1b67920 28735776 rdx 0x7ffff695d468 140737330402408 rsi 0x7ffff695d468 140737330402408 rdi 0x7ffff4803ae0 140737295432416 rbp 0x7ffffffedfa0 140737488281504 rsp 0x7ffffffedf80 140737488281472 r8 0x0 0 r9 0x0 0 r10 0x7ffff69629f0 140737330424304 r11 0x7ffff6962908 140737330424072 r12 0x7ffffffee2d0 140737488282320 r13 0x7ffff7e88778 140737352599416 r14 0x7ffffffee080 140737488281728 r15 0x7ffff7e88740 140737352599360 rip 0x92b798 <JSObject::allocKindForTenure(js::Nursery const&) const+24> => 0x92b798 <JSObject::allocKindForTenure(js::Nursery const&) const+24>: mov (%rax),%rdx 0x92b79b <JSObject::allocKindForTenure(js::Nursery const&) const+27>: cmp %rcx,%rdx Managed to find a single-file testcase for this now :)

Assuming sec-high due to the crash address being read here.

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/52d7c9292ecf user: Jan de Mooij date: Sat Nov 21 14:33:13 2015 +0100 summary: Bug 1132183 - Make |this| a real binding, remove lazy this computation. r=efaust,shu This iteration took 7.407 seconds to run.

Jan, is bug 1132183 a likely regressor?

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #6) > Jan, is bug 1132183 a likely regressor? No. The test uses 'this' and that bug changed how 'this' is implemented, but it should be possible to write a test that fails at an earlier revision. The problem here is MRest codegen reading the frame's formals directly but MarkThisAndArguments not always marking the formals. We do mark the formals explicitly if the function uses 'arguments' and we just need to do something similar for 'this'. Will post a patch later.

(In reply to Jan de Mooij [:jandem] from comment #7) > We do mark the formals > explicitly if the function uses 'arguments' and we just need to do something > similar for 'this'. Er, s/'this'/rest/

I can still reproduce using the testcase in comment 2: Summary: Crash [@ JSObject::allocKindForTenure] Build version: mozilla-central revision 7c83da46ea74 Build flags: --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug Runtime options: --fuzzing-safe --no-threads --ion-eager Testcase: function runTestCase() testcase(); setJitCompilerOption("ion.warmup.trigger", 40); gczeal(2); function testcase(... of) { function foo() testcase(foo); this; foo(); } runTestCase();

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

(In reply to Fuzzing Team from comment #12) > JSBugMon: The testcase found in this bug no longer reproduces (tried > revision 7c83da46ea74). Sigh. I think this is an intermittent bug, though fairly reliably reproducible on some machines.

Treat functions with rest parameters like functions with an arguments object: don't spill to their formal argument slots and always mark the formals. To avoid duplicating this logic in multiple places, I added a helper method, script->mayReadFrameArgsDirectly().

This causes all sorts of other crashes that don't have any REST-associated function in the stack anywhere. Please land asap, thanks!

Comment on attachment 8703121 [details] [diff] [review] Patch Review of attachment 8703121 [details] [diff] [review]: ----------------------------------------------------------------- This sounds good to me.

Comment on attachment 8703121 [details] [diff] [review] Patch [Security approval request comment] * How easily could an exploit be constructed based on the patch? It's not trivial to trigger a GC right there, but with some effort it might be possible for someone to figure out the problem and find a reliable way to do that. * Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. * Which older supported branches are affected by this flaw? 38+ * If not all supported branches, which bug introduced the flaw? Bug 934502. * Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should apply or be easy to backport. * How likely is this patch to cause regressions; how much testing does it need? Unlikely.

sec-approval+ for trunk. Please prepare and nominate branch patches (including ESR38) as well after checkin.

Comment on attachment 8703121 [details] [diff] [review] Patch Approval Request Comment [Feature/regressing bug #]: Bug 934502. [User impact if declined]: Security bugs, crashes. [Describe test coverage new/current, TreeHerder]: Fixes the crash. [Risks and why]: Low risk. Furthermore, rest parameters are relatively new so they are not used much on the web. [String/UUID change made/needed]: None.

JSBugMon: This bug has been automatically verified fixed.

Comment on attachment 8703121 [details] [diff] [review] Patch Sec-fix that was verified, taking it to beta, aurora, esr38.

This doesn't apply cleanly to aurora or beta (so probably definitely doesn't apply cleanly to esr38). Can we get rebased patches?

https://hg.mozilla.org/releases/mozilla-aurora/rev/6b77bca9e1dc https://hg.mozilla.org/releases/mozilla-beta/rev/51f069f6fe91 https://hg.mozilla.org/releases/mozilla-esr38/rev/f31d643afd41

JSBugMon: This bug has been automatically verified fixed on Fx45