Closed
Bug 1242111
Opened 9 years ago
Closed 9 years ago
Assertion failure: !script->baselineScript()->active(), at js/src/gc/Zone.cpp:227
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: gkw, Assigned: jimb)
Details
(Keywords: assertion, regression, testcase, Whiteboard: [fuzzblocker][jsbugmon:update])
Attachments
(1 file)
3.94 KB,
patch
|
jimb
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 7104d650a97d (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --ion-eager):
// Adapted from randomly chosen test: js/src/jit-test/tests/debug/bug1240546.js
var g = newGlobal();
g.debuggeeGlobal = [];
g.eval("(" + function() {
oomAfterAllocations(57);
Debugger(debuggeeGlobal).onEnterFrame = function() {}
} + ")()");
Backtrace:
0 js-dbg-64-dm-clang-darwin-7104d650a97d 0x000000010095f2a1 JS::Zone::discardJitCode(js::FreeOp*) + 433 (Zone.cpp:227)
1 js-dbg-64-dm-clang-darwin-7104d650a97d 0x00000001005a56eb js::gc::GCRuntime::beginSweepingZoneGroup() + 2763 (FindSCCs.h:33)
2 js-dbg-64-dm-clang-darwin-7104d650a97d 0x00000001005a6d38 js::gc::GCRuntime::beginSweepPhase(bool) + 1432 (jsgc.cpp:5380)
3 js-dbg-64-dm-clang-darwin-7104d650a97d 0x00000001005aab97 js::gc::GCRuntime::incrementalCollectSlice(js::SliceBudget&, JS::gcreason::Reason) + 951 (SliceBudget.h:77)
4 js-dbg-64-dm-clang-darwin-7104d650a97d 0x00000001005ab77c js::gc::GCRuntime::gcCycle(bool, js::SliceBudget&, JS::gcreason::Reason) + 460 (jsgc.cpp:6355)
5 js-dbg-64-dm-clang-darwin-7104d650a97d 0x00000001005ac135 js::gc::GCRuntime::collect(bool, js::SliceBudget, JS::gcreason::Reason) + 757 (jsgc.cpp:6455)
6 js-dbg-64-dm-clang-darwin-7104d650a97d 0x000000010059be76 js::gc::GCRuntime::gc(JSGCInvocationKind, JS::gcreason::Reason) + 86 (jsgc.cpp:6514)
7 js-dbg-64-dm-clang-darwin-7104d650a97d 0x000000010053eb9e js::DestroyContext(JSContext*, js::DestroyContextMode) + 462 (Utility.h:384)
8 js-dbg-64-dm-clang-darwin-7104d650a97d 0x0000000100004abc main + 12284 (js.cpp:308)
9 js-dbg-64-dm-clang-darwin-7104d650a97d 0x0000000100000ea4 start + 52
This is happening often just enough to warrant a [fuzzblocker] status.
![]() |
Reporter | |
Comment 1•9 years ago
|
||
Seems to go back further than m-c rev 54be5416ae5d. I guess the reason why this just popped up now is because the test for bug 1240546 only just landed.
Setting needinfo? from our Debugger gurus, Jim and Nick.
Flags: needinfo?(nfitzgerald)
Flags: needinfo?(jimb)
Whiteboard: [fuzzblocker][jsbugmon:update,bisect] → [fuzzblocker][jsbugmon:update]
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jimb
Assignee | ||
Comment 4•9 years ago
|
||
The test injects an OOM when a HashTable tries to allocate a smaller table because the current table is underloaded. The error is ignored, because this is supposed to be a benign failure. This all occurs during GC, which is not supposed to OOM.
Assignee | ||
Comment 5•9 years ago
|
||
Cancel that; this test runs with OOM_failAlways set, so there are twenty OOMs injected into the program before it crashes; the failure I was looking at in comment 4 is probably innocent.
Assignee | ||
Comment 6•9 years ago
|
||
Okay, this is looking more plausible.
Note that the --no-threads and --ion-eager flags are required to see the crash; and since it depends on a specific allocation failing, you probably want to reproduce with the exact changeset suggested.
The test case injects OOMs starting with the 57th allocation onwards. If I let the 57th allocation pass, but begin failing with the 58th, the test case doesn't crash.
The 57th allocation is an attempt to grow a vector, as requested by the call to AppendAndInvalidateScript in UpdateExecutionObservabilityOfScriptsInZone. The crash is a failure of the assertion:
MOZ_ASSERT_IF(script->hasBaselineScript(), !script->baselineScript()->active());
Since there is a call to FinishDiscardBaselineScript just after the call to AppendAndInvalidateScript, and that specific allocation must fail to trigger the bug, I suspect that we are not backing out of UpdateExecutionObservabilityOfScriptsInZone correctly.
Shu, would you be willing to take a look at this?
Flags: needinfo?(shu)
Comment 7•9 years ago
|
||
Thanks to Jim's analysis the fix was easy.
Attachment #8711919 -
Flags: review?(jimb)
Updated•9 years ago
|
Flags: needinfo?(shu)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8711919 [details] [diff] [review]
Handle OOM in UpdateExecutionObservabilityOfScriptsInZone.
Review of attachment 8711919 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me - thanks for the fix!
I worry that the regression test will stop being useful almost immediately, as the magic number '57' will become wrong. If it's easy to make it sweep a range of allocation counts (as with 'oomTest' or something like that) and still catch the crash, that might give us more robust coverage. But that's not a condition for r=me.
Attachment #8711919 -
Flags: review?(jimb) → review+
Comment 10•9 years ago
|
||
Comment 11•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/23f22124b285
https://hg.mozilla.org/mozilla-central/rev/834c46411298
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment 12•9 years ago
|
||
Too late for assertion fixes in 46.
You need to log in
before you can comment on or make changes to this bug.
Description
•