Add back fs.findObject and fs.readFile in loadLibData

[hoodmane]

Pyodide is using this hook. But if there is some good alternative, we are happy to switch to using it.

The logic we pass for this hook is here:
https://github.com/pyodide/pyodide/blob/main/src/js/dynload.ts#L21-L77

@ryanking13

[sbc100]

Can you call the native _dlopen function, since that is where a lot of this logic lives now?

[hoodmane]

Copying my response from the other location:

There's a few issues. One is asynchronous compilation: dlopen is synchronous I think, so it has to instantiate the module synchronously, which fails on the main thread for large modules.

A second problem is resolving dynamic library dependencies. If we dlopen a library a.so that depends on b.so then the default lookup logic does something we don't want.

[vgvassilev]

@hoodmane, can you elaborate on why this patch was necessary? I cannot imagine a use-case except maybe if allowing to load libraries from the emscripten virtual file system...

PS: dlopen should be thread safe but dlerror might not be on some platforms.

[hoodmane]

if allowing to load libraries from the emscripten virtual file system

Well yes that is exactly what we are trying to do. We'd like it if the linker would look at RPATH and LDPATH to locate the libraries from the file system.

[hoodmane]

Pyodide is still carrying this patch here:
https://github.com/pyodide/pyodide/blob/main/emsdk/patches/0001-Add-back-fs.findObject-and-fs.readFile-in-loadLibDat.patch

[vgvassilev]

What is the reason this did not make it upstream?

[hoodmane]

@sbc100 rejected this patch in some other di. I am having trouble finding it right now and annoyingly we didn't cross reference it earlier I just said "Copying my response from the other location". Maybe it was in a Pyodide issue?

But I think that essentially the problem with this patch is that it doesn't make dlopen and similar C functions respect something like a LDPATH. It only works if you call loadDynamicLibrary from JavaScript and pass an fs argument that knows how you want to handle LDPATH/RPATH.

Proper RPATH support requires an LLVM change, though a fairly simple one. There's a discussion about that here:
#22126
I am probably going to try to push it forward quite soon since this is causing trouble for me right now. LDPATH should be a simpler change.

@sbc100 what would you think about adding code here that looks at ENV.LDPATH and searches those directories in the emscripten file system? That seems like it would be a reasonable change to me?

[sbc100]

@sbc100 rejected this patch in some other di. I am having trouble finding it right now and annoyingly we didn't cross reference it earlier I just said "Copying my response from the other location". Maybe it was in a Pyodide issue?

But I think that essentially the problem with this patch is that it doesn't make dlopen and similar C functions respect something like a LDPATH. It only works if you call loadDynamicLibrary from JavaScript and pass an fs argument that knows how you want to handle LDPATH/RPATH.

Proper RPATH support requires an LLVM change, though a fairly simple one. There's a discussion about that here: #22126 I am probably going to try to push it forward quite soon since this is causing trouble for me right now. LDPATH should be a simpler change.

@sbc100 what would you think about adding code here that looks at ENV.LDPATH and searches those directories in the emscripten file system? That seems like it would be a reasonable change to me?

Sounds reasonable to me yes. I assume you mean LD_LIBRRARY_PATH rather than LDPATH?

LD_LIBRARY_PATH is already supported on the native code size by the way. See resolve_path in system/lib/libc/dynlink.c.

Is one of the problems here that we want this code to work for loading DLLs before the main module is up and running? i.e. before we can run any native code?

[anutosh491]

Pinging for any progress here.

Interested in the usecase here (possibly not through a patch and through having something upstream)
Had opened pyodide/pyodide#5420 to discuss the same.