Closed Bug 949283 Opened 10 years ago Closed 10 years ago

[jsdbg2] Are DebugScopes::missingScopes keys getting fixed up when scope objects are tenured?

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: jimb, Assigned: terrence)

Details

Attachments

(3 files)

The proposed patch for bug 948459 includes an assertion that, when a frame is popped, we've removed all entries from DebugScope::liveScopes that refer to that frame. However, that assertion fails on the root analysis / GGC build on try.

Running changeset 641ac2610a47 on x86-64 Linux, a debug, exact rooting, ggc build, the following command crashes for me:

$ JS_GC_ZEAL=7 /home/jimb/moz/dbg/js/src/obj~/js --ion-eager -f /home/jimb/moz/dbg/js/src/jit-test/lib/prolog.js --js-cache-per-process --js-cache /home/jimb/moz/dbg/js/src/jit-test/.js-cache -e "const platform='linux2'; const libdir='/home/jimb/moz/dbg/js/src/jit-test/lib/'; const scriptdir='/home/jimb/moz/dbg/js/src/jit-test/tests/debug/'" -f /home/jimb/moz/dbg/js/src/jit-test/tests/debug/Environment-identity-03.js
Assertion failure: e.front().value() != frame, at /home/jimb/moz/dbg/js/src/vm/ScopeObject.cpp:1838
Segmentation fault (core dumped)
$ 

In GDB, I noticed that we have two debug scope objects, both Call objects, being added for the same stack frame, with no missingScopes removals in between:

Breakpoint 2, js::DebugScopes::addDebugScope (cx=0x18f7480, si=..., debugScope=(js::DebugScopeObject &) @0x7fffebc8cb00 [object Proxy])
    at /home/jimb/moz/dbg/js/src/vm/ScopeObject.cpp:1748
$102 = (const js::ScopeIter &) @0x7fffffff7080: {cx = 0x18f7480, frame_ = {ptr_ = 0x7fffffff942a}, cur_ = (JSObject *) 0x7ffff0102a90 [object Call] delegate, 
  block_ = 0x0, type_ = 0x0, hasScopeObject_ = 0x0, _mCheckNotUsedAsTemporary = {statementDone = 0x1}}
...
(gdb) c
Continuing.

Breakpoint 2, js::DebugScopes::addDebugScope (cx=0x18f7480, si=..., debugScope=(js::DebugScopeObject &) @0x7fffebc8cc80 [object Proxy])
    at /home/jimb/moz/dbg/js/src/vm/ScopeObject.cpp:1748
$105 = (const js::ScopeIter &) @0x7fffffff7060: {cx = 0x18f7480, frame_ = {ptr_ = 0x7fffffff942a}, cur_ = (JSObject *) 0x7fffebc8cb40 [object Call] delegate, 
  block_ = 0x0, type_ = 0x0, hasScopeObject_ = 0x0, _mCheckNotUsedAsTemporary = {statementDone = 0x1}}

Note that both of them have a stack frame of 0x7fffffff942a, and both are referring to Call objects --- which should never happen, since GetDebugScopeForMissing is supposed to find any extant DebugScopeObject for that frame.

I haven't chased this down yet, but it makes me suspicious that the first Call object, at 0x7ffff0102a90, is in the nursery:

(gdb) p/x cx->runtime_->gcNurseryStart_
$109 = 0x7ffff0100000
(gdb) p/x cx->runtime_->gcNurseryEnd_
$110 = 0x7ffff10ffff8
(gdb) 

Perhaps the problem is that the Call object was tenured, but the entry in missingScopes was not rehashed.
Here's the assertion that catches the bug.
With that patch applied, I can reproduce this bug using existing jit-tests as follows:

$ JS_GC_ZEAL=7 setarch x86_64 -R python jit-test/jit_test.py obj~/js --args=--ion-eager --show-output Environment-identity-03
Assertion failure: e.front().value() != frame, at /home/jimb/moz/dbg/js/src/vm/ScopeObject.cpp:1838
Exit code: -11
FAIL - debug/Environment-identity-03.js
[0|1|0|0] 100% ==========================================================>|   0.5s
FAILURES:
    /home/jimb/moz/dbg/js/src/jit-test/tests/debug/Environment-identity-03.js
TIMEOUTS:
$
Here is my lame attempt to fix the problem; I don't really know what I'm doing, and this seems to die with complaints from js::PointerHasher that it is being asked to 'match' poisoned pointers.
We're missing a post barrier on the put to missingScopes at ScopeObject.cpp:1722. Probably fallout from making the barriers on these manual. Thanks for finding this Jim!
Assignee: nobody → terrence
Ah, I explicitly skipped it because I was not aware that ScopeIterKey could contain nursery things. Given that I implemented ScopeIterKey in bug 949283 specifically because it can contain nursery things, however.... /me facepalms.
I totally missed this post-barrier before.
Attachment #8346881 - Flags: review?(jcoppeard)
Comment on attachment 8346881 [details] [diff] [review]
postbarrier_missingScopes-v0.diff

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

Ugh, yes, looks good.
Attachment #8346881 - Flags: review?(jcoppeard) → review+
https://hg.mozilla.org/mozilla-central/rev/0aef240c39d3
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.