Bug 1420355 - Don't initialize DMD if the DMD environment variable is not given. r?njn draft
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 28 Nov 2017 08:10:07 +0900
changeset 704804 3ca7b958c8db0819ebdd54fd705ff87d76b5ebf7
parent 704803 2c3c0cb917fe8aa3b93d4a7d0aa82eebc36ca577
child 704805 58692386211959bd041f09f855907f598a9e24b4
push id91252
push userbmo:mh+mozilla@glandium.org
push dateWed, 29 Nov 2017 00:22:21 +0000
reviewersnjn
bugs1420355
milestone59.0a1
Bug 1420355 - Don't initialize DMD if the DMD environment variable is not given. r?njn This makes things slightly more inconvenient (having to set two environment variables instead of one for the simplest case) until a few patches down the line, when DMD is statically linked, at which point it will get down to one environment variable every time.
memory/replace/dmd/DMD.cpp
memory/replace/dmd/test/script-diff-dark-matter-expected.txt
memory/replace/dmd/test/script-diff-dark-matter2.json
memory/replace/dmd/test/script-ignore-alloc-fns-expected.txt
memory/replace/dmd/test/script-ignore-alloc-fns.json
memory/replace/dmd/test/test_dmd.js
python/mozbuild/mozbuild/mach_commands.py
testing/awsy/mach_commands.py
testing/mochitest/runtests.py
testing/mochitest/runtestsremote.py
--- a/memory/replace/dmd/DMD.cpp
+++ b/memory/replace/dmd/DMD.cpp
@@ -1251,17 +1251,17 @@ FreeCallback(void* aPtr, Thread* aT, Dea
     GCStackTraces();
   }
 }
 
 //---------------------------------------------------------------------------
 // malloc/free interception
 //---------------------------------------------------------------------------
 
