Bug 1560667 - Annotate more callers. r=jonco
authorSteve Fink <sfink@mozilla.com>
Wed, 02 Oct 2019 03:19:58 +0000
changeset 495916 dfecb23de876e3aa1dfa4907af1859cf3fd023e8
parent 495915 55b80c2e2228aeddd724898e6f2511b9099c5f24
child 495917 5a8a7c48c850766084ca90175b9641e34e2fdc6b
push id114140
push userdvarga@mozilla.com
push dateWed, 02 Oct 2019 18:04:51 +0000
treeherdermozilla-inbound@32eb0ea893f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1560667
milestone71.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1560667 - Annotate more callers. r=jonco This includes some annotations to isOverridableField() that are only necessary at this point in the patch stack; the entire function will be removed shortly. Differential Revision: https://phabricator.services.mozilla.com/D46565
js/src/devtools/rootAnalysis/annotations.js
xpcom/base/CycleCollectedJSRuntime.cpp
--- a/js/src/devtools/rootAnalysis/annotations.js
+++ b/js/src/devtools/rootAnalysis/annotations.js
@@ -66,51 +66,61 @@ function indirectCallCannotGC(fullCaller
     // to be throwing a JS exception.
     if (name == "callback" && caller.includes("js::ErrorToException"))
         return true;
 
     // The math cache only gets called with non-GC math functions.
     if (name == "f" && caller.includes("js::MathCache::lookup"))
         return true;
 
+    // It would probably be better to somehow rewrite PR_CallOnce(foo) into a
+    // call of foo, but for now just assume that nobody is crazy enough to use
+    // PR_CallOnce with a function that can GC.
+    if (name == "func" && caller == "PR_CallOnce")
+        return true;
+
     return false;
 }
 
 // Ignore calls through functions pointers with these types
 var ignoreClasses = {
     "JSStringFinalizer" : true,
     "SprintfState" : true,
     "SprintfStateStr" : true,
     "JSLocaleCallbacks" : true,
     "JSC::ExecutableAllocator" : true,
     "PRIOMethods": true,
     "_MD_IOVector" : true,
     "malloc_table_t": true, // replace_malloc
     "malloc_hook_table_t": true, // replace_malloc
+    "mozilla::MallocSizeOf": true,
+    "MozMallocSizeOf": true,
 };
 
 // Ignore calls through TYPE.FIELD, where TYPE is the class or struct name containing
 // a function pointer field named FIELD.
 var ignoreCallees = {
     "js::Class.trace" : true,
     "js::Class.finalize" : true,
     "JSClassOps.trace" : true,
     "JSClassOps.finalize" : true,
     "JSRuntime.destroyPrincipals" : true,
     "icu_50::UObject.__deleting_dtor" : true, // destructors in ICU code can't cause GC
     "mozilla::CycleCollectedJSRuntime.DescribeCustomObjects" : true, // During tracing, cannot GC.
     "mozilla::CycleCollectedJSRuntime.NoteCustomGCThingXPCOMChildren" : true, // During tracing, cannot GC.
     "PLDHashTableOps.hashKey" : true,
+    "PLDHashTableOps.clearEntry" : true,
     "z_stream_s.zfree" : true,
     "z_stream_s.zalloc" : true,
     "GrGLInterface.fCallback" : true,
     "std::strstreambuf._M_alloc_fun" : true,
     "std::strstreambuf._M_free_fun" : true,
     "struct js::gc::Callback<void (*)(JSContext*, void*)>.op" : true,
     "mozilla::ThreadSharedFloatArrayBufferList::Storage.mFree" : true,
+    "mozilla::SizeOfState.mMallocSizeOf": true,
 };
 
 function fieldCallCannotGC(csu, fullfield)
 {
     if (csu in ignoreClasses)
         return true;
     if (fullfield in ignoreCallees)
         return true;
@@ -247,23 +257,46 @@ var ignoreFunctions = {
     // VTune internals that lazy-load a shared library and make IndirectCalls.
     "iJIT_IsProfilingActive" : true,
     "iJIT_NotifyEvent": true,
 
     // The big hammers.
     "PR_GetCurrentThread" : true,
     "calloc" : true,
 
+    // This will happen early enough in initialization to not matter.
+    "_PR_UnixInit" : true,
+
     "uint8 nsContentUtils::IsExpandedPrincipal(nsIPrincipal*)" : true,
 
     "void mozilla::AutoProfilerLabel::~AutoProfilerLabel(int32)" : true,
 
     // Stores a function pointer in an AutoProfilerLabelData struct and calls it.
     // And it's in mozglue, which doesn't have access to the attributes yet.
     "void mozilla::ProfilerLabelEnd(mozilla::Tuple<void*, unsigned int>*)" : true,
+
+    // This gets into PLDHashTable function pointer territory, and should get
+    // set up early enough to not do anything when it matters anyway.
+    "mozilla::LogModule* mozilla::LogModule::Get(int8*)": true,
+
+    // This annotation is correct, but the reasoning is still being hashed out
+    // in bug 1582326 comment 8 and on.
+    "nsCycleCollector.cpp:nsISupports* CanonicalizeXPCOMParticipant(nsISupports*)": true,
+
+    // PLDHashTable again
+    "void mozilla::DeadlockDetector<T>::Add(const T*) [with T = mozilla::BlockingResourceBase]": true,
+
+    // OOM handling during logging
+    "void mozilla::detail::log_print(mozilla::LogModule*, int32, int8*)": true,
+
+    // This would need to know that the nsCOMPtr refcount will not go to zero.
+    "uint8 XPCJSRuntime::DescribeCustomObjects(JSObject*, JSClass*, int8[72]*)[72]) const": true,
+
+    // As the comment says "Refcount isn't zero, so Suspect won't delete anything."
+    "uint64 nsCycleCollectingAutoRefCnt::incr(void*, nsCycleCollectionParticipant*) [with void (* suspect)(void*, nsCycleCollectionParticipant*, nsCycleCollectingAutoRefCnt*, bool*) = NS_CycleCollectorSuspect3; uintptr_t = long unsigned int]": true,
 };
 
 function extraGCFunctions() {
     return ["ffi_call"].filter(f => f in readableNames);
 }
 
 function isProtobuf(name)
 {
@@ -309,24 +342,29 @@ function ignoreGCFunction(mangled)
     // and rely on only the dynamic checks provided by AutoAssertCannotGC.
     if (isHeapSnapshotMockClass(fun) || isGTest(fun))
         return true;
 
     // Templatized function
     if (fun.includes("void nsCOMPtr<T>::Assert_NoQueryNeeded()"))
         return true;
 
+    // Bug 1577915 - Sixgill is ignoring a template param that makes its CFG
+    // impossible.
+    if (fun.includes("UnwrapObjectInternal") && fun.includes("mayBeWrapper = false"))
+        return true;
+
     // These call through an 'op' function pointer.
     if (fun.includes("js::WeakMap<Key, Value, HashPolicy>::getDelegate("))
         return true;
 
     // TODO: modify refillFreeList<NoGC> to not need data flow analysis to
     // understand it cannot GC. As of gcc 6, the same problem occurs with
     // tryNewTenuredThing, tryNewNurseryObject, and others.
-    if (/refillFreeList|tryNew/.test(fun) && /\(js::AllowGC\)0u/.test(fun))
+    if (/refillFreeList|tryNew/.test(fun) && /\(js::AllowGC\)0/.test(fun))
         return true;
 
     return false;
 }
 
 function stripUCSAndNamespace(name)
 {
     name = name.replace(/(struct|class|union|const) /g, "");
@@ -421,16 +459,37 @@ function isOverridableField(staticCSU, c
     if (field == "GetIsMainThread")
         return false;
     if (field == "GetThreadFromPRThread")
         return false;
     if (field == "DocAddSizeOfIncludingThis")
         return false;
     if (field == "ConstructUbiNode")
         return false;
+
+    // Fields on the [builtinclass] nsIPrincipal
+    if (field == "GetSiteOrigin")
+        return false;
+    if (field == "GetDomain")
+        return false;
+    if (field == "GetBaseDomain")
+        return false;
+    if (field == "GetOriginNoSuffix")
+        return false;
+
+    // Fields on nsIURI
+    if (field == "GetScheme")
+        return false;
+    if (field == "GetAsciiHostPort")
+        return false;
+    if (field == "GetAsciiSpec")
+        return false;
+    if (field == "SchemeIs")
+        return false;
+
     if (staticCSU == 'nsIXPCScriptable' && field == "GetScriptableFlags")
         return false;
     if (staticCSU == 'nsIXPConnectJSObjectHolder' && field == 'GetJSObject')
         return false;
     if (staticCSU == 'nsIXPConnect' && field == 'GetSafeJSContext')
         return false;
 
     // nsIScriptSecurityManager is not [builtinclass], but smaug says "the
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -1210,16 +1210,18 @@ void CycleCollectedJSRuntime::NurseryWra
 void CycleCollectedJSRuntime::NurseryWrapperPreserved(JSObject* aWrapper) {
   mPreservedNurseryObjects.InfallibleAppend(
       JS::PersistentRooted<JSObject*>(mJSRuntime, aWrapper));
 }
 
 void CycleCollectedJSRuntime::DeferredFinalize(
     DeferredFinalizeAppendFunction aAppendFunc, DeferredFinalizeFunction aFunc,
     void* aThing) {
+  // Tell the analysis that the function pointers will not GC.
+  JS::AutoSuppressGCAnalysis suppress;
   if (auto entry = mDeferredFinalizerTable.LookupForAdd(aFunc)) {
     aAppendFunc(entry.Data(), aThing);
   } else {
     entry.OrInsert(
         [aAppendFunc, aThing]() { return aAppendFunc(nullptr, aThing); });
   }
 }