Closed Bug 1215774 Opened 9 years ago Closed 8 years ago

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


(Core :: Graphics, defect)

Not set



Tracking Status
firefox43 --- unaffected
firefox44 --- affected
firefox45 --- fixed


(Reporter: cpeterson, Assigned: lsalzman, Mentored)


(Blocks 1 open bug)


(Keywords: regression, Whiteboard: [good first bug][lang=c++][gfx-noted])


(1 file)

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():
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).
Mentor: cpeterson
OS: Unspecified → Mac OS X
Whiteboard: [good first bug][lang=c++]
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
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+
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
No longer depends on: 1229518
You need to log in before you can comment on or make changes to this bug.