Assertion failure: !isInList(), at jsweakmap.cpp involving oomAfterAllocations

RESOLVED FIXED in Firefox 42

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gkw, Assigned: fitzgen)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla42
x86_64
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox39 affected, firefox42 fixed)

Details

(Whiteboard: [jsbugmon:update,ignore])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
__defineGetter__("x", Debugger)
Math.exp(x)
oomAfterAllocations(6)
print(x)

asserts js debug shell on m-c changeset 0a8b3b67715a with --fuzzing-safe --no-threads --no-ion at Assertion failure: !isInList(), at jsweakmap.cpp.

Configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh ~/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build -R ~/trees/mozilla-central" -r 0a8b3b67715a

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a2f5fa870c8a
user:        Boris Zbarsky
date:        Fri Jul 04 01:24:54 2014 -0400
summary:     Bug 966452 part 1.  Refactor the js_ReportUncaughtException to produce a (message, JSErrorReport*) pair before reporting.  r=waldo and including the fix for bug 1034616 to fix JS tests to deal with this, r=jorendorff.  r=terrence on the AutoStableStringChars bits

Bug 966452 caused an assertion failure with a different name to happen.

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/c5bb52570f8f
user:        Nick Fitzgerald
date:        Mon Jul 21 18:16:13 2014 -0700
summary:     Bug 993085 - Part 1: Add the Debugger.Memory.prototype.trackingAllocationSites accessor property r=jimb

Bug 993085 then caused this particular assertion failure to happen.

Nick, is bug 993085 a likely regressor?
Flags: needinfo?(nfitzgerald)
(Reporter)

Comment 1

3 years ago
Created attachment 8569322 [details]
stack

(lldb) bt 5
* thread #1: tid = 0x3afa93, 0x0000000100831f12 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::WeakMapBase::~WeakMapBase(this=<unavailable>) + 146 at jsweakmap.cpp:39, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000100831f12 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::WeakMapBase::~WeakMapBase(this=<unavailable>) + 146 at jsweakmap.cpp:39
    frame #1: 0x00000001001eb5c9 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::Debugger::~Debugger() [inlined] js::HashMap<JS::Zone*, unsigned long, js::DefaultHasher<JS::Zone*>, js::RuntimeAllocPolicy>::~HashMap() + 25 at jsweakmap.h:111
    frame #2: 0x00000001001eb5b0 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::Debugger::~Debugger() [inlined] js::DebuggerWeakMap<JSScript*, false>::~DebuggerWeakMap(this=0x0000000102875938) + 14 at Debugger.h:58
    frame #3: 0x00000001001eb5a2 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::Debugger::~Debugger() [inlined] js::DebuggerWeakMap<JSObject*, false>::~DebuggerWeakMap(this=<unavailable>, this=<unavailable>, this=0x0000000102875938, this=<unavailable>) at Debugger.h:58
    frame #4: 0x00000001001eb5a2 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::Debugger::~Debugger(this=0x0000000102875800) + 242 at Debugger.cpp:398
(lldb)
It seems likely, but I am not sure what this assertion is asserting or why. Will look into it.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Flags: needinfo?(nfitzgerald)
Something strange going on here. UniquePtr<Debugger> is trying to free some pointer even though release was explicitly called (and it isn't the pointer it used to hold either).

Something going wrong somewhere.
Created attachment 8570234 [details] [diff] [review]
Fix assertion failure with Debugger and oomAfterAllocations
Attachment #8570234 - Flags: review?(shu)
Not caused by my patch, but fun stuff!

Despite `// |jit| allow-oom` this test still fails because this is an "unhandlable oom" -- any way around this?
For posterity:

16:51 < sfink> fitzgen: if you look at js/public/Utility.h, you can see the JS_OOM_POSSIBLY_FAIL macro. You could #define JS_OOM_BREAKPOINT to have an easy way of breaking on when that triggers (due to the oomAfterAllocations), then step 
               through and see how it handles the oom.
16:52 < sfink> or more specifically, #define JS_OOM_BREAKPOINT at the top of Utility.h, then set a breakpoint on js_failedAllocBreakpoint
Created attachment 8570719 [details] [diff] [review]
Fix assertion failure with Debugger and oomAfterAllocations
Attachment #8570719 - Flags: review?(shu)
Attachment #8570234 - Attachment is obsolete: true
Attachment #8570234 - Flags: review?(shu)

Comment 9

3 years ago
Comment on attachment 8570719 [details] [diff] [review]
Fix assertion failure with Debugger and oomAfterAllocations

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

LGTM, nice catch.
Attachment #8570719 - Flags: review?(shu) → review+
Keywords: checkin-needed
sorry had to back this out for test failures like https://treeherder.mozilla.org/img/logviewerIcon.svg
Flags: needinfo?(nfitzgerald)
(In reply to Carsten Book [:Tomcat] from comment #12)
> sorry had to back this out for test failures like
> https://treeherder.mozilla.org/img/logviewerIcon.svg

Care to share the link to the failures ;)
Flags: needinfo?(nfitzgerald) → needinfo?(cbook)
Created attachment 8572029 [details] [diff] [review]
Fix assertion failure with Debugger and oomAfterAllocations
Attachment #8570719 - Attachment is obsolete: true
Attachment #8572029 - Flags: review+
Keywords: checkin-needed
Flags: needinfo?(cbook)
Turns out those WinXP debug jittest timeouts in the Try run were real. Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/461dfb926e39
(Reporter)

Updated

3 years ago
Flags: needinfo?(nfitzgerald)
Is there any way to run the jit-tests with -o on tbpl?
Flags: needinfo?(nfitzgerald) → needinfo?(gary)
(Reporter)

Comment 20

3 years ago
I'm not sure I'm the right person for this, perhaps someone else might know?
Flags: needinfo?(gary)
(In reply to Nick Fitzgerald [:fitzgen] from comment #19)
> Is there any way to run the jit-tests with -o on tbpl?

The best way I can think of is to edit js/src/devtools/automation/autospider.sh. Before the line

  $COMMAND_PREFIX $MAKE check-jit-test || exit 1

add in

  export JITTEST_EXTRA_ARGS="-o"

(and if you only care about jit-tests, comment out the other tests nearby for faster turnaround.)
> 13:48:06     INFO -  TEST-PASS | tests\jit-test\jit-test\tests\debug\bug-1136806.js | Success (code 3, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --no-sse3 --no-threads")

Hmm only one TEST-PASS for this test, when I am pretty sure it should be running with 4 different configurations, like the rest of the tests seem to.

No TEST-FAIL or any useful output though...
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 701c4a82fc56).
https://hg.mozilla.org/mozilla-central/rev/71b89d5b16d2
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.