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 778287 - G_MODULE_EXPORT and -fvisibility=hidden
G_MODULE_EXPORT and -fvisibility=hidden
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gmodule
2.48.x
Other Linux
: Normal minor
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-02-07 14:19 UTC by github
Modified: 2017-03-23 14:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gmodule: Add the visibility attribute to G_MODULE_EXPORT on gcc (2.38 KB, patch)
2017-02-13 11:50 UTC, Philip Withnall
committed Details | Review

Description github 2017-02-07 14:19:11 UTC
G_MODULE_EXPORT should contain __attribute__((visibility("default"))) to export the symbol when compiling with gcc flag -fvisibility=hidden

Test case:
-------
#include <gmodule.h>
G_MODULE_EXPORT const char version[] = "1.2.3";
-------
gcc $(pkg-config --cflags glib-2.0) glib-visibility.c -shared && nm -D

[...]
0000000000000629 R version

gcc $(pkg-config --cflags glib-2.0) glib-visibility.c -shared -fvisibility=hidden && nm -D

[no symbol named version]
Comment 1 Philip Withnall 2017-02-08 10:36:54 UTC
It should probably be defined the same way that _GLIB_EXTERN is defined in configure.ac (which is what GLib uses internally for exporting its API when -fvisibility=hidden is enabled). See bug #688681 for the details on that one.

I can’t see a reason why adding __attribute__((visibility("default"))) to symbols which are declared as G_MODULE_EXPORT should cause problems. It would change the effective symbol visibility in projects which compile with anything other than -fvisibility=default (for example, -fvisibility=internal), but that’s what we want.
Comment 2 Emmanuele Bassi (:ebassi) 2017-02-08 11:09:34 UTC
If your project changes the visibility rules, then I think it's your responsibility to add the annotation itself, instead of modifying G_MODULE_EXPORT for everyone. The visibility rules inside GLib do not propagate to dependent libraries.
Comment 3 Philip Withnall 2017-02-08 11:27:21 UTC
(In reply to Emmanuele Bassi (:ebassi) from comment #2)
> If your project changes the visibility rules, then I think it's your
> responsibility to add the annotation itself, instead of modifying
> G_MODULE_EXPORT for everyone. The visibility rules inside GLib do not
> propagate to dependent libraries.

G_MODULE_EXPORT is not used within GLib: it’s orthogonal to _GLIB_EXTERN. It’s defined for external modules to use to publicly export their symbols. This bug is precisely the use case it’s for.
Comment 4 github 2017-02-08 14:08:35 UTC
I was writing a plugin for wireshark when I hit this bug, but I wasn't using any build system. I am happy with any solution now when we have had a discussion about it.
Comment 5 Philip Withnall 2017-02-13 11:50:10 UTC
Created attachment 345611 [details] [review]
gmodule: Add the visibility attribute to G_MODULE_EXPORT on gcc

For versions of GCC which support it (≥ 4), define G_MODULE_EXPORT as
__attribute__((visibility("default"))). This is normally a no-op, unless
compiling with -fvisibility=hidden, in which case it marks a symbol to
be publicly exported from the library, which is what G_MODULE_EXPORT is
for. Previously G_MODULE_EXPORT has only worked on Windows.

The compatibility check for whether the compiler supports
__attribute__((visibility)) is based on the __GNUC__ define, and is
similar to the check done in configure.ac for defining G_GNUC_INTERNAL.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 6 Philip Withnall 2017-02-13 11:54:09 UTC
Here’s a potential patch for the problem. I don’t have any compilers which don’t support __attribute__((visibility)) though, so I can’t really test that the feature check works. It will break with a compiler which defines __GNUC__ ≥ 4 but doesn’t support __attribute__((visibility)).
Comment 7 Allison Karlitskaya (desrt) 2017-03-23 12:21:48 UTC
Review of attachment 345611 [details] [review]:

This makes sense to me.  +1
Comment 8 Philip Withnall 2017-03-23 14:59:16 UTC
Attachment 345611 [details] pushed as 5533293 - gmodule: Add the visibility attribute to G_MODULE_EXPORT on gcc