From c85915b7b68a789472fa4e906bdce1d669e48845 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Tue, 13 Dec 2022 12:17:17 -0800 Subject: [PATCH] Allow JS -> native dependencies to be specified in __deps. NFC Now that LLD_REPORT_UNDEFINED is always on, we can depend on JS library processing before link time. Extend the existing symbol list to be a list of symbols + native dependencies. This way we can begin to move reverse dependencies out of deps_info.py and potentially one day completely remove that file. For now it is still needed because indirect dependencies can't be specified in the JS library code yet. Replaces (and inspired by): #15982 --- emcc.py | 38 +++++++++++++++++++++++++----------- src/jsifier.js | 17 +++++++++------- src/library.js | 2 +- src/library_browser.js | 3 ++- src/library_sdl.js | 11 ++++++++--- src/library_uuid.js | 1 + test/test_other.py | 44 ++++++++++++------------------------------ tools/building.py | 7 +++---- tools/deps_info.py | 18 +++++++---------- 9 files changed, 71 insertions(+), 70 deletions(-) diff --git a/emcc.py b/emcc.py index 222547aace2fc..da3faf3df3bb1 100755 --- a/emcc.py +++ b/emcc.py @@ -28,6 +28,7 @@ import json import logging import os +import pprint import re import shlex import shutil @@ -50,6 +51,7 @@ from tools.minimal_runtime_shell import generate_minimal_runtime_html import tools.line_endings from tools import feature_matrix +from tools import deps_info from tools import js_manipulation from tools import wasm2c from tools import webassembly @@ -542,14 +544,29 @@ def build_symbol_list(filename): """Only called when there is no existing symbol list for a given content hash. """ library_syms = generate_js_symbols() - write_file(filename, '\n'.join(library_syms) + '\n') + lines = [] + + for name, deps in library_syms.items(): + if deps: + lines.append('%s: %s' % (name, ','.join(deps))) + else: + lines.append(name) + write_file(filename, '\n'.join(lines) + '\n') # We need to use a separate lock here for symbol lists because, unlike with system libraries, # it's normally for these file to get pruned as part of normal operation. This means that it # can be deleted between the `cache.get()` then the `read_file`. with filelock.FileLock(cache.get_path(cache.get_path('symbol_lists.lock'))): filename = cache.get(f'symbol_lists/{content_hash}.txt', build_symbol_list) - library_syms = read_file(filename).splitlines() + lines = read_file(filename).splitlines() + library_syms = {} + for line in lines: + if ':' in line: + name, deps = line.split(':') + library_syms[name] = deps.strip().split(',') + else: + library_syms[line] = [] + # Limit of the overall size of the cache to 100 files. # This code will get test coverage since a full test run of `other` or `core` @@ -1315,9 +1332,15 @@ def run(args): logger.debug('stopping after linking to object file') return 0 + js_syms = {} + if not settings.SIDE_MODULE: + js_syms = get_all_js_syms() + deps_info.append_deps_info(js_syms) + logger.debug("JS symbols: " + pprint.pformat(js_syms)) + phase_calculate_system_libraries(state, linker_arguments, linker_inputs, newargs) - phase_link(linker_arguments, wasm_target) + phase_link(linker_arguments, wasm_target, js_syms) # Special handling for when the user passed '-Wl,--version'. In this case the linker # does not create the output file, but just prints its version and exits with 0. @@ -3060,7 +3083,7 @@ def phase_calculate_system_libraries(state, linker_arguments, linker_inputs, new @ToolchainProfiler.profile_block('link') -def phase_link(linker_arguments, wasm_target): +def phase_link(linker_arguments, wasm_target, js_syms): logger.debug(f'linking: {linker_arguments}') # Make a final pass over settings.EXPORTED_FUNCTIONS to remove any @@ -3070,13 +3093,6 @@ def phase_link(linker_arguments, wasm_target): settings.REQUIRED_EXPORTS = dedup_list(settings.REQUIRED_EXPORTS) settings.EXPORT_IF_DEFINED = dedup_list(settings.EXPORT_IF_DEFINED) - # if EMCC_DEBUG=2 then we must link now, so the temp files are complete. - # if using the wasm backend, we might be using vanilla LLVM, which does not allow our - # fastcomp deferred linking opts. - # TODO: we could check if this is a fastcomp build, and still speed things up here - js_syms = None - if settings.ERROR_ON_UNDEFINED_SYMBOLS and not settings.SIDE_MODULE: - js_syms = get_all_js_syms() building.link_lld(linker_arguments, wasm_target, external_symbols=js_syms) diff --git a/src/jsifier.js b/src/jsifier.js index c80e2a42997fe..5664d271f784b 100644 --- a/src/jsifier.js +++ b/src/jsifier.js @@ -68,6 +68,7 @@ function isDefined(symName) { function runJSify(symbolsOnly = false) { const libraryItems = []; + const symbolDeps = {}; let postSets = []; LibraryManager.load(); @@ -245,9 +246,16 @@ function ${name}(${args}) { return; } + const deps = LibraryManager.library[symbol + '__deps'] || []; + if (!Array.isArray(deps)) { + error(`JS library directive ${symbol}__deps=${deps.toString()} is of type ${typeof deps}, but it should be an array!`); + return; + } + if (symbolsOnly) { if (!isJsOnlySymbol(symbol) && LibraryManager.library.hasOwnProperty(symbol)) { - librarySymbols.push(symbol); + externalDeps = deps.filter((d) => !(d in LibraryManager.library) && typeof d === 'string'); + symbolDeps[symbol] = externalDeps; } return; } @@ -319,11 +327,6 @@ function ${name}(${args}) { const original = LibraryManager.library[symbol]; let snippet = original; - const deps = LibraryManager.library[symbol + '__deps'] || []; - if (!Array.isArray(deps)) { - error(`JS library directive ${symbol}__deps=${deps.toString()} is of type ${typeof deps}, but it should be an array!`); - return; - } const isUserSymbol = LibraryManager.library[symbol + '__user']; deps.forEach((dep) => { @@ -542,7 +545,7 @@ function ${name}(${args}) { } if (symbolsOnly) { - print(JSON.stringify(librarySymbols)); + print(JSON.stringify(symbolDeps)); return; } diff --git a/src/library.js b/src/library.js index c5ac6e27663d1..f6e235a00f81a 100644 --- a/src/library.js +++ b/src/library.js @@ -2111,7 +2111,7 @@ mergeInto(LibraryManager.library, { list: [], map: {} }, - setprotoent__deps: ['$Protocols', '$writeAsciiToMemory'], + setprotoent__deps: ['$Protocols', '$writeAsciiToMemory', 'malloc'], setprotoent: function(stayopen) { // void setprotoent(int stayopen); diff --git a/src/library_browser.js b/src/library_browser.js index 555277ab12c00..a8160844ded6b 100644 --- a/src/library_browser.js +++ b/src/library_browser.js @@ -804,6 +804,7 @@ var LibraryBrowser = { }, emscripten_run_preload_plugins_data__proxy: 'sync', + emscripten_run_preload_plugins_data__deps: ['malloc'], emscripten_run_preload_plugins_data__sig: 'viiiiii', emscripten_run_preload_plugins_data: function(data, size, suffix, arg, onload, onerror) { {{{ runtimeKeepalivePush() }}} @@ -1355,7 +1356,7 @@ var LibraryBrowser = { return info.awaited; }, - emscripten_get_preloaded_image_data__deps: ['$PATH_FS'], + emscripten_get_preloaded_image_data__deps: ['$PATH_FS', 'malloc'], emscripten_get_preloaded_image_data__proxy: 'sync', emscripten_get_preloaded_image_data__sig: 'iiii', emscripten_get_preloaded_image_data: function(path, w, h) { diff --git a/src/library_sdl.js b/src/library_sdl.js index d52e85e992c2a..d3f9796a40ffe 100644 --- a/src/library_sdl.js +++ b/src/library_sdl.js @@ -1348,7 +1348,7 @@ var LibrarySDL = { return SDL.version; }, - SDL_Init__deps: ['$zeroMemory'], + SDL_Init__deps: ['$zeroMemory', 'malloc', 'free', 'memcpy'], SDL_Init__proxy: 'sync', SDL_Init__sig: 'ii', SDL_Init__docs: '/** @param{number=} initFlags */', @@ -1846,15 +1846,18 @@ var LibrarySDL = { SDL_SetError: function() {}, SDL_malloc__sig: 'ii', + SDL_malloc__deps: ['malloc'], SDL_malloc: function(size) { return _malloc(size); }, SDL_free__sig: 'vi', + SDL_free__deps: ['free'], SDL_free: function(ptr) { _free(ptr); }, + SDL_CreateRGBSurface__deps: ['malloc', 'free'], SDL_CreateRGBSurface__proxy: 'sync', SDL_CreateRGBSurface__sig: 'iiiiiiiii', SDL_CreateRGBSurface: function(flags, width, height, depth, rmask, gmask, bmask, amask) { @@ -2067,6 +2070,7 @@ var LibrarySDL = { SDL_PushEvent__proxy: 'sync', SDL_PushEvent__sig: 'ii', + SDL_PushEvent__deps: ['malloc'], SDL_PushEvent: function(ptr) { var copy = _malloc({{{ C_STRUCTS.SDL_KeyboardEvent.__size__ }}}); _memcpy(copy, ptr, {{{ C_STRUCTS.SDL_KeyboardEvent.__size__ }}}); @@ -2117,6 +2121,7 @@ var LibrarySDL = { // An Emscripten-specific extension to SDL: Some browser APIs require that they are called from within an event handler function. // Allow recording a callback that will be called for each received event. emscripten_SDL_SetEventHandler__proxy: 'sync', + emscripten_SDL_SetEventHandler__deps: ['malloc'], emscripten_SDL_SetEventHandler__sig: 'vii', emscripten_SDL_SetEventHandler: function(handler, userdata) { SDL.eventHandler = handler; @@ -2245,7 +2250,7 @@ var LibrarySDL = { return flags; // We support JPG, PNG, TIF because browsers do }, - IMG_Load_RW__deps: ['SDL_LockSurface', 'SDL_FreeRW', '$PATH_FS'], + IMG_Load_RW__deps: ['SDL_LockSurface', 'SDL_FreeRW', '$PATH_FS', 'malloc'], IMG_Load_RW__proxy: 'sync', IMG_Load_RW__sig: 'iii', IMG_Load_RW: function(rwopsID, freeSrc) { @@ -2413,7 +2418,7 @@ var LibrarySDL = { // SDL_Audio - SDL_OpenAudio__deps: ['$autoResumeAudioContext', '$safeSetTimeout'], + SDL_OpenAudio__deps: ['$autoResumeAudioContext', '$safeSetTimeout', 'malloc'], SDL_OpenAudio__proxy: 'sync', SDL_OpenAudio__sig: 'iii', SDL_OpenAudio: function(desired, obtained) { diff --git a/src/library_uuid.js b/src/library_uuid.js index 0f649d0922960..6a4bb4d9b8b65 100644 --- a/src/library_uuid.js +++ b/src/library_uuid.js @@ -24,6 +24,7 @@ mergeInto(LibraryManager.library, { }, // Copies the 'compact' UUID variable from src to dst. + uuid_copy__deps: ['memcpy'], uuid_copy: function(dst, src) { // void uuid_copy(uuid_t dst, const uuid_t src); _memcpy(dst, src, 16); diff --git a/test/test_other.py b/test/test_other.py index 06168bbccf3db..0eedb07c6f3d4 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -6735,26 +6735,22 @@ def test_no_missing_symbols(self): # simple hello world should not show any miss mergeInto(LibraryManager.library, { my_js__deps: ['main'], my_js: (function() { - return function() { - console.log("hello " + _nonexistingvariable); - }; + return () => console.log("hello " + _nonexistingvariable); }()), }); ''') - create_file('test.cpp', '''\ + create_file('test.c', '''\ #include #include -extern "C" { - extern void my_js(); -} +void my_js(); int main() { my_js(); return EXIT_SUCCESS; } ''') - self.run_process([EMXX, 'test.cpp', '--js-library', 'library_foo.js']) + self.run_process([EMCC, 'test.c', '--js-library', 'library_foo.js']) # but we do error on a missing js var create_file('library_foo_missing.js', ''' @@ -6767,16 +6763,16 @@ def test_no_missing_symbols(self): # simple hello world should not show any miss }()), }); ''') - err = self.expect_fail([EMXX, 'test.cpp', '--js-library', 'library_foo_missing.js']) - self.assertContained('undefined symbol: nonexistingvariable', err) + err = self.expect_fail([EMCC, 'test.c', '--js-library', 'library_foo_missing.js']) + self.assertContained('wasm-ld: error: symbol exported via --export not found: nonexistingvariable', err) # and also for missing C code, of course (without the --js-library, it's just a missing C method) - err = self.expect_fail([EMXX, 'test.cpp']) + err = self.expect_fail([EMCC, 'test.c']) self.assertContained('undefined symbol: my_js', err) - def test_js_lib_to_system_lib(self): - # memset is in compiled code, so a js library __deps can't access it. it - # would need to be in deps_info.json or EXPORTED_FUNCTIONS + def test_js_lib_native_deps(self): + # Verify that memset (which lives in compiled code), can be specified as a JS library + # dependency. create_file('lib.js', r''' mergeInto(LibraryManager.library, { depper__deps: ['memset'], @@ -6794,29 +6790,13 @@ def test_js_lib_to_system_lib(self): } int main(int argc, char** argv) { - char buffer[11]; - buffer[10] = '\0'; - // call by a pointer, to force linking of memset, no llvm intrinsic here - volatile auto ptr = memset; - (*ptr)(buffer, 'a', 10); + char buffer[11] = { 0 }; depper(buffer); puts(buffer); } ''') - err = self.expect_fail([EMXX, 'test.cpp', '--js-library', 'lib.js']) - self.assertContained('_memset may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library', err) - - # without the dep, and with EXPORTED_FUNCTIONS, it works ok - create_file('lib.js', r''' -mergeInto(LibraryManager.library, { - depper: function(ptr) { - _memset(ptr, 'd'.charCodeAt(0), 10); - }, -}); -''') - self.run_process([EMXX, 'test.cpp', '--js-library', 'lib.js', '-sEXPORTED_FUNCTIONS=_main,_memset']) - self.assertContained('dddddddddd', self.run_js('a.out.js')) + self.do_runf('test.cpp', 'dddddddddd\n', emcc_args=['--js-library', 'lib.js']) def test_realpath(self): create_file('src.c', r''' diff --git a/tools/building.py b/tools/building.py index f17ad3064e47b..616c100539812 100644 --- a/tools/building.py +++ b/tools/building.py @@ -148,7 +148,7 @@ def link_to_object(args, target): def lld_flags_for_executable(external_symbols): cmd = [] - if external_symbols: + if settings.ERROR_ON_UNDEFINED_SYMBOLS: undefs = shared.get_temp_files().get('.undefined').name utils.write_file(undefs, '\n'.join(external_symbols)) cmd.append('--allow-undefined-file=%s' % undefs) @@ -176,9 +176,8 @@ def lld_flags_for_executable(external_symbols): # Strip the leading underscores c_exports = [demangle_c_symbol_name(e) for e in c_exports] c_exports += settings.EXPORT_IF_DEFINED - if external_symbols: - # Filter out symbols external/JS symbols - c_exports = [e for e in c_exports if e not in external_symbols] + # Filter out symbols external/JS symbols + c_exports = [e for e in c_exports if e not in external_symbols] for export in c_exports: cmd.append('--export-if-defined=' + export) diff --git a/tools/deps_info.py b/tools/deps_info.py index 85755a5b62fb5..f6f4a74eed54a 100644 --- a/tools/deps_info.py +++ b/tools/deps_info.py @@ -53,15 +53,10 @@ _deps_info = { 'alarm': ['_emscripten_timeout'], 'setitimer': ['_emscripten_timeout'], - 'Mix_LoadWAV_RW': ['fileno'], - 'SDL_CreateRGBSurface': ['malloc', 'free'], 'SDL_GL_GetProcAddress': ['malloc'], - 'SDL_Init': ['malloc', 'free', 'memcpy'], 'SDL_LockSurface': ['malloc', 'free'], 'SDL_OpenAudio': ['malloc', 'free'], 'SDL_PushEvent': ['malloc', 'free'], - 'SDL_free': ['free'], - 'SDL_malloc': ['malloc', 'free'], '_embind_register_class': ['free'], '_embind_register_enum_value': ['free'], '_embind_register_function': ['free'], @@ -86,8 +81,6 @@ 'emscripten_async_wget_data': ['malloc', 'free'], 'emscripten_create_worker': ['malloc', 'free'], 'emscripten_get_compiler_setting': ['malloc'], - 'emscripten_get_preloaded_image_data': ['malloc'], - 'emscripten_get_preloaded_image_data_from_FILE': ['fileno'], 'emscripten_get_window_title': ['malloc'], 'emscripten_idb_async_load': ['malloc', 'free'], 'emscripten_idb_load': ['malloc', 'free'], @@ -100,7 +93,6 @@ 'emscripten_longjmp': ['malloc', 'free', 'saveSetjmp', 'setThrew'], 'emscripten_pc_get_file': ['malloc', 'free'], 'emscripten_pc_get_function': ['malloc', 'free'], - 'emscripten_run_preload_plugins_data': ['malloc'], 'emscripten_run_script_string': ['malloc', 'free'], 'emscripten_set_batterychargingchange_callback_on_thread': ['malloc'], 'emscripten_set_batterylevelchange_callback_on_thread': ['malloc'], @@ -176,19 +168,23 @@ # directly include invokes in deps_info.py, so we list it as a setjmp's # dependency. 'setjmp': ['malloc', 'free', 'saveSetjmp', 'setThrew'], - 'setprotoent': ['malloc'], 'syslog': ['malloc', 'ntohs', 'htons'], 'vsyslog': ['malloc', 'ntohs', 'htons'], 'timegm': ['malloc'], 'tzset': ['malloc'], - 'uuid_compare': ['memcmp'], - 'uuid_copy': ['memcpy'], 'wgpuBufferGetMappedRange': ['malloc', 'free'], 'wgpuBufferGetConstMappedRange': ['malloc', 'free'], 'emscripten_glGetString': ['malloc'], } +def append_deps_info(js_symbol_deps): + for key, value in js_symbol_deps.items(): + if value: + _deps_info.setdefault(key, []) + _deps_info[key] += value + + def get_deps_info(): if not settings.WASM_EXCEPTIONS and settings.LINK_AS_CXX: _deps_info['__cxa_begin_catch'] = ['__cxa_is_pointer_type', '__cxa_free_exception']