1778767 - UBSan reports undefined behavior from glib's `g_clear_pointer`

Status: RESOLVED FIXED

(Core :: Graphics, defect, P2)

Description

[Jim Blandy :jimb]

UBSan complains about conversions between function pointer types in the Wayland code. The conversions seem unlikely to have any significance in the real world, but it's impossible to run under Wayland with UBSan turned on without fixing them.

Comment 1

[Jim Blandy :jimb]

Attachment: Bug 1778767: Define MozClearPointer, a type-safe replacement for g_clear_pointer. r?stransky

Comment 2

[Jim Blandy :jimb]

The deeper mystery is: why am I apparently the only one who's noticed this?

Comment 3

[Jim Blandy :jimb]

Hi, Martin - could you review this patch?

Comment 5

[Jim Blandy :jimb]

(Bug 1778564 does have some useful notes, though.)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc1377cd7adb
Define MozClearPointer, a type-safe replacement for g_clear_pointer. r=stransky,jgilbert

Comment 7

[Iulian Moraru]

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

Comment 8

[Cosmin Sabou [:CosminS]]

Backed out from central for causing bug 1779364 and new desktop nightlies incoming: https://hg.mozilla.org/mozilla-central/rev/04b02c492dab34ec35205f2338b27dc9469969fc

Comment 9

[Jan Alexander Steffens [:heftig]]

One of the g_clear_pointer calls wasn't replaced with MozClearPointer, but with wl_callback_destroy directly, removing the null check and allowing the function to be called with a null pointer.

https://hg.mozilla.org/mozilla-central/rev/cc1377cd7adb#l3.31

Comment 10

[Jan Alexander Steffens [:heftig]]

As an alternative, what about setting GLIB_VERSION_MAX_ALLOWED to at least GLIB_VERSION_2_58? It seems that makes the macro safe, as it no longer casts the function pointer.

PS: Never mind, this doesn't apply to C++ unless GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_68.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56c92687fce0
Define MozClearPointer, a type-safe replacement for g_clear_pointer. r=stransky,jgilbert

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/56c92687fce0