Skip to content

Basic dynamic loading support #32

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

Merged
merged 13 commits into from
Jun 26, 2011
Merged

Basic dynamic loading support #32

merged 13 commits into from
Jun 26, 2011

Conversation

max99x
Copy link
Contributor

@max99x max99x commented Jun 25, 2011

I've added a basic implementation of dlfcn.h and a compilation mode for dynamically loaded libraries. I doubt this is going to cut it for serious applications, but it works for the few trivial cases I've tried.

Notes:

  1. The preamble is not included when compiling for dynamic loading; it should use the parent process's core runtime and memory management.
  2. The shell and postamble for shared libs are abbreviated.
  3. Library functions in dynamically loaded modules are not defined if the parent has them defined. A problem here is that if the parent has a library function foo() undefined and dynamically loads two modules that define it, they will still define it locally, a copy for each module. I'm not sure if this is the right way to go about it. Should a dlopen() call with the RTLD_GLOBAL flag export identifiers to the actual global scope instead of the parent's Module? Should it do that for library functions?

Also included is support for exporting non-function globals.

I'll add a test case or two, hopefully tomorrow. In the meantime if you could take a look and tell me if anything looks amiss, I'd appreciate it.

@kripken
Copy link
Member

kripken commented Jun 25, 2011

Very nice! I'll have time to look at this a little later, but one idea I had meanwhile: I think you can work around the library problem by just including the entire library in the parent. That is, include all the library in the main file, and none of it in the shared libraries.

This is obviously inefficient since you include lots of functions that aren't needed. However the entire size of the whole library.js is really not that big. So solving this properly can be left for later, meanwhile this is a trivial workaround, and it will keep library calls fast.

@max99x
Copy link
Contributor Author

max99x commented Jun 25, 2011

I think combining the two would be one way to go. This way the libraries are a little more portable, since they can define any functions that they need but can't find, but can still be fast if the parent includes the whole library.

To keep it flexible, I can add a setting so one can manually configure how to include the library:

  1. Include everything.
  2. Include only the necessary parts.
  3. Include the necessary parts but have them defined only if the parent doesn't have them.
  4. Include nothing.

Of course that means an extra burden on the user to configure it correctly, but emscripten doesn't strike me as a project aimed at beginners, at least in its current state.

@kripken
Copy link
Member

kripken commented Jun 25, 2011

I am less concerned about user complexity, more about code complexity. More options is more complicated to get right. But if you already have it working or think it isn't hard, that's cool.

Ok, I read the code, focusing on integration with emscripten. I didn't take a very careful look inside the dlopen() implementation yet.

Looks great, but some minor comments:

  1. Please do === when comparing stuff (like |== undefined|). It's safer.
  2. Why is there |typeof ' + ident + ' == "undefined"| and not just a comparison === to undefined?
  3. Putting a var definition inside an if clause doesn't work. var definitions are function scope. If there isn't a good solution here, then we still have the option discussed before to just ignore library stuff (add an option "compile as something that will load shared libs", and include ALL of library there, and do not include library stuff at all when building as shared).
  4. Please change |is_error| and so forth to |isError| etc.
  5. When pushing to FUNCTION_TABLE, you need to also push a 0 after it. Valid indexes into that array must be even, for some odd low-level compiler reason (there is a comment explaining more in the code near Function.getIndex I believe).
  6. Please default BUILD_AS_SHARED_LIB to 0.

And one more substantial one:

  1. When functions are indexed and placed in FUNCTION_TABLE, we put the absolute values in the calls. So |void ptr = (void)someFunction;| will end up as |ptr = 56| (assuming the index is 56). This is fine in the parent file. But in the child, you are appending to an existing FUNCTION_TABLE, and it isn't clear what the absolute indexes will be. They will need to be calculated at library load time.

One way is to generate not absolute offsets but some identifier, like {{{FUNCTION_INDEX:5}}} etc in shared libs. Then you replace all of those (for 5) with the same absolute value (calculated from the known FUNCTION_TABLE at runtime) and likewise for the rest. There are probably better ways though, this is just off the top of my head.

@max99x
Copy link
Contributor Author

max99x commented Jun 25, 2011

Please do === when comparing stuff (like |== undefined|). It's safer.

Yeah, will do. I'm used to CoffeeScript, which converts it automatically.

Why is there |typeof ' + ident + ' == "undefined"| and not just a comparison === to undefined?

Otherwise it would raise a ReferenceError. Alternatively, we could do 'this.' + ident + ' === undefined'.

Putting a var definition inside an if clause doesn't work. var definitions are function scope.

My bad. If we're using globals in the parent anyway, we can explicitly define globals in the lib using this.ident = ....

Please change |is_error| and so forth to |isError| etc.

Will do.

When pushing to FUNCTION_TABLE, you need to also push a 0 after it. Valid indexes into that array must be even, for some odd low-level compiler reason (there is a comment explaining more in the code near Function.getIndex I believe).

Will do. I still need to look into the case where both the parent and the lib define FUNCTION_TABLE entries. Perhaps there's a conflict there somewhere.

Please default BUILD_AS_SHARED_LIB to 0.

Oh, I thought I did. Will fix.

['memset', 'malloc', 'free'].forEach(function(ident) {
if (INCLUDE_FULL_LIBRARY) {
assert(!BUILD_AS_SHARED_LIB, 'Cannot have both INCLUDE_FULL_LIBRARY and BUILD_AS_SHARED_LIB set.')
var libFuncsToInclude = Object.keys(Library);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use keys(), which we define ourselves. Object.keys is not present in older engines.

@kripken
Copy link
Member

kripken commented Jun 26, 2011

Great stuff! :)

Ok, this is ready to be merged, pending on (1) the few minor comments I just added as notes, and (2) that enough automatic tests pass.

kripken added a commit that referenced this pull request Jun 26, 2011
Basic dynamic loading support
@kripken kripken merged commit 196f994 into emscripten-core:master Jun 26, 2011
tlively pushed a commit to tlively/emscripten that referenced this pull request Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants