Bug 1903037 part 1 - Support leak checking in MFBT code when included in SpiderMonkey. r=mccr8,glandium

`mozilla::StringBuffer` uses `RefCountLogger`, but this was a no-op when included in
SpiderMonkey code (`MOZILLA_INTERNAL_API` is not defined in that case). This resulted
in leakcheck failures when string buffers are used directly in SpiderMonkey.

This patch changes the calls to `NS_LogAddRef` and `NS_LogRelease` to go through a
function pointer. This also makes it possible to use a different implementation in
SpiderMonkey shell builds if we ever want to.

Differential Revision: https://phabricator.services.mozilla.com/D213968

js/public/RefCounted.h

@@ -14,11 +14,7 @@
 
 // These types implement the same interface as mozilla::(Atomic)RefCounted and
 // must be used instead of mozilla::(Atomic)RefCounted for everything in
-// SpiderMonkey. There are two reasons:
+// SpiderMonkey. This is because Release() needs to call js_delete, not delete.
-//  - Release() needs to call js_delete, not delete
-//  - SpiderMonkey does not have MOZILLA_INTERNAL_API defined which can lead
-//    to ODR violations that show up as spurious leak reports when ref-counted
-//    types are allocated by SpiderMonkey and released by Gecko (or vice versa).
 
 namespace js {
 

js/src/build/moz.build

@@ -15,6 +15,8 @@ LIBRARY_DEFINES["EXPORT_JS_API"] = True
 # to static mozglue missing, see bug 1588340 for more information.
 LIBRARY_DEFINES["MOZ_HAS_MOZGLUE"] = True
 
+LIBRARY_DEFINES["MOZ_SUPPORT_LEAKCHECKING"] = True
+
 # JavaScript must be built shared, even for static builds, as it is used by
 # other modules which are always built shared. Failure to do so results in
 # the js code getting copied into xpinstall and jsd as well as mozilla-bin,

mfbt/RefCounted.cpp

@@ -0,0 +1,36 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "mozilla/RefCounted.h"
+
+namespace mozilla::detail {
+
+#ifdef MOZ_REFCOUNTED_LEAK_CHECKING
+MFBT_DATA LogAddRefFunc gLogAddRefFunc = nullptr;
+MFBT_DATA LogReleaseFunc gLogReleaseFunc = nullptr;
+MFBT_DATA size_t gNumStaticCtors = 0;
+MFBT_DATA const char* gLastStaticCtorTypeName = nullptr;
+
+MFBT_API void RefCountLogger::SetLeakCheckingFunctions(
+    LogAddRefFunc aLogAddRefFunc, LogReleaseFunc aLogReleaseFunc) {
+  if (gNumStaticCtors > 0) {
+    // RefCountLogger was used before this point. Print a warning, similar to
+    // ASSERT_ACTIVITY_IS_LEGAL. We do this here because SpiderMonkey standalone
+    // and shell builds don't call this function and we don't want to report any
+    // warnings in that case.
+    fprintf(stderr,
+            "RefCounted objects addrefed/released (static ctor?) total: %zu, "
+            "last type: %s\n",
+            gNumStaticCtors, gLastStaticCtorTypeName);
+    gNumStaticCtors = 0;
+    gLastStaticCtorTypeName = nullptr;
+  }
+  gLogAddRefFunc = aLogAddRefFunc;
+  gLogReleaseFunc = aLogReleaseFunc;
+}
+#endif  // MOZ_REFCOUNTED_LEAK_CHECKING
+
+}  // namespace mozilla::detail

mfbt/RefCounted.h

@@ -24,11 +24,7 @@
 #  include <atomic>
 #endif  // __wasi__
 
-#if defined(MOZILLA_INTERNAL_API)
+#if defined(MOZ_SUPPORT_LEAKCHECKING) && defined(NS_BUILD_REFCNT_LOGGING)
-#  include "nsXPCOM.h"
-#endif
-
-#if defined(MOZILLA_INTERNAL_API) && defined(NS_BUILD_REFCNT_LOGGING)
 #  define MOZ_REFCOUNTED_LEAK_CHECKING
 #endif
 
@@ -58,13 +54,26 @@ namespace mozilla {
  * Note that when deriving from RefCounted or AtomicRefCounted, you
  * should add MOZ_DECLARE_REFCOUNTED_TYPENAME(ClassName) to the public
  * section of your class, where ClassName is the name of your class.
- *
- * Note: SpiderMonkey should use js::RefCounted instead since that type
- * will use appropriate js_delete and also not break ref-count logging.
  */
 namespace detail {
 const MozRefCountType DEAD = 0xffffdead;
 
+#ifdef MOZ_REFCOUNTED_LEAK_CHECKING
+// When this header is included in SpiderMonkey code, NS_LogAddRef and
+// NS_LogRelease are not available. To work around this, we call these
+// functions through a function pointer set by SetLeakCheckingFunctions.
+// Note: these are globals because GCC on Linux reports undefined-reference
+// errors when they're static members of the RefCountLogger class.
+using LogAddRefFunc = void (*)(void* aPtr, MozRefCountType aNewRefCnt,
+                               const char* aTypeName, uint32_t aClassSize);
+using LogReleaseFunc = void (*)(void* aPtr, MozRefCountType aNewRefCnt,
+                                const char* aTypeName);
+extern MFBT_DATA LogAddRefFunc gLogAddRefFunc;
+extern MFBT_DATA LogReleaseFunc gLogReleaseFunc;
+extern MFBT_DATA size_t gNumStaticCtors;
+extern MFBT_DATA const char* gLastStaticCtorTypeName;
+#endif
+
 // When building code that gets compiled into Gecko, try to use the
 // trace-refcount leak logging facilities.
 class RefCountLogger {
@@ -78,10 +87,20 @@ class RefCountLogger {
     const void* pointer = aPointer;
     const char* typeName = aPointer->typeName();
     uint32_t typeSize = aPointer->typeSize();
-    NS_LogAddRef(const_cast<void*>(pointer), aRefCount, typeName, typeSize);
+    if (gLogAddRefFunc) {
+      gLogAddRefFunc(const_cast<void*>(pointer), aRefCount, typeName, typeSize);
+    } else {
+      gNumStaticCtors++;
+      gLastStaticCtorTypeName = typeName;
+    }
 #endif
   }
 
+#ifdef MOZ_REFCOUNTED_LEAK_CHECKING
+  static MFBT_API void SetLeakCheckingFunctions(LogAddRefFunc aLogAddRefFunc,
+                                                LogReleaseFunc aLogReleaseFunc);
+#endif
+
   // Created by `RefCounted`-like classes to log a successful Release call in
   // the Gecko leak-logging system. The constructor should be invoked before the
   // refcount is decremented to avoid invoking `typeName()` with a zero
@@ -100,7 +119,12 @@ class RefCountLogger {
     void logRelease(MozRefCountType aRefCount) {
 #ifdef MOZ_REFCOUNTED_LEAK_CHECKING
       MOZ_ASSERT(aRefCount != DEAD);
-      NS_LogRelease(const_cast<void*>(mPointer), aRefCount, mTypeName);
+      if (gLogReleaseFunc) {
+        gLogReleaseFunc(const_cast<void*>(mPointer), aRefCount, mTypeName);
+      } else {
+        gNumStaticCtors++;
+        gLastStaticCtorTypeName = mTypeName;
+      }
 #endif
     }
 

mfbt/moz.build

@@ -177,6 +177,7 @@ UNIFIED_SOURCES += [
     "JSONWriter.cpp",
     "Poison.cpp",
     "RandomNum.cpp",
+    "RefCounted.cpp",
     "SHA1.cpp",
     "TaggedAnonymousMemory.cpp",
     "UniquePtrExtensions.cpp",
@@ -209,6 +210,9 @@ OS_LIBS += CONFIG["LIBATOMIC_LIBS"]
 
 DEFINES["LZ4LIB_VISIBILITY"] = ""
 
+# For RefCounted.cpp
+DEFINES["MOZ_SUPPORT_LEAKCHECKING"] = True
+
 # This is kind of gross because this is not a subdirectory,
 # but pure_virtual requires mfbt to build and some projects
 # don't use mfbt.

toolkit/library/moz.build

@@ -9,6 +9,7 @@
 def Libxul_defines():
     LIBRARY_DEFINES["MOZILLA_INTERNAL_API"] = True
     LIBRARY_DEFINES["IMPL_LIBXUL"] = True
+    LIBRARY_DEFINES["MOZ_SUPPORT_LEAKCHECKING"] = True
     if not CONFIG["JS_SHARED_LIBRARY"]:
         LIBRARY_DEFINES["STATIC_EXPORTABLE_JS_API"] = True
 

xpcom/base/nsTraceRefcnt.cpp

@@ -811,6 +811,11 @@ EXPORT_XPCOM_API(void)
 NS_LogInit() {
   NS_SetMainThread();
 
+#if defined(NS_BUILD_REFCNT_LOGGING)
+  mozilla::detail::RefCountLogger::SetLeakCheckingFunctions(NS_LogAddRef,
+                                                            NS_LogRelease);
+#endif
+
   // FIXME: This is called multiple times, we should probably not allow that.
   if (++gInitCount) {
     nsTraceRefcnt::SetActivityIsLegal(true);