Bug 1380096 - Don't require that sMainThreadRunnableName is null-terminated, r=erahm
☠☠ backed out by 88e332b0ef13 ☠ ☠
authorMichael Layzell <michael@thelayzells.com>
Tue, 11 Jul 2017 16:52:09 -0400
changeset 607738 8fe427b20f9074c88387cacd01c55b2e06c96394
parent 607737 e3e1e82bcb3dd70b4de49707f7ed0ea4d0b2d07b
child 607739 703825995ef40591a932983f7a812c335f50ec75
push id68095
push userbmo:rbarker@mozilla.com
push dateWed, 12 Jul 2017 20:01:47 +0000
reviewerserahm
bugs1380096
milestone56.0a1
Bug 1380096 - Don't require that sMainThreadRunnableName is null-terminated, r=erahm MozReview-Commit-ID: 9Zo1vjmKzZC
xpcom/threads/ThreadStackHelper.cpp
xpcom/threads/nsThread.cpp
xpcom/threads/nsThread.h
--- a/xpcom/threads/ThreadStackHelper.cpp
+++ b/xpcom/threads/ThreadStackHelper.cpp
@@ -133,30 +133,31 @@ ThreadStackHelper::GetStacksInternal(Sta
 
 #ifdef MOZ_THREADSTACKHELPER_PSEUDO
   ScopedSetPtr<Stack> stackPtr(mStackToFill, aStack);
 #endif
 #ifdef MOZ_THREADSTACKHELPER_NATIVE
   ScopedSetPtr<NativeStack> nativeStackPtr(mNativeStackToFill, aNativeStack);
 #endif
 
-  char nameBuffer[1000] = {0};
+  Vector<char, 1000> nameBuffer;
   auto callback = [&, this] (void** aPCs, size_t aCount, bool aIsMainThread) {
     // NOTE: We cannot allocate any memory in this callback, as the target
     // thread is suspended, so we first copy it into a stack-allocated buffer,
     // and then once the target thread is resumed, we can copy it into a real
     // nsCString.
     //
     // Currently we only store the names of runnables which are running on the
     // main thread, so we only want to read sMainThreadRunnableName and copy its
     // value in the case that we are currently suspending the main thread.
     if (aIsMainThread && nsThread::sMainThreadRunnableName) {
-      strncpy(nameBuffer, nsThread::sMainThreadRunnableName, sizeof(nameBuffer));
-      // Make sure the string is null-terminated.
-      nameBuffer[sizeof(nameBuffer) - 1] = '\0';
+      MOZ_ALWAYS_TRUE(
+        nameBuffer.append(nsThread::sMainThreadRunnableName->BeginReading(),
+                          std::min(uint32_t(nameBuffer.sMaxInlineStorage),
+                                   uint32_t(nsThread::sMainThreadRunnableName->Length()))));
     }
 
 #ifdef MOZ_THREADSTACKHELPER_PSEUDO
     if (mStackToFill) {
       FillStackBuffer();
     }
 #endif
 
@@ -172,18 +173,18 @@ ThreadStackHelper::GetStacksInternal(Sta
 
   if (mStackToFill || mNativeStackToFill) {
     profiler_suspend_and_sample_thread(mThreadId,
                                        callback,
                                        /* aSampleNative = */ !!aNativeStack);
   }
 
   // Copy the name buffer allocation into the output string.
-  if (nameBuffer[0] != 0) {
-    aRunnableName = nameBuffer;
+  if (nameBuffer.length() > 0) {
+    aRunnableName.AssignASCII(nameBuffer.begin(), nameBuffer.length());
   }
 #endif
 }
 
 bool
 ThreadStackHelper::PrepareStackBuffer(Stack& aStack)
 {
   // Return false to skip getting the stack and return an empty stack
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -94,17 +94,17 @@ using namespace mozilla;
 static LazyLogModule sThreadLog("nsThread");
 #ifdef LOG
 #undef LOG
 #endif
 #define LOG(args) MOZ_LOG(sThreadLog, mozilla::LogLevel::Debug, args)
 
 NS_DECL_CI_INTERFACE_GETTER(nsThread)
 
-const char* nsThread::sMainThreadRunnableName = nullptr;
+const nsACString* nsThread::sMainThreadRunnableName = nullptr;
 
 //-----------------------------------------------------------------------------
 // Because we do not have our own nsIFactory, we have to implement nsIClassInfo
 // somewhat manually.
 
 class nsThreadClassInfo : public nsIClassInfo
 {
 public:
@@ -1417,25 +1417,27 @@ nsThread::ProcessNextEvent(bool aMayWait
           // overdrawn our idle budget, if it's negative it will go in
           // the 0 bucket of the histogram.
           idleTimer.emplace(name, mNextIdleDeadline);
         }
       }
 
       // If we're on the main thread, we want to record our current runnable's
       // name in a static so that BHR can record it.
-      const char* restoreRunnableName = nullptr;
+      const nsACString* restoreRunnableName = nullptr;
       auto clear = MakeScopeExit([&] {
         if (MAIN_THREAD == mIsMainThread) {
+          MOZ_ASSERT(NS_IsMainThread());
           sMainThreadRunnableName = restoreRunnableName;
         }
       });
       if (MAIN_THREAD == mIsMainThread) {
+        MOZ_ASSERT(NS_IsMainThread());
         restoreRunnableName = sMainThreadRunnableName;
-        sMainThreadRunnableName = name.get();
+        sMainThreadRunnableName = &name;
       }
 #endif
 
       event->Run();
     } else if (aMayWait) {
       MOZ_ASSERT(ShuttingDown(),
                  "This should only happen when shutting down");
       rv = NS_ERROR_UNEXPECTED;
--- a/xpcom/threads/nsThread.h
+++ b/xpcom/threads/nsThread.h
@@ -88,17 +88,17 @@ public:
   {
     kMaybeReport,
     kForceReport
   };
 
   static bool SaveMemoryReportNearOOM(ShouldSaveMemoryReport aShouldSave);
 #endif
 
-  static const char* sMainThreadRunnableName;
+  static const nsACString* sMainThreadRunnableName;
 
 private:
   void DoMainThreadSpecificProcessing(bool aReallyWait);
 
   // Returns a null TimeStamp if we're not in the idle period.
   mozilla::TimeStamp GetIdleDeadline();
   void GetIdleEvent(nsIRunnable** aEvent, mozilla::MutexAutoLock& aProofOfLock);
   void GetEvent(bool aWait, nsIRunnable** aEvent,