Skip to content

Commit

Permalink
Fix linking against static FriBidi on Windows
Browse files Browse the repository at this point in the history
On Windows, symbols from dynamic-link libraries are replaced by
pointers that are filled at runtime after loading the libraries.
These pointers are given names prefixed by __imp_ and declared
in an "import library" that accompanies the main library and is
statically linked into consumers that want to use the DLL.
For a function symbol, an additional stub function is generated
with the original name that simply forwards to a matching __imp_
function-pointer call.

In C, an imported function can be declared in two ways:

    void foo();

ends up calling the stub and wasting a jump at runtime; while

    __declspec(dllimport) void foo();

replaces foo() calls by (*__imp_foo)() at compile-time (not link-time),
which ends up being a simple optimization.

For imported objects, stubs are impossible, so only
the __imp_ pointers exist and dllimport is mandatory.

Many libraries, including libass, export only functions
and avoid dllimport in their header files for simplicity.

In contrast, FriBidi exports some functions and some objects, so it
cannot neglect dllimport. Its solution is to declare all symbols in its
header files with dllimport except if FRIBIDI_LIB_STATIC is defined.

When libass is compiled without FRIBIDI_LIB_STATIC and linked against a
FriBidi static library, references to FriBidi imports try to use __imp_
symbols (due to the dllimport declarations), but they do not exist,
so the linking step fails.

In general, libass cannot know whether it will be linked to static
or dynamic FriBidi. For example, one can build a static libass.lib
to embed in a larger project that uses a shared FriBidi.dll. So we cannot
tell on our own, without the builder's input, whether we would need
to go through __imp_ object pointers or reference objects directly.

However, libass does not use any objects from FriBidi. We use only functions,
and functions are always safe to use without dllimport, because at link-time
the symbol is sure to resolve either to the actual function in the static
library or to the stub function in the import library.

As a result, we can simply always define FRIBIDI_LIB_STATIC to drop the
dllimport declarations, and we will always produce static and dynamic
libraries that work equally well with static and dynamic FriBidi.

This logic is Windows-specific, and it is plausible that FRIBIDI_LIB_STATIC
may break other platforms, so define it only on Windows.

Note: FriBidi tries to add the FRIBIDI_LIB_STATIC macro when using
`pkg-config --static --cflags`. However, this works only in one of the many
pkg-config implementations, pkgconf, and notably not [yet] in the original
freedesktop.org pkg-config. Even if it did work consistently, we would still
need either to assume we always want --static or to make a possibly incorrect
guess as to what kind of linking will ultimately be used for FriBidi.
And even if --static is always fine on Windows, we might want to avoid it
on other platforms, complicating build logic that people using other build
systems would need to replicate.
  • Loading branch information
astiob committed Apr 12, 2022
1 parent cbeea94 commit c5ee66d
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions libass/ass_compat.h
Expand Up @@ -26,6 +26,12 @@
#define inline __inline
#endif

// Work around build failures on Windows with static FriBidi.
// Possible because we only use FriBidi functions, not objects.
#if defined(_WIN32) && !defined(FRIBIDI_LIB_STATIC)
#define FRIBIDI_LIB_STATIC
#endif

#ifndef HAVE_STRDUP
char *ass_strdup_fallback(const char *s); // definition in ass_utils.c
#define strdup ass_strdup_fallback
Expand Down

1 comment on commit c5ee66d

@eli-schwartz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had cause to mention this a couple times recently-ish, so while I'm at it I figured I should probably say something here...

I noticed this commit referenced at one point in the Meson PR, and found it fascinating for a couple reasons.

First off -- this is an amazingly well written commit message, I wish all commit messages were this good, so, thank you.

Second -- thank you for explaining how dllexport/dllimport work. It's a topic that has driven me crazy for a long time. I don't use Windows, and I'm, practically, pretty limited in my capacity to see what/how MSVC does when it does its thing. Microsoft docs are, unfortunately, basically useless on the topic.

Your excellent description of the problem and its scope, requirements, and resolutions, has been inspirational and illuminating to me.

Please sign in to comment.