-
Notifications
You must be signed in to change notification settings - Fork 201
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
add stubs for dlopen, dlsym, etc. #443
Conversation
This adds weak exports for the POSIX `dlopen`, `dlsym`, `dlclose`, and `dlerror` functions, allowing code which uses those features to compile. The implementations are stubs which always fail since there is currently no official standard for runtime dynamic linking. Since the symbols are weak, they can be overriden with useful, runtime-specific implementations, e.g. based on host functions or statically-generated tables (see https://github.com/dicej/component-linking-demo for an example of the latter). Signed-off-by: Joel Dice <[email protected]>
0236773
to
3a14878
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also fine merging this as-is, but wanted to get some feedback on the following: it feels like this pattern, "provide some maybe-working, weak symbols," is what the libwasi-emulated-*.a
libraries are for. @sunfishcode, @sbc100: any opinions on this?
In order to override the weak symbols one would need to provide strong symbols, which already works today doesn't it (i.e. without this change). What is the advantage of having these failing weak symbols? |
(Sorry for being slow to reply; busy week) The main advantage is being able to compile projects that use |
I would imagine that most projects that use Do you have a examples of a codebases that would benefit from these stubs? i.e. Do not have a way to disable the |
Ah, but I don't want to disable them. I've actually forked CPython to enable Beyond CPython, I want to be able to compile any library that uses Granted, if you don't have a working |
Sorry I must be misunderstanding something. If you are going to provide a working implementation at link time I don't see why this change is useful. Can't you already do that without this change? |
I think I see now.. you are talking about that case where libc itself is a dynamic library.. and you want to link against one version of libc (that doesn't have dlopen) but then run with version that does have it? I.e. you are going to run against a different version of libc.so to the one that you linked against. Presumably that also means that this patch is really only useful in the case the libc is dynamically linked? I'm still not 100% convinced this is that right way forward.. but I think I'm seeing you use case now. What do others thing about this approach? @sunfishcode ? |
Yes, that's correct. And I'm happy to restrict this to the dynamically linked version of libc if that's feasible. |
Oops, I should have read more carefully. I'm actually going to use the exact same libc.so in both cases, but also link in my special libdl-for-shared-everything-linking.so so that its version of dlopen etc. is used instead of libc.so's version. |
Here's an excerpt from the Makefile I'm using that does this, in case it's helpful: $(BUILD_DIR)/bar.wasm: \
$(BUILD_DIR)/libbar.so \
$(BUILD_DIR)/libfoo.so \
$(BUILD_DIR)/libdl.so \
$(BUILD_DIR)/libpython3.11.so \
$(LIBC) \
$(LIBCXX) \
$(LIBCXXABI) \
$(NUMPY_LIBRARIES) \
$(WASI_ADAPTER)
$(WASM_TOOLS_CLI) component link \
--adapt wasi_snapshot_preview1=$(WASI_ADAPTER) \
--dl-openable $(BUILD_DIR)/libfoo.so \
$(BUILD_DIR)/libbar.so \
$(BUILD_DIR)/libdl.so \
$(BUILD_DIR)/libpython3.11.so \
$(LIBC) \
$(LIBCXX) \
$(LIBCXXABI) \
$(shell echo $(NUMPY_LIBRARIES) | \
sed 's_$(BUILD_DIR)/\([^ ]*\)_--dl-openable /build/\1=$(BUILD_DIR)/\1_g') \
-o $@ In this case, libdl.so is customized for how |
i'm a bit confused. do you mean you don't really need these stubs? |
#define RTLD_DI_LINKMAP 2 | ||
#define RTLD_GLOBAL 256 | ||
#define RTLD_LAZY 1 | ||
#define RTLD_LOCAL 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it straightforward to use a non-zero value?
an implementation can still choose to ignore it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dicej ping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't notice these comments earlier. The values in these files come straight from libc-top-half/musl/include/dlfcn.h, which appears to remain unchanged since it was imported from musl libc. glibc
seems to use the same values for these constants also. I'm not necessarily opposed to changing the values, but sticking with the musl values seems like a safe default here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today I learned that RTLD_GLOBAL
is the default on MacOS, but RTLD_LOCAL
is the default on Linux (which I guess is why RTLD_LOCAL
is zero on musl and glibc but nonzero on MacOS). I guess we ought to decide that the default should be for wasi-libc
if we ever ship more than a stub implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unless you want to decide the default right now, we can postpone the decision by making it a non-zero value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed an update changing it to a non-zero value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand how we can make both of these non-zero. This is a binary choice between global or local symbol binding, its not possible to choose neither. The question is what the default should be when neither is specified and the answer has to be one or the other.
I suggest we go with that musl's defaults for now given that we are largely a port of musl here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, I understand now. Please ignore my last comment. Having RTLD_LOCAL
and RTLD_GLOBAL
both be non-zero allows the implementation to detect the default case and choose one or the other at runtime. SGTM
@@ -1499,6 +1499,15 @@ | |||
#define RRFIXEDSZ NS_RRFIXEDSZ | |||
#define RRQ 01 | |||
#define RS_HIPRI 0x01 | |||
#define RTLD_DEFAULT ((void *)0) | |||
#define RTLD_DI_LINKMAP 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this make sense w/o dlinfo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dicej ping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't know why this is defined unconditionally in dlfcn.h. We could move it into the #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
block in that file if desired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed an update to address this.
IIRC that use case if dynamic linking. They want link (at build time) against a |
It might be helpful to step back and look at the big picture here. Many (most?) popular high-level languages have some way to do FFI calls to C ABI native code loaded via Python is perhaps the most notable of these since many popular packages rely heavily on native extensions. Those packages are distributed as platform-specific "wheels", each of which includes the Python code plus one or more shared libraries built for that specific platform. The wheels are usually published to pypi.org, the default package index and repository used by The immediate motivation behind #429 and WebAssembly/wasi-sdk#338 was to lay the foundation for introducing a new WASI platform for Python wheels alongside the existing ones such as MacOS/ARM64 and Windows/x64. Such wheels would contain shared libraries built using a future release of In order to actually use the shared libraries in these wheels, CPython needs some way to load them, and the easiest way is to build it with As I mentioned earlier, the intention here is that the stubs will be replaced (either at runtime or component composition time) with a working implementation appropriate to the environment in which they're used. One such implementation might load libraries Emscripten-style, via a URL. Another might search the filesystem. To summarize: my goal is to make it easy to create and use WASI shared libraries -- including via |
my impression is that it's a bit tricky to replace only a part of a shared library. (libc.so) how about providing a stub libdl.so as a separate library from libc? |
Sounds good to me. Ideally I'd like to include it as part of |
That does sounds little more logical I guess, since then you replacing the entire libdl.so and not just part of libc.so. I guess we could even make this a dynamic-only library since there would not be any use the static one. But that also begs the question, why not just supply your working libdl.so and link time and link against that, which would take wasi-libc out of the picture completely. I guess one argument would be that there could be other users who could benefit from a shared libdl.so.. not just python.. but I'm not sure we are that stage yet. I don't feel strongly either way. |
Yes, there are three options here with respect to CPython:
|
Does CPython gracefully handle failure of Or are you imagining that all users of CPython+wasi is inject a working version of dlopen at runtime? (Presumably those who build without dyanmic linking have no way to do this). |
Yes, it does handle it gracefully -- the same way it would if it couldn't find the native extension library on the filesystem. I.e. it will behave the same way it does today: trying to load a native extension will fail because it is running in an environment where loading native extensions is not possible. BTW, keep in mind that 99% of users will not be building CPython from source -- they will be using an official, pre-built release, in which case build-time feedback isn't relevant to them.
All CPython users who want to use native extensions with CPython+wasi will inject a working version of dlopen, yes (though not necessarily at runtime -- possibly at composition time). |
SGTM.. sounds like a good plan. Out of interest when you talk about injecting a working version of "dlopen" at "composition time" is that just the "libdl" functions that get linked in, or are you also talking about the shared libraries that are opened by "dlopen" ? Are they somehow determined ahead of time and kind of pre-linked? i.e. is your dlopen actually able to load stuff at runtime off the wire? |
Off topic now: How about I found out more about |
Sounds great -- here's a talk I did about I've chatted with Hood Chatham in detail about Pyodide, and it's been an inspiration for a lot of what I've done with |
The This implementation of |
Ah, yes that is what I think I remembered about this use case. It seems like that "pseudo-dynamic linking" in this case could probably be optimized by doing that link statically use wasm-ld and building things as |
.so files have a number of advantages over .a files:
|
Good points! It still might be interesting as to measure how much using |
Hey @sbc100 👋🏻 I agree that for use cases where having The use cases we're currently looking at unfortunately fundamentally require Given that, do you think we could get this approach in? If anyone in the future has use cases that lend themselves to static linking, we'd be happy to help facilitate that work, e.g. by providing reviews and support. |
Sure thing, I just have couple more nits. I think mostly agreed to go with above proposal to putting these stubs in libdl.so, so that still needs to happen. |
Per review feedback, it's easier to simply replace libdl.so with a working implementation at runtime than it is to override a handful of symbols in libc. Note that I've both added libdl.so and replaced the empty libdl.a we were previously creating with one that contains the stubs. I'm thinking we might as well be consistent about what symbols the .so and the .a contain. Otherwise, e.g. the CPython build gets confused when the dlfcn.h says `dlopen` etc. exist but libdl.a is empty. Signed-off-by: Joel Dice <[email protected]>
@sbc100 thanks for the detailed feedback. I believe I've addressed everything. BTW, I noticed that |
For WASI, we use flag values which match MacOS rather than musl. This gives `RTLD_LOCAL` a non-zero value, avoiding ambiguity and allowing us to defer the decision of whether `RTLD_LOCAL` or `RTLD_GLOBAL` should be the default when neither is specified. We also avoid declaring `dladdr`, `dlinfo`, and friends on WASI since they are neither supported nor stubbed at this time. Signed-off-by: Joel Dice <[email protected]>
libc-top-half/musl/include/dlfcn.h
Outdated
#define RTLD_LOCAL 0x4 | ||
#define RTLD_GLOBAL 0x8 | ||
#define RTLD_NOLOAD 0x10 | ||
#define RTLD_NODELETE 0x80 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why re-define all of these just because you want to make RTLD_LOCAL non-zero?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have to. I just figured we should try to consistently match some existing system that is already widely used, and MacOS was the first one I found that had a non-zero RTLD_LOCAL. I don't have any strong feelings about it, though. Would you prefer I change only RTLD_LOCAL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should always prefer the minimal set of changes from musl... unless there is a good reason not to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed another update that uses the musl values except for RTLD_LOCAL.
This minimizes the divergence from upstream while still giving us the flexibility to choose a default value later. Signed-off-by: Joel Dice <[email protected]>
Signed-off-by: Joel Dice <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks to me like all the concerns are addressed; I was generally in favor of this from the beginning but it looks like the discussion improved some parts. Thanks @dicej for working through all of this and giving clear explanations! Since there seems to be consensus on the main idea, I'll go ahead and merge this and we can work out any details in future PRs.
This adds weak exports for the POSIX
dlopen
,dlsym
,dlclose
, anddlerror
functions, allowing code which uses those features to compile. The implementations are stubs which always fail since there is currently no official standard for runtime dynamic linking.Since the symbols are weak, they can be overriden with useful, runtime-specific implementations, e.g. based on host functions or statically-generated tables (see https://github.com/dicej/component-linking-demo for an example of the latter).