Bug 1127201 (part 1) - Let MOZ_ASSERT take a string variable as the second arg. r=Waldo.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 04 Feb 2015 19:42:29 -0800
changeset 227755 27ba3e43fb81b90883e137e2457fcf3f7658143c
parent 227754 31ef1bac5d92115b643e21d6d411ae5743b609a6
child 227756 7fa65aa297a48f9f568e58e38534f1c028624d64
push id28238
push userkwierso@gmail.com
push dateFri, 06 Feb 2015 00:55:16 +0000
treeherdermozilla-central@7c5f187b65bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1127201
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1127201 (part 1) - Let MOZ_ASSERT take a string variable as the second arg. r=Waldo. ASSERT_UNLESS_FUZZING() (which is defined multiple times!) caused problems due when __VA_ARGS__ was empty which is most of the time. So I just disallowed the optional string, which was only used in a small fraction of the occurrences. I don't particularly like this patch. I'm not convinced its any better than just removing the nsPrintfCString()s like I did earlier, but I've done it to at least show what's involved.
dom/indexedDB/ActorsParent.cpp
dom/ipc/Blob.cpp
dom/ipc/ContentProcessManager.cpp
dom/ipc/nsIContentParent.cpp
ipc/glue/BackgroundParentImpl.cpp
mfbt/Assertions.h
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -92,19 +92,19 @@ namespace dom {
 namespace indexedDB {
 
 using namespace mozilla::dom::quota;
 using namespace mozilla::ipc;
 
 #define DISABLE_ASSERTS_FOR_FUZZING 0
 
 #if DISABLE_ASSERTS_FOR_FUZZING
-#define ASSERT_UNLESS_FUZZING(...) do { } while (0)
+#define ASSERT_UNLESS_FUZZING() do { } while (0)
 #else
-#define ASSERT_UNLESS_FUZZING(...) MOZ_ASSERT(false, __VA_ARGS__)
+#define ASSERT_UNLESS_FUZZING() MOZ_ASSERT(false)
 #endif
 
 namespace {
 
 class Cursor;
 class Database;
 struct DatabaseActorInfo;
 class DatabaseLoggingInfo;
--- a/dom/ipc/Blob.cpp
+++ b/dom/ipc/Blob.cpp
@@ -50,19 +50,19 @@
 
 #ifdef OS_POSIX
 #include "chrome/common/file_descriptor_set_posix.h"
 #endif
 
 #define DISABLE_ASSERTS_FOR_FUZZING 0
 
 #if DISABLE_ASSERTS_FOR_FUZZING
-#define ASSERT_UNLESS_FUZZING(...) do { } while (0)
+#define ASSERT_UNLESS_FUZZING() do { } while (0)
 #else
-#define ASSERT_UNLESS_FUZZING(...) MOZ_ASSERT(false, __VA_ARGS__)
+#define ASSERT_UNLESS_FUZZING() MOZ_ASSERT(false)
 #endif
 
 #define PRIVATE_REMOTE_INPUT_STREAM_IID \
   {0x30c7699f, 0x51d2, 0x48c8, {0xad, 0x56, 0xc0, 0x16, 0xd7, 0x6f, 0x71, 0x27}}
 
 namespace mozilla {
 namespace dom {
 
--- a/dom/ipc/ContentProcessManager.cpp
+++ b/dom/ipc/ContentProcessManager.cpp
@@ -10,19 +10,19 @@
 
 #include "mozilla/StaticPtr.h"
 #include "mozilla/ClearOnShutdown.h"
 
 #include "nsPrintfCString.h"
 
 // XXX need another bug to move this to a common header.
 #ifdef DISABLE_ASSERTS_FOR_FUZZING
-#define ASSERT_UNLESS_FUZZING(...) do { } while (0)
+#define ASSERT_UNLESS_FUZZING() do { } while (0)
 #else
-#define ASSERT_UNLESS_FUZZING(...) MOZ_ASSERT(false, __VA_ARGS__)
+#define ASSERT_UNLESS_FUZZING() MOZ_ASSERT(false)
 #endif
 
 namespace mozilla {
 namespace dom {
 
 static uint64_t gTabId = 0;
 
 /* static */
@@ -73,17 +73,17 @@ ContentProcessManager::RemoveContentProc
 bool
 ContentProcessManager::AddGrandchildProcess(const ContentParentId& aParentCpId,
                                             const ContentParentId& aChildCpId)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   auto iter = mContentParentMap.find(aParentCpId);
   if (NS_WARN_IF(iter == mContentParentMap.end())) {
-    ASSERT_UNLESS_FUZZING("Parent process should be already in map!");
+    ASSERT_UNLESS_FUZZING();
     return false;
   }
   iter->second.mChildrenCpId.insert(aChildCpId);
   return true;
 }
 
 bool
 ContentProcessManager::GetParentProcessId(const ContentParentId& aChildCpId,
@@ -150,28 +150,28 @@ ContentProcessManager::AllocateTabId(con
   struct RemoteFrameInfo info;
 
   const IPCTabAppBrowserContext& appBrowser = aContext.appBrowserContext();
   // If it's a PopupIPCTabContext, it's the case that a TabChild want to
   // open a new tab. aOpenerTabId has to be it's parent frame's opener id.
   if (appBrowser.type() == IPCTabAppBrowserContext::TPopupIPCTabContext) {
     auto remoteFrameIter = iter->second.mRemoteFrames.find(aOpenerTabId);
     if (remoteFrameIter == iter->second.mRemoteFrames.end()) {
-      ASSERT_UNLESS_FUZZING("Failed to find parent frame's opener id.");
+      ASSERT_UNLESS_FUZZING();
       return TabId(0);
     }
 
     info.mOpenerTabId = remoteFrameIter->second.mOpenerTabId;
 
     const PopupIPCTabContext &ipcContext = appBrowser.get_PopupIPCTabContext();
     MOZ_ASSERT(ipcContext.opener().type() == PBrowserOrId::TTabId);
 
     remoteFrameIter = iter->second.mRemoteFrames.find(ipcContext.opener().get_TabId());
     if (remoteFrameIter == iter->second.mRemoteFrames.end()) {
-      ASSERT_UNLESS_FUZZING("Failed to find tab id.");
+      ASSERT_UNLESS_FUZZING();
       return TabId(0);
     }
 
     info.mContext = remoteFrameIter->second.mContext;
   }
   else {
     MaybeInvalidTabContext tc(aContext);
     if (!tc.IsValid()) {
--- a/dom/ipc/nsIContentParent.cpp
+++ b/dom/ipc/nsIContentParent.cpp
@@ -21,19 +21,19 @@
 #include "nsFrameMessageManager.h"
 #include "nsIJSRuntimeService.h"
 #include "nsPrintfCString.h"
 
 using namespace mozilla::jsipc;
 
 // XXX need another bug to move this to a common header.
 #ifdef DISABLE_ASSERTS_FOR_FUZZING
-#define ASSERT_UNLESS_FUZZING(...) do { } while (0)
+#define ASSERT_UNLESS_FUZZING() do { } while (0)
 #else
-#define ASSERT_UNLESS_FUZZING(...) MOZ_ASSERT(false, __VA_ARGS__)
+#define ASSERT_UNLESS_FUZZING() MOZ_ASSERT(false)
 #endif
 
 namespace mozilla {
 namespace dom {
 
 nsIContentParent::nsIContentParent()
 {
   mMessageManager = nsFrameMessageManager::NewProcessMessageManager(this);
@@ -72,37 +72,37 @@ nsIContentParent::CanOpenBrowser(const I
 {
   const IPCTabAppBrowserContext& appBrowser = aContext.appBrowserContext();
 
   // We don't trust the IPCTabContext we receive from the child, so we'll bail
   // if we receive an IPCTabContext that's not a PopupIPCTabContext.
   // (PopupIPCTabContext lets the child process prove that it has access to
   // the app it's trying to open.)
   if (appBrowser.type() != IPCTabAppBrowserContext::TPopupIPCTabContext) {
-    ASSERT_UNLESS_FUZZING("Unexpected IPCTabContext type.  Aborting AllocPBrowserParent.");
+    ASSERT_UNLESS_FUZZING();
     return false;
   }
 
   const PopupIPCTabContext& popupContext = appBrowser.get_PopupIPCTabContext();
   if (popupContext.opener().type() != PBrowserOrId::TPBrowserParent) {
-    ASSERT_UNLESS_FUZZING("Unexpected PopupIPCTabContext type.  Aborting AllocPBrowserParent.");
+    ASSERT_UNLESS_FUZZING();
     return false;
   }
 
   auto opener = static_cast<TabParent*>(popupContext.opener().get_PBrowserParent());
   if (!opener) {
-    ASSERT_UNLESS_FUZZING("Got null opener from child; aborting AllocPBrowserParent.");
+    ASSERT_UNLESS_FUZZING();
     return false;
   }
 
   // Popup windows of isBrowser frames must be isBrowser if the parent
   // isBrowser.  Allocating a !isBrowser frame with same app ID would allow
   // the content to access data it's not supposed to.
   if (!popupContext.isBrowserElement() && opener->IsBrowserElement()) {
-    ASSERT_UNLESS_FUZZING("Child trying to escalate privileges!  Aborting AllocPBrowserParent.");
+    ASSERT_UNLESS_FUZZING();
     return false;
   }
 
   MaybeInvalidTabContext tc(aContext);
   if (!tc.IsValid()) {
     NS_ERROR(nsPrintfCString("Child passed us an invalid TabContext.  (%s)  "
                              "Aborting AllocPBrowserParent.",
                              tc.GetInvalidReason()).get());
--- a/ipc/glue/BackgroundParentImpl.cpp
+++ b/ipc/glue/BackgroundParentImpl.cpp
@@ -18,19 +18,19 @@
 #include "mozilla/layout/VsyncParent.h"
 #include "nsNetUtil.h"
 #include "nsRefPtr.h"
 #include "nsThreadUtils.h"
 #include "nsTraceRefcnt.h"
 #include "nsXULAppAPI.h"
 
 #ifdef DISABLE_ASSERTS_FOR_FUZZING
-#define ASSERT_UNLESS_FUZZING(...) do { } while (0)
+#define ASSERT_UNLESS_FUZZING() do { } while (0)
 #else
-#define ASSERT_UNLESS_FUZZING(...) MOZ_ASSERT(false)
+#define ASSERT_UNLESS_FUZZING() MOZ_ASSERT(false)
 #endif
 
 using mozilla::ipc::AssertIsOnBackgroundThread;
 
 namespace {
 
 void
 AssertIsInMainProcess()
--- a/mfbt/Assertions.h
+++ b/mfbt/Assertions.h
@@ -118,39 +118,49 @@ TerminateProcess(void* hProcess, unsigne
 #else
 #define MOZ_STATIC_ASSERT_IF(cond, expr, reason)  static_assert(!(cond) || (expr), reason)
 #endif
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+static MOZ_COLD MOZ_NEVER_INLINE void
+MOZ_ReportAssertionFailureHelper(const char* aStr1, const char* aStr2,
+                                 const char* aStr3, const char* aFilename,
+                                 int aLine)
+  MOZ_PRETEND_NORETURN_FOR_STATIC_ANALYSIS
+{
+#ifdef ANDROID
+  __android_log_print(ANDROID_LOG_FATAL, "MOZ_Assert",
+                      "Assertion failure: %s%s%s, at %s:%d\n",
+                      aStr1, aStr2, aStr3, aFilename, aLine);
+#else
+  fprintf(stderr, "Assertion failure: %s%s%s, at %s:%d\n", aStr1, aStr2, aStr3,
+          aFilename, aLine);
+#ifdef MOZ_DUMP_ASSERTION_STACK
+  nsTraceRefcnt::WalkTheStack(stderr);
+#endif
+  fflush(stderr);
+#endif
+}
+
 /*
  * Prints |aStr| as an assertion failure (using aFilename and aLine as the
  * location of the assertion) to the standard debug-output channel.
  *
  * Usually you should use MOZ_ASSERT or MOZ_CRASH instead of this method.  This
  * method is primarily for internal use in this header, and only secondarily
  * for use in implementing release-build assertions.
  */
 static MOZ_COLD MOZ_ALWAYS_INLINE void
 MOZ_ReportAssertionFailure(const char* aStr, const char* aFilename, int aLine)
   MOZ_PRETEND_NORETURN_FOR_STATIC_ANALYSIS
 {
-#ifdef ANDROID
-  __android_log_print(ANDROID_LOG_FATAL, "MOZ_Assert",
-                      "Assertion failure: %s, at %s:%d\n",
-                      aStr, aFilename, aLine);
-#else
-  fprintf(stderr, "Assertion failure: %s, at %s:%d\n", aStr, aFilename, aLine);
-#ifdef MOZ_DUMP_ASSERTION_STACK
-  nsTraceRefcnt::WalkTheStack(stderr);
-#endif
-  fflush(stderr);
-#endif
+  MOZ_ReportAssertionFailureHelper(aStr, "", "", aFilename, aLine);
 }
 
 static MOZ_COLD MOZ_ALWAYS_INLINE void
 MOZ_ReportCrash(const char* aStr, const char* aFilename, int aLine)
   MOZ_PRETEND_NORETURN_FOR_STATIC_ANALYSIS
 {
 #ifdef ANDROID
   __android_log_print(ANDROID_LOG_FATAL, "MOZ_CRASH",
@@ -265,21 +275,21 @@ MOZ_ReportCrash(const char* aStr, const 
 /*
  * MOZ_ASSERT(expr [, explanation-string]) asserts that |expr| must be truthy in
  * debug builds.  If it is, execution continues.  Otherwise, an error message
  * including the expression and the explanation-string (if provided) is printed,
  * an attempt is made to invoke any existing debugger, and execution halts.
  * MOZ_ASSERT is fatal: no recovery is possible.  Do not assert a condition
  * which can correctly be falsy.
  *
- * The optional explanation-string, if provided, must be a string literal
- * explaining the assertion.  It is intended for use with assertions whose
- * correctness or rationale is non-obvious, and for assertions where the "real"
- * condition being tested is best described prosaically.  Don't provide an
- * explanation if it's not actually helpful.
+ * The optional explanation-string, if provided, must be a |const char*|
+ * (string literal or a variable) explaining the assertion.  It is intended for
+ * use with assertions whose correctness or rationale is non-obvious, and for
+ * assertions where the "real" condition being tested is best described
+ * prosaically.  Don't provide an explanation if it's not actually helpful.
  *
  *   // No explanation needed: pointer arguments often must not be NULL.
  *   MOZ_ASSERT(arg);
  *
  *   // An explanation can be helpful to explain exactly how we know an
  *   // assertion is valid.
  *   MOZ_ASSERT(state == WAITING_FOR_RESPONSE,
  *              "given that <thingA> and <thingB>, we must have...");
@@ -363,17 +373,18 @@ struct AssertionConditionType
       MOZ_REALLY_CRASH(); \
     } \
   } while (0)
 /* Now the two-argument form. */
 #define MOZ_ASSERT_HELPER2(expr, explain) \
   do { \
     MOZ_VALIDATE_ASSERT_CONDITION_TYPE(expr); \
     if (MOZ_UNLIKELY(!(expr))) { \
-      MOZ_ReportAssertionFailure(#expr " (" explain ")", __FILE__, __LINE__); \
+      MOZ_ReportAssertionFailureHelper(#expr " (", explain, ")", \
+                                       __FILE__, __LINE__); \
       MOZ_REALLY_CRASH(); \
     } \
   } while (0)
 
 #define MOZ_RELEASE_ASSERT_GLUE(a, b) a b
 #define MOZ_RELEASE_ASSERT(...) \
   MOZ_RELEASE_ASSERT_GLUE( \
     MOZ_PASTE_PREFIX_AND_ARG_COUNT(MOZ_ASSERT_HELPER, __VA_ARGS__), \