# HG changeset patch # User Gabriele Svelto # Date 1537962874 0 # Node ID 8d6329af610eb827b07f3cc5782fa1c77af2a271 # Parent 7617cbea4dc83a6c1ac1a34dce55ff866989a85f Bug 1490240 - Fix the remaining compiler warnings in the crash reporter r=ted,glandium Differential Revision: https://phabricator.services.mozilla.com/D5741 diff --git a/mozglue/misc/interceptor/PatcherNopSpace.h b/mozglue/misc/interceptor/PatcherNopSpace.h --- a/mozglue/misc/interceptor/PatcherNopSpace.h +++ b/mozglue/misc/interceptor/PatcherNopSpace.h @@ -147,49 +147,50 @@ public: ReadOnlyTargetFunction readOnlyTargetFn( this->ResolveRedirectedAddress(aTargetFn)); if (!WriteHook(readOnlyTargetFn, aHookDest, aOrigFunc)) { return false; } - mPatchedFns.append(reinterpret_cast(readOnlyTargetFn.GetBaseAddress())); - return true; + return mPatchedFns.append( + reinterpret_cast(readOnlyTargetFn.GetBaseAddress())); } bool WriteHook(const ReadOnlyTargetFunction& aFn, intptr_t aHookDest, void** aOrigFunc) { // Ensure we can read and write starting at fn - 5 (for the long jmp we're // going to write) and ending at fn + 2 (for the short jmp up to the long // jmp). These bytes may span two pages with different protection. WritableTargetFunction writableFn(aFn.Promote(7, -5)); if (!writableFn) { return false; } // Check that the 5 bytes before the function are NOP's or INT 3's, const uint8_t nopOrBp[] = { 0x90, 0xCC }; - if (!writableFn.VerifyValuesAreOneOf(nopOrBp)) { + if (!writableFn.template VerifyValuesAreOneOf(nopOrBp)) { return false; } // ... and that the first 2 bytes of the function are mov(edi, edi). // There are two ways to encode the same thing: // // 0x89 0xff == mov r/m, r // 0x8b 0xff == mov r, r/m // // where "r" is register and "r/m" is register or memory. // Windows seems to use 0x8B 0xFF. We include 0x89 0xFF out of paranoia. // (These look backwards because little-endian) const uint16_t possibleEncodings[] = { 0xFF8B, 0xFF89 }; - if (!writableFn.VerifyValuesAreOneOf(possibleEncodings, 5)) { + if (!writableFn.template VerifyValuesAreOneOf( + possibleEncodings, 5)) { return false; } // Write a long jump into the space above the function. writableFn.WriteByte(0xe9); // jmp if (!writableFn) { return false; } diff --git a/mozglue/misc/interceptor/TargetFunction.h b/mozglue/misc/interceptor/TargetFunction.h --- a/mozglue/misc/interceptor/TargetFunction.h +++ b/mozglue/misc/interceptor/TargetFunction.h @@ -15,16 +15,37 @@ #include "mozilla/Unused.h" #include "mozilla/Vector.h" #include namespace mozilla { namespace interceptor { +#if defined(_M_IX86) + +template bool +CommitAndWriteShortInternal(const T& aMMPolicy, void* aDest, uint16_t aValue); + +template <> inline bool +CommitAndWriteShortInternal( + const MMPolicyInProcess& aMMPolicy, void* aDest, uint16_t aValue) +{ + return aMMPolicy.WriteAtomic(aDest, aValue); +} + +template <> inline bool +CommitAndWriteShortInternal( + const MMPolicyOutOfProcess& aMMPolicy, void* aDest, uint16_t aValue) +{ + return aMMPolicy.Write(aDest, &aValue, sizeof(uint16_t)); +} + +#endif // defined(_M_IX86) + template class MOZ_STACK_CLASS WritableTargetFunction final { class AutoProtect final { using ProtectParams = Tuple; public: @@ -228,34 +249,16 @@ public: mAccumulatedStatus = false; return; } mOffset += sizeof(uint16_t); } #if defined(_M_IX86) -private: - template - bool CommitAndWriteShortInternal(const T& aMMPolicy, void* aDest, uint16_t aValue); - - template <> - bool CommitAndWriteShortInternal(const MMPolicyInProcess& aMMPolicy, - void* aDest, uint16_t aValue) - { - return aMMPolicy.WriteAtomic(aDest, aValue); - } - - template <> - bool CommitAndWriteShortInternal(const MMPolicyOutOfProcess& aMMPolicy, - void* aDest, uint16_t aValue) - { - return aMMPolicy.Write(aDest, &aValue, sizeof(uint16_t)); - } - public: /** * Commits any dirty writes, and then writes a short, atomically if possible. * This call may succeed in both inproc and outproc cases, but atomicity * is only guaranteed in the inproc case. */ bool CommitAndWriteShort(const uint16_t aValue) { @@ -590,92 +593,91 @@ private: static const size_t kInlineStorage = 32; #endif const MMPolicyOutOfProcess& mMMPolicy; Vector mLocalBytes; uint8_t const * const mBase; }; +template class TargetBytesPtr; + +template<> +class TargetBytesPtr +{ +public: + typedef TargetBytesPtr Type; + + static Type Make(const MMPolicyInProcess& aMMPolicy, const void* aFunc) + { + return TargetBytesPtr(aMMPolicy, aFunc); + } + + static Type CopyFromOffset(const TargetBytesPtr& aOther, + const uint32_t aOffsetFromOther) + { + return TargetBytesPtr(aOther, aOffsetFromOther); + } + + ReadOnlyTargetBytes* operator->() + { + return &mTargetBytes; + } + + TargetBytesPtr(TargetBytesPtr&& aOther) + : mTargetBytes(std::move(aOther.mTargetBytes)) + { + } + + TargetBytesPtr(const TargetBytesPtr& aOther) + : mTargetBytes(aOther.mTargetBytes) + { + } + + TargetBytesPtr& operator=(const TargetBytesPtr&) = delete; + TargetBytesPtr& operator=(TargetBytesPtr&&) = delete; + +private: + TargetBytesPtr(const MMPolicyInProcess& aMMPolicy, const void* aFunc) + : mTargetBytes(aMMPolicy, aFunc) + { + } + + TargetBytesPtr(const TargetBytesPtr& aOther, + const uint32_t aOffsetFromOther) + : mTargetBytes(aOther.mTargetBytes, aOffsetFromOther) + { + } + + ReadOnlyTargetBytes mTargetBytes; +}; + +template <> +class TargetBytesPtr +{ +public: + typedef std::shared_ptr> Type; + + static Type Make(const MMPolicyOutOfProcess& aMMPolicy, const void* aFunc) + { + return std::make_shared>( + aMMPolicy, aFunc); + } + + static Type CopyFromOffset(const Type& aOther, + const uint32_t aOffsetFromOther) + { + return std::make_shared>( + *aOther, aOffsetFromOther); + } +}; + template class MOZ_STACK_CLASS ReadOnlyTargetFunction final { - template - class TargetBytesPtr; - - template<> - class TargetBytesPtr - { - public: - typedef TargetBytesPtr Type; - - static Type Make(const MMPolicyInProcess& aMMPolicy, const void* aFunc) - { - return std::move(TargetBytesPtr(aMMPolicy, aFunc)); - } - - static Type CopyFromOffset(const TargetBytesPtr& aOther, - const uint32_t aOffsetFromOther) - { - return std::move(TargetBytesPtr(aOther, aOffsetFromOther)); - } - - ReadOnlyTargetBytes* operator->() - { - return &mTargetBytes; - } - - TargetBytesPtr(TargetBytesPtr&& aOther) - : mTargetBytes(std::move(aOther.mTargetBytes)) - { - } - - TargetBytesPtr(const TargetBytesPtr& aOther) - : mTargetBytes(aOther.mTargetBytes) - { - } - - TargetBytesPtr& operator=(const TargetBytesPtr&) = delete; - TargetBytesPtr& operator=(TargetBytesPtr&&) = delete; - - private: - TargetBytesPtr(const MMPolicyInProcess& aMMPolicy, const void* aFunc) - : mTargetBytes(aMMPolicy, aFunc) - { - } - - TargetBytesPtr(const TargetBytesPtr& aOther, - const uint32_t aOffsetFromOther) - : mTargetBytes(aOther.mTargetBytes, aOffsetFromOther) - { - } - - ReadOnlyTargetBytes mTargetBytes; - }; - - template <> - class TargetBytesPtr - { - public: - typedef std::shared_ptr> Type; - - static Type Make(const MMPolicyOutOfProcess& aMMPolicy, const void* aFunc) - { - return std::move(std::make_shared>( - aMMPolicy, aFunc)); - } - - static Type CopyFromOffset(const Type& aOther, - const uint32_t aOffsetFromOther) - { - return std::move(std::make_shared>( - *aOther, aOffsetFromOther)); - } - }; - public: ReadOnlyTargetFunction(const MMPolicy& aMMPolicy, const void* aFunc) : mTargetBytes(TargetBytesPtr::Make(aMMPolicy, aFunc)) , mOffset(0) { } ReadOnlyTargetFunction(const MMPolicy& aMMPolicy, FARPROC aFunc) diff --git a/toolkit/crashreporter/breakpad-client/linux/moz.build b/toolkit/crashreporter/breakpad-client/linux/moz.build --- a/toolkit/crashreporter/breakpad-client/linux/moz.build +++ b/toolkit/crashreporter/breakpad-client/linux/moz.build @@ -25,14 +25,11 @@ if CONFIG['OS_TARGET'] == 'Linux' or CON if CONFIG['OS_TARGET'] == 'Android': DEFINES['ANDROID_NDK_MAJOR_VERSION'] = CONFIG['ANDROID_NDK_MAJOR_VERSION'] DEFINES['ANDROID_NDK_MINOR_VERSION'] = CONFIG['ANDROID_NDK_MINOR_VERSION'] LOCAL_INCLUDES += [ '/toolkit/crashreporter/google-breakpad/src/common/android/include', ] -# We allow warnings for third-party code that can be updated from upstream. -AllowCompilerWarnings() - FINAL_LIBRARY = 'xul' include('/toolkit/crashreporter/crashreporter.mozbuild') diff --git a/toolkit/crashreporter/breakpad-client/mac/handler/moz.build b/toolkit/crashreporter/breakpad-client/mac/handler/moz.build --- a/toolkit/crashreporter/breakpad-client/mac/handler/moz.build +++ b/toolkit/crashreporter/breakpad-client/mac/handler/moz.build @@ -6,18 +6,15 @@ UNIFIED_SOURCES += [ 'breakpad_nlist_64.cc', 'dynamic_images.cc', 'exception_handler.cc', 'minidump_generator.cc', ] -# We allow warnings for third-party code that can be updated from upstream. -AllowCompilerWarnings() - FINAL_LIBRARY = 'xul' LOCAL_INCLUDES += [ '/toolkit/crashreporter/breakpad-client', '/toolkit/crashreporter/google-breakpad/src', ] diff --git a/toolkit/crashreporter/client/crashreporter_win.cpp b/toolkit/crashreporter/client/crashreporter_win.cpp --- a/toolkit/crashreporter/client/crashreporter_win.cpp +++ b/toolkit/crashreporter/client/crashreporter_win.cpp @@ -278,17 +278,18 @@ static string FormatLastError() // strip off any trailing newlines string::size_type n = message.find_last_not_of("\r\n"); if (n < message.size() - 1) { message.erase(n+1); } } else { char buf[64]; - sprintf(buf, "Unknown error, error code: 0x%08x", err); + sprintf(buf, "Unknown error, error code: 0x%08x", + static_cast(err)); message += buf; } return message; } #define TS_DRAW 2 #define BP_CHECKBOX 3 @@ -343,28 +344,16 @@ static void GetRelativeRect(HWND hwnd, H static void SetDlgItemVisible(HWND hwndDlg, UINT item, bool visible) { HWND hwnd = GetDlgItem(hwndDlg, item); ShowWindow(hwnd, visible ? SW_SHOW : SW_HIDE); } -static void SetDlgItemDisabled(HWND hwndDlg, UINT item, bool disabled) -{ - HWND hwnd = GetDlgItem(hwndDlg, item); - LONG style = GetWindowLong(hwnd, GWL_STYLE); - if (!disabled) - style |= WS_DISABLED; - else - style &= ~WS_DISABLED; - - SetWindowLong(hwnd, GWL_STYLE, style); -} - /* === Crash Reporting Dialog === */ static void StretchDialog(HWND hwndDlg, int ydiff) { RECT r; GetWindowRect(hwndDlg, &r); r.bottom += ydiff; MoveWindow(hwndDlg, r.left, r.top, diff --git a/toolkit/crashreporter/client/moz.build.1490240.later b/toolkit/crashreporter/client/moz.build.1490240.later new file mode 100644 --- /dev/null +++ b/toolkit/crashreporter/client/moz.build.1490240.later @@ -0,0 +1,14 @@ +--- moz.build ++++ moz.build +@@ -94,11 +94,8 @@ DEFINES['BIN_SUFFIX'] = '"%s"' % CONFIG[ + + RCINCLUDE = 'crashreporter.rc' + + # Don't use the STL wrappers in the crashreporter clients; they don't + # link with -lmozalloc, and it really doesn't matter here anyway. + DisableStlWrapping() + + include('/toolkit/crashreporter/crashreporter.mozbuild') +- +-if CONFIG['CC_TYPE'] == 'clang-cl': +- AllowCompilerWarnings() # workaround for bug 1090497 diff --git a/toolkit/crashreporter/google-breakpad/src/common/mac/moz.build b/toolkit/crashreporter/google-breakpad/src/common/mac/moz.build --- a/toolkit/crashreporter/google-breakpad/src/common/mac/moz.build +++ b/toolkit/crashreporter/google-breakpad/src/common/mac/moz.build @@ -37,16 +37,13 @@ SOURCES += [ 'bootstrap_compat.cc', 'HTTPMultipartUpload.m', 'MachIPC.mm', 'string_utilities.cc', ] Library('breakpad_mac_common_s') -# We allow warnings for third-party code that can be updated from upstream. -AllowCompilerWarnings() - FINAL_LIBRARY = 'xul' CMFLAGS += ['-std=c99'] include('/toolkit/crashreporter/crashreporter.mozbuild') diff --git a/toolkit/crashreporter/google-breakpad/src/common/moz.build b/toolkit/crashreporter/google-breakpad/src/common/moz.build --- a/toolkit/crashreporter/google-breakpad/src/common/moz.build +++ b/toolkit/crashreporter/google-breakpad/src/common/moz.build @@ -65,14 +65,11 @@ if CONFIG['OS_TARGET'] == 'Android': 'android/breakpad_getcontext.S', ] LOCAL_INCLUDES += [ '/toolkit/crashreporter/google-breakpad/src/common/android/include', ] Library('breakpad_common_s') -# We allow warnings for third-party code that can be updated from upstream. -AllowCompilerWarnings() - FINAL_LIBRARY = 'xul' include('/toolkit/crashreporter/crashreporter.mozbuild') diff --git a/toolkit/crashreporter/moz.build.1490240.later b/toolkit/crashreporter/moz.build.1490240.later new file mode 100644 --- /dev/null +++ b/toolkit/crashreporter/moz.build.1490240.later @@ -0,0 +1,14 @@ +--- moz.build ++++ moz.build +@@ -136,11 +136,8 @@ crash_annotations = GENERATED_FILES['Cra + crash_annotations.script = 'generate_crash_reporter_sources.py:emit_header' + crash_annotations.inputs = [ + 'CrashAnnotations.h.in', + 'CrashAnnotations.yaml', + ] + + with Files('**'): + BUG_COMPONENT = ('Toolkit', 'Crash Reporting') +- +-if CONFIG['CC_TYPE'] == 'clang-cl': +- AllowCompilerWarnings() # workaround for bug 1090497 diff --git a/toolkit/crashreporter/nsExceptionHandler.cpp b/toolkit/crashreporter/nsExceptionHandler.cpp --- a/toolkit/crashreporter/nsExceptionHandler.cpp +++ b/toolkit/crashreporter/nsExceptionHandler.cpp @@ -7,16 +7,17 @@ #include "nsExceptionHandler.h" #include "nsExceptionHandlerUtils.h" #include "nsAppDirectoryServiceDefs.h" #include "nsDirectoryServiceDefs.h" #include "nsDirectoryService.h" #include "nsDataHashtable.h" #include "mozilla/ArrayUtils.h" +#include "mozilla/DebugOnly.h" #include "mozilla/EnumeratedRange.h" #include "mozilla/Services.h" #include "nsIObserverService.h" #include "mozilla/Unused.h" #include "mozilla/Printf.h" #include "mozilla/Sprintf.h" #include "mozilla/SyncRunnable.h" #include "mozilla/TimeStamp.h" @@ -1299,16 +1300,18 @@ ReserveBreakpadVM() static void FreeBreakpadVM() { if (gBreakpadReservedVM) { VirtualFree(gBreakpadReservedVM, 0, MEM_RELEASE); } } +#if defined(XP_WIN) + /** * Filters out floating point exceptions which are handled by nsSigHandlers.cpp * and should not be handled as crashes. * * Also calls FreeBreakpadVM if appropriate. */ static bool FPEFilter(void* context, EXCEPTION_POINTERS* exinfo, MDRawAssertionInfo* assertion) @@ -1343,16 +1346,18 @@ ChildFPEFilter(void* context, EXCEPTION_ { bool result = FPEFilter(context, exinfo, assertion); if (result) { PrepareChildExceptionTimeAnnotations(context); } return result; } +#endif // defined(XP_WIN) + static MINIDUMP_TYPE GetMinidumpType() { MINIDUMP_TYPE minidump_type = static_cast( MiniDumpWithFullMemoryInfo | MiniDumpWithUnloadedModules); #ifdef NIGHTLY_BUILD // This is Nightly only because this doubles the size of minidumps based @@ -1392,16 +1397,18 @@ static bool ShouldReport() envvar = PR_GetEnv("MOZ_CRASHREPORTER_FULLDUMP"); if (envvar && *envvar) { return false; } return true; } +#if !defined(XP_WIN) + static bool Filter(void* context) { mozilla::IOInterposer::Disable(); return true; } static bool @@ -1409,16 +1416,18 @@ ChildFilter(void* context) { bool result = Filter(context); if (result) { PrepareChildExceptionTimeAnnotations(context); } return result; } +#endif // !defined(XP_WIN) + static void TerminateHandler() { MOZ_CRASH("Unhandled exception"); } #if !defined(MOZ_WIDGET_ANDROID) @@ -1618,19 +1627,20 @@ nsresult SetExceptionHandler(nsIFile* aX #ifdef _WIN64 // Tell JS about the new filter before we disable SetUnhandledExceptionFilter SetJitExceptionHandler(); #endif // protect the crash reporter from being unloaded gBlockUnhandledExceptionFilter = true; gKernel32Intercept.Init("kernel32.dll"); - bool ok = stub_SetUnhandledExceptionFilter.Set(gKernel32Intercept, - "SetUnhandledExceptionFilter", - &patched_SetUnhandledExceptionFilter); + DebugOnly ok = + stub_SetUnhandledExceptionFilter.Set(gKernel32Intercept, + "SetUnhandledExceptionFilter", + &patched_SetUnhandledExceptionFilter); #ifdef DEBUG if (!ok) printf_stderr ("SetUnhandledExceptionFilter hook failed; crash reporter is vulnerable.\n"); #endif #endif // store application start time diff --git a/toolkit/crashreporter/test/moz.build.1490240.later b/toolkit/crashreporter/test/moz.build.1490240.later new file mode 100644 --- /dev/null +++ b/toolkit/crashreporter/test/moz.build.1490240.later @@ -0,0 +1,14 @@ +--- moz.build ++++ moz.build +@@ -54,11 +54,8 @@ TEST_HARNESS_FILES.xpcshell.toolkit.cras + + include('/toolkit/crashreporter/crashreporter.mozbuild') + + NO_PGO = True + + # Temporary workaround for an issue in upstream breakpad + if CONFIG['CC_TYPE'] in ('msvc', 'clang-cl'): + CXXFLAGS += ['-wd4334'] +- +-if CONFIG['CC_TYPE'] == 'clang-cl': +- AllowCompilerWarnings() # workaround for bug 1090497 diff --git a/toolkit/crashreporter/test/nsTestCrasher.cpp b/toolkit/crashreporter/test/nsTestCrasher.cpp --- a/toolkit/crashreporter/test/nsTestCrasher.cpp +++ b/toolkit/crashreporter/test/nsTestCrasher.cpp @@ -170,17 +170,17 @@ void Crash(int16_t how) case CRASH_X64CFI_EPILOG: { auto m = GetWin64CFITestMap(); if (m.find(how) == m.end()) { break; } auto pfnTest = m[how]; auto pfnLauncher = m[CRASH_X64CFI_LAUNCHER]; ReserveStack(); - pfnLauncher(0, pfnTest); + pfnLauncher(0, reinterpret_cast(pfnTest)); break; } #endif // XP_WIN && HAVE_64BIT_BUILD && !defined(__MINGW32__) default: break; } }