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 642823 - Forbidden pointer conversion in GOptionEntry
Forbidden pointer conversion in GOptionEntry
Status: RESOLVED NOTABUG
Product: glib
Classification: Platform
Component: general
2.28.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks: 589197
 
 
Reported: 2011-02-20 17:46 UTC by Kjell Ahlstedt
Modified: 2011-03-08 14:31 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Kjell Ahlstedt 2011-02-20 17:46:12 UTC
Overview:
The description of GOptionEntry says:
"If the arg type is G_OPTION_ARG_CALLBACK, then arg_data must point to a
GOptionArgFunc callback function."

GOptionEntry::arg_type of type gpointer (i.e. void*) is used for storing either
a pointer to a data location or a pointer to a function. This dual use violates
strict ISO C rules. If you compile glib/goption.c and glib/tests/
option-context.c with gcc version 4.4.5 with the flag -pedantic, some results
are as follows.

  CC     goption.lo
goption.c: In function ‘parse_arg’:
goption.c:1298: warning: ISO C forbids conversion of object pointer to function pointer type

  CC     option-context.o
option-context.c: In function ‘callback_test1’:
option-context.c:631: warning: ISO C forbids initialization between function pointer and ‘void *’

This violation of the ISO C rules (and the ISO C++ rules) became apparent when
I wrapped the G_OPTION_ARG_CALLBACK functionality in C++ code in glibmm. See
bug 589197 comment 11 and following comments. The glibmm code is compiled with
gcc flags -pedantic -Werror -O2 (and more flags).

There are indirect ways to convert between a function pointer and an object
pointer, but you don't want to be forced to use any of them. There are at least
two tricks that can be used with the gcc compiler (without changing the
compiler flags), but I'm not 100% sure they can be used with all other relevant
compilers.

It would be much better if glib used separate struct members for storing a
pointer to function and a pointer to object. I can see two alternatives for
GOptionEntry.

1. Make arg_data a union:
   union
   {
     gpointer dp;
     GOptionArgFunc fp;
   } arg_data;

2. Add another element to GOptionEntry:
   GOptionArgFunc arg_func;

Both alternatives break API/ABI. I don't know if there is a solution that
does not break API/ABI and conforms to the ISO C standard.

   Steps to reproduce:

   Actual results:

   Expected results:

   Build date and platform: Ubuntu 10.10, source code of glib, glibmm, etc.
      built with jhbuild on 2011-02-17.

   Additional information:
Why is C and C++ restricted in this way? Probably because there are processor
architectures where code and data reside in different address spaces. An
address to a location in code memory is not necessarily the same size as an
address to a location in data memory.

Example 1: Microchip's PIC processors (small processors for embedded
applications), where a code address can be 12 bits and a data address 8 bits
(just an example, it differs between models).

Example 2: 16-bit Intel processors in some memory models, where a data address
can be 16 bits, a code address 20 or 32 bits.

Probably these examples are not relevant to glib, but they show, I believe,
why the C and C++ standards draw a sharp dividing line between the two groups
of addresses.
Comment 1 Matthias Clasen 2011-02-21 16:10:09 UTC
We do this all over the place, and it is working in practice.
There is only a very slim change that we are going to go on a quest of ISO-C-correctness. None of the platforms you mention are relevant for glib or gtk.
Comment 2 Murray Cumming 2011-02-23 10:05:26 UTC
But do we do this elsewhere in the public API? I don't think so.

This is a (small) real issue because it makes it harder to use the GOptionEntry API in an application that itself _does_ strive for correctness. For instance, we had to hack around this to build glibmm without compiler warnings, which we've been doing for years.
Comment 3 Kjell Ahlstedt 2011-02-27 12:41:01 UTC
I compiled all of glib and gtk+ with the compiler flag -pedantic. It showed
that both glib and gtk+ frequently convert between object pointers and function
pointers, but it's mostly done internally. The only examples I found where the
API forces an application to make such conversions are GOptionEntry::arg_data
and GHook::func. GHook is not used in glibmm.
Comment 4 Dan Winship 2011-02-28 13:33:44 UTC
If you want to fix the syntactic issue (to make picky compilers happy) without worrying about platforms where the pointers actually are different sizes, I think this should work:

  #define G_OPTION_ARG_FUNC(f) GSIZE_TO_POINTER (GPOINTER_TO_SIZE (f))
Comment 5 Kjell Ahlstedt 2011-03-08 10:49:06 UTC
I don't think there is a reasonable API-preserving fix to this issue. Judging
from comment 1 it's very unlikely that there will be a fix even when API/ABI
can be broken.

The following two URLs point at interesting info on the C++ standard:
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#195
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#573

It's possible that in a future version of the C++ standard, conversion between
a function pointer and an object pointer with reinterpret_cast will "evoke
conditionally-supported behavior", i.e. a compiler will be allowed to allow it.

I close this bug now. Feel free to reopen it, if you think the issue shall be
reconsidered when API/ABI can be broken.

In reply to comment 4:
Yes, this is one way to make the compiler happy, but it won't work on a
platform where the size of a function pointer is greater than that of void*.
Then you are bound to lose information when you convert a function pointer to
a void*, however you do it. But that's not really the point of this bug.
Comment 6 Dan Winship 2011-03-08 14:31:20 UTC
(In reply to comment #5)
> Yes, this is one way to make the compiler happy, but it won't work on a
> platform where the size of a function pointer is greater than that of void*.

...which isn't the case on any supported version of Windows, and which POSIX explicitly forbids, so you're covered on all glib-supported platforms.