After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 793272 - [PATCH] fix GCC 8.0's -Wcast-function-type warnings
[PATCH] fix GCC 8.0's -Wcast-function-type warnings
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: build
unspecified
Other Linux
: High normal
: 2.56
Assigned To: gtkdev
gtkdev
: 793175 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2018-02-07 18:31 UTC by Lubomir Rintel
Modified: 2018-07-17 14:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH 1/3] gtype: cast *_init functions to void(*)(void) first (2.53 KB, patch)
2018-02-07 18:31 UTC, Lubomir Rintel
committed Details | Review
[PATCH 2/3] gmem: avoid the -Wcast-function-type warning (1.27 KB, patch)
2018-02-07 18:31 UTC, Lubomir Rintel
reviewed Details | Review
[PATCH 3/3] all: fix build with CFLAGS=-Werror=cast-function-type (45.90 KB, patch)
2018-02-07 18:32 UTC, Lubomir Rintel
rejected Details | Review

Description Lubomir Rintel 2018-02-07 18:31:02 UTC
Created attachment 368088 [details] [review]
[PATCH 1/3] gtype: cast *_init functions to void(*)(void) first

glib's header files triggering a warning is somewhat inconvenient. I'm attaching two patches that fix that.

The third patch adds casts that would suppress the warning all around the glib source tree. It doesn't look too good -- perhaps it just shouldn't be done. Nevertheless it illustrates what would need to be done.
Comment 1 Lubomir Rintel 2018-02-07 18:31:44 UTC
Created attachment 368089 [details] [review]
[PATCH 2/3] gmem: avoid the -Wcast-function-type warning
Comment 2 Lubomir Rintel 2018-02-07 18:32:19 UTC
Created attachment 368090 [details] [review]
[PATCH 3/3] all: fix build with CFLAGS=-Werror=cast-function-type
Comment 3 Philip Withnall 2018-02-08 12:23:55 UTC
*** Bug 793175 has been marked as a duplicate of this bug. ***
Comment 4 Philip Withnall 2018-02-08 12:25:20 UTC
Review of attachment 368090 [details] [review]:

No, that’s an unmaintainable approach to fixing this problem. I would rather disable the warning (-Wno-cast-function-type) internally for the GLib build than do this. However, I still need to investigate other solutions.
Comment 5 Philip Withnall 2018-02-16 15:34:27 UTC
Review of attachment 368088 [details] [review]:

I’m a bit sad that we have to do this, as type checking is generally a good thing. However, the vast majority of users of GObject never use the second parameter to the *InitFunc functions (indeed, I didn’t know it existed until now), so pragmatically we need to cast away this warning.

The approach of casting it away in the helper macros seems reasonable: any user who is calling g_type_register_static_simple() directly (for example) will still get the full type checking.
Comment 6 Philip Withnall 2018-02-16 15:37:06 UTC
Review of attachment 368089 [details] [review]:

What was the type of the destroy notify function you were using in this case? From my reading of https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wno-cast-function-type, GDestroyNotify should match any single-parameter, void-return function. Were you passing a function which has additional parameters? I’m less happy about allowing that, since those additional parameters will get undefined values off the stack (depending on the calling convention in use).
Comment 7 Philip Withnall 2018-02-16 15:40:31 UTC
Review of attachment 368090 [details] [review]:

Did any of these warnings highlight legitimate bugs where a function of the wrong type was being cast to something unexpected?

I’m tempted to say that GLib doesn’t support being built with -Wcast-function-type (but should only install public headers which don’t cause warnings with -Wcast-function-type), since we rely on undefined behaviour to do with function casting quite a lot. (On the processor architectures we care about, it’s all defined and safe for us to use, in the processor specifications; but not in the C standard.)
Comment 8 Philip Withnall 2018-02-16 15:51:11 UTC
Comment on attachment 368088 [details] [review]
[PATCH 1/3] gtype: cast *_init functions to void(*)(void) first

First patch pushed to master and glib-2-54.
Comment 9 Ernestas Kulik 2018-02-16 17:30:04 UTC
Review of attachment 368088 [details] [review]:

::: gobject/gtype.h
@@ +2009,3 @@
                                        g_intern_static_string (#TypeName), \
                                        sizeof (TypeName##Interface), \
+                                       (GClassInitFunc)(GVoidFunc)type_name##_default_init, \

This is causing build failures in some modules.
Comment 10 Philip Withnall 2018-02-16 17:40:55 UTC
Review of attachment 368088 [details] [review]:

::: gobject/gtype.h
@@ +2009,3 @@
                                        g_intern_static_string (#TypeName), \
                                        sizeof (TypeName##Interface), \
+                                       (GClassInitFunc)(GVoidFunc)type_name##_default_init, \

Particularly:
/usr/include/glib-2.0/gobject/gtype.h:2011:57: error: ‘GVoidFunc’ undeclared (first use in this function)
                                        (GClassInitFunc)(GVoidFunc)type_name##_default_init, \

From http://build.gnome.org/continuous/buildmaster/builds/2018/02/16/48/build/log-nautilus.txt.

I’ll push a fix; I missed the fact that this one differed when I did the review. :-(
Comment 11 Philip Withnall 2018-02-16 17:46:45 UTC
Fix for that pushed as 55e1c6185f74170980067f2b247152b400ab62d2 (master) and cb451011c3b2f584f2bb51fe2deb0abc073b59b8 (glib-2-54).
Comment 12 Philip Withnall 2018-02-22 12:50:49 UTC
Lubomir, do you have answers to the questions in comment #6 and comment #7? I’d like to get this bug resolved for the GLib 2.56 release, and the code freeze is coming up on 2018-03-05.
Comment 13 Philip Withnall 2018-03-13 11:46:57 UTC
Since there’s been no response to my questions, and I can’t reproduce the original problems (GCC 8 is not widely available yet; I’m on F27 and I’m not upgrading to Rawhide just to reproduce this), I’m going to close this.

Once GCC 8 is more widely available, if there are still problems with -Wcast-function-type, I’m sure we can revisit them.
Comment 14 Mathieu Bridon 2018-07-17 14:29:52 UTC
I just found this still happend with glibc from master (at least up to 449dcca106d1f22ee279e4c4054e1907ebd2402c)

I originally opened the bug against gsound: https://gitlab.gnome.org/GNOME/gsound/issues/1

A simple way to reproduce the build failure if you don't have access to GCC8 is to use Flatpak:

$ git clone git@gitlab.gnome.org:GNOME/gnome-clocks  # Clocks master builds gsound master against glib master
$ cd gnome-clocks
$ git checkout bochecha/flatpak-gitlab   # Only necessary until https://gitlab.gnome.org/GNOME/gnome-clocks/merge_requests/2 is merged
$ flatpak-builder --force-clean --ccache app data/flatpak/org.gnome.clocks.json

Can we reopen this issue?

To answer your question in comment 6

> What was the type of the destroy notify function you were using in this case?

I think in the case of gsound, it is ca_proplist_destroy: https://developer.gnome.org/libcanberra/unstable/libcanberra-canberra.html#ca-proplist-destroy
Comment 15 Emmanuele Bassi (:ebassi) 2018-07-17 14:57:38 UTC
(In reply to Mathieu Bridon from comment #14)

> Can we reopen this issue?

Please, open a new issue on GitLab:

  https://gitlab.gnome.org/GNOME/glib/issues/new