Skip to content

Commit

Permalink
STANDALONE_WASM: separate crt1 for commands and reactors (#11167)
Browse files Browse the repository at this point in the history
Unlike without STANDALONE_WASM users are expected to either define
a main or explicitly opt out.
This change and paves to way to moving to llvm's new main mangling
method: https://reviews.llvm.org/D70700

Fixes:  #9640
  • Loading branch information
sbc100 authored May 18, 2020
1 parent 6511178 commit fcfa4d8
Show file tree
Hide file tree
Showing 13 changed files with 64 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ var LibraryManager = {
}

if (BOOTSTRAPPING_STRUCT_INFO) {
libraries = ['library_formatString.js'];
libraries = ['library_formatString.js', 'library_stack_trace.js'];
}

// Deduplicate libraries to avoid processing any library file multiple times
Expand Down
4 changes: 4 additions & 0 deletions src/postamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,11 @@ function callMain(args) {
#endif

#if STANDALONE_WASM
#if EXPECT_MAIN
var entryFunction = Module['__start'];
#else
var entryFunction = Module['__initialize'];
#endif
#else
var entryFunction = Module['_main'];
#endif
Expand Down
8 changes: 1 addition & 7 deletions system/lib/libc/crt1.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@

extern void __wasm_call_ctors(void) __attribute__((weak));

// TODO(sbc): We shouldn't even link this file if there is no main:
// https://github.com/emscripten-core/emscripten/issues/9640
extern int main(int argc, char** argv) __attribute__((weak));
extern int main(int argc, char** argv);

// If main() uses argc/argv, then no __original_main is emitted, and then
// this definition is used, which loads those values and sends them to main.
Expand Down Expand Up @@ -66,10 +64,6 @@ void _start(void) {
__wasm_call_ctors();
}

if (!main) {
return;
}

int r = __original_main();

exit(r);
Expand Down
14 changes: 14 additions & 0 deletions system/lib/libc/crt1_reactor.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright 2020 The Emscripten Authors. All rights reserved.
* Emscripten is available under two separate licenses, the MIT license and the
* University of Illinois/NCSA Open Source License. Both these licenses can be
* found in the LICENSE file.
*/

extern void __wasm_call_ctors(void) __attribute__((weak));

void _initialize(void) {
if (__wasm_call_ctors) {
__wasm_call_ctors();
}
}
3 changes: 0 additions & 3 deletions tests/other/metadce/mem_no_main_O3_STANDALONE_WASM.exports

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
_initialize
foo
memory
33 changes: 13 additions & 20 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1595,7 +1595,9 @@ def test_iostream_ctors(self):
}
''', 'bugfree code')

@also_with_standalone_wasm
# Marked as impure since the WASI reactor modules (modules without main)
# are not yet suppored by the wasm engines we test against.
@also_with_impure_standalone_wasm
def test_ctors_no_main(self):
self.emcc_args.append('--no-entry')
self.do_run_in_out_file_test('tests', 'core', 'test_ctors_no_main')
Expand Down Expand Up @@ -8852,22 +8854,6 @@ def test_safe_stack_dylink(self):
}
''', ['abort(stack overflow)', '__handle_stack_overflow'], assert_returncode=None)

@also_with_standalone_wasm
def test_undefined_main(self):
if self.get_setting('LLD_REPORT_UNDEFINED'):
self.skipTest('LLD_REPORT_UNDEFINED does not allow implicit undefined main')
if self.get_setting('STRICT'):
self.skipTest('STRICT does not allow implicit undefined main')
# By default in emscripten we allow main to be undefined. Its used when
# building library code that has no main.
# TODO(sbc): Simplify the code by making this an opt-in feature.
# https://github.com/emscripten-core/emscripten/issues/9640
src = '''
#include <emscripten.h>
EMSCRIPTEN_KEEPALIVE void foo() {}
'''
self.build(src, self.get_dir(), 'test.c')

def test_fpic_static(self):
self.emcc_args.append('-fPIC')
self.do_run_in_out_file_test('tests', 'core', 'test_hello_world')
Expand Down Expand Up @@ -8909,16 +8895,23 @@ def test_minimal_runtime_emscripten_get_exported_function(self):
self.emcc_args += ['-lexports.js', '-s', 'MINIMAL_RUNTIME=1']
self.do_run_in_out_file_test('tests', 'core', 'test_get_exported_function')

