Closed Bug 1215774 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Graphics, defect)

Unspecified
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox43 --- unaffected
firefox44 --- affected
firefox45 --- fixed

People

(Reporter: cpeterson, Assigned: lsalzman, Mentored)

References

(Blocks 1 open bug)

Details

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

Attachments

(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():

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: 5 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.