GNOME Bugzilla – Bug 793272
[PATCH] fix GCC 8.0's -Wcast-function-type warnings
Last modified: 2018-07-17 14:57:38 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.
Created attachment 368089 [details] [review] [PATCH 2/3] gmem: avoid the -Wcast-function-type warning
Created attachment 368090 [details] [review] [PATCH 3/3] all: fix build with CFLAGS=-Werror=cast-function-type
*** Bug 793175 has been marked as a duplicate of this bug. ***
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.
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.
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).
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 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.
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.
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. :-(
Fix for that pushed as 55e1c6185f74170980067f2b247152b400ab62d2 (master) and cb451011c3b2f584f2bb51fe2deb0abc073b59b8 (glib-2-54).
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.
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.
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
(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