def test_auto_detect_main(self):
if not self.get_setting('LLD_REPORT_UNDEFINED') and not self.get_setting('STRICT'):
# Marked as impure since the WASI reactor modules (modules without main)
# are not yet suppored by the wasm engines we test against.
@also_with_impure_standalone_wasm
def test_undefined_main(self):
# Traditionally in emscripten we allow main to be undefined. This allows programs with a main
# and libraries without a main to be compiled identically.
# However we are trying to move away from that model to a more explicit opt-out model. See:
# https://github.com/emscripten-core/emscripten/issues/9640
if not self.get_setting('LLD_REPORT_UNDEFINED') and not self.get_setting('STRICT') and not self.get_setting('STANDALONE_WASM'):
self.do_run_in_out_file_test('tests', 'core', 'test_ctors_no_main')

# Disabling IGNORE_MISSING_MAIN should cause link to fail due to missing main
self.set_setting('IGNORE_MISSING_MAIN', 0)
err = self.expect_fail([PYTHON, EMCC, path_from_root('tests', 'core', 'test_ctors_no_main.cpp')] + self.get_emcc_args())
self.assertContained('error: entry symbol not defined (pass --no-entry to suppress): main', err)

# We can fix the error either by adding --no-entry or by setting EXPORTED_FUNCTIONS to empty
# If we pass --no-entry or set EXPORTED_FUNCTIONS to empty should never see any errors
self.emcc_args.append('--no-entry')
self.do_run_in_out_file_test('tests', 'core', 'test_ctors_no_main')

Expand Down
2 changes: 1 addition & 1 deletion tests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -8379,7 +8379,7 @@ def test_metadce_hello_fastcomp(self, *args):
'O3_standalone_narg': ('mem_no_argv.c', ['-O3', '-s', 'STANDALONE_WASM'],
[], [], 6309), # noqa
# without main, no support code for argc/argv is emitted either
'O3_standalone_lib': ('mem_no_main.c', ['-O3', '-s', 'STANDALONE_WASM'],
'O3_standalone_lib': ('mem_no_main.c', ['-O3', '-s', 'STANDALONE_WASM', '--no-entry'],
[], [], 6309), # noqa
# Growth support code is in JS, no significant change in the wasm
'O3_grow': ('mem.c', ['-O3', '-s', 'ALLOW_MEMORY_GROWTH'],
Expand Down
7 changes: 5 additions & 2 deletions tools/shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -1582,9 +1582,12 @@ def lld_flags_for_executable(external_symbol_list):
'-z', 'stack-size=%s' % Settings.TOTAL_STACK,
'--initial-memory=%d' % Settings.INITIAL_MEMORY,
]
use_start_function = Settings.STANDALONE_WASM

if not use_start_function:
if Settings.STANDALONE_WASM:
# when Settings.EXPECT_MAIN is set we fall back to wasm-ld default of _start
if not Settings.EXPECT_MAIN:
cmd += ['--entry=_initialize']
else:
if Settings.EXPECT_MAIN and not Settings.IGNORE_MISSING_MAIN:
cmd += ['--entry=main']
else:
Expand Down
23 changes: 22 additions & 1 deletion tools/system_libs.py
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,24 @@ def can_build(self):
return super(crt1, self).can_build() and shared.Settings.WASM_BACKEND


class crt1_reactor(MuslInternalLibrary):
name = 'crt1_reactor'
cflags = ['-O2']
src_dir = ['system', 'lib', 'libc']
src_files = ['crt1_reactor.c']

force_object_files = True

def get_ext(self):
return '.o'

def can_use(self):
return super(crt1_reactor, self).can_use() and shared.Settings.STANDALONE_WASM

def can_build(self):
return super(crt1_reactor, self).can_build() and shared.Settings.WASM_BACKEND


class libc_extras(NoBCLibrary, MuslInternalLibrary):
"""This library is separate from libc itself for fastcomp only so that the
constructor it contains can be DCE'd. Such tricks are not needed wih the
Expand Down Expand Up @@ -1557,7 +1575,10 @@ def add_library(lib):
libs_to_link.append((lib.get_path(), need_whole_archive))

if shared.Settings.STANDALONE_WASM:
add_library(system_libs_map['crt1'])
if shared.Settings.EXPECT_MAIN:
add_library(system_libs_map['crt1'])
else:
add_library(system_libs_map['crt1_reactor'])

for forced in force_include:
add_library(system_libs_map[forced])
Expand Down

0 comments on commit fcfa4d8

Please sign in to comment.