Closed
Bug 1480835
Opened 6 years ago
Closed 6 years ago
DEBUG-only hazard in AutoEntryScript constructor
Categories
(Core :: JavaScript: GC, defect)
Core
JavaScript: GC
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: sfink, Assigned: sfink)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
This is a straightforward one. Here's the code:
AutoEntryScript::AutoEntryScript(JSObject* aObject,
const char* aReason,
bool aIsMainThread)
: AutoEntryScript(xpc::NativeGlobal(aObject), aReason, aIsMainThread)
{
MOZ_ASSERT(!js::IsCrossCompartmentWrapper(aObject));
}
Hazard is pasted below. But the problem is that xpc::NativeGlobal can GC, but we use aObject in the assertion in the body.
The easiest solution would be to delete the assertion.
Function '_ZN7mozilla3dom15AutoEntryScriptC4EP8JSObjectPKcb$void mozilla::dom::AutoEntryScript::AutoEntryScript(JSObject*, int8*, uint8)' has unrooted 'aObject' of type 'JSObject*' live across GC call '_ZN7mozilla3dom15AutoEntryScriptC1EP15nsIGlobalObjectPKcb$void mozilla::dom::AutoEntryScript::AutoEntryScript(nsIGlobalObject*, int8*, uint8)' at dom/script/ScriptSettings.cpp:672
ScriptSettings.cpp:672: Call(1,2, __temp_1 := NativeGlobal(aObject*))
ScriptSettings.cpp:672: Call(2,3, this*.AutoEntryScript(__temp_1*,aReason*,aIsMainThread*)) [[GC call]]
ScriptSettings.cpp:674: Call(3,4, __temp_3 := IsCrossCompartmentWrapper(aObject*))
GC Function: _ZN7mozilla3dom15AutoEntryScriptC1EP15nsIGlobalObjectPKcb$void mozilla::dom::AutoEntryScript::AutoEntryScript(nsIGlobalObject*, int8*, uint8)
_ZN7mozilla3dom15AutoEntryScriptC4EP15nsIGlobalObjectPKcb
_ZN7mozilla3dom9AutoJSAPIC2EP15nsIGlobalObjectbNS0_24ScriptSettingsStackEntry4TypeE
void mozilla::dom::AutoJSAPI::AutoJSAPI(nsIGlobalObject*, uint8, uint32)
void mozilla::dom::AutoJSAPI::InitInternal(nsIGlobalObject*, JSObject*, JSContext*, uint8)
uint8 JS_GetProperty(JSContext*, JS::Handle<JSObject*>, int8*, JS::MutableHandle<JS::Value>)
uint8 JS_GetPropertyById(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)
uint8 JS_ForwardGetPropertyTo(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)
uint8 js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)
IndirectCall: op
Assignee | ||
Comment 1•6 years ago
|
||
This code is from bug 1474541. In particular, see bug 1474541 comment 13.
Jan - I didn't look too closely, but it seems like what you mention in (2) is now done? The problem is that I'm fixing some problems in the hazard analysis related to losing call edges between constructor and destructor variants, and that reveals a hazard here. Is it ok to just kill off the assert now?
Attachment #8997500 -
Flags: review?(jdemooij)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → sphink
Status: NEW → ASSIGNED
Comment 2•6 years ago
|
||
Comment on attachment 8997500 [details] [diff] [review]
Remove now-redundant assertion that is a rooting hazard
Review of attachment 8997500 [details] [diff] [review]:
-----------------------------------------------------------------
Good find. This is fine, thanks!
Attachment #8997500 -
Flags: review?(jdemooij) → review+
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/309fe3a601ac
Remove now-redundant assertion that is a rooting hazard, r=jandem
Comment 4•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•