search

libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.71k stars 1.8k forks source link

Library lacks symbol versions #2786

Closed SDLBugzilla closed 1 year ago

SDLBugzilla commented 3 years ago

This bug report was migrated from our old Bugzilla tracker.

Reported in version: 2.0.6 Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2018-01-10 15:43:41 +0000, Jan Engelhardt wrote:

In SDL 2.0.5, SDL_RenderSetIntegerScale was added, but neither the SONAME was changed, nor were symbol versions used to indicate that there is an ABI boundary. This means that package managers which scan ELF files for dependencies will not pick up the news, leading to a system where a program built with SDL-2.0.6 is installable on SDL 2.0.3 systems, but is unrunnable.

$ LD_BIND_NOW=1 chocolate-doom chocolate-doom: symbol lookup error: chocolate-doom: undefined symbol: SDL_RenderSetIntegerScale

Please add symbol versions. Something like

/ sdl.sym / SDL_2.0.0 { global: SDL_xxx; SDL_yyy; ...; local: *; }; SDL_2.0.5 { global: SDL_RenderSetIntegerScale; } SDL_2.0.0;

and use with gcc -Wl,--version-script=sdl.sym during build.

icculus commented 2 years ago

Thanks to build-scripts/fnsince.pl, we could actually do this now if we want to.

Putting in 2.0.20 milestone (but not committing to actually fixing this yet).

slouken commented 2 years ago

@smcv, what changes are needed to switch to versioned symbols? Is this a reasonable change to make without a major version bump?

smcv commented 2 years ago

Yes, I had meant to open an issue about this.

It's OK to add symbol versioning without a major version bump, because it is a backwards-compatible change. However, it is not a forwards-compatible change: any program or dependent library compiled against the new SDL will need a version that has versioned symbols, even if all the symbols it uses already existed in old versions of SDL.

Similarly, if we want SDL-built-by-CMake and SDL-built-by-Autotools to be drop-in replacements for each other, then versioned symbols need to be enabled at the same time and on the same platforms for both build systems. My suggestion would be to enable versioned symbols on GNU/Linux (Linux with glibc) unconditionally, and not enable them on other platforms; and if other platforms want to join in, they can send patches.

There are two different levels of symbol versioning we could have.

Single version-definition (basic versioned symbols), for library coexistence

A basic form of ELF symbol versioning avoids some bugs that can happen when two SONAMEs of the same library collide, and the recursive dependency tree of an executable pulls in both the old SONAME and the new SONAME. This isn't generally a problem on Windows, because Windows symbol resolution is a tree, but it can be a problem on Linux (and other ELF platforms) because ELF symbol resolution usually happens in a single flat global namespace, in which a symbol can "interpose" in front of a symbol of the same name. Interposing is necessary to make LD_PRELOAD work, but often undesirable.

The simplest form of symbol versioning is to put the same version (often the same string as the SONAME) on every public symbol. Linux distributions often prefer libraries to add at least this basic level of versioned symbols before breaking ABI, so that if the old and new versions both end up in the same process's address space, they won't fight. For instance, it's OK for both libpng16.so.16 and libpng12.so.0 to end up indirectly linked into the same process, because their symbols are tagged with PNG16_0 and PNG12_0 respectively. libdbus has a similar setup.

The easiest way to do this with a recent-ish binutils is to link with -Wl,--default-symver, which uses the SONAME of the library as the name of the version definition (verdef). That results in function names like SDL_GameControllerName@libSDL2-2.0.so.0 which won't collide with a potential future SDL_GameControllerName@libSDL3.so.0 in a potential future SDL 3. A real example of this: https://gitlab.gnome.org/GNOME/json-glib/-/merge_requests/28

Multiple version-definition (complex versioned symbols), for version-tracking

The more complex form that was requested by the original reporter of this issue involves keeping track of which version of the library added each symbol, with a new version definition (verdef) for each release that added new ABI, so that you can objdump a compiled binary and take a fairly good guess at what is the minimum version it needs. The libraries shipped by util-linux are a prominent example of this. libostree is another good example. https://github.com/json-c/json-c/pull/639 is an example of a MR that added this to an existing library.

Some people are big fans of this, but despite being a distro library maintainer (one of the very people this is meant to benefit), I personally don't think this is all that good a tradeoff: it's a nice-to-have, but the amount of work that has to be done upstream is significant.

