// Random chosen test: js/src/jit-test/tests/debug/Source-invisible.js newGlobal({ invisibleToDebugger: true }) // Random chosen test: js/src/jit-test/tests/debug/Debugger-findObjects-05.js x = (new Debugger).findObjects() asserts js debug shell on m-c changeset 8c02f3280d0c with --no-ion --no-threads at Assertion failure: !k->compartment()->options_.invisibleToDebugger(), at vm/Debugger.h. Debug 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 --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests This was found by combining random jit-tests together with jsfunfuzz, the specific file(s) is/are: http://hg.mozilla.org/mozilla-central/file/8c02f3280d0c/js/src/jit-test/tests/debug/Source-invisible.js http://hg.mozilla.org/mozilla-central/file/8c02f3280d0c/js/src/jit-test/tests/debug/Debugger-findObjects-05.js autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/8f19523dc6bf user: Nick Fitzgerald date: Wed Oct 15 19:21:00 2014 +0200 summary: Bug 1082761 - Add Debugger.prototype.findObjects; r=jimb Nick, is bug 1082761 a likely regressor?

This is unlikely to be s-s.

(lldb) bt 5 * thread #1: tid = 0x38cdaf, 0x00000001006bf86a js-dbg-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`bool js::DebuggerWeakMap<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*>, false>::relookupOrAdd<JS::Rooted<JSObject*>, js::NativeObject*>(js::detail::HashTable<js::HashMapEntry<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*> >, js::HashMap<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*>, js::DefaultHasher<js::PreBarriered<JSObject*> >, js::RuntimeAllocPolicy>::MapHashPolicy, js::RuntimeAllocPolicy>::AddPtr&, JS::Rooted<JSObject*> const&, js::NativeObject* const&) [inlined] JSObject::lastProperty(this=<unavailable>, this=<unavailable>) const + 28 at jsobj.h:129, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x00000001006bf86a js-dbg-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`bool js::DebuggerWeakMap<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*>, false>::relookupOrAdd<JS::Rooted<JSObject*>, js::NativeObject*>(js::detail::HashTable<js::HashMapEntry<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*> >, js::HashMap<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*>, js::DefaultHasher<js::PreBarriered<JSObject*> >, js::RuntimeAllocPolicy>::MapHashPolicy, js::RuntimeAllocPolicy>::AddPtr&, JS::Rooted<JSObject*> const&, js::NativeObject* const&) [inlined] JSObject::lastProperty(this=<unavailable>, this=<unavailable>) const + 28 at jsobj.h:129 frame #1: 0x00000001006bf84e js-dbg-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`bool js::DebuggerWeakMap<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*>, false>::relookupOrAdd<JS::Rooted<JSObject*>, js::NativeObject*>(js::detail::HashTable<js::HashMapEntry<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*> >, js::HashMap<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*>, js::DefaultHasher<js::PreBarriered<JSObject*> >, js::RuntimeAllocPolicy>::MapHashPolicy, js::RuntimeAllocPolicy>::AddPtr&, JS::Rooted<JSObject*> const&, js::NativeObject* const&) [inlined] JSObject::compartment() const at jsobj.h:176 frame #2: 0x00000001006bf84e js-dbg-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`bool js::DebuggerWeakMap<js::PreBarriered<JSObject*>, js::RelocatablePtr<JSObject*>, false>::relookupOrAdd<JS::Rooted<JSObject*>, js::NativeObject*>(this=<unavailable>, p=<unavailable>, k=<unavailable>, v=<unavailable>) + 542 at Debugger.h:96 frame #3: 0x000000010062c7a4 js-dbg-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`js::Debugger::wrapDebuggeeValue(this=0x00000001030ec600, cx=0x0000000101d02710, vp=<unavailable>) + 1348 at Debugger.cpp:827 frame #4: 0x00000001006393fb js-dbg-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`js::Debugger::findObjects(cx=0x0000000101d02710, argc=<unavailable>, vp=0x000000010307e6a8) + 699 at Debugger.cpp:3709 (lldb)

Will have a fix up soon.

Comment on attachment 8532711 [details] [diff] [review] find-objects-invisible-to-debugger.patch Review of attachment 8532711 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure I understand this bug. Why is findObjects traversing into a non-debuggee global anyhow? I see that the traversal considers *all* CCW wrapped objects as starting nodes. Should this be further filtered by those wrapped objects belong to one of the compartments in the debuggee compartment set? Please re-request r? with an explanation if this patch is indeed correct. ::: js/src/vm/Debugger.h @@ +232,5 @@ > Observing = 1 > }; > > + // Return true if the given compartment is a debuggee, false otherwise. > + bool Nit: no need to put the return type on a different line for an inline def. @@ +236,5 @@ > + bool > + isDebuggee(const JSCompartment *compartment) const { > + return compartment && > + compartment->isDebuggee() && > + debuggees.has(compartment->maybeGlobal()); When would the compartment be null? Nit: line up with the first line, like: return compartment && compartment->isDebuggee() && ...

(In reply to Shu-yu Guo [:shu] from comment #5) > Comment on attachment 8532711 [details] [diff] [review] > find-objects-invisible-to-debugger.patch > > Review of attachment 8532711 [details] [diff] [review]: > ----------------------------------------------------------------- > > I'm not sure I understand this bug. > > Why is findObjects traversing into a non-debuggee global anyhow? I see that > the traversal considers *all* CCW wrapped objects as starting nodes. Should > this be further filtered by those wrapped objects belong to one of the > compartments in the debuggee compartment set? Talked to you a little about this last night. Regarding CCWs as roots and filtering: yes, but I left it as a follow up (bug 1108149). We should just use JS::ubi::RootList which uses JS_TraceIncomingCCWs and does this stuff properly. Regarding traversing to non-debuggee objects: we need to abandon referents in the traversal, which I was also putting off as a follow up (just wanted to fix the crash here). The result is that we traverse and then iterate over more things than we need to, but our behavior is still correct and safe, here. > @@ +236,5 @@ > > + bool > > + isDebuggee(const JSCompartment *compartment) const { > > + return compartment && > > + compartment->isDebuggee() && > > + debuggees.has(compartment->maybeGlobal()); > > When would the compartment be null? Should I just assert that it is non null instead? Or make it a reference parameter and do the asserting at the caller?

Comment on attachment 8532711 [details] [diff] [review] find-objects-invisible-to-debugger.patch Review of attachment 8532711 [details] [diff] [review]: ----------------------------------------------------------------- r=me with asserting compartment being non-null. No need to make it a reference. For sake of convention, we pass around JSCompartment as a pointer everywhere.

Updated and carrying over shu's r+.