1215774 - gfx/cairo/cairo/src/cairo.c:181:2 [-Wnull-dereference] indirection of non-volatile null pointer will be deleted, not trap

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Chris Peterson [:cpeterson]]

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

Comment 1

[Chris Peterson [:cpeterson]]

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).

Comment 2

[obongofon]

hello

Comment 3

[Lee Salzman [:lsalzman]]

Attachment: use abort() to abort on error in Cairo

Just use abort() here since it is does not require invasively adding more Mozilla headers and is used elsewhere in Cairo for this purpose.

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Is abort() actually doing the regular, proper, standard abort()?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

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.

Comment 6

[Lee Salzman [:lsalzman]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f1e43b87dad

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b760702408be

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b760702408be