If you're going this way, then it is invalid (a bug) to have symbols that conditionally exist under some but not all build configurations, even more than with no versioned symbols or with the simpler form.

Possible transitions

These transitions are valid:

These transitions are invalid:

slouken commented 2 years ago

If you simply use the SONAME for all functions, then don't you have to provide a symbol with each SONAME where it's valid? Or does the linker automatically use newer versions of the function if an application references the symbol with an older SONAME version?

smcv commented 2 years ago

There's only one SONAME for each compatible family of versions of the library: it's something like libSDL2-2.0.so.0, not libSDL2-2.0.so.0.18.2.

$ objdump -T -x /usr/lib/x86_64-linux-gnu/libSDL2-2.0.so.0.18.2
...
Dynamic Section:
...
  SONAME               libSDL2-2.0.so.0

The SONAME only changes when the library breaks backwards compatibility, for instance 1.2.x were libSDL-1.2.so.0, and SDL 3 might be libSDL3.so.0 or something.

slouken commented 2 years ago

Okay, so in our case the SONAME would be libSDL2-2.0.so.0 indefinitely, which is not helpful for anyone trying to do useful symbol versioning, correct?

smcv commented 2 years ago

The SONAME is libSDL2-2.0.so.0 until the ABI breaks (presumably SDL 3), at which point it changes. That means the simple form of symbol versioning is good for resolving collisions between SDL 2 and SDL 3 (which does have value!), but you're right that it doesn't help a library user to determine what minimal version of SDL their binary needs.

slouken commented 2 years ago

There shouldn't be any collisions between SDL 2 and SDL 3, as they'll have different major versions and applications linked to one won't run if that library version isn't present.

It sounds like the original poster was asking for individual symbol versions.

@icculus, is this something we want to add now that we have the perl script with information on symbol versions?

slouken commented 2 years ago

FYI, we're planning to create sdl20-compat for older applications to run on SDL 3, similar to the way sdl12-compat works.

icculus commented 2 years ago

@icculus, is this something we want to add now that we have the perl script with information on symbol versions?

Based on this comment...

However, it is not a forwards-compatible change: any program or dependent library compiled against the new SDL will need a version that has versioned symbols, even if all the symbols it uses already existed in old versions of SDL.

...we absolutely don't want this for SDL2. Maybe for SDL3 since it could be there from the start...?

I have to read this discussion way more closely to understand what the benefit of versioning is here, but I'll report back.

smcv commented 2 years ago

There shouldn't be any collisions between SDL 2 and SDL 3

They'll presumably both export a SDL_GetVersion symbol, for example? That's a collision: if SDL 2 and SDL 3 end up in the same process via different dependency chains, it's arbitrary which one will end up providing that symbol.

smcv commented 2 years ago

is this something we want to add now that we have the perl script with information on symbol versions?

Please note that changing the version associated with a symbol is an ABI break, so if there is a release with the wrong version associated with a particular symbol, then it must stay wrong indefinitely. If you do the more elaborate form of versioning, then I would recommend committing the version script to git, so that incompatible changes are easy for reviewers to spot.

sulix commented 2 years ago

Personally, I'd really like to preserve forward compatibility where possible (so definitely don't change anything for SDL2).

For SDL3, I'd still rather not have the symbol versions change with each minor release, as I don't think we've hit any problems with SDL2 that this would've solved, and not having the forward compatibility can be a pain and lead to nasty workarounds.

That said, I'd not have a problem with a symbol version that only changes with the major version, which seems to be the suggestion of using --default-symver and the SONAME. It would be nice if SDL2 and SDL3 could coexist in the same process without things blowing up (though I'd still discourage people from doing so deliberately).

Some remaining questions from a symbol-version novice:

smcv commented 2 years ago

It would be nice if SDL2 and SDL3 could coexist in the same process without things blowing up

Sorry, if you want this, then they both need at least --default-symver. If that's not an option, then having --default-symver in SDL 3 would at least let you load SDL 3 and 4 into the same namespace without explosions.

How would this affect people using dlopen()/dlsym() to load SDL?

dlopen() is unaffected. dlsym() will find symbols of any arbitrary version, as though the symbol-versioning wasn't there. dlvsym() (GNU-specific) lets you specify a (symbol, version) pair.

Symbol collisions are generally less of a problem for dlopen() with a non-null handle, because you can use flags like RTLD_LOCAL and RTLD_DEEPBIND that are not available for ordinary DT_NEEDED linking.

