Bug 1324093 - Part 1: Move MOZ_REALLY_CRASH's null-deref and TerminateProcess into a never-inline function. r=froydnj
☠☠ backed out by a991fee52cc9 ☠ ☠
authorDavid Major <dmajor@mozilla.com>
Fri, 13 Jan 2017 13:58:22 +1300
changeset 329237 d3cc90db1908a37934800e45e3b83a1883ca138b
parent 329236 865a34953fb530f32a31fa1915be9a0aa1f3941a
child 329238 0ee273b613dbec2dee5df5ac4de8cd88c7056c87
push id31201
push usercbook@mozilla.com
push dateFri, 13 Jan 2017 09:20:42 +0000
treeherdermozilla-central@91f5293e9a89 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1324093
milestone53.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 1324093 - Part 1: Move MOZ_REALLY_CRASH's null-deref and TerminateProcess into a never-inline function. r=froydnj The C versus C++ distinction was only there so that Android could make sure it used the global ::abort. I didn't see the need to maintain the distinction for Windows. (Besides, with this change we're no longer doing textual inclusion of "TerminateProcess" in the macro, so people can't take over the name.) Linux's abort sequence wasn't long enough to be troublesome, so I left it alone. MozReview-Commit-ID: 9Q6f0WOnzws
mfbt/Assertions.h
--- a/mfbt/Assertions.h
+++ b/mfbt/Assertions.h
@@ -42,17 +42,17 @@ AnnotateMozCrashReason(const char* reaso
 #  define MOZ_CRASH_ANNOTATE(...) AnnotateMozCrashReason(__VA_ARGS__)
 #else
 #  define MOZ_CRASH_ANNOTATE(...) do { /* nothing */ } while (0)
 #endif
 
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
-#ifdef WIN32
+#ifdef _MSC_VER
    /*
     * TerminateProcess and GetCurrentProcess are defined in <winbase.h>, which
     * further depends on <windef.h>.  We hardcode these few definitions manually
     * because those headers clutter the global namespace with a significant
     * number of undesired macros and symbols.
     */
 MOZ_BEGIN_EXTERN_C
 __declspec(dllimport) int __stdcall
@@ -187,52 +187,43 @@ MOZ_ReportCrash(const char* aStr, const 
  */
 #if defined(_MSC_VER)
    /*
     * On MSVC use the __debugbreak compiler intrinsic, which produces an inline
     * (not nested in a system function) breakpoint.  This distinctively invokes
     * Breakpad without requiring system library symbols on all stack-processing
     * machines, as a nested breakpoint would require.
     *
+    * We use __LINE__ to prevent the compiler from folding multiple crash sites
+    * together, which would make crash reports hard to understand.
+    *
     * We use TerminateProcess with the exit code aborting would generate
     * because we don't want to invoke atexit handlers, destructors, library
     * unload handlers, and so on when our process might be in a compromised
     * state.
     *
     * We don't use abort() because it'd cause Windows to annoyingly pop up the
     * process error dialog multiple times.  See bug 345118 and bug 426163.
     *
-    * We follow TerminateProcess() with a call to MOZ_NoReturn() so that the
-    * compiler doesn't hassle us to provide a return statement after a
-    * MOZ_REALLY_CRASH() call.
-    *
     * (Technically these are Windows requirements, not MSVC requirements.  But
     * practically you need MSVC for debugging, and we only ship builds created
     * by MSVC, so doing it this way reduces complexity.)
     */
 
-__declspec(noreturn) __inline void MOZ_NoReturn() {}
+static MOZ_COLD MOZ_NORETURN MOZ_NEVER_INLINE void MOZ_NoReturn(int aLine)
+{
+  *((volatile int*) NULL) = aLine;
+  TerminateProcess(GetCurrentProcess(), 3);
+}
 
-#  ifdef __cplusplus
-#    define MOZ_REALLY_CRASH() \
-       do { \
-         ::__debugbreak(); \
-         *((volatile int*) NULL) = __LINE__; \
-         ::TerminateProcess(::GetCurrentProcess(), 3); \
-         ::MOZ_NoReturn(); \
-       } while (0)
-#  else
-#    define MOZ_REALLY_CRASH() \
-       do { \
-         __debugbreak(); \
-         *((volatile int*) NULL) = __LINE__; \
-         TerminateProcess(GetCurrentProcess(), 3); \
-         MOZ_NoReturn(); \
-       } while (0)
-#  endif
+#  define MOZ_REALLY_CRASH() \
+     do { \
+       __debugbreak(); \
+       MOZ_NoReturn(__LINE__); \
+     } while (0)
 #else
 #  ifdef __cplusplus
 #    define MOZ_REALLY_CRASH() \
        do { \
          *((volatile int*) NULL) = __LINE__; \
          ::abort(); \
        } while (0)
 #  else