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

RESOLVED FIXED in Firefox 45

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: cpeterson, Assigned: lsalzman, Mentored)

Tracking

(Blocks 1 bug, {regression})

unspecified
mozilla45
Unspecified
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 unaffected, firefox44 affected, firefox45 fixed)

Details

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

Attachments

(1 attachment)

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).
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
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+
https://hg.mozilla.org/mozilla-central/rev/b760702408be
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Depends on: 1229518
No longer depends on: 1229518
You need to log in before you can comment on or make changes to this bug.