(And the SDL_DYNAMIC_API?)

I think you'd ideally want versions of SDL that have versioned symbols to use dlvsym() to pick up the correct major version.

How linux/glibc/GNU ld-specific is this, and could it lead to incompatibilities there (would SDL break on non-glibc systems, etc)?

As far as I'm aware, it's glibc-specific. The conservative assumption would be to enable versioned symbols if the platform is Linux with glibc, but not enable them anywhere else (for example kFreeBSD with glibc, or Linux with musl). If other libc implementations have versioned symbols that work the same way they do on Linux with glibc, then people who have tested on those platforms can send pull requests.

--default-symver seems to be GNU-ld-specific and isn't supported by lld, which is annoying. Applying the same version, but via a linker script rather than the command-line option, would be compatible but more code.

sulix commented 2 years ago

My gut feeling is that the advantages we'd get out of symbol versioning are sufficiently niche (SDL3 and SDL4 being loaded into the same binary, on Linux) that it's not gaining us much. And while it seems like it shouldn't cause any serious problems either, it doesn't feel particularly compelling to me.

And if we make a more serious use of it (per-symbol, or enabling it for SDL2), that comes with corresponding hits to forward compatibility.

As I see it, the advantages of symbol versioning would be:

I presume there's some other advantages I'm missing…

Of those above, though, we only get the first one with the "simple symbol versioning". It's also the more compelling one, I think, but not worth breaking forward compatibility early to use it in SDL2, IMHO. Which means that at least we don't have to decide until SDL3 is coming out…

tl;dr: My feeling is that symbol versioning isn't worth it, but I wouldn't hate the simple symbol versioning in SDL3+.

slouken commented 2 years ago

Okay, thank you everyone for the context. I think this is something we'll revisit for SDL 3.

smcv commented 2 years ago

It seems like something the package managers could fix by looking at all the symbol names, but I digress…

Yes, that's what we do in Debian: there's a list of known symbols in debian/*.symbols in the packaging, and when we package a new upstream version, we update that list. The ability to do that is why I personally don't value the more complex approach very highly, because we can get most of its benefit another way.

jengelh commented 1 year ago

It seems like something the package managers could fix by looking at all the symbol names, but I digress…

They could, but I fear that would presumably add a lot of processing to package managers. A rough estimate I project is that systems would have take +200% the time to process the new data. (going from 112k to 362k "dependencies" to match between any two packages).

there's a list of known symbols in debian/*.symbols in the packaging, and when we package a new upstream version, we update that list

But that's distro-specific, which is unfortunate (as in, different distros would do their own symbol lists or so). ELF verdefs could help everyone, even those without a package manager and just the bare dynamic loader.

jengelh commented 1 year ago

It's OK to add symbol versioning without a major version bump, because it is a backwards-compatible change. However, it is not a forwards-compatible change: any program or dependent library compiled against the new SDL will need a version that has versioned symbols, even if all the symbols it uses already existed in old versions of SDL.

At least under glibc, running with old unversioned libs is fine:

$ ./make.sh 
+ gcc -fPIC lib.c -shared -o symless/liba.so
+ gcc -fPIC lib.c -shared -o symful/liba.so -Wl,--version-script=syms
+ gcc main.c -o main -Lsymful -la
+ LD_LIBRARY_PATH=/dev/shm/test/symful
+ ./main
hi world
+ LD_LIBRARY_PATH=/dev/shm/test/symless
+ ./main
./main: /dev/shm/test/symless/liba.so: no version information available (required by ./main)
hi world
smcv commented 1 year ago

At least under glibc, running with old unversioned libs is fine

It's fine until it isn't: there's a reason glibc logs a warning in your example. In simple cases it'll work, but it's best not to rely on that.

smcv commented 1 year ago

In simple cases it'll work, but it's best not to rely on that.

As an example of the sort of subtle behaviour change that can happen: if symless/liba.so does not import or export any versioned symbols, and handle is a dlopen() handle for symless/liba.so, then dlvsym (handle, "do_something", "any_verdef") will return do_something(); but if liba.so does import at least one versioned symbol, then dlvsym (handle, "do_something", "any_verdef") will fail to find do_something() and return null.

I think ld.so might be more tolerant than dlvsym() here, but having predictable behaviour is better than relying on subtle edge cases, and anyway we are likely going to want to dlopen() and dlvsym() symbols out of SDL at least some of the time.

In a trivial test-build with nearly-empty source files, liba.so might legitimately not import any versioned symbols (although I discovered that the answer can depend on factors like whether you built it with gcc or with clang!); but in real life, SDL on Linux is going to import versioned symbols from at least glibc, so it will have at least some version information.

madebr commented 1 year ago

Does adding a version script allow use of both SDL2 and SDL3 in the same process by different shared libraries?

The following cmake script tests this and fails, using #6613 for SDL3.

CMake project testing linking SDL2 and SDL3 in the same process ```cmake cmake_minimum_required(VERSION 3.0) project(sdl2and3 C) find_package(SDL2 REQUIRED) find_package(SDL3 REQUIRED) file(WRITE use_sdl2.c [[ #include "SDL.h" #include int use_sdl2() { SDL_version version; SDL_SetMainReady(); SDL_GetVersion(&version); if (version.major == 2) { fprintf(stderr, "use_sdl2: OK (got %u.%u.%u)\n", version.major, version.minor, version.patch); return 0; } else { fprintf(stderr, "use_sdl2: BAD (got %u.%u.%u)\n", version.major, version.minor, version.patch); return 1; } } ]]) file(WRITE use_sdl2.sym [[ { global: use_sdl2; local: *; }; ]]) file(WRITE use_sdl3.c [[ #include #include int use_sdl3() { SDL_version version; SDL_SetMainReady(); SDL_GetVersion(&version); if (version.major == 3) { fprintf(stderr, "use_sdl3: OK (got %u.%u.%u)\n", version.major, version.minor, version.patch); return 0; } else { fprintf(stderr, "use_sdl3: BAD (got %u.%u.%u)\n", version.major, version.minor, version.patch); return 1; } } ]]) file(WRITE use_sdl3.sym [[ { global: use_sdl3; local: *; }; ]]) file(WRITE test_sdl2.c [[ extern int use_sdl2(void); int main(int argc, char*argv[]) { return use_sdl2(); } ]]) file(WRITE test_sdl3.c [[ extern int use_sdl3(void); int main(int argc, char*argv[]) { return use_sdl3(); } ]]) file(WRITE test_sdl2_and_sdl3.c [[ extern int use_sdl2(void); extern int use_sdl3(void); int main(int argc, char*argv[]) { return use_sdl2() | use_sdl3(); } ]]) file(WRITE test_sdl2_dlopen.c [[ #include typedef int (*use_fn)(void); use_fn use_sdl2; int main(int argc, char*argv[]) { void *handle2; int rc; handle2 = dlopen("libuse_sdl2.so.0", RTLD_NOW); use_sdl2 = dlsym(handle2, "use_sdl2"); rc = use_sdl2(); dlclose(handle2); return rc; } ]]) file(WRITE test_sdl3_dlopen.c [[ #include typedef int (*use_fn)(void); use_fn use_sdl3; int main(int argc, char*argv[]) { void *handle3; int rc; handle3 = dlopen("libuse_sdl3.so.0", RTLD_NOW); use_sdl3 = dlsym(handle3, "use_sdl3"); rc = use_sdl3(); dlclose(handle3); return rc; } ]]) file(WRITE test_sdl2_and_sdl3_dlopen.c [[ #include typedef int (*use_fn)(void); use_fn use_sdl2; use_fn use_sdl3; int main(int argc, char*argv[]) { void *handle2; void *handle3; int rc; handle2 = dlopen("libuse_sdl2.so.0", RTLD_NOW); use_sdl2 = dlsym(handle2, "use_sdl2"); handle3 = dlopen("libuse_sdl3.so.0", RTLD_NOW); use_sdl3 = dlsym(handle3, "use_sdl3"); rc = use_sdl2() | use_sdl3(); dlclose(handle3); dlclose(handle2); return rc; } ]]) file(WRITE test_sdl3_and_sdl2_dlopen.c [[ #include typedef int (*use_fn)(void); use_fn use_sdl2; use_fn use_sdl3; int main(int argc, char*argv[]) { void *handle2; void *handle3; int rc; handle3 = dlopen("libuse_sdl3.so.0", RTLD_NOW); use_sdl3 = dlsym(handle3, "use_sdl3"); handle2 = dlopen("libuse_sdl2.so.0", RTLD_NOW); use_sdl2 = dlsym(handle2, "use_sdl2"); rc = use_sdl2() | use_sdl3(); dlclose(handle2); dlclose(handle3); return rc; } ]]) add_library(use_sdl2 SHARED use_sdl2.c) target_link_libraries(use_sdl2 PRIVATE SDL2::SDL2) set_target_properties(use_sdl2 PROPERTIES SOVERSION "0") target_link_options(use_sdl2 PRIVATE "-Wl,--version-script=${PROJECT_SOURCE_DIR}/use_sdl2.sym") set_target_properties(use_sdl2 PROPERTIES LINK_DEPENDS "${PROJECT_SOURCE_DIR}/use_sdl2.sym") add_library(use_sdl3 SHARED use_sdl3.c) target_link_libraries(use_sdl3 PRIVATE SDL3::SDL3) set_target_properties(use_sdl3 PROPERTIES SOVERSION "0") target_link_options(use_sdl3 PRIVATE "-Wl,--version-script=${PROJECT_SOURCE_DIR}/use_sdl3.sym") set_target_properties(use_sdl3 PROPERTIES LINK_DEPENDS "${PROJECT_SOURCE_DIR}/use_sdl3.sym") add_executable(test_sdl2 test_sdl2.c) target_link_libraries(test_sdl2 PRIVATE use_sdl2) add_executable(test_sdl3 test_sdl3.c) target_link_libraries(test_sdl3 PRIVATE use_sdl3) add_executable(test_sdl2_and_sdl3 test_sdl2_and_sdl3.c) target_link_libraries(test_sdl2_and_sdl3 PRIVATE use_sdl2 use_sdl3) add_executable(test_sdl3_and_sdl2 test_sdl2_and_sdl3.c) target_link_libraries(test_sdl3_and_sdl2 PRIVATE use_sdl3 use_sdl2) add_executable(test_sdl2_dlopen test_sdl2_dlopen.c) target_link_libraries(test_sdl2_dlopen PRIVATE ${CMAKE_DL_LIBS}) add_executable(test_sdl3_dlopen test_sdl3_dlopen.c) target_link_libraries(test_sdl3_dlopen PRIVATE ${CMAKE_DL_LIBS}) add_executable(test_sdl2_and_sdl3_dlopen test_sdl2_and_sdl3_dlopen.c) target_link_libraries(test_sdl2_and_sdl3_dlopen PRIVATE ${CMAKE_DL_LIBS}) add_executable(test_sdl3_and_sdl2_dlopen test_sdl3_and_sdl2_dlopen.c) target_link_libraries(test_sdl3_and_sdl2_dlopen PRIVATE ${CMAKE_DL_LIBS}) enable_testing() add_test(test_sdl2 test_sdl2) add_test(test_sdl3 test_sdl3) add_test(test_sdl2_and_sdl3 test_sdl2_and_sdl3) add_test(test_sdl3_and_sdl2 test_sdl3_and_sdl2) add_test(test_sdl2_dlopen test_sdl2_dlopen) add_test(test_sdl3_dlopen test_sdl3_dlopen) add_test(test_sdl2_and_sdl3_dlopen test_sdl2_and_sdl3_dlopen) add_test(test_sdl3_and_sdl2_dlopen test_sdl3_and_sdl2_dlopen) set_tests_properties(test_sdl2_dlopen test_sdl3_dlopen test_sdl2_and_sdl3_dlopen test_sdl3_and_sdl2_dlopen PROPERTIES ENVIRONMENT "LD_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}") ```

In this project, test_sdl2_and_sdl3 and test_sdl3_and_sdl2 fail.

Edit, when using dlopen to open use_sdl2 and use_sdl3, then it works fine. But that should already have been the case, as this is also done by dynapi/sdl*-compat.

smcv commented 1 year ago

Does adding a version script allow use of both SDL2 and SDL3 in the same process by different shared libraries?

Only if both SDL2 and SDL3 have versioned symbols. I suggested adding versioned symbols to SDL2 in the past, but maintainers didn't like undermining SDL2's usual forwards-compatibility.

Adding versioned symbols to SDL3, either the simple version with a single verdef or the more complex version with multiple verdefs, would eventually allow use of both SDL3 and SDL4 in the same process. I think it's worth doing for that future benefit. We don't get the full benefit right now, but if we keep rejecting it on the basis that the immediate benefit is too small, then we'll never get there.

madebr commented 1 year ago

A version script has been added to SDL3. Thanks @smcv for the guidance.