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.
This commit is contained in:
Nicholas Nethercote 2015-02-04 19:42:29 -08:00
parent 84eb5e1b6e
commit f9f2575aa6
6 changed files with 45 additions and 34 deletions

View File

@ -97,9 +97,9 @@ 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 {

View File

@ -55,9 +55,9 @@
#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 \

View File

@ -15,9 +15,9 @@
// 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 {
@ -78,7 +78,7 @@ ContentProcessManager::AddGrandchildProcess(const ContentParentId& aParentCpId,
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);
@ -155,7 +155,7 @@ ContentProcessManager::AllocateTabId(const TabId& aOpenerTabId,
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);
}
@ -166,7 +166,7 @@ ContentProcessManager::AllocateTabId(const TabId& aOpenerTabId,
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);
}

View File

@ -26,9 +26,9 @@ 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 {
@ -77,19 +77,19 @@ nsIContentParent::CanOpenBrowser(const IPCTabContext& aContext)
// (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;
}
@ -97,7 +97,7 @@ nsIContentParent::CanOpenBrowser(const IPCTabContext& aContext)
// 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;
}

View File

@ -23,9 +23,9 @@
#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;

View File

@ -123,6 +123,26 @@ __declspec(dllimport) void* __stdcall GetCurrentProcess(void);
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.
@ -135,17 +155,7 @@ 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
@ -270,11 +280,11 @@ __declspec(noreturn) __inline void MOZ_NoReturn() {}
* 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);
@ -368,7 +378,8 @@ struct AssertionConditionType
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)