Bug 1364613 - Disable replace-malloc on hazard builds. r?sfink draft
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 16 May 2017 08:20:46 +0900
changeset 578561 af46c56bde58f7dfe2bed234ba67f1948120a8ac
parent 578139 648fe1aaa3ce005b7b00de5a692b7984e5995b09
child 628752 2b512c1c85ba2e2a0710bad4e9cbd51d5fb66bfd
push id58964
push userbmo:mh+mozilla@glandium.org
push dateTue, 16 May 2017 06:27:00 +0000
reviewerssfink
bugs1364613, 1361258
milestone55.0a1
Bug 1364613 - Disable replace-malloc on hazard builds. r?sfink This avoids some known hazard from replace-malloc itself, and unhides --disable-replace-malloc hazards if there are any (and there is one from bug 1361258), which wouldn't be caught until riding trains (replace-malloc being only enabled on nightly). The hazard from bug 1361258 that disappears is this one: Error: Indirect call malloc_hook_table_t.jemalloc_thread_local_arena_hook Location: replace_jemalloc_thread_local_arena @memory/replace/replace/ReplaceMalloc.cpp#261 Stack Trace: jemalloc_thread_local_arena @ memory/build/replace_malloc.c#287 Gecko_SetJemallocThreadLocalArena @ layout/style/ServoBindings.cpp#2062 The new hazard from that bug is: Error: Variable assignment jemalloc.c:arenas_map Location: jemalloc_thread_local_arena @memory/mozjemalloc/jemalloc.c#3068 Stack Trace: Gecko_SetJemallocThreadLocalArena @ layout/style/ServoBindings.cpp#2048 Where arenas_map is a thread-local variable, so there really is no hazard.
browser/config/mozconfigs/linux64/hazards
js/src/devtools/rootAnalysis/analyzeHeapWrites.js
--- a/browser/config/mozconfigs/linux64/hazards
+++ b/browser/config/mozconfigs/linux64/hazards
@@ -26,16 +26,18 @@ mk_add_options MOZ_OBJDIR=obj-analyzed
 # (--enable-debug, --enable-tests) in the trickiest way possible
 # (--enable-optimize) to maximize the chance of seeing tricky static orderings.
 ac_add_options --enable-debug
 ac_add_options --enable-tests
 ac_add_options --enable-optimize
 ac_add_options --with-compiler-wrapper=$TOOLTOOL_DIR/sixgill/usr/libexec/sixgill/scripts/wrap_gcc/basecc
 ac_add_options --without-ccache
 
+ac_add_options --disable-replace-malloc
+
 CFLAGS="$CFLAGS -Wno-attributes"
 CPPFLAGS="$CPPFLAGS -Wno-attributes"
 CXXFLAGS="$CXXFLAGS -Wno-attributes"
 
 export PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig:/usr/share/pkgconfig
 . $topsrcdir/build/unix/mozconfig.gtk
 
 . "$topsrcdir/build/mozconfig.common.override"
--- a/js/src/devtools/rootAnalysis/analyzeHeapWrites.js
+++ b/js/src/devtools/rootAnalysis/analyzeHeapWrites.js
@@ -96,39 +96,29 @@ function checkOverridableVirtualCall(ent
 
     dumpError(entry, location, "AddRef/Release on nsISupports");
 }
 
 function checkIndirectCall(entry, location, callee)
 {
     var name = entry.name;
 
-    // replace_malloc indirects through this table.
-    if (callee.startsWith('malloc_table_t.'))
-        return;
-
     // These hash table callbacks should be threadsafe.
     if (/PLDHashTable/.test(name) && (/matchEntry/.test(callee) || /hashKey/.test(callee)))
         return;
     if (/PL_HashTable/.test(name) && /keyCompare/.test(callee))
         return;
 
     dumpError(entry, location, "Indirect call " + callee);
 }
 
 function checkVariableAssignment(entry, location, variable)
 {
     var name = entry.name;
 
-    // Malloc related state.
-    if (/replace_malloc_initialized/.test(variable))
-        return;
-    if (name == "replace_init")
-        return;
-
     dumpError(entry, location, "Variable assignment " + variable);
 }
 
 // Annotations for function parameters, based on function name and parameter
 // name + type.
 function treatAsSafeArgument(entry, varName, csuName)
 {
     var whitelist = [
@@ -348,30 +338,30 @@ function ignoreContents(entry)
         /MOZ_ReportAssertionFailure/,
         /MOZ_ReportCrash/,
         /AnnotateMozCrashReason/,
         /InvalidArrayIndex_CRASH/,
         /NS_ABORT_OOM/,
 
         // These ought to be threadsafe.
         "NS_DebugBreak",
-        "replace_free", "replace_malloc",
         /mozalloc_handle_oom/,
         /^NS_Log/, /log_print/, /LazyLogModule::operator/,
         /SprintfLiteral/, "PR_smprintf", "PR_smprintf_free",
         /NS_DispatchToMainThread/, /NS_ReleaseOnMainThread/,
         /NS_NewRunnableFunction/, /NS_Atomize/,
         /nsCSSValue::BufferFromString/,
         /NS_strdup/,
         /Assert_NoQueryNeeded/,
         /imgRequestProxy::GetProgressTracker/, // Uses an AutoLock
         /Smprintf/,
         "malloc",
         "free",
         "realloc",
+        "jemalloc_thread_local_arena",
         /profiler_register_thread/,
         /profiler_unregister_thread/,
 
         // These all create static strings in local storage, which is threadsafe
         // to do but not understood by the analysis yet.
         / EmptyString\(\)/,
         /nsCSSProps::LookupPropertyValue/,
         /nsCSSProps::ValueToKeyword/,
@@ -388,17 +378,16 @@ function ignoreContents(entry)
         /nsCSSValue::SetCalcValue/,
         /CSSValueSerializeCalcOps::Append/,
         "Gecko_CSSValue_SetFunction",
         "Gecko_CSSValue_SetArray",
         "Gecko_EnsureMozBorderColors",
         "Gecko_ClearMozBorderColors",
         "Gecko_AppendMozBorderColors",
         "Gecko_CopyMozBorderColors",
-        "Gecko_SetJemallocThreadLocalArena",
         "Gecko_SetNullImageValue",
 
         // Needs main thread assertions or other fixes.
         /UndisplayedMap::GetEntryFor/,
         /nsStyleContext::CalcStyleDifferenceInternal/,
         /EffectCompositor::GetServoAnimationRule/,
         /LookAndFeel::GetColor/,
         "Gecko_CopyStyleContentsFrom",