Bug 1282737 - Use a Shmem to store abort message from child. r?billm,aklotz draft
authorKan-Ru Chen <kanru@kanru.info>
Fri, 01 Jul 2016 12:22:11 +0800
changeset 383064 407140f52ad8f533f00248efaca35c408542a9b8
parent 383063 739ed29a7098d6692a3375c8af2c60874bef9093
child 524372 abd9b4e7f4ec33323002718b4a73ee22e36bbac8
push id21912
push userbmo:kchen@mozilla.com
push dateFri, 01 Jul 2016 04:28:59 +0000
reviewersbillm, aklotz
bugs1282737
milestone50.0a1
Bug 1282737 - Use a Shmem to store abort message from child. r?billm,aklotz This patch aims to bring AbortMessage annotation back to the child process that is thread-safe and IPDL-safe. It uses a Shmem as the abort message buffer that is shared between parent and child. When child aborts, the parent process can safely reopen the sealed Shmem segments and read the abort message from it. The child process may put arbitrary data in the buffer so the data is still escaped in the parent process. MozReview-Commit-ID: 6XYwleEZYK8
dom/ipc/ContentParent.cpp
dom/ipc/CrashReporterChild.cpp
dom/ipc/CrashReporterChild.h
dom/ipc/CrashReporterParent.cpp
dom/ipc/CrashReporterParent.h
dom/ipc/PCrashReporter.ipdl
toolkit/crashreporter/nsExceptionHandler.cpp
toolkit/crashreporter/nsExceptionHandler.h
toolkit/crashreporter/test/unit_ipc/test_content_annotation.js
xpcom/base/nsDebugImpl.cpp
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -3650,17 +3650,19 @@ ContentParent::AllocPCrashReporterParent
 #endif
 }
 
 bool
 ContentParent::RecvPCrashReporterConstructor(PCrashReporterParent* actor,
                                              const NativeThreadId& tid,
                                              const uint32_t& processType)
 {
-  static_cast<CrashReporterParent*>(actor)->SetChildData(tid, processType);
+  auto reporter = static_cast<CrashReporterParent*>(actor);
+  reporter->SetChildData(tid, processType);
+  reporter->InitAbortMessageStore();
   return true;
 }
 
 bool
 ContentParent::DeallocPCrashReporterParent(PCrashReporterParent* crashreporter)
 {
   delete crashreporter;
   return true;
--- a/dom/ipc/CrashReporterChild.cpp
+++ b/dom/ipc/CrashReporterChild.cpp
@@ -8,16 +8,33 @@
 #include "CrashReporterChild.h"
 #include "nsXULAppAPI.h"
 
 using mozilla::plugins::PluginModuleChild;
 
 namespace mozilla {
 namespace dom {
 
+bool
+CrashReporterChild::RecvInitAbortMessageStore(mozilla::ipc::Shmem&& aShmem)
+{
+  mAbortMessageStore = Move(aShmem);
+  return true;
+}
+
+bool
+CrashReporterChild::PutAbortMessage(const nsCString& aMessage)
+{
+  size_t buflen = mAbortMessageStore.Size<char>();
+  char* buf = mAbortMessageStore.get<char>();
+  strncpy(buf, aMessage.get(), buflen - 1);
+  buf[buflen - 1] = '\0';
+  return true;
+}
+
 /*static*/
 PCrashReporterChild*
 CrashReporterChild::GetCrashReporter()
 {
   const ManagedContainer<PCrashReporterChild>* reporters = nullptr;
   switch (XRE_GetProcessType()) {
     case GeckoProcessType_Content: {
       ContentChild* child = ContentChild::GetSingleton();
--- a/dom/ipc/CrashReporterChild.h
+++ b/dom/ipc/CrashReporterChild.h
@@ -3,30 +3,37 @@
 /* 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/. */
 
 #ifndef mozilla_dom_CrashReporterChild_h
 #define mozilla_dom_CrashReporterChild_h
 
 #include "mozilla/dom/PCrashReporterChild.h"
+#include "mozilla/ipc/Shmem.h"
 
 namespace mozilla {
 namespace dom {
 
 class CrashReporterChild :
   public PCrashReporterChild
 {
 public:
   CrashReporterChild() {
     MOZ_COUNT_CTOR(CrashReporterChild);
   }
   ~CrashReporterChild() {
     MOZ_COUNT_DTOR(CrashReporterChild);
   }
 
   static PCrashReporterChild* GetCrashReporter();
+
+  bool PutAbortMessage(const nsCString& aMessage);
+
+private:
+  bool RecvInitAbortMessageStore(mozilla::ipc::Shmem&& aShmem) override;
+  mozilla::ipc::Shmem mAbortMessageStore;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_CrashReporterChild_h
--- a/dom/ipc/CrashReporterParent.cpp
+++ b/dom/ipc/CrashReporterParent.cpp
@@ -92,16 +92,29 @@ void
 CrashReporterParent::SetChildData(const NativeThreadId& tid,
                                   const uint32_t& processType)
 {
   mInitialized = true;
   mMainThread = tid;
   mProcessType = processType;
 }
 
+void
+CrashReporterParent::InitAbortMessageStore()
+{
+  AllocShmem(kAbortMessageStoreSize, SharedMemory::TYPE_BASIC, &mAbortMessageStore);
+  memset(mAbortMessageStore.get<char>(), 0, kAbortMessageStoreSize);
+  // toSend is zeroed out by SendInitAbortMessageStore. mAbortMessageStore
+  // can be used to resurrect the shmem later.
+  Shmem toSend(mAbortMessageStore);
+  if (!SendInitAbortMessageStore(toSend)) {
+    NS_WARNING("can not initialize CrashReporter emergency storage");
+  }
+}
+
 #ifdef MOZ_CRASHREPORTER
 bool
 CrashReporterParent::GenerateCrashReportForMinidump(nsIFile* minidump,
                                                     const AnnotationTable* processNotes)
 {
   if (!CrashReporter::GetIDFromMinidump(minidump, mChildDumpID)) {
     return false;
   }
@@ -139,16 +152,22 @@ CrashReporterParent::GenerateChildData(c
   char startTime[32];
   snprintf_literal(startTime, "%lld", static_cast<long long>(mStartTime));
   mNotes.Put(NS_LITERAL_CSTRING("StartupTime"), nsDependentCString(startTime));
 
   if (!mAppNotes.IsEmpty()) {
     mNotes.Put(NS_LITERAL_CSTRING("Notes"), mAppNotes);
   }
 
+  mAbortMessageStore.ResurrectReadOnly();
+  nsDependentCString abortMessage(mAbortMessageStore.get<char>());
+  if (abortMessage.Length()) {
+    mNotes.Put(NS_LITERAL_CSTRING("AbortMessage"), abortMessage);
+  }
+
   // Append these notes to the end of the extra file based on the current
   // dump id we obtained from CreatePairedMinidumps.
   bool ret = CrashReporter::AppendExtraData(mChildDumpID, mNotes);
   if (ret && processNotes) {
     ret = CrashReporter::AppendExtraData(mChildDumpID, *processNotes);
   }
 
   if (!ret) {
--- a/dom/ipc/CrashReporterParent.h
+++ b/dom/ipc/CrashReporterParent.h
@@ -4,31 +4,34 @@
  * 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/. */
 
 #ifndef mozilla_dom_CrashReporterParent_h
 #define mozilla_dom_CrashReporterParent_h
 
 #include "mozilla/dom/PCrashReporterParent.h"
 #include "mozilla/dom/TabMessageUtils.h"
+#include "mozilla/ipc/Shmem.h"
 #include "nsIFile.h"
 #ifdef MOZ_CRASHREPORTER
 #include "nsExceptionHandler.h"
 #include "nsDataHashtable.h"
 #endif
 
 namespace mozilla {
 namespace dom {
 
 class CrashReporterParent : public PCrashReporterParent
 {
 #ifdef MOZ_CRASHREPORTER
   typedef CrashReporter::AnnotationTable AnnotationTable;
 #endif
 public:
+  static const size_t kAbortMessageStoreSize = 4096;
+
   CrashReporterParent();
   virtual ~CrashReporterParent();
 
 #ifdef MOZ_CRASHREPORTER
 
   /*
    * Attempt to create a bare-bones crash report, along with extra process-
    * specific annotations present in the given AnnotationTable. Calls
@@ -128,16 +131,22 @@ public:
 
   /*
    * Initialize this reporter with data from the child process.
    */
   void
   SetChildData(const NativeThreadId& id, const uint32_t& processType);
 
   /*
+   * Initialize the abort message storage shared with the child process.
+   */
+  void
+  InitAbortMessageStore();
+
+  /*
    * Returns the ID of the child minidump.
    * GeneratePairedMinidump or GenerateCrashReport must be called first.
    */
   const nsString& ChildDumpID() const {
     return mChildDumpID;
   }
 
   /*
@@ -175,16 +184,17 @@ public:
   nsCString mAppNotes;
   nsString mChildDumpID;
   // stores the child main thread id
   NativeThreadId mMainThread;
   time_t mStartTime;
   // stores the child process type
   uint32_t mProcessType;
   bool mInitialized;
+  mozilla::ipc::Shmem mAbortMessageStore;
 };
 
 #ifdef MOZ_CRASHREPORTER
 template<class Toplevel>
 inline bool
 CrashReporterParent::GeneratePairedMinidump(Toplevel* t)
 {
   mozilla::ipc::ScopedProcessHandle child;
--- a/dom/ipc/PCrashReporter.ipdl
+++ b/dom/ipc/PCrashReporter.ipdl
@@ -16,16 +16,18 @@ struct Mapping {
   nsCString file_id;
   uintptr_t start_address;
   size_t mapping_length;
   size_t file_offset;
 };
 
 async protocol PCrashReporter {
   manager PContent or PPluginModule or PGMP;
+child:
+  async InitAbortMessageStore(Shmem shmem);
 parent:
   async AnnotateCrashReport(nsCString key, nsCString data);
   async AppendAppNotes(nsCString data);
   async __delete__();
 };
 
 }
 }
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -2249,16 +2249,43 @@ nsresult AppendAppNotesToCrashReport(con
   }
 
   MutexAutoLock lock(*notesFieldLock);
 
   notesField->Append(data);
   return AnnotateCrashReport(NS_LITERAL_CSTRING("Notes"), *notesField);
 }
 
+nsresult AnnotateAbortMessage(const nsACString& message)
+{
+  if (!GetEnabled())
+    return NS_ERROR_NOT_INITIALIZED;
+
+  if (XRE_IsParentProcess()) {
+    return AnnotateCrashReport(NS_LITERAL_CSTRING("AbortMessage"), message);
+  }
+
+  nsCString escapedData;
+  nsresult rv = EscapeAnnotation(NS_LITERAL_CSTRING("AbortMessage"),
+                                 message, escapedData);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  {
+    MutexAutoLock lock(*crashReporterAPILock);
+
+    auto* reporter =
+      static_cast<CrashReporterChild*>(CrashReporterChild::GetCrashReporter());
+    reporter->PutAbortMessage(escapedData);
+  }
+
+  return NS_OK;
+}
+
 nsresult CrashReporterHelperRunnable::Run()
 {
   // We expect this to be in the child process' main thread.  If it isn't,
   // something is happening we didn't design for.
   MOZ_ASSERT(!XRE_IsParentProcess());
   MOZ_ASSERT(NS_IsMainThread());
 
   // Don't just leave the assert, paranoid about infinite recursion
@@ -3604,16 +3631,19 @@ SetRemoteExceptionHandler(const nsACStri
                      nullptr,    // no minidump callback
                      nullptr,    // no callback context
                      google_breakpad::ExceptionHandler::HANDLER_ALL,
                      GetMinidumpType(),
                      NS_ConvertASCIItoUTF16(crashPipe).get(),
                      nullptr);
   gExceptionHandler->set_handle_debug_exceptions(true);
 
+  NS_ASSERTION(!crashReporterAPILock, "Shouldn't have a lock yet");
+  crashReporterAPILock = new Mutex("crashReporterAPILock");
+
   mozalloc_set_oom_abort_handler(AnnotateOOMAllocationSize);
 
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }
 
 //--------------------------------------------------
 #elif defined(XP_LINUX)
@@ -3655,16 +3685,19 @@ SetRemoteExceptionHandler()
 
   if (gDelayedAnnotations) {
     for (uint32_t i = 0; i < gDelayedAnnotations->Length(); i++) {
       gDelayedAnnotations->ElementAt(i)->Run();
     }
     delete gDelayedAnnotations;
   }
 
+  NS_ASSERTION(!crashReporterAPILock, "Shouldn't have a lock yet");
+  crashReporterAPILock = new Mutex("crashReporterAPILock");
+
   mozalloc_set_oom_abort_handler(AnnotateOOMAllocationSize);
 
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }
 
 //--------------------------------------------------
 #elif defined(XP_MACOSX)
@@ -3681,16 +3714,19 @@ SetRemoteExceptionHandler(const nsACStri
   gExceptionHandler = new google_breakpad::
     ExceptionHandler("",
                      ChildFilter,
                      nullptr,    // no minidump callback
                      nullptr,    // no callback context
                      true,       // install signal handlers
                      crashPipe.BeginReading());
 
+  NS_ASSERTION(!crashReporterAPILock, "Shouldn't have a lock yet");
+  crashReporterAPILock = new Mutex("crashReporterAPILock");
+
   mozalloc_set_oom_abort_handler(AnnotateOOMAllocationSize);
 
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }
 #endif  // XP_WIN
 
 
@@ -3981,16 +4017,20 @@ CreateAdditionalChildMinidump(ProcessHan
   return true;
 }
 
 bool
 UnsetRemoteExceptionHandler()
 {
   delete gExceptionHandler;
   gExceptionHandler = nullptr;
+
+  delete crashReporterAPILock;
+  crashReporterAPILock = nullptr;
+
   return true;
 }
 
 #if defined(MOZ_WIDGET_ANDROID)
 void AddLibraryMapping(const char* library_name,
                        uintptr_t   start_address,
                        size_t      mapping_length,
                        size_t      file_offset)
--- a/toolkit/crashreporter/nsExceptionHandler.h
+++ b/toolkit/crashreporter/nsExceptionHandler.h
@@ -68,16 +68,19 @@ nsresult SetMinidumpPath(const nsAString
 
 // AnnotateCrashReport, RemoveCrashReportAnnotation and
 // AppendAppNotesToCrashReport may be called from any thread in a chrome
 // process, but may only be called from the main thread in a content process.
 nsresult AnnotateCrashReport(const nsACString& key, const nsACString& data);
 nsresult RemoveCrashReportAnnotation(const nsACString& key);
 nsresult AppendAppNotesToCrashReport(const nsACString& data);
 
+// AnnotateAbortMessage may be called from any thread.
+nsresult AnnotateAbortMessage(const nsACString& aMessage);
+
 // NOTE: If you change this definition, also change the definition in Assertions.h
 // as it is intended to be defining this same function.
 void AnnotateMozCrashReason(const char* aReason);
 void AnnotateOOMAllocationSize(size_t size);
 nsresult SetGarbageCollecting(bool collecting);
 void SetEventloopNestingLevel(uint32_t level);
 
 void AnnotateTexturesSize(size_t size);
--- a/toolkit/crashreporter/test/unit_ipc/test_content_annotation.js
+++ b/toolkit/crashreporter/test/unit_ipc/test_content_annotation.js
@@ -10,13 +10,15 @@ function run_test()
   // Try crashing with a runtime abort
   do_content_crash(function() {
                      crashType = CrashTestUtils.CRASH_RUNTIMEABORT;
                      crashReporter.annotateCrashReport("TestKey", "TestValue");
                      crashReporter.appendAppNotesToCrashReport("!!!foo!!!");
                    },
                    function(mdump, extra) {
                      do_check_eq(extra.TestKey, "TestValue");
+                     do_check_true('AbortMessage' in extra);
+                     do_check_neq(extra.AbortMessage.indexOf("Intentional crash"), -1);
                      do_check_true('StartupTime' in extra);
                      do_check_true('ProcessType' in extra);
                      do_check_neq(extra.Notes.indexOf("!!!foo!!!"), -1);
                    });
 }
--- a/xpcom/base/nsDebugImpl.cpp
+++ b/xpcom/base/nsDebugImpl.cpp
@@ -384,19 +384,19 @@ NS_DebugBreak(uint32_t aSeverity, const 
       // Updating crash annotations in the child causes us to do IPC. This can
       // really cause trouble if we're asserting from within IPC code. So we
       // have to do without the annotations in that case.
       if (XRE_IsParentProcess()) {
         nsCString note("xpcom_runtime_abort(");
         note += buf.buffer;
         note += ")";
         CrashReporter::AppendAppNotesToCrashReport(note);
-        CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("AbortMessage"),
-                                           nsDependentCString(buf.buffer));
       }
+      // Updating abort message is safe from the child process.
+      CrashReporter::AnnotateAbortMessage(nsDependentCString(buf.buffer));
 #endif  // MOZ_CRASHREPORTER
 
 #if defined(DEBUG) && defined(_WIN32)
       RealBreak();
 #endif
 #if defined(DEBUG)
       nsTraceRefcnt::WalkTheStack(stderr);
 #endif