Skip -lfoo expansion for real dynamic libraries. NFC #25871
Conversation
The code here expands `-lfoo` to `/path/to/libfoo.so` for fake shared libraries so that the linker can find them (it won't look for .so files otherwise when static linking). Also, on
| Ping.. |
| # Attempting to link statically against a side module (libside.so) should fail. | ||
| err = self.expect_fail([EMCC, '-L.', '-lside']) | ||
| self.assertContained(r'error: attempted static link of dynamic object \.[/\\]libside.so', err, regex=True) | ||
| self.assert_fail([EMCC, '-L.', '-lside'], 'wasm-ld: error: unable to find library -lside') |
There was a problem hiding this comment.
Is the old error message not clearer?
There was a problem hiding this comment.
Maybe, but the new error is more consistent with how lld reports this error which I think is a good thing.
I think its better to pass argument directly to lld (or clang) where we can, and remove special logic like this from the emcc driver.
There was a problem hiding this comment.
(I also dont think this is a very common error that is likely to show up much)
| # Attempting to link statically against a side module (libside.so) should fail. | ||
| err = self.expect_fail([EMCC, '-L.', '-lside']) | ||
| self.assertContained(r'error: attempted static link of dynamic object \.[/\\]libside.so', err, regex=True) | ||
| self.assert_fail([EMCC, '-L.', '-lside'], 'wasm-ld: error: unable to find library -lside') |
| | ||
| static_lib = f'lib{lib}.a' | ||
| if not settings.RELOCATABLE and not find_library(static_lib, options.lib_dirs): | ||
| if not settings.RELOCATABLE and not settings.MAIN_MODULE and not find_library(static_lib, options.lib_dirs): |
There was a problem hiding this comment.
When we are building with real dynamic linking (i.e. MAIN_MODULE or RELOCATOABLE) we don't need to do any of this since the wasm-ld will just find a use the dynamic libraries.
We only need to do this when fake dylibs are being used.
There was a problem hiding this comment.
Makes sense, but why wasn't it needed before?
There was a problem hiding this comment.
Expanding these -l flags is harmless enough in all cases. Its just work that is not necessary when actually dynamic linking (since wasm-ld will do it in that case).
There was a problem hiding this comment.
lgtm, but I'm still confused here...
Even with enabling dynamic linking, you can statically link libraries to the main module? That is, why does dynamic linking mode differ from non-dynamic linking mode, with respect to static linking..?
There was a problem hiding this comment.
I suppose you could have some fake dynamic libraries and some real ones I guess? But if we want to support then then we should remove both settings.RELOCATABLE and settings.MAIN_MODULE here. It seems like a rare case though. Normally one is either doing real dynamic linking of fake dynamic linking, not some hybrid of the two.
There was a problem hiding this comment.
Oh wait.. I remember why this is not necessary in the dynamic linking case.
This is because wasm-ld will search the library path for .so files on its own in that case. No need to expand anything here in those cases.
There was a problem hiding this comment.
I see, thanks, so wasm-ld behaves differently in that case...
2 participants
The code here expands
-lfooto/path/to/libfoo.sofor fake shared libraries so that the linker can find them (it won't look for .so files otherwise when static linking).