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)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox46 --- wontfix
firefox47 --- fixed

People

(Reporter: gkw, Assigned: jimb)

Details

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

Attachments

(1 file)

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.
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]
I can reproduce.
Flags: needinfo?(jimb)
Assignee: nobody → jimb
Thanks, Jim!
Flags: needinfo?(nfitzgerald)
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.
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.
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)
Thanks to Jim's analysis the fix was easy.
Attachment #8711919 - Flags: review?(jimb)
Flags: needinfo?(shu)
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+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Too late for assertion fixes in 46.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: