Assertion failure: !k->compartment()->options_.invisibleToDebugger() - crash when closing a Hello window with the Browser Console open

RESOLVED FIXED in mozilla38

Status

()

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

People

(Reporter: standard8, Assigned: jimb)

Tracking

({crash, regression})

Trunk
mozilla38
crash, regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
I updated to today's fx-team and now I'm getting assertion failures.

STR:

1) Build debug fx-team (I'm on a868809113e1)
2) Start the build, open the browser debugger (note, I'm in non-e10s mode)
3) Open the Hello panel, open (or start a conversation).
4) Once the conversation has opened, close the window.

Expected Results:

-> Closes fine, no issues

Actual Results:

Assertion failure: !k->compartment()->options_.invisibleToDebugger(), at /Users/mark/loop/gecko-dev/js/src/vm/Debugger.h:100

It doesn't happen with the browser console closed.

Stack:

0   XUL                           	0x000000010511b88a bool js::DebuggerWeakMap<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&) + 618 (Debugger.h:100)
1   XUL                           	0x00000001050db7b7 js::Debugger::wrapDebuggeeValue(JSContext*, JS::MutableHandle<JS::Value>) + 1415 (Debugger.cpp:841)
2   XUL                           	0x00000001050e89ce js::Debugger::makeGlobalObjectReference(JSContext*, unsigned int, JS::Value*) + 190 (Debugger.cpp:3772)
3   XUL                           	0x000000010513303c js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) + 636 (jscntxtinlines.h:228)
4   XUL                           	0x0000000105158e30 js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) + 752 (Interpreter.cpp:554)
5   XUL                           	0x00000001050b4c0e js::DirectProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const + 190 (DirectProxyHandler.cpp:79)
6   XUL                           	0x00000001050b485d js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const + 477 (CrossCompartmentWrapper.cpp:286)
7   XUL                           	0x00000001050bc698 js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) + 248 (Proxy.cpp:401)
8   XUL                           	0x00000001050be9c0 js::proxy_Call(JSContext*, unsigned int, JS::Value*) + 96 (Proxy.cpp:792)
9   XUL                           	0x000000010513323c js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) + 1148 (jscntxtinlines.h:228)
10  XUL                           	0x0000000105158e30 js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) + 752 (Interpreter.cpp:554)
11  XUL                           	0x0000000104c6000c js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) + 2252 (BaselineIC.cpp:9496)

Updated

3 years ago
Flags: needinfo?(standard8)
(Reporter)

Updated

3 years ago
Flags: needinfo?(standard8)
(Reporter)

Updated

3 years ago
Keywords: crash
Summary: Assertion failure: !k->compartment()->options_.invisibleToDebugger() - when closing a Hello window with the Browser Console open → Assertion failure: !k->compartment()->options_.invisibleToDebugger() - crash when closing a Hello window with the Browser Console open
Component: Developer Tools: Console → JavaScript Engine
Product: Firefox → Core
Jim, maybe you know what's wrong here?
Flags: needinfo?(jimb)
(Assignee)

Comment 2

3 years ago
Beautiful. Here's an easier way to reproduce the crash in the JavaScript shell:

$ cat ~/moz/not-so-invisible.js
var g = newGlobal({ invisibleToDebugger: true });
var dbg = new Debugger;

dbg.makeGlobalObjectReference(g);
$ ~/moz/dbg/obj-bug/dist/bin/js -f ~/moz/not-so-invisible.js
Assertion failure: !k->compartment()->options_.invisibleToDebugger(), at /home/jimb/moz/dbg/js/src/vm/Debugger.h:100
Segmentation fault (core dumped)
$ 

All calls to UnwrapOneUnchecked and UncheckedUnwrap in js/src/vm/Debugger.cpp should be checking that the newly unwrapped object is not in an invisibleToDebugger compartment.
Flags: needinfo?(jimb)
(Assignee)

Comment 3

3 years ago
But that's just to get an error thrown, instead of a crash. There's still a bug in the browser console, in that it's trying to use Debugger to inspect stuff that's invisible to it.
(Assignee)

Updated

3 years ago
Assignee: nobody → jimb
(Reporter)

Comment 4

3 years ago
I also saw a similar failure yesterday when I was using the browser toolbox with the inspector to inspect the browser chrome.
(Assignee)

Comment 5

3 years ago
(In reply to Mark Banner (:standard8) from comment #4)
> I also saw a similar failure yesterday when I was using the browser toolbox
> with the inspector to inspect the browser chrome.

I'm pretty sure my reproduction recipe in comment 2 is correct, so no need for more details, as long as the assertion message is the same, and the devtools are involved.
(Assignee)

Comment 6

3 years ago
Standard8: Sorry I missed you on IRC. I'm working on this. Should be quick.
(Assignee)

Comment 7

3 years ago
Created attachment 8553381 [details] [diff] [review]
Make Debugger decline invisible-to-Debugger globals as debuggees and as Debugger.Object referents.

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=546916c83e1d
Attachment #8553381 - Flags: review?(sphink)
Comment on attachment 8553381 [details] [diff] [review]
Make Debugger decline invisible-to-Debugger globals as debuggees and as Debugger.Object referents.

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

Do you know if we need the public domain boilerplate at the top of test scripts?

::: js/src/vm/Debugger.cpp
@@ +3790,5 @@
>          return false;
>  
> +    // If we create a global for an invisible compartment, then from it we can
> +    // reach function objects, scripts, environments, etc. etc. none of which
> +    // we're ever supposed to see.

Did you you intend to to duplicate etc. etc.?
Attachment #8553381 - Flags: review?(sphink) → review+
(Assignee)

Comment 9

3 years ago
(In reply to Steve Fink [:sfink, :s:] from comment #8)
> Do you know if we need the public domain boilerplate at the top of test
> scripts?

Out of all the files in js/src/jit-tests, only 75 have that boilerplate. I don't know if it's necessary...

> Did you you intend to to duplicate etc. etc.?

Yes, but maybe it's too affected.
(Assignee)

Comment 10

3 years ago
Try push looked okay. Changed wording.
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ba8480c8608
Flags: in-testsuite+
Target Milestone: --- → mozilla38
(In reply to Jim Blandy :jimb from comment #9)
> (In reply to Steve Fink [:sfink, :s:] from comment #8)
> > Do you know if we need the public domain boilerplate at the top of test
> > scripts?
> 
> Out of all the files in js/src/jit-tests, only 75 have that boilerplate. I
> don't know if it's necessary...

Not worth worrying about for this bug, then.
(Assignee)

Comment 12

3 years ago
For what it's worth, here's the relevant page:
https://www.mozilla.org/MPL/license-policy.html

"PD Test Code is Test Code which is Mozilla Code, which does not carry an explicit license header, and which was either committed to the Mozilla repository on or after 23rd September 2014, or was committed before that date but all contributors up to that date were Mozilla employees, contractors or interns. PD Test Code is made available under the Creative Commons Public Domain Dedication. Test Code which has not been demonstrated to be PD Test Code should be considered to be under the MPL 2."
https://hg.mozilla.org/mozilla-central/rev/4ba8480c8608
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.