Closed Bug 1480843 Opened Last year Closed Last year

Avoid hazard by controlling order of operations so that we GC before using any GC heap pointers

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: sfink, Assigned: sfink)

Details

Attachments

(1 file)

New hazard revealed by analysis bugfixes:

Function '_ZL20ReadScriptOrFunctionP20nsIObjectInputStreamP9JSContextPP8JSScriptPP8JSObject$nsXPConnect.cpp:uint32 ReadScriptOrFunction(nsIObjectInputStream*, JSContext*, JSScript**, JSObject**)' has unrooted '__temp_7' of type 'JSObject*' live across GC call '_ZN3xpc16CompilationScopeEv$JSObject* xpc::CompilationScope()' at js/xpconnect/src/nsXPConnect.cpp:1029
    nsXPConnect.cpp:1029: Call(13,14, __temp_7 := CurrentGlobalOrNull(cx*))
    nsXPConnect.cpp:1029: Call(14,15, __temp_8 := CompilationScope()) [[GC call]]
    nsXPConnect.cpp:1029: Assume(15,16, (__temp_8* !=p{JSObject} __temp_7*), true)
GC Function: _ZN3xpc16CompilationScopeEv$JSObject* xpc::CompilationScope()
    JSObject* XPCJSRuntime::LoaderGlobal()
    JSObject* mozJSComponentLoader::GetSharedGlobal(JSContext*)
    void mozJSComponentLoader::CreateLoaderGlobal(JSContext*, nsTSubstring<char>*, JS::MutableHandle<JSObject*>)
    uint32 xpc::InitClassesWithNewWrappedGlobal(JSContext*, nsISupports*, nsIPrincipal*, uint32, JS::RealmOptions*, JS::MutableHandle<JSObject*>)
    uint32 XPCWrappedNative::WrapNewGlobal(xpcObjectHelper*, nsIPrincipal*, uint8, JS::RealmOptions*, XPCWrappedNative**)
    nsIXPCScriptable.GetJSClass
    unresolved nsIXPCScriptable.GetJSClass

Note that the exact stack there is somewhat bogus; I could annotate away the GetJSClass thing. But the loader global is lazily created, it looks like, so even though it's probably guaranteed to exist at this point there's no simple annotation that'll fix this.
This is a little unfortunate because we can no longer change this MOZ_RELEASE_ASSERT to MOZ_ASSERT without incurring the wrath of -Wunused, but there are greater hardships in life.
Attachment #8997510 - Flags: review?(bzbarsky)
Comment on attachment 8997510 [details] [diff] [review]
Avoid hazard by controlling order of operations so that we GC before using any GC heap pointers

Yeah, if we MOZ_ASSERT then we can DebugOnly as needed.  Not a big deal.

r=me
Attachment #8997510 - Flags: review?(bzbarsky) → review+
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9345ad05c0c5
Avoid hazard by controlling order of operations so that we GC before using any GC heap pointers, r=bz
https://hg.mozilla.org/mozilla-central/rev/9345ad05c0c5
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.