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 797185 - Fix API export/import decorators in general and "inconsistent DLL linkage" with MSVC on Windows
Fix API export/import decorators in general and "inconsistent DLL linkage" wi...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-09-21 10:21 UTC by Tim-Philipp Müller
Modified: 2018-09-25 08:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
common: gst-glib-gen.mak: include config.h in generated enumtypes and marshal .c files (2.03 KB, patch)
2018-09-21 10:23 UTC, Tim-Philipp Müller
committed Details | Review
gstconfig.h: add GST_API_IMPORT define (1.10 KB, patch)
2018-09-21 10:24 UTC, Tim-Philipp Müller
committed Details | Review
libs: fix 'inconsistent DLL linkage' warnings on Windows (9.99 KB, patch)
2018-09-21 10:25 UTC, Tim-Philipp Müller
committed Details | Review
libs: figure out right export define in configure (12.93 KB, patch)
2018-09-21 10:26 UTC, Tim-Philipp Müller
committed Details | Review
tests: netclock-replay: fix build with new api export/import (1.96 KB, patch)
2018-09-21 10:26 UTC, Tim-Philipp Müller
committed Details | Review
meson: use library() for libgstcheck instead of always building a shared lib (1.24 KB, patch)
2018-09-21 10:27 UTC, Tim-Philipp Müller
committed Details | Review
gst-plugins-base: libs: fix API export/import and 'inconsistent linkage' on MSVC (33.87 KB, patch)
2018-09-21 10:31 UTC, Tim-Philipp Müller
committed Details | Review

Description Tim-Philipp Müller 2018-09-21 10:21:10 UTC
Quick write-up about the "recent" (just before 1.14) and upcoming changes about how public symbols are exported in the various shared GStreamer libraries. Mostly so there's something to reference from the various commits and a place to track regressions, if any.

# Building shared libraries

On *nix systems or with gcc/clang-based toolchains (like mingw), pretty much all symbols will be exported by default unless they were explicitly marked as static or internal/hidden. In GStreamer we have in the past just exported all symbols that match a certain regular expression ('starts with gst_|GST_' more or less). This regexp exporting is not standardised across different toolchains, so a bit iffy to make work consistently across the board.

With the MSVC toolchain however, by default no symbols will be exported. There symbols have to be exported explicitly. This could be done via a version/map file of some sort (like the .def files we used to have in win32/common/libgstxyz.def), or it can be done explicitly by marking symbols to be exported with __declspec(dllexport). The .def file approach is not so practical when the list of exported symbols is not always the same but may vary depending on the configuration or what dependencies are available exactly.

On *nix systems one does not have to import external symbols explicitly, it is enough to have a declaration in a header file. On Windows/MSVC this is mostly fine as well, at least for functions, but variables are treated specially and they must be imported explicitly via __declspec(dllimport).

In GStreamer we have recently added 'decorators' to all public exported API. Initially this was just GST_EXPORT for all public API in all libraries. However, that caused problems and lots of 'inconsistent dll linkage' warnings and other compiler and linker warnings on the Windows MSVC builds.

The reason is as follows: Consider the case where we compile libgstbase-1.0. This shared library will *export* a variety of symbols such as gst_base_sink_* for example. At the same time it will implicitly or explicitly *import* (use) a variety of public symbols from libgstreamer-1.0. Now when building libgstbase-1.0 we will just #include headers such as gst.h an gstbasesink.h, and there all public functions would be marked with the same export decorator GST_EXPORT. If we define -DGST_EXPORTS then the GST_EXPORT will be turned into __declspec(dllexport) on Windows/MSVC. But as we can't have different values for GST_EXPORT depending on which headers we include, all public API will be marked for exporting, even the one from libgstreamer-1.0 which we want to *import*. This causes the warnings.

Having discovered this issue we therefore changed the API decorators into per-library GST_*_API decorators just before the 1.14.0 release, so we now have GST_API and GST_BASE_API etc. They still all mapped to GST_EXPORT however, so we didn't solve the problem yet back then, but we laid the foundation to be able to define them to separate things (e.g. GST_API = import, GS_BASE_API = export) later.

The last step will be achieved by passing -DBUILDING_GST and -DBUILDING_GST_BASE etc. when building each library. That define can then be used in the header files by the API decorators to evaluate to either the export or the import macro.

