Closed Bug 976021 Opened 6 years ago Closed 6 years ago

GenerationalGC: Crash [@ GetGCThingRuntime] with poisoned pointer

Categories

(Core :: JavaScript Engine, defect, major)

x86
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: decoder, Assigned: terrence)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(1 file)

The following testcase crashes on mozilla-central built with --enable-exact-rooting --enable-gcgenerational, revision d9c58306bfbc (run with --fuzzing-safe --ion-eager):


for (;; RegExp = Array(RegExp, [""])) {}
Crash trace:

Program received signal SIGSEGV, Segmentation fault.
GetGCThingRuntime (thing=0x2b2b2b2b) at ../../dist/include/js/HeapAPI.h:133
133         return *reinterpret_cast<JS::shadow::Runtime **>(addr);
(gdb) bt 32
#0  GetGCThingRuntime (thing=0x2b2b2b2b) at ../../dist/include/js/HeapAPI.h:133
#1  isTenured (this=0x2b2b2b2b) at  js/src/gc/Heap.h:1055
#2  js::gc::Cell::arenaHeader (this=0x2b2b2b2b) at  js/src/gc/Heap.h:964
#3  0x080b0817 in tenuredZone (this=0x2b2b2b2b) at  js/src/gc/Heap.h:1024
#4  zone (this=0x2b2b2b2b) at  js/src/gc/Barrier.h:185
#5  js::gc::BarrieredCell<js::ObjectImpl>::zone (this=0xf69fffa0) at  js/src/vm/ObjectImpl.h:1540
#6  0x08191afe in js::GCMarker::processMarkStackTop (this=0x9479278, budget=...) at  js/src/gc/Marking.cpp:1468
#7  0x08157684 in js::GCMarker::drainMarkStack (this=0x9479278, budget=...) at  js/src/gc/Marking.cpp:1554
#8  0x084a8b6e in DrainMarkStack (phase=js::gcstats::PHASE_MARK, sliceBudget=..., rt=0x9478ff8) at  js/src/jsgc.cpp:4067
#9  IncrementalCollectSlice (rt=0x9478ff8, budget=156190872, reason=JS::gcreason::ALLOC_TRIGGER, gckind=js::GC_NORMAL) at  js/src/jsgc.cpp:4627
#10 0x084aad00 in GCCycle (rt=0x9478ff8, incremental=<optimized out>, budget=0, gckind=js::GC_NORMAL, reason=JS::gcreason::ALLOC_TRIGGER) at  js/src/jsgc.cpp:4791
#11 0x084ab3df in Collect (rt=0x9478ff8, incremental=<optimized out>, budget=0, gckind=js::GC_NORMAL, reason=JS::gcreason::ALLOC_TRIGGER) at  js/src/jsgc.cpp:4929
#12 0x084abaf7 in Collect (reason=JS::gcreason::ALLOC_TRIGGER, gckind=js::GC_NORMAL, budget=0, incremental=true, rt=0x9478ff8) at  js/src/jsgc.cpp:4859
#13 GCSlice (reason=JS::gcreason::ALLOC_TRIGGER, gckind=js::GC_NORMAL, rt=0x9478ff8, millis=<optimized out>) at  js/src/jsgc.cpp:4974
#14 js::gc::GCIfNeeded (cx=0x94855d8) at  js/src/jsgc.cpp:5074
#15 0x083f6f65 in js_InvokeOperationCallback (cx=0x94855d8) at  js/src/jscntxt.cpp:1024
#16 js_HandleExecutionInterrupt (cx=0x94855d8) at  js/src/jscntxt.cpp:1051
#17 0xf66efa63 in ?? ()
Whiteboard: [jsbugmon:ignore] → [jsbugmon:update,bisect]
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 975959
Sorry, wrong bug.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/357895a6aaac
user:        Jon Coppeard
date:        Wed Feb 05 14:09:41 2014 +0000
summary:     Bug 965745 - Always patch loop backedges in Ion if interrupt flag is set r=jandem

This iteration took 369.611 seconds to run.
The bisected bug only uncovered the pre-existing bug. This is very similar to 975959: barriers are omitted from MCallOptimize's Array call inlining because the new Array will "always" be in the nursery. Sadly this ends up not happening because the interpreter path hits the cache and object allocation from the cache is NoGC, which falls back to the tenured heap -- e.g. bug 919544. I think the right solution here is to finish up the patch on that bug.
Assignee: nobody → terrence
Of course even once we fix the first bug, the MNewArray is initialized with an initial generation based on the type, actively flouting the comment.
Add the requisite barriers. I tested with appropriate printfs to ensure these don't actually get exercised in any octane benchmarks, so performance shouldn't be an issue.
Attachment #8383356 - Flags: review?(jdemooij)
Comment on attachment 8383356 [details] [diff] [review]
fix_new_array_call_with_many_args-v0.diff

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

Good catch.

::: js/src/jit/MCallOptimize.cpp
@@ +298,5 @@
>  
> +            // There is normally no need for a post barrier on these writes
> +            // because the new array will be in the nursery. However, this
> +            // assumption is volated if we specifically requested pre-tenuring.
> +            if (templateObject->type()->initialHeap(constraints()) == gc::TenuredHeap)

Nit: it's slightly more efficient to do ins->initialHeap() == gc::TenuredHeap, so that we don't potentially add a constraint for each element.
Attachment #8383356 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/49b9cdf425f7
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.