Respect runtime paths when loading shared libraries

[ryanking13]

This updates loadDynamicLibrary to locate shared libraries from the file system if possible.

When locating a shared library with a relative path, first attempt to find it in the LD_LIBRARY_PATH directories. If it is not found there, search for it in the runtime path directories. If it is not found, fall back to the original Module['locateFile'].

Followup to #23805.

[ryanking13]

The CI is green now, but two issues are remaining:

1. I wasn't able to remove $FS and $PATH dependency. ifdef hacks doesn't seem to work. And marking those dependencies seems to always link libfs and libpath (even when they are in the ifdef FILESYSTEM block), increasing the output size a lot for the codesize_hello_dylink test.

2. It seems like wasmfs doesn't yet implement FS.lookupPath. So I ended up checking FS.lookupPath existence and early return.

[sbc100]

There is existing logic in system/lib/libc/dynlink.c which searches LD_LIBRARY_PATH so we would somehow avoid searching that path twice.

[ryanking13]

There is existing logic in system/lib/libc/dynlink.c which searches LD_LIBRARY_PATH so we would somehow avoid searching that path twice.

Yes, but that only happens in top-level libraries, I guess? Here is the code flow I understand:

1. A user call dlopen("lib.so") in their c codebase
2. LD_LIBRARY_PATH will be searched inside dlopen, replace the lib.so with the absolute path, and call _emscripten_dlopen_js js function with the resolved path.
3. _emscripten_dlopen_js will internally call loadDynamicLibrary with the absolute path (== loadDynamicLibrary('/abs/path/of/lib.so') )
4. Since the absolute path is given, we don't need to search LD_LIBRARY_PATH and load the file directly.
5. loadDynamicLibrary starts to load the dependencies of lib.so, e.g. dep1.so ( == loadDynamicLibrary('dep1.so'))
6. We don't know where to locate dep1.so. Therefore, search LD_LIBRARY_PATH and rpaths of lib.so to locate dep1.so.

[sbc100]

Yes, I think something like that would be good idea. If we do go that route I might suggest that we implement _dylink_resolve_path_js first as a separate NFC (Non-Functional Change) and then add rpath support as a followup.

[hoodmane]

Okay I addressed all comments and tests seem to be passing locally for me. So this is back to being ready for review.

[hoodmane]

test_ld_library_path_pthread is broken because FS.readFile() doesn't work there. I guess we have to solve it with proxying.

[sbc100]

test_ld_library_path_pthread is broken because FS.readFile() doesn't work there. I guess we have to solve it with proxying.

Hmm.. yes maybe use FS_readFile and mark that with __proxy attribute.

[hoodmane]

But the arguments to/return value of FS_readFile aren't pointers.

[sbc100]

LGTM with some final comments.

Thanks for bearing with me on this one.. i know it was a lot of back and forth.

[sbc100]

test_ld_library_path_pthread is broken because FS.readFile() doesn't work there. I guess we have to solve it with proxying.

Hmm.. yes maybe use FS_readFile and mark that with __proxy attribute.

Oh I see.. so we cannot proxy it :(

Maybe just skip the test for now? And we can try to fix it as a followup? My understanding was that all this loading should happen on the main thread anyway since he original dlopen is always proxied back, but maybe I'm misremembering.

[sbc100]

test_ld_library_path_pthread is broken because FS.readFile() doesn't work there. I guess we have to solve it with proxying.

Hmm.. yes maybe use FS_readFile and mark that with __proxy attribute.

Oh I see.. so we cannot proxy it :(

Maybe just skip the test for now? And we can try to fix it as a followup? My understanding was that all this loading should happen on the main thread anyway since he original dlopen is always proxied back, but maybe I'm misremembering.

Yes, IIRC all the initial loading should happen on the main thread and then background threads should should be able mirror all the same loads but hopfully we can avoid re-resolving all the pathnames on the background threads since them main thread already resolved them.

[hoodmane]

Shouldn't the side threads hit the if (handle) { condition and never make it to this logic?

[hoodmane]

Okay I skipped test_ld_library_path in pthreads and added a WASMFS variant to it.

[ryanking13]

Thanks so much for finishing the PR @hoodmane @sbc100! I know it was a lot of work and I really appreciate it. While this change is not visible to the users right now, I believe this is going to be a big progress in Pyodide.