clang reports the following warning _cairo_error() (from bug 1207750): gfx/cairo/cairo/src/cairo.c:181:2 [-Wnull-dereference] indirection of non-volatile null pointer will be deleted, not trap The crash should use a volatile NULL pointer deref like the MOZ_REALLY_CRASH() macro. Or just use abort(): https://dxr.mozilla.org/mozilla-central/source/mfbt/Assertions.h#241
This is a good first bug for a C++ developer using Mac OS X (because this bug is a clang compiler warning and clang is the default compiler on OS X).
Whiteboard: [good first bug][lang=c++] → [good first bug][lang=c++][gfx-noted]
Just use abort() here since it is does not require invasively adding more Mozilla headers and is used elsewhere in Cairo for this purpose.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Attachment #8690231 - Flags: review?(jmuizelaar)
Is abort() actually doing the regular, proper, standard abort()?
Comment on attachment 8690231 [details] [diff] [review] use abort() to abort on error in Cairo Review of attachment 8690231 [details] [diff] [review]: ----------------------------------------------------------------- Here's why we don't use abort on Windows: * 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 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.) However, I think it's fine to use in our case.
Attachment #8690231 - Flags: review?(jmuizelaar) → review+
You need to log in before you can comment on or make changes to this bug.