-static void Init(malloc_table_t* aMallocTable);
+static bool Init(malloc_table_t* aMallocTable);
 
 } // namespace dmd
 } // namespace mozilla
 
 static void*
 replace_malloc(size_t aSize)
 {
   using namespace mozilla::dmd;
@@ -1363,21 +1363,22 @@ replace_free(void* aPtr)
   FreeCallback(aPtr, t, &db);
   MaybeAddToDeadBlockTable(db);
   gMallocTable.free(aPtr);
 }
 
 void
 replace_init(malloc_table_t* aMallocTable, ReplaceMallocBridge** aBridge)
 {
-  mozilla::dmd::Init(aMallocTable);
+  if (mozilla::dmd::Init(aMallocTable)) {
 #define MALLOC_FUNCS MALLOC_FUNCS_MALLOC_BASE
 #define MALLOC_DECL(name, ...) aMallocTable->name = replace_ ## name;
 #include "malloc_decls.h"
-  *aBridge = mozilla::dmd::gDMDBridge;
+    *aBridge = mozilla::dmd::gDMDBridge;
+  }
 }
 
 namespace mozilla {
 namespace dmd {
 
 //---------------------------------------------------------------------------
 // Options (Part 2)
 //---------------------------------------------------------------------------
@@ -1434,19 +1435,16 @@ Options::GetBool(const char* aArg, const
 
 Options::Options(const char* aDMDEnvVar)
   : mDMDEnvVar(aDMDEnvVar ? InfallibleAllocPolicy::strdup_(aDMDEnvVar)
                           : nullptr)
   , mMode(Mode::DarkMatter)
   , mStacks(Stacks::Partial)
   , mShowDumpStats(false)
 {
-  // It's no longer necessary to set the DMD env var to "1" if you want default
-  // options (you can leave it undefined) but we still accept "1" for
-  // backwards compatibility.
   char* e = mDMDEnvVar;
   if (e && strcmp(e, "1") != 0) {
     bool isEnd = false;
     while (!isEnd) {
       // Consume leading whitespace.
       while (isspace(*e)) {
         e++;
       }
@@ -1546,41 +1544,42 @@ postfork()
     gStateLock->Unlock();
   }
 }
 
 // WARNING: this function runs *very* early -- before all static initializers
 // have run.  For this reason, non-scalar globals such as gStateLock and
 // gStackTraceTable are allocated dynamically (so we can guarantee their
 // construction in this function) rather than statically.
-static void
+static bool
 Init(malloc_table_t* aMallocTable)
 {
+  // DMD is controlled by the |DMD| environment variable.
+  const char* e = getenv("DMD");
+
+  if (!e) {
+    return false;
+  }
+  // Initialize the function table first, because StatusMsg uses
+  // InfallibleAllocPolicy::malloc_, which uses it.
   gMallocTable = *aMallocTable;
+
+  StatusMsg("$DMD = '%s'\n", e);
+
   gDMDBridge = InfallibleAllocPolicy::new_<DMDBridge>();
 
 #ifndef XP_WIN
   // Avoid deadlocks when forking by acquiring our state lock prior to forking
   // and releasing it after forking. See |LogAlloc|'s |replace_init| for
   // in-depth details.
   //
   // Note: This must run after attempting an allocation so as to give the
   // system malloc a chance to insert its own atfork handler.
   pthread_atfork(prefork, postfork, postfork);
 #endif
-
-  // DMD is controlled by the |DMD| environment variable.
-  const char* e = getenv("DMD");
-
-  if (e) {
-    StatusMsg("$DMD = '%s'\n", e);
-  } else {
-    StatusMsg("$DMD is undefined\n");
-  }
-
   // Parse $DMD env var.
   gOptions = InfallibleAllocPolicy::new_<Options>(e);
 
   gStateLock = InfallibleAllocPolicy::new_<Mutex>();
 
   gBernoulli = (FastBernoulliTrial*)
     InfallibleAllocPolicy::malloc_(sizeof(FastBernoulliTrial));
   ResetBernoulli();
@@ -1599,16 +1598,17 @@ Init(malloc_table_t* aMallocTable)
     // Create this even if the mode isn't Cumulative (albeit with a small
     // size), in case the mode is changed later on (as is done by SmokeDMD.cpp,
     // for example).
     gDeadBlockTable = InfallibleAllocPolicy::new_<DeadBlockTable>();
     size_t tableSize = gOptions->IsCumulativeMode() ? 8192 : 4;
     MOZ_ALWAYS_TRUE(gDeadBlockTable->init(tableSize));
   }
 
+  return true;
 }
 
 //---------------------------------------------------------------------------
 // Block reporting and unreporting
 //---------------------------------------------------------------------------
 
 static void
 ReportHelper(const void* aPtr, bool aReportedOnAlloc)
--- a/memory/replace/dmd/test/script-diff-dark-matter-expected.txt
+++ b/memory/replace/dmd/test/script-diff-dark-matter-expected.txt
@@ -2,17 +2,17 @@
 # dmd.py --filter-stacks-for-testing -o script-diff-dark-matter-actual.txt script-diff-dark-matter1.json script-diff-dark-matter2.json
 
 Invocation 1 {
   $DMD = '--mode=dark-matter'
   Mode = 'dark-matter'
 }
 
 Invocation 2 {
-  $DMD is undefined
+  $DMD = '1'
   Mode = 'dark-matter'
 }
 
 #-----------------------------------------------------------------
 
 Twice-reported {
   -1 blocks in heap block record 1 of 1
   -1,088 bytes (-1,064 requested / -24 slop)
--- a/memory/replace/dmd/test/script-diff-dark-matter2.json
+++ b/memory/replace/dmd/test/script-diff-dark-matter2.json
@@ -1,12 +1,12 @@
 {
  "version": 5,
  "invocation": {
-  "dmdEnvVar": null,
+  "dmdEnvVar": "1",
   "mode": "dark-matter"
  },
  "blockList": [
   {"req": 4096, "alloc": "A", "num": 4},
 
   {"req": 8192, "alloc": "B"},
   {"req": 8192, "alloc": "B"},
 
--- a/memory/replace/dmd/test/script-ignore-alloc-fns-expected.txt
+++ b/memory/replace/dmd/test/script-ignore-alloc-fns-expected.txt
@@ -1,13 +1,13 @@
 #-----------------------------------------------------------------
 # dmd.py --filter-stacks-for-testing -o script-ignore-alloc-fns-actual.txt --ignore-alloc-fns script-ignore-alloc-fns.json
 
 Invocation {
-  $DMD is undefined
+  $DMD = '1'
   Mode = 'dark-matter'
 }
 
 #-----------------------------------------------------------------
 
 # no twice-reported heap blocks
 
 #-----------------------------------------------------------------
--- a/memory/replace/dmd/test/script-ignore-alloc-fns.json
+++ b/memory/replace/dmd/test/script-ignore-alloc-fns.json
@@ -1,12 +1,12 @@
 {
  "version": 5,
  "invocation": {
-  "dmdEnvVar": null,
+  "dmdEnvVar": "1",
   "mode": "dark-matter"
  },
  "blockList": [
   {"req": 1048576,           "alloc": "A"},
   {"req": 65536,             "alloc": "B"},
   {"req": 8000, "slop": 192, "alloc": "C"},
   {"req": 2500,              "alloc": "D"}
  ],
--- a/memory/replace/dmd/test/test_dmd.js
+++ b/memory/replace/dmd/test/test_dmd.js
@@ -131,16 +131,17 @@ function run_test() {
   // These tests do complete end-to-end testing of DMD, i.e. both the C++ code
   // that generates the JSON output, and the script that post-processes that
   // output.
   //
   // Run these synchronously, because test() updates the complete*.json files
   // in-place (to fix stacks) when it runs dmd.py, and that's not safe to do
   // asynchronously.
 
+  gEnv.set('DMD', '1');
   gEnv.set(gEnv.get("DMD_PRELOAD_VAR"), gEnv.get("DMD_PRELOAD_VALUE"));
 
   runProcess(gDmdTestFile, []);
 
   function test2(aTestName, aMode) {
     let name = "complete-" + aTestName + "-" + aMode;
     jsonFile = FileUtils.getFile("CurWorkD", [name + ".json"]);
     test(name, [jsonFile.path]);
--- a/python/mozbuild/mozbuild/mach_commands.py
+++ b/python/mozbuild/mozbuild/mach_commands.py
@@ -940,16 +940,18 @@ class RunProgram(MachCommandBase):
                     "MOZ_REPLACE_MALLOC_LIB": dmd_lib,
                 },
             }
 
             arch = self.substs['OS_ARCH']
 
             if dmd_params:
                 env_vars[arch]["DMD"] = " ".join(dmd_params)
+            else:
+                env_vars[arch]["DMD"] = "1"
 
             extra_env.update(env_vars.get(arch, {}))
 
         return self.run_process(args=args, ensure_exit_code=False,
             pass_thru=True, append_env=extra_env)
 
 @CommandProvider
 class Buildsymbols(MachCommandBase):
--- a/testing/awsy/mach_commands.py
+++ b/testing/awsy/mach_commands.py
@@ -165,16 +165,18 @@ class MachCommands(MachCommandBase):
                 "WINNT": {
                     "MOZ_REPLACE_MALLOC_LIB": dmd_lib,
                 },
             }
 
             arch = self.substs['OS_ARCH']
             for k, v in env_vars[arch].iteritems():
                 os.environ[k] = v
+            if 'DMD' not in os.environ:
+                os.environ['DMD'] = '1'
 
             # Also add the bin dir to the python path so we can use dmd.py
             if bin_dir not in sys.path:
                 sys.path.append(bin_dir)
 
         for k, v in kwargs.iteritems():
             setattr(args, k, v)
 
--- a/testing/mochitest/runtests.py
+++ b/testing/mochitest/runtests.py
@@ -1629,16 +1629,19 @@ toolbar#nav-bar {
         if hasattr(options, "topsrcdir"):
             browserEnv["MOZ_DEVELOPER_REPO_DIR"] = options.topsrcdir
         if hasattr(options, "topobjdir"):
             browserEnv["MOZ_DEVELOPER_OBJ_DIR"] = options.topobjdir
 
         if options.headless:
             browserEnv["MOZ_HEADLESS"] = '1'
 
+        if options.dmd:
+            browserEnv["DMD"] = os.environ.get('DMD', '1')
+
         # These variables are necessary for correct application startup; change
         # via the commandline at your own risk.
         browserEnv["XPCOM_DEBUG_BREAK"] = "stack"
 
         # interpolate environment passed with options
         try:
             browserEnv.update(
                 dict(
--- a/testing/mochitest/runtestsremote.py
+++ b/testing/mochitest/runtestsremote.py
@@ -275,16 +275,18 @@ class MochiRemote(MochitestDesktop):
         # remove desktop environment not used on device
         if "XPCOM_MEM_BLOAT_LOG" in browserEnv:
             del browserEnv["XPCOM_MEM_BLOAT_LOG"]
         # override mozLogs to avoid processing in MochitestDesktop base class
         self.mozLogs = None
         browserEnv["MOZ_LOG_FILE"] = os.path.join(
             self.remoteMozLog,
             self.mozLogName)
+        if options.dmd:
+            browserEnv['DMD'] = '1'
         return browserEnv
 
     def runApp(self, *args, **kwargs):
         """front-end automation.py's `runApp` functionality until FennecRunner is written"""
 
         # automation.py/remoteautomation `runApp` takes the profile path,
         # whereas runtest.py's `runApp` takes a mozprofile object.
         if 'profileDir' not in kwargs and 'profile' in kwargs: