Skip to content

Incorrect library name passed to linker when linking with versioned dynamic libraries #14689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
khovansky-al opened this issue Jul 19, 2021 · 9 comments
Labels

Comments

@khovansky-al
Copy link

After upgrading emscripten in nixpkgs, I found that zlib example is not building, failing at the step of seemingly building examples that have to be linked with just produced library.
Looking at the produced linker command, it appears that the library name passed to wasm-ld is incorrect.
This happened on emscripten 2.0.25 but the offending code appears to be unchanged between that and current main.

Here's the invoked command (with unrelated parts omitted for brevity):

/.../bin/emcc -O3 -D_LARGEFILE64_SOURCE=1 -DHAVE_HIDDEN -o examplesh example.o -L. libz.so.1.2.11

wasm-ld: error: unable to find library -lz.so.1.2
emcc: error: '/.../bin/wasm-ld -o examplesh.wasm example.o -L. -lz.so.1.2 [...]

Library file produced during compilation is called libz.so.1.2.11 and is symlinked as

ln -s libz.so.1.2.11 libz.so
ln -s libz.so.1.2.11 libz.so.1

From these logs it's clear that .11 part is getting chopped, as if it was an extension.
I went digging in emcc.py and found that the call to unsuffixed_basename is likely to be incorrect, as it doesn't have the special logic for ignoring the library version extension part and acts as a normal basename.

After applying a local hack to do libname = strip_prefix(arg[:arg.rfind(file_suffix)], 'lib') instead (taking only the name part until the format suffix), the linking succeeded.

I'm not sure whether it should be passing version and what kind of a long-term solution would maintainers prefer so I refrained from opening a PR.

@sbc100
Copy link
Collaborator

sbc100 commented Jul 23, 2021

This does indeed look like a bug. emscripten should not be turning explicit libraries references such as libz.so.1.2.11 into -l flags .. it should just pass them directly through.

BTW, when you like explictly with a library such as libz.so.1.2.11 you don't also need the -L. flag .. the -L flags only effect the -l searches, they do not effect libraries specifed via their full filename such as libz.so.1.2.11. That is unrelated to this bug of course.

@sbc100
Copy link
Collaborator

sbc100 commented Jul 23, 2021

It looks like the offending code is:

emscripten/emcc.py

Lines 1235 to 1240 in 94b70c5

if file_suffix in DYNAMICLIB_ENDINGS and not building.is_bitcode(arg) and not building.is_wasm(arg):
# For shared libraries that are neither bitcode nor wasm, assuming its local native
# library and attempt to find a library by the same name in our own library path.
# TODO(sbc): Do we really need this feature? See test_other.py:test_local_link
libname = strip_prefix(unsuffixed_basename(arg), 'lib')
add_link_flag(state, i, '-l' + libname)

What kind of file is libz.so.1.2.11? It looks like its not a wasm or a bitcode file which is why its hitting this code path. I think we should simply remove this feature.. it doesn't seem very useful to me and can be very confusing.

@sbc100
Copy link
Collaborator

sbc100 commented Jul 23, 2021

The commit that caused your use case to start failing was #14355, although the previous behaviour was to not pass anything to the linker at all :-/

sbc100 added a commit that referenced this issue Jul 23, 2021
This is rather an odd feature and I think we should notify users if they
are relying on it.  Honestly I doubt many are.

See #14689 for an example of how confusing it can be when it goes wrong.

Also, make sure we strip off the full suffix in the case of `.so.1.2.3`
suffixes.
sbc100 added a commit that referenced this issue Jul 23, 2021
This is rather an odd feature and I think we should notify users if they
are relying on it.  Honestly I doubt many are.

See #14689 for an example of how confusing it can be when it goes wrong.

Also, make sure we strip off the full suffix in the case of `.so.1.2.3`
suffixes.
sbc100 added a commit that referenced this issue Jul 23, 2021
This is rather an odd feature and I think we should notify users if they
are relying on it.  Honestly I doubt many are.

See #14689 for an example of how confusing it can be when it goes wrong.

Also, make sure we strip off the full suffix in the case of `.so.1.2.3`
suffixes.
sbc100 added a commit that referenced this issue Jul 23, 2021
This is rather an odd feature and I think we should notify users if they
are relying on it.  Honestly I doubt many are.

See #14689 for an example of how confusing it can be when it goes wrong.

Also, make sure we strip off the full suffix in the case of `.so.1.2.3`
suffixes.
@sbc100
Copy link
Collaborator

sbc100 commented Jul 26, 2021

I think this issue should be fixed by #14746.,

However I'm still curious about whether libz.so.1.2.11 exists in your working directory? It seems that it doesn't? Or that it is perhaps not a wasm file?

@sbc100
Copy link
Collaborator

sbc100 commented Jul 26, 2021

What does file libz.so.1.2.11 say?

@khovansky-al
Copy link
Author

Thank you for all the explanations and a rapid fix.
The file definitely exists, that was the first thing I checked. However it never crossed my mind to check what's inside the file.
file libz.so.1.2.11 says it's a text file and peeking inside, it appears to be the JS glue generated by emscripten. Really curious how this happened.

You can look at the build commands executed for the library here, though it's nothing special: https://github.com/NixOS/nixpkgs/blob/456ac9c646c6bcdbf81c63e42f8941904da2435a/pkgs/top-level/emscripten-packages.nix#L148

This is a stable master but I didn't change configure, build and install phases when trying to upgrade.

@sbc100
Copy link
Collaborator

sbc100 commented Jul 27, 2021

Its possible that something on the emscripten side changed to ouput a JS file called libz.so.1.2.11 ... but what imagine probably happened is that that file was previously being completely ignored but that it always contains the wrong contents.

Can you go back to a working build (a working version of emsdk) and verify that this is the case?

If I'm wrong and that contents of libz.so.1.2.11 did change with the upgrade then perhaps you can bisect to find the exact change the caused it?: https://emscripten.org/docs/contributing/developers_guide.html#bisecting

@khovansky-al
Copy link
Author

Looks like the contents did change. Current nixpkgs build of emscriptenPackages.zlib reports:

file /nix/store/mqw28l0m51jivabn27r6a1nm6mz772hc-zlib-1.2.11/lib/libz.so.1.2.11 
/.../libz.so.1.2.11: WebAssembly (wasm) binary module version 0x1 (MVP)

This is under a very old emscripten 2.0.1. I'll try bisecting and see when it changes.

@stale
Copy link

stale bot commented Jul 31, 2022

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants