Closed
Bug 1218900
Opened 9 years ago
Closed 9 years ago
Assertion failure: phaseNestingDepth == 0, at gc/Statistics.cpp
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla45
People
(Reporter: gkw, Assigned: jonco)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
readline = function() {};
// Adapted from randomly chosen test: js/src/jit-test/tests/basic/testGuardCalleeSneakAttack2.js
Function.prototype.toString = function() {
for (var i = 0; i < 2; i++) {
this()
}
};
oomTest(getBacktrace({
thisprops: true
}))
asserts js debug shell on m-c changeset d53a52b39a95 with --fuzzing-safe --no-threads --ion-eager at Assertion failure: phaseNestingDepth == 0, at gc/Statistics.cpp
Configure options:
CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --disable-threadsafe --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests
python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic" -r d53a52b39a95
autoBisect shows this is probably related to the following changeset:
The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/9c365490d4ce
user: Jon Coppeard
date: Tue Oct 13 13:37:07 2015 +0100
summary: Bug 1212469 - Make oomTest() into a shell function r=nbp
Jon, is bug 1212469 a likely regressor?
Flags: needinfo?(jcoppeard)
Reporter | ||
Comment 1•9 years ago
|
||
(lldb) bt 5
* thread #1: tid = 0x1f0ec4, 0x00000001008e40c4 js-dbg-64-dm-darwin-d53a52b39a95`js::gcstats::Statistics::startTimingMutator(this=<unavailable>) + 132 at Statistics.cpp:1024, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
* frame #0: 0x00000001008e40c4 js-dbg-64-dm-darwin-d53a52b39a95`js::gcstats::Statistics::startTimingMutator(this=<unavailable>) + 132 at Statistics.cpp:1024
frame #1: 0x000000010000dbc2 js-dbg-64-dm-darwin-d53a52b39a95`StartTimingMutator(cx=<unavailable>, argc=<unavailable>, vp=0x00007fff5fbfcdc8) + 98 at js.cpp:1627
frame #2: 0x0000000100676cb2 js-dbg-64-dm-darwin-d53a52b39a95`js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [inlined] js::CallJSNative(cx=0x0000000102c45400, native=(js-dbg-64-dm-darwin-d53a52b39a95`StartTimingMutator(JSContext*, unsigned int, JS::Value*) at js.cpp:1619))(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 174 at jscntxtinlines.h:235
frame #3: 0x0000000100676c04 js-dbg-64-dm-darwin-d53a52b39a95`js::Invoke(cx=0x0000000102c45400, args=0x00007fff5fbfcd70, construct=<unavailable>) + 596 at Interpreter.cpp:489
frame #4: 0x000000010069c4fd js-dbg-64-dm-darwin-d53a52b39a95`js::Invoke(cx=0x0000000102c45400, thisv=0x00007fff5fbfcf58, fval=<unavailable>, argc=<unavailable>, argv=<unavailable>, rval=<unavailable>) + 669 at Interpreter.cpp:542
(lldb)
Assignee | ||
Comment 2•9 years ago
|
||
Not related to bug 1212469. I can remove the call to oomTest() and still get the failure.
BTW making Function.prototype.toString call |this()| is really evil...
Assignee | ||
Comment 3•9 years ago
|
||
Patch to make Statistics::startTimingMutator() return false if called at the wrong time rather than asserting.
Assignee: nobody → jcoppeard
Attachment #8679941 -
Flags: review?(sphink)
Comment 4•9 years ago
|
||
Comment on attachment 8679941 [details] [diff] [review]
bug1218900-mutator-timing
Review of attachment 8679941 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/gc/Statistics.cpp
@@ +1021,5 @@
> Statistics::startTimingMutator()
> {
> + // Should only be called from outside of GC.
> + if (phaseNestingDepth != 0)
> + return false;
This bug shows that the comment is incorrect. The intention was to prevent mistakes where somehow startTimingMutator would be called from within a GC, which would mess up the phase nesting. But it's additionally guarding against nested invocations of itself. As in, this script shows the same crash:
startTimingMutator();
startTimingMutator();
So I think the right fix would be to change this to
if (phaseNestingDepth != 0) {
// Should only be called from outside of GC.
MOZ_ASSERT(phaseNestingDepth == 1);
MOZ_ASSERT(phaseNesting[0] == PHASE_MUTATOR);
return false;
}
Attachment #8679941 -
Flags: review?(sphink) → review+
Reporter | ||
Comment 5•9 years ago
|
||
autoBisect shows this is probably related to the following changeset:
The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/b261745c586a
user: Steve Fink
date: Tue Nov 18 11:26:11 2014 -0800
summary: Bug 1088831 - Count storebuffer overflows, account for minor GCs, and implement timed regions, r=jonco
Yes, bug 1088831 does seem to be the regressor. (I bisected using a testcase without oomTest)
Comment 7•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment 8•9 years ago
|
||
bugherder uplift |
status-b2g-v2.5:
--- → fixed
Comment 9•9 years ago
|
||
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
status-b2g-v2.5:
fixed → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•