Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1151,11 +1151,20 @@ def check(input_file):

if shared.Settings.MAIN_MODULE:
assert not shared.Settings.SIDE_MODULE
if shared.Settings.MAIN_MODULE != 2:
if shared.Settings.MAIN_MODULE == 1:
# Main modules must provide all JS library functions to side modules, as they
# may require them (in main module mode 2, the user must specify which
# using the normal exporting method).
shared.Settings.INCLUDE_FULL_LIBRARY = 1
shared.Settings.EXPORT_ALL = 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to do this both for SIDE_MODULE=1 and MAIN_MODULE=1, down below on ling 1170 would make sense to me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This still needs doing I think.

elif shared.Settings.SIDE_MODULE:
assert not shared.Settings.MAIN_MODULE
options.memory_init_file = False # memory init file is not supported with asm.js side modules, must be executable synchronously (for dlopen)
# memory init file is not supported with asm.js side modules, must be executable synchronously (for dlopen)
options.memory_init_file = False
# fastcomp dynamic linking is a little odd in that EXPORT_ALL matters
# even for side modules.
if not shared.Settings.WASM_BACKEND and shared.Settings.SIDE_MODULE == 1:
shared.Settings.EXPORT_ALL = 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can just do this once down on line 1166, no? Right below where we set LINKABLE = 1?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still needs doing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, done.


if shared.Settings.MAIN_MODULE or shared.Settings.SIDE_MODULE:
assert shared.Settings.ASM_JS, 'module linking requires asm.js output (-s ASM_JS=1)'
Expand Down
9 changes: 6 additions & 3 deletions src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -785,9 +785,12 @@ var NODE_CODE_CACHING = 0;
// there, you are in effect removing it).
var EXPORTED_FUNCTIONS = ['_main'];

// If true, we export all the symbols. Note that this does *not* affect LLVM, so
// it can still eliminate functions as dead. This just exports them on the
// Module object.
// If true, we export all the symbols that are present in JS onto the Module
// object. This does not affect which symbols will be present - it does not
// prevent DCE or cause anything to be included in linking. It only does
// Module['X'] = X;
// for all X that end up in the JS file. This is useful to export the JS
// library functions on Module, for things like dynamic linking.
var EXPORT_ALL = 0;

// Export all bindings generator functions (prefixed with emscripten_bind_). This
Expand Down
5 changes: 5 additions & 0 deletions src/support.js
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,11 @@ function loadWebAssemblyModule(binary, flags) {
return obj[prop] = function() {
if (!fp) {
var f = resolveSymbol(name, 'function', legalized);
#if ASSERTIONS
// this resolved symbol must exist, as we are about to try to
// add it to the table; error clearly here instead of there.
assert(f, 'could not resolve: ' + name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

resolveSymbol already includes an assert which more detail than this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I forgot to build with assertions when I tested this I think...

#endif
fp = addFunction(f, sig);
}
return fp;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
___errno_location
__apply_relocations
_main
_malloc
dynCall_X
dynCall_ii
dynCall_iiii
dynCall_iiiii
stackAlloc
___errno_location
__apply_relocations
_main
_malloc
dynCall_X
dynCall_ii
dynCall_iiii
dynCall_iiiii
stackAlloc
26 changes: 13 additions & 13 deletions tests/other/metadce/hello_world_fastcomp_O3_MAIN_MODULE_2.imports
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
STACKTOP
___wasi_fd_write
__memory_base
__table_base
_emscripten_get_heap_size
_emscripten_memcpy_big
_emscripten_resize_heap
abort
fb
gb
memory
setTempRet0
table
STACKTOP
___wasi_fd_write
__memory_base
__table_base
_emscripten_get_heap_size
_emscripten_memcpy_big
_emscripten_resize_heap
abort
fb
gb
memory
setTempRet0
table
26 changes: 13 additions & 13 deletions tests/other/metadce/hello_world_fastcomp_O3_MAIN_MODULE_2.sent
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
STACKTOP
___wasi_fd_write
__memory_base
__table_base
_emscripten_get_heap_size
_emscripten_memcpy_big
_emscripten_resize_heap
abort
fb
gb
memory
setTempRet0
table
STACKTOP
___wasi_fd_write
__memory_base
__table_base
_emscripten_get_heap_size
_emscripten_memcpy_big
_emscripten_resize_heap
abort
fb
gb
memory
setTempRet0
table
5 changes: 0 additions & 5 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2555,12 +2555,10 @@ def test_nestedstructs(self):
def prep_dlfcn_lib(self):
self.clear_setting('MAIN_MODULE')
self.set_setting('SIDE_MODULE')
self.set_setting('EXPORT_ALL')

def prep_dlfcn_main(self):
self.set_setting('MAIN_MODULE')
self.clear_setting('SIDE_MODULE')
self.set_setting('EXPORT_ALL')

create_test_file('lib_so_pre.js', '''
if (!Module['preRun']) Module['preRun'] = [];
Expand Down Expand Up @@ -3566,9 +3564,6 @@ def test_dlfcn_feature_in_lib(self, js_engines):
self.do_run(src, 'float: 42.\n', js_engines=js_engines)

def dylink_test(self, main, side, expected=None, header=None, main_emcc_args=[], force_c=False, need_reverse=True, auto_load=True, **kwargs):
# shared settings
self.set_setting('EXPORT_ALL', 1)

if header:
create_test_file('header.h', header)

Expand Down
4 changes: 2 additions & 2 deletions tests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -6667,7 +6667,7 @@ def test_main_module_without_exceptions_message(self):
def build_main(args):
print(args)
with env_modify({'EMCC_FORCE_STDLIBS': 'libc++abi'}):
run_process([PYTHON, EMCC, 'main.cpp', '-s', 'MAIN_MODULE=1', '-s', 'EXPORT_ALL',
run_process([PYTHON, EMCC, 'main.cpp', '-s', 'MAIN_MODULE=1',
'--embed-file', 'libside.wasm'] + args)

build_main([])
Expand Down Expand Up @@ -8220,7 +8220,7 @@ def test_metadce_cxx_fastcomp(self, *args):
# don't compare the # of functions in a main module, which changes a lot
# TODO(sbc): Investivate why the number of exports is order of magnitude
# larger for wasm backend.
'main_module_2': (['-O3', '-s', 'MAIN_MODULE=2'], 12, [], [], 10770, 12, 10, None), # noqa
'main_module_2': (['-O3', '-s', 'MAIN_MODULE=2'], 12, [], [], 10652, 12, 10, None), # noqa
})
@no_fastcomp()
def test_metadce_hello(self, *args):
Expand Down