Example:

#ifdef BUILDING_GST_BASE
#define GST_BASE_API export
#else
#define GST_BASE_API import
#endif

The exact export define will depend on a number of things, such as the toolchain used and the operating system. It can be one of (for now):

  - __declspec(dllexport) extern
  - extern __attribute__ ((visibility ("default")))
  - extern

We will decide which one it is in configure and/or meson.build, and add an GST_API_EXPORT define to config.h that maps to the right thing (we do not want to pass this via the command line, that's pretty much impossible to get right with autotools + libtool due to all the layers and quoting we'd have to handle).

The import define is easier, we can just hardcode that to one of

  - __declspec(dllimport) extern
  - extern

depending on whether a Windows/MSVC toolchain is used or not. We will do that in gst/gstconfig.h as GST_API_IMPORT.

Then we end up with:

#ifdef BUILDING_GST
#define GST_API GST_API_EXPORT  /* from config.h */
#else
#define GST_API GST_API_IMPORT  /* from gst/gstconfig.h */
#endif

#ifdef BUILDING_GST_BASE
#define GST_BASE_API GST_API_EXPORT  /* from config.h */
#else
#define GST_BASE_API GST_API_IMPORT  /* from gst/gstconfig.h */
#endif

So when building libgstbase-1.0 BUILDING_GST_BASE will be defined and BUILDING_GST will not be defined, and GST_API will map to GST_API_IMPORT and GST_BASE_API to GST_API_EXPORT.

Since the export define comes from config.h we need to make sure that all .c files include config.h as first thing, so the GST_API_EXPORT decorator is defined before they include any header files. Failure to do so will result in easy to spot compiler errors.

We could have stuck with the .def file approach, but that was never really very good. People would always forget to add new API, and it doesn't work for libs like libgstgl-1.0 where what is included depends on the build options or operating system (which is already the case for the other libs too really just ignored for now).

With the decorator system we can handle all of this correctly and consistently.

We have also switched to disabling export of all symbols by default on *nix with gcc/clang toolchains by passing -fvisibility=hidden where supported. This way only symbols that are decorated with __attribute__ ((visibility ("default"))) get exported.

This way we can make sure we get consistent exports on all systems/toolchains and will quickly notice if an export is missing somewhere.

The only thing that's a bit messy is libgstcheck which includes an internally built copy of libcheck whose symbols we leak in libgstcheck for historical reasons. There we export based on a static list of functions for now with autotools (with meson it's not a problem).

# Building static libraries

To link to GStreamer statically on Windows, one must define GST_STATIC_COMPILATION or the prototypes will cause the compiler to search for the symbol inside a DLL. (N.B. we pretty much only support everything-gst-static or everything-gst-dynamic but do not support mix'n'match.)

The fact that we have to define GST_STATIC_COMPILATION on Windows also means we can't re-use objects between a shared library and a static library on Windows/MSVC, but have to build everything twice, but that's a job for the build system to handle (cf. https://github.com/mesonbuild/meson/issues/4133).

We don't need to worry about the export define when building a static library, if it maps to dllexport it will just be ignored in the static library case.
Comment 1 Tim-Philipp Müller 2018-09-21 10:23:15 UTC
Created attachment 373719 [details] [review]
common: gst-glib-gen.mak: include config.h in generated enumtypes and marshal .c files

This is needed to get the export define the GST_*_API markers map to when compiling gst libs.
Comment 2 Tim-Philipp Müller 2018-09-21 10:24:42 UTC
Created attachment 373720 [details] [review]
gstconfig.h: add GST_API_IMPORT define

This is for use by the various GST_*_API decorators and
will be what they get defined to when a library API is being
used by external users of that library (not the library itself
whilst it's being compiled).
    
In most cases it will simply map to a plain 'extern' but on
Windows with MSVC it will need to map to __declspec(dllimport).
For functions this is not strictly needed, but for exported
variables it is.
Comment 3 Tim-Philipp Müller 2018-09-21 10:25:35 UTC
Created attachment 373721 [details] [review]
libs: fix 'inconsistent DLL linkage' warnings on Windows

For each lib we build export its own API in headers when we're
building it, otherwise import the API from the headers.

This fixes linker warnings on Windows when building with MSVC.

The problem was that we had defined all GST_*_API decorators
unconditionally to GST_EXPORT. This was intentional and only
supposed to be temporary, but caused linker warnings because
we tell the linker that we want to export all symbols even
those from externall DLLs, and when the linker notices that
they were in external DLLS and not present locally it warns.

What we need to do when building each library is: export
the library's own symbols and import all other symbols. To
this end we define e.g. BUILDING_GST_FOO and then we define
the GST_FOO_API decorator either to export or to import
symbols depending on whether BUILDING_GST_FOO is set or not.
That way external users of each library API automatically
get the import.
Comment 4 Tim-Philipp Müller 2018-09-21 10:26:20 UTC
Created attachment 373722 [details] [review]
libs: figure out right export define in configure

Add new GST_API_EXPORT in config.h and use that for GST_*_API
decorators instead of GST_EXPORT.

The right export define depends on the toolchain and whether
we're using -fvisibility=hidden or not, so it's better to set it
to the right thing directly than hard-coding a compiler whitelist
in the public header.

We put the export define into config.h instead of passing it via the
command line to the compiler because it might contain spaces and brackets
and in the autotools scenario we'd have to pass that through multiple
layers of plumbing and Makefile/shell escaping and we're just not going
to be *that* lucky.

The export define is only used if we're compiling our lib, not by external
users of the lib headers, so it's not a problem to put it into config.h

Also, this means all .c files of libs need to include config.h
to get the export marker defined, so fix up a few that didn't
include config.h.

This commit depends on a common submodule commit that makes gst-glib-gen.mak
add an #include "config.h" to generated enum/marshal .c files for the
autotools build.
Comment 5 Tim-Philipp Müller 2018-09-21 10:26:59 UTC
Created attachment 373723 [details] [review]
tests: netclock-replay: fix build with new api export/import

Can't mix/match imports and exports from the same library
here, so just include all .c files needed instead and don't
link to gstnet at all then.
Comment 6 Tim-Philipp Müller 2018-09-21 10:27:48 UTC
Created attachment 373724 [details] [review]
meson: use library() for libgstcheck instead of always building a shared lib

Otherwise we try to build a shared lib when we build the rest
of GStreamer statically, which won't work because we pass
-DGST_STATIC_COMPILATION when building statically, which means
we won't dllimport public symbols from our libs which means
that on Windows the unit tests will fail to link to libgstcheck.
Comment 7 Tim-Philipp Müller 2018-09-21 10:31:33 UTC
Created attachment 373725 [details] [review]
gst-plugins-base: libs: fix API export/import and 'inconsistent linkage' on MSVC

For each lib we build export its own API in headers when we're
building it, otherwise import the API from the headers.

This fixes linker warnings on Windows when building with MSVC.

The problem was that we had defined all GST_*_API decorators
unconditionally to GST_EXPORT. This was intentional and only
supposed to be temporary, but caused linker warnings because
we tell the linker that we want to export all symbols even
those from externall DLLs, and when the linker notices that
they were in external DLLS and not present locally it warns.

What we need to do when building each library is: export
the library's own symbols and import all other symbols. To
this end we define e.g. BUILDING_GST_FOO and then we define
the GST_FOO_API decorator either to export or to import
symbols depending on whether BUILDING_GST_FOO is set or not.
That way external users of each library API automatically
get the import.

While we're at it, add new GST_API_EXPORT in config.h and use
that for GST_*_API decorators instead of GST_EXPORT.

The right export define depends on the toolchain and whether
we're using -fvisibility=hidden or not, so it's better to set it
to the right thing directly than hard-coding a compiler whitelist
in the public header.

We put the export define into config.h instead of passing it via the
command line to the compiler because it might contain spaces and brackets
and in the autotools scenario we'd have to pass that through multiple
layers of plumbing and Makefile/shell escaping and we're just not going
to be *that* lucky.

The export define is only used if we're compiling our lib, not by external
users of the lib headers, so it's not a problem to put it into config.h

Also, this means all .c files of libs need to include config.h
to get the export marker defined, so fix up a few that didn't
include config.h.

This commit depends on a common submodule commit that makes gst-glib-gen.mak
add an #include "config.h" to generated enum/marshal .c files for the
autotools build.
Comment 8 Tim-Philipp Müller 2018-09-24 13:50:52 UTC
core:

commit 50c32da91fb0c398be23aff7b37ac3dc516323be (HEAD -> master, origin/master, origin/HEAD)
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Wed Sep 19 19:37:38 2018 +0100

    meson: use library() for libgstcheck instead of always building a shared lib
    
    Otherwise we try to build a shared lib when we build the rest
    of GStreamer statically, which won't work because we pass
    -DGST_STATIC_COMPILATION when building statically, which means
    we won't dllimport public symbols from our libs which means
    that on Windows the unit tests will fail to link to libgstcheck.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797185

commit af5717b3646927013902c690f46162554fd547c3
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Sun Aug 26 01:23:23 2018 +0200

    tests: netclock-replay: fix build with new api export/import
    
    Can't mix/match imports and exports from the same library
    here, so just include all .c files needed instead and don't
    link to gstnet at all then.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797185

commit 57c8e0146f0e203058c95721527cf50a1dd19f72
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Sat Aug 25 23:56:01 2018 +0200

    libs: figure out right export define in configure
    
    Add new GST_API_EXPORT in config.h and use that for GST_*_API
    decorators instead of GST_EXPORT.
    
    The right export define depends on the toolchain and whether
    we're using -fvisibility=hidden or not, so it's better to set it
    to the right thing directly than hard-coding a compiler whitelist
    in the public header.
    
    We put the export define into config.h instead of passing it via the
    command line to the compiler because it might contain spaces and brackets
    and in the autotools scenario we'd have to pass that through multiple
    layers of plumbing and Makefile/shell escaping and we're just not going
    to be *that* lucky.
    
    The export define is only used if we're compiling our lib, not by external
    users of the lib headers, so it's not a problem to put it into config.h
    
    Also, this means all .c files of libs need to include config.h
    to get the export marker defined, so fix up a few that didn't
    include config.h.
    
    This commit depends on a common submodule commit that makes gst-glib-gen.mak
    add an #include "config.h" to generated enum/marshal .c files for the
    autotools build.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797185

commit 46ed0f0489896824f45694b5d8fbdfaac8de6504
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Sat Aug 25 23:09:12 2018 +0200

    libs: fix 'inconsistent DLL linkage' warnings on Windows
    
    For each lib we build export its own API in headers when we're
    building it, otherwise import the API from the headers.
    
    This fixes linker warnings on Windows when building with MSVC.
    
    The problem was that we had defined all GST_*_API decorators
    unconditionally to GST_EXPORT. This was intentional and only
    supposed to be temporary, but caused linker warnings because
    we tell the linker that we want to export all symbols even
    those from externall DLLs, and when the linker notices that
    they were in external DLLS and not present locally it warns.
    
    What we need to do when building each library is: export
    the library's own symbols and import all other symbols. To
    this end we define e.g. BUILDING_GST_FOO and then we define
    the GST_FOO_API decorator either to export or to import
    symbols depending on whether BUILDING_GST_FOO is set or not.
    That way external users of each library API automatically
    get the import.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797185

commit 50038bed79e056eee109572db03113999cc7eb38
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Sat Aug 25 22:53:07 2018 +0200

    gstconfig.h: add GST_API_IMPORT define
    
    This is for use by the various GST_*_API decorators and
    will be what they get defined to when a library API is being
    used by external users of that library (not the library itself
    whilst it's being compiled).
    
    In most cases it will simply map to a plain 'extern' but on
    Windows with MSVC it will need to map to __declspec(dllimport).
    For functions this is not strictly needed, but for exported
    variables it is.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797185



base:

commit dc29bc4e13ea1c25b1179efe362dd5b2bc071fd5 (HEAD -> master, origin/master, origin/HEAD)
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Sat Apr 28 14:50:11 2018 +0100

    libs: fix API export/import and 'inconsistent linkage' on MSVC
    
    For each lib we build export its own API in headers when we're
    building it, otherwise import the API from the headers.
    
    This fixes linker warnings on Windows when building with MSVC.
    
    The problem was that we had defined all GST_*_API decorators
    unconditionally to GST_EXPORT. This was intentional and only
    supposed to be temporary, but caused linker warnings because
    we tell the linker that we want to export all symbols even
    those from externall DLLs, and when the linker notices that
    they were in external DLLS and not present locally it warns.
    
    What we need to do when building each library is: export
    the library's own symbols and import all other symbols. To
    this end we define e.g. BUILDING_GST_FOO and then we define
    the GST_FOO_API decorator either to export or to import
    symbols depending on whether BUILDING_GST_FOO is set or not.
    That way external users of each library API automatically
    get the import.
    
    While we're at it, add new GST_API_EXPORT in config.h and use
    that for GST_*_API decorators instead of GST_EXPORT.
    
    The right export define depends on the toolchain and whether
    we're using -fvisibility=hidden or not, so it's better to set it
    to the right thing directly than hard-coding a compiler whitelist
    in the public header.
    
    We put the export define into config.h instead of passing it via the
    command line to the compiler because it might contain spaces and brackets
    and in the autotools scenario we'd have to pass that through multiple
    layers of plumbing and Makefile/shell escaping and we're just not going
    to be *that* lucky.
    
    The export define is only used if we're compiling our lib, not by external
    users of the lib headers, so it's not a problem to put it into config.h
    
    Also, this means all .c files of libs need to include config.h
    to get the export marker defined, so fix up a few that didn't
    include config.h.
    
    This commit depends on a common submodule commit that makes gst-glib-gen.mak
    add an #include "config.h" to generated enum/marshal .c files for the
    autotools build.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797185

bad:

commit 96ac822b67b56aacd3bc530ca6cba1d38f90895c
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Mon Sep 24 14:40:31 2018 +0100

    player: fix deprecated api declaration

commit b6411ae74cc7319ef368f5b4a6a872231b49620a
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Mon Sep 24 11:52:22 2018 +0100

    libs: fix API export/import and 'inconsistent linkage' on MSVC
    
    For each lib we build export its own API in headers when we're
    building it, otherwise import the API from the headers.
    
    This fixes linker warnings on Windows when building with MSVC.
    
    The problem was that we had defined all GST_*_API decorators
    unconditionally to GST_EXPORT. This was intentional and only
    supposed to be temporary, but caused linker warnings because
    we tell the linker that we want to export all symbols even
    those from externall DLLs, and when the linker notices that
    they were in external DLLS and not present locally it warns.
    
    What we need to do when building each library is: export
    the library's own symbols and import all other symbols. To
    this end we define e.g. BUILDING_GST_FOO and then we define
    the GST_FOO_API decorator either to export or to import
    symbols depending on whether BUILDING_GST_FOO is set or not.
    That way external users of each library API automatically
    get the import.
    
    While we're at it, add new GST_API_EXPORT in config.h and use
    that for GST_*_API decorators instead of GST_EXPORT.
    
    The right export define depends on the toolchain and whether
    we're using -fvisibility=hidden or not, so it's better to set it
    to the right thing directly than hard-coding a compiler whitelist
    in the public header.
    
    We put the export define into config.h instead of passing it via the
    command line to the compiler because it might contain spaces and brackets
    and in the autotools scenario we'd have to pass that through multiple
    layers of plumbing and Makefile/shell escaping and we're just not going
    to be *that* lucky.
    
    The export define is only used if we're compiling our lib, not by external
    users of the lib headers, so it's not a problem to put it into config.h
    
    Also, this means all .c files of libs need to include config.h
    to get the export marker defined, so fix up a few that didn't
    include config.h.
    
    This commit depends on a common submodule commit that makes gst-glib-gen.mak
    add an #include "config.h" to generated enum/marshal .c files for the
    autotools build.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797185


rtsp-server:

commit 62d4c0b179555fa0189d2c8a8e2d81da04a5bc3d
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Mon Sep 24 09:36:21 2018 +0100

    libs: fix API export/import and 'inconsistent linkage' on MSVC
    
    Export rtsp-server library API in headers when we're building the
    library itself, otherwise import the API from the headers.
    
    This fixes linker warnings on Windows when building with MSVC.
    
    Fix up some missing config.h includes when building the lib which
    is needed to get the export api define from config.h
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797185