Closed Bug 1240736 Opened 5 years ago Closed 5 years ago

Assertion failure: zone->runtimeFromAnyThread()->isHeapBusy(), at js/src/jsgcinlines.h:248 with OOM

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED DUPLICATE of bug 1244412
Tracking Status
firefox46 --- wontfix

People

(Reporter: decoder, Assigned: terrence)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [fuzzblocker] [jsbugmon:])

Attachments

(1 file, 1 obsolete file)

32.75 KB, application/javascript
Details
The following testcase crashes on mozilla-central revision 8cb42e7a16b4 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-offthread-compile=off --ion-check-range-analysis --baseline-eager --ion-extra-checks):

See attachment.


Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0832f70d in js::gc::ZoneCellIterUnderGC::ZoneCellIterUnderGC (this=0xffffa520, zone=0xf7a43000, kind=js::gc::OBJECT_LIMIT) at js/src/jsgcinlines.h:248
#0  0x0832f70d in js::gc::ZoneCellIterUnderGC::ZoneCellIterUnderGC (this=0xffffa520, zone=0xf7a43000, kind=js::gc::OBJECT_LIMIT) at js/src/jsgcinlines.h:248
#1  0x08895bcc in JS::Zone::discardJitCode (this=0xf7a43000, fop=0xf7a3bd7c) at js/src/gc/Zone.cpp:225
#2  0x087c5594 in js::AutoClearTypeInferenceStateOnOOM::~AutoClearTypeInferenceStateOnOOM (this=0xffffa604, __in_chrg=<optimized out>) at js/src/vm/TypeInference.cpp:4427
#3  0x087f529a in mozilla::Maybe<js::AutoClearTypeInferenceStateOnOOM>::reset (this=this@entry=0xffffa600) at js/src/debug32/dist/include/mozilla/Maybe.h:373
#4  0x087c58a7 in ~Maybe (this=0xffffa600, __in_chrg=<optimized out>) at js/src/debug32/dist/include/mozilla/Maybe.h:92
#5  js::ObjectGroup::sweep (this=this@entry=0xf594d148, oom=0xffffa604, oom@entry=0x0) at js/src/vm/TypeInference.cpp:4243
#6  0x081831dd in maybeSweep (oom=0x0, this=0xf594d148) at js/src/vm/ObjectGroup-inl.h:26
#7  flags (this=0xf594d148) at js/src/vm/ObjectGroup-inl.h:32
#8  js::ObjectGroup::unknownProperties (this=0xf594d148) at js/src/vm/ObjectGroup-inl.h:67
#9  0x086fb120 in js::TrackPropertyTypes (obj=obj@entry=0xf5950040, id=..., id@entry=..., cx=<optimized out>) at js/src/vm/TypeInference-inl.h:348
#10 0x08711b63 in AddTypePropertyId (value=..., id=..., obj=0xf5950040, cx=0xf7a7b020) at js/src/vm/TypeInference-inl.h:420
#11 setSlotWithType (overwriting=<optimized out>, value=..., shape=<optimized out>, cx=0xf7a7b020, this=0xf5950040) at js/src/vm/NativeObject-inl.h:286
#12 NativeSetExistingDataProperty (cx=cx@entry=0xf7a7b020, obj=..., obj@entry=..., shape=shape@entry=..., v=v@entry=..., result=..., receiver=...) at js/src/vm/NativeObject.cpp:2040
#13 0x08712ee2 in SetExistingProperty (result=..., shape=..., pobj=..., receiver=..., v=..., id=..., obj=..., cx=0xf7a7b020) at js/src/vm/NativeObject.cpp:2262
#14 js::NativeSetProperty (cx=cx@entry=0xf7a7b020, obj=obj@entry=..., id=id@entry=..., value=value@entry=..., receiver=receiver@entry=..., qualified=qualified@entry=js::Qualified, result=...) at js/src/vm/NativeObject.cpp:2323
#15 0x08272cf5 in SetProperty (result=..., receiver=..., v=..., id=..., obj=..., cx=0xf7a7b020) at js/src/vm/NativeObject.h:1488
#16 js::PutProperty (cx=0xf7a7b020, obj=..., id=..., v=..., strict=false) at js/src/jsobj.h:934
#17 0x086b6035 in js::DefFunOperation (cx=0xf7a7b020, script=..., scopeChain=..., funArg=...) at js/src/vm/Interpreter.cpp:4238
#18 0xf7fc9f71 in ?? ()
#19 0xf7fc8c5c in ?? ()
#20 0x082416bf in EnterBaseline (cx=0xf0ae70d0, cx@entry=0xf7a7b020, data=...) at js/src/jit/BaselineJIT.cpp:135
[...]
#27 0x080e2b14 in runOffThreadScript (cx=0xf7a7b020, argc=0, vp=0xffffafb8) at js/src/shell/js.cpp:3715
#28 0xf7c6d98c in ?? ()
#29 0x0832298c in EnterIon (data=..., cx=0xf7a7b020) at js/src/jit/Ion.cpp:2721
[...]
#34 0x085606d0 in JS_CallFunction (cx=cx@entry=0xf7a7b020, obj=..., fun=fun@entry=..., args=..., rval=rval@entry=...) at js/src/jsapi.cpp:2857
#35 0x086e562a in OOMTest (cx=0xf7a7b020, argc=1, vp=0xffffb700) at js/src/builtin/TestingFunctions.cpp:1218
[...]
#50 0x080ec042 in Evaluate (cx=0xf7a7b020, argc=1, vp=0xffffc270) at js/src/shell/js.cpp:1404
#51 0xf7fd26c6 in ?? ()
#52 0xf584a830 in ?? ()
#53 0xf7c82dcb in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
eax	0x0	0
ebx	0x9840694	159647380
ecx	0xf7e3b88c	-136071028
edx	0x0	0
esi	0xf7a43000	-140234752
edi	0xffffa520	-23264
ebp	0xffffa508	4294944008
esp	0xffffa4c0	4294943936
eip	0x832f70d <js::gc::ZoneCellIterUnderGC::ZoneCellIterUnderGC(JS::Zone*, js::gc::AllocKind)+1021>
=> 0x832f70d <js::gc::ZoneCellIterUnderGC::ZoneCellIterUnderGC(JS::Zone*, js::gc::AllocKind)+1021>:	movl   $0xf8,0x0
   0x832f717 <js::gc::ZoneCellIterUnderGC::ZoneCellIterUnderGC(JS::Zone*, js::gc::AllocKind)+1031>:	call   0x80fe980 <abort()>


The test is sensitive to whitespace and comment line removal and takes about 5 seconds to run which makes it impractical to reduce it further. This happens quite often though, marking as fuzzblocker, in particular because the assert looks like a more generic GC assert that could mask other problems.
Attached file Testcase (obsolete) —
Attached file Testcase
Fix attached test.
Attachment #8709425 - Attachment is obsolete: true
cc'ing our gc folks
Whiteboard: [jsbugmon:update,bisect][fuzzblocker] → [fuzzblocker] [jsbugmon:bisect]
JSBugMon: Cannot process bug: Error: Failed to isolate test from comment
Whiteboard: [fuzzblocker] [jsbugmon:bisect] → [fuzzblocker] [jsbugmon:]
JSBugMon: Bisection requested, failed due to error: Error: Failed to isolate test from comment
Got it to repro in rr. Taking.
Assignee: nobody → terrence
Wow, this is a hard path to hit. The fuzzers once again impress. The proximate cause of this is Brian's incremental type sweeping changes from ages ago, although I'm not sure this particular case would have worked even then.

Here is my interpretation of what is going on:

The type forest is a big place, filled with weak pointers. We used to visit this entire structure non-incrementally in beginSweep, generally blowing out our slice budget. So what Brian's patch did was move to an incremental scheme. At beginSweep, we bump the "generation" on the TI Zone info. Then when sweeping a Script or an ObjectGroup we clean up the associated parts of the type tree. This is still non-incremental, but only visits the dead parts of the tree non-incrementally. The live parts of the tree still need to get visited, however. This happens via a read-barrier on the ObjectGroup's properties that checks the local generation against the TI generation and does sweeping work if they do not match.

This all works fine. What's going off the rails here is that we are hitting the OOM path while we are doing sweeping for a live ObjectGroup. Since these happen outside the GC, the OOM path's attempts to iterate the heap are failing. What these assertions are protecting against is a situation where the background finalization thread mutates the same arena that the foreground is currently iterating over. Before we added these assertions, there were several cases where the foreground could crash by observing a partially dead thing, so these assertions are actually quite useful in practice. I think in this particular case, however, none of the iterated arenas are background finalized, so the current code is actually safe.

I think the proper solution here is to relax the checks such that we are allowed to visit arena kinds that are not background finalized from outside the GC.

Brian, does all of the above make sense? Am I misreading or misremembering any important details?
Status: NEW → ASSIGNED
Flags: needinfo?(bhackett1024)
(In reply to Terrence Cole [:terrence] from comment #7)
> Brian, does all of the above make sense? Am I misreading or misremembering
> any important details?

This all looks right.
Flags: needinfo?(bhackett1024)
Naveed, can you find an assignee?
Flags: needinfo?(nihsanullah)
This is basically the same issue as bug 1244412.  Let's see if terrence r+s my patch in that bug before duping... comment 7 implies that the issue may be more subtle than I realised.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1244412
Jonco is on it and marked as dupe clearing NI
Flags: needinfo?(nihsanullah)
You need to log in before you can comment on or make changes to this bug.