Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Apr 16, 2021

This means we can reason about the symbols which are or are not defined across
the entire program which allows for ERROR_ON_UNDEFINED_SYMBOLS to be
enabled for any build that includes all its side modules on the command line.

@sbc100 sbc100 force-pushed the read_side_module_exports branch 2 times, most recently from 14bcbdf to ab85af3 Compare April 16, 2021 04:14
@sbc100 sbc100 requested a review from dschuff April 16, 2021 04:15
@sbc100 sbc100 force-pushed the read_side_module_exports branch 2 times, most recently from 8817d7f to ee6d97a Compare April 16, 2021 17:31
@sbc100 sbc100 force-pushed the read_side_module_exports branch 3 times, most recently from 62db801 to a6bfc13 Compare April 16, 2021 20:44
Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

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

Can you also update the documentation? Once this works (is that with this PR?) we should document it as the primary way to create dynamic libraries (btw does this mean we don't need the MAIN_MODULE and SIDE_MODULE flags anymore?)

@sbc100 sbc100 force-pushed the read_side_module_exports branch from a6bfc13 to 4c3840d Compare April 16, 2021 21:04
@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 16, 2021

(btw does this mean we don't need the MAIN_MODULE and SIDE_MODULE flags anymore?)

Not yet but that would be really really cool.. lets do it!

  • MAIN_MODULE -> include a dylib on the command line when linking
  • SIDE_MODULE -> -shared

@sbc100 sbc100 force-pushed the read_side_module_exports branch from 4c3840d to 29d987f Compare April 16, 2021 21:13
This means we can reason about the symbols which are or are not defined
across the entire program which allows for ERROR_ON_UNDEFINED_SYMBOLS to
be enabled for any build that includes all its side modules on the
command line.
@sbc100 sbc100 force-pushed the read_side_module_exports branch from 29d987f to cded507 Compare April 16, 2021 21:17
@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 16, 2021

Added a ChangeLog entry.

@sbc100 sbc100 enabled auto-merge (squash) April 16, 2021 21:18
@sbc100 sbc100 merged commit 6ca2853 into main Apr 16, 2021
@sbc100 sbc100 deleted the read_side_module_exports branch April 16, 2021 22:04
@dschuff
Copy link
Member

dschuff commented Apr 16, 2021

(btw does this mean we don't need the MAIN_MODULE and SIDE_MODULE flags anymore?)

Not yet but that would be really really cool.. lets do it!

  • MAIN_MODULE -> include a dylib on the command line when linking
  • SIDE_MODULE -> -shared

I wonder what the least "surprising" way to do MAIN_MODULE is. on other platforms, dynamic executables are the default, but not for wasm. If someone or their build system is including libs on the command line that we are ignoring now, I guess someone could accidentally end up with a dynamic executable? I guess that doesn't seem too likely though, since they would also have to be using SIDE_MODULE.
We should probably also check the behavior of CMake and see if our CMake support needs to be updated.

@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 16, 2021

on other platforms, dynamic executables are the default, but not for wasm.

Actually on linux (gcc/clang, etc) the behaviour is "make a static executable unless there is one ore more dynamic libraries in the link, in which case make a dynamic executable". You don't actually need -static unless you have shared objects in your libraries path that you want to explicitly ignore and force the use of the static libraries. If the linker only sees static libraries it will build a static executable.

So I guess I'm proposing that we match this behaviour.. that major difference being that we don't provide any shared libraries for system libs.. so most users will end up with static by default.

@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 16, 2021

This was surprising to me actually:

$ cat test.c
void _start() {}
$ clang -nostdlib test.c
$ file a.out 
a.out: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, BuildID[sha1]=94968d2d1c7dfd08c5b9c950ad85d6ae4d42a410, not stripped

Here we see the compiler defaulting to static linking (without any -static)

@dschuff
Copy link
Member

dschuff commented Apr 16, 2021

OK, you got me there :D
Still, the dynamic libc in basically every distro means that every "normal" build with no special flags is dynamic.

@dschuff
Copy link
Member

dschuff commented Apr 16, 2021

BTW, I think that your proposal will probably be fine. But we should make sure that CMake is doing something reasonable in the default cases, and maybe it would be good to have some easy way for a user to tell whether an executable is dynamic, to help them (and us) diagnose problems when things go wrong.

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.

3 participants