From b64a46e81b814d8ccb543500daa184451b0c69ce Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Mon, 8 May 2023 18:23:22 -0700 Subject: [PATCH] Move dlopen file operations into native code. NFC This allows the file data to be read by just a single thread (the calling thread) and shared with all the others via shared memory. Fixes: #19245 --- emcc.py | 6 --- src/generated_struct_info32.json | 6 ++- src/generated_struct_info64.json | 6 ++- src/library_dylink.js | 37 ++++++---------- src/struct_info_internal.json | 2 + system/lib/libc/dynlink.c | 43 +++++++++++++++---- system/lib/libc/musl/src/internal/dynlink.h | 6 +++ .../metadce/test_metadce_hello_dylink.jssize | 2 +- test/test_other.py | 41 ++++++++++++------ 9 files changed, 94 insertions(+), 55 deletions(-) diff --git a/emcc.py b/emcc.py index d0d09f294acc3..ef558e531f25e 100755 --- a/emcc.py +++ b/emcc.py @@ -2307,12 +2307,6 @@ def phase_linker_setup(options, state, newargs): settings.SYSCALLS_REQUIRE_FILESYSTEM = 0 settings.JS_LIBRARIES.append((0, 'library_wasmfs.js')) settings.REQUIRED_EXPORTS += ['_wasmfs_read_file'] - if settings.MAIN_MODULE: - # Dynamic library support uses JS API internals, so include it all - # TODO: rewriting more of the dynamic linking support code into wasm could - # avoid this. also, after we remove the old FS, we could write a - # more specific API for wasmfs/dynamic linking integration perhaps - settings.FORCE_FILESYSTEM = 1 if settings.FORCE_FILESYSTEM: # Add exports for the JS API. Like the old JS FS, WasmFS by default # includes just what JS parts it actually needs, and FORCE_FILESYSTEM is diff --git a/src/generated_struct_info32.json b/src/generated_struct_info32.json index 1a160f0d19462..acb5641fd5618 100644 --- a/src/generated_struct_info32.json +++ b/src/generated_struct_info32.json @@ -1321,12 +1321,14 @@ "d_type": 18 }, "dso": { - "__size__": 28, + "__size__": 36, + "file_data": 28, + "file_data_size": 32, "flags": 4, "mem_addr": 12, "mem_allocated": 8, "mem_size": 16, - "name": 28, + "name": 36, "table_addr": 20, "table_size": 24 }, diff --git a/src/generated_struct_info64.json b/src/generated_struct_info64.json index 4a8b96dd5f6bc..973b4220d0200 100644 --- a/src/generated_struct_info64.json +++ b/src/generated_struct_info64.json @@ -1321,12 +1321,14 @@ "d_type": 18 }, "dso": { - "__size__": 48, + "__size__": 64, + "file_data": 48, + "file_data_size": 56, "flags": 8, "mem_addr": 16, "mem_allocated": 12, "mem_size": 24, - "name": 48, + "name": 64, "table_addr": 32, "table_size": 40 }, diff --git a/src/library_dylink.js b/src/library_dylink.js index b897f5720a325..fa06a62fbbb15 100644 --- a/src/library_dylink.js +++ b/src/library_dylink.js @@ -905,10 +905,6 @@ var LibraryDylink = { // - if flags.loadAsync=true, the loading is performed asynchronously and // loadDynamicLibrary returns corresponding promise. // - // - if flags.fs is provided, it is used as FS-like interface to load library data. - // By default, when flags.fs=undefined, native loading capabilities of the - // environment are used. - // // If a library was already loaded, it is not loaded a second time. However // flags.global and flags.nodelete are handled every time a load request is made. // Once a library becomes "global" or "nodelete", it cannot be removed or unloaded. @@ -960,12 +956,13 @@ var LibraryDylink = { // libName -> libData function loadLibData() { // for wasm, we can use fetch for async, but for fs mode we can only imitate it - if (flags.fs && flags.fs.findObject(libName)) { - var libData = flags.fs.readFile(libName, {encoding: 'binary'}); - if (!(libData instanceof Uint8Array)) { - libData = new Uint8Array(libData); + if (handle) { + var data = {{{ makeGetValue('handle', C_STRUCTS.dso.file_data, '*') }}}; + var dataSize = {{{ makeGetValue('handle', C_STRUCTS.dso.file_data_size, '*') }}}; + if (data && dataSize) { + var libData = HEAP8.slice(data, data + dataSize); + return flags.loadAsync ? Promise.resolve(libData) : libData; } - return flags.loadAsync ? Promise.resolve(libData) : libData; } var libFile = locateFile(libName); @@ -987,7 +984,7 @@ var LibraryDylink = { // lookup preloaded cache first if (preloadedWasm[libName]) { #if DYLINK_DEBUG - err('using preloaded module for: ' + libName); + dbg('using preloaded module for: ' + libName); #endif var libModule = preloadedWasm[libName]; return flags.loadAsync ? Promise.resolve(libModule) : libModule; @@ -1059,7 +1056,7 @@ var LibraryDylink = { }, // void* dlopen(const char* filename, int flags); - $dlopenInternal__deps: ['$FS', '$ENV', '$dlSetError', '$PATH'], + $dlopenInternal__deps: ['$ENV', '$dlSetError', '$PATH'], $dlopenInternal: function(handle, jsflags) { // void *dlopen(const char *file, int mode); // http://pubs.opengroup.org/onlinepubs/009695399/functions/dlopen.html @@ -1079,7 +1076,6 @@ var LibraryDylink = { global, nodelete: Boolean(flags & {{{ cDefs.RTLD_NODELETE }}}), loadAsync: jsflags.loadAsync, - fs: jsflags.fs, } if (jsflags.loadAsync) { @@ -1104,18 +1100,12 @@ var LibraryDylink = { _dlopen_js: function(handle) { #if ASYNCIFY return Asyncify.handleSleep((wakeUp) => { - var jsflags = { - loadAsync: true, - fs: FS, // load libraries from provided filesystem - } + var jsflags = { loadAsync: true } var promise = dlopenInternal(handle, jsflags); promise.then(wakeUp).catch(() => wakeUp(0)); }); #else - var jsflags = { - loadAsync: false, - fs: FS, // load libraries from provided filesystem - } + var jsflags = { loadAsync: false } return dlopenInternal(handle, jsflags); #endif }, @@ -1125,8 +1115,8 @@ var LibraryDylink = { _emscripten_dlopen_js: function(handle, onsuccess, onerror, user_data) { /** @param {Object=} e */ function errorCallback(e) { - var filename = UTF8ToString({{{ makeGetValue('handle', C_STRUCTS.dso.name, '*') }}}); - dlSetError('Could not load dynamic lib: ' + filename + '\n' + e); + var filename = UTF8ToString(handle + {{{ C_STRUCTS.dso.name }}}); + dlSetError('Could not load dynamic libX: ' + filename + '\n' + e); {{{ runtimeKeepalivePop() }}} callUserCallback(() => {{{ makeDynCall('vpp', 'onerror') }}}(handle, user_data)); } @@ -1136,7 +1126,8 @@ var LibraryDylink = { } {{{ runtimeKeepalivePush() }}} - var promise = dlopenInternal(handle, { loadAsync: true }); + var jsflags = { loadAsync: true } + var promise = dlopenInternal(handle, jsflags); if (promise) { promise.then(successCallback, errorCallback); } else { diff --git a/src/struct_info_internal.json b/src/struct_info_internal.json index e1d9999ea8958..fa59ad4aaa8c1 100644 --- a/src/struct_info_internal.json +++ b/src/struct_info_internal.json @@ -40,6 +40,8 @@ "mem_size", "table_addr", "table_size", + "file_data", + "file_data_size", "name" ] } diff --git a/system/lib/libc/dynlink.c b/system/lib/libc/dynlink.c index fea12b76fcd09..bfb8416a1204d 100644 --- a/system/lib/libc/dynlink.c +++ b/system/lib/libc/dynlink.c @@ -11,6 +11,7 @@ #define _GNU_SOURCE #include #include +#include #include #include #include @@ -19,6 +20,7 @@ #include #include #include +#include #include #include @@ -81,6 +83,8 @@ static thread_local struct dlevent* thread_local_tail = &main_event; static pthread_mutex_t write_lock = PTHREAD_MUTEX_INITIALIZER; static thread_local bool skip_dlsync = false; +static void dlsync(); + static void do_write_lock() { // Once we have the lock we want to avoid automatic code sync as that would // result in a deadlock. @@ -155,6 +159,14 @@ static void load_library_done(struct dso* p) { p->table_addr, p->table_size); new_dlevent(p, -1); +#ifdef _REENTRANT + // Block until all other threads have loaded this module. + dlsync(); +#endif + if (p->file_data) { + free(p->file_data); + p->file_data_size = 0; + } } static struct dso* load_library_start(const char* name, int flags) { @@ -169,6 +181,29 @@ static struct dso* load_library_start(const char* name, int flags) { p->flags = flags; strcpy(p->name, name); + // If the file exists in the filesystem, load it here into linear memory which + // makes the data available to JS, and to other threads. This data gets + // free'd later once all threads have loaded the DSO. + struct stat statbuf; + if (stat(name, &statbuf) == 0 && S_ISREG(statbuf.st_mode)) { + int fd = open(name, O_RDONLY); + if (fd >= 0) { + off_t size = lseek(fd, 0, SEEK_END); + if (size != (off_t)-1) { + lseek(fd, 0, SEEK_SET); + p->file_data = malloc(size); + if (p->file_data) { + if (read(fd, p->file_data, size) == size) { + p->file_data_size = size; + } else { + free(p->file_data); + } + } + } + close(fd); + } + } + return p; } @@ -424,10 +459,6 @@ static void dlopen_onsuccess(struct dso* dso, void* user_data) { dso->mem_addr, dso->mem_size); load_library_done(dso); -#ifdef _REENTRANT - // Block until all other threads have loaded this module. - dlsync(); -#endif do_write_unlock(); data->onsuccess(data->user_data, dso); free(data); @@ -526,10 +557,6 @@ static struct dso* _dlopen(const char* file, int flags) { } dbg("dlopen_js: success: %p", p); load_library_done(p); -#ifdef _REENTRANT - // Block until all other threads have loaded this module. - dlsync(); -#endif end: dbg("dlopen(%s): done: %p", file, p); do_write_unlock(); diff --git a/system/lib/libc/musl/src/internal/dynlink.h b/system/lib/libc/musl/src/internal/dynlink.h index de668f4a3845d..a90658bbb45d7 100644 --- a/system/lib/libc/musl/src/internal/dynlink.h +++ b/system/lib/libc/musl/src/internal/dynlink.h @@ -29,6 +29,12 @@ struct dso { void* table_addr; size_t table_size; + // For DSO load events, where the DSO comes from a file on disc, this + // is a pointer the file data read in by the laoding thread and shared with + // others. + uint8_t* file_data; + size_t file_data_size; + // Flexible array; must be final element of struct char name[]; }; diff --git a/test/other/metadce/test_metadce_hello_dylink.jssize b/test/other/metadce/test_metadce_hello_dylink.jssize index fb3b566636284..a86975277b10a 100644 --- a/test/other/metadce/test_metadce_hello_dylink.jssize +++ b/test/other/metadce/test_metadce_hello_dylink.jssize @@ -1 +1 @@ -28151 +28007 diff --git a/test/test_other.py b/test/test_other.py index e6648fc7ca136..3854ddd6df5dd 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -6371,7 +6371,13 @@ def test_RUNTIME_LINKED_LIBS(self): self.assertBinaryEqual('main.wasm', 'main2.wasm') - def test_ld_library_path(self): + @parameterized({ + '': ([],), + 'pthread': (['-g', '-pthread', '-Wno-experimental', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'],), + }) + def test_ld_library_path(self, args): + if args: + self.setup_node_pthreads() create_file('hello1.c', r''' #include @@ -6456,17 +6462,17 @@ def test_ld_library_path(self): return 0; } ''') - self.run_process([EMCC, '-o', 'hello1.wasm', 'hello1.c', '-sSIDE_MODULE']) - self.run_process([EMCC, '-o', 'hello2.wasm', 'hello2.c', '-sSIDE_MODULE']) - self.run_process([EMCC, '-o', 'hello3.wasm', 'hello3.c', '-sSIDE_MODULE']) - self.run_process([EMCC, '-o', 'hello4.wasm', 'hello4.c', '-sSIDE_MODULE']) + self.run_process([EMCC, '-o', 'hello1.wasm', 'hello1.c', '-sSIDE_MODULE'] + args) + self.run_process([EMCC, '-o', 'hello2.wasm', 'hello2.c', '-sSIDE_MODULE'] + args) + self.run_process([EMCC, '-o', 'hello3.wasm', 'hello3.c', '-sSIDE_MODULE'] + args) + self.run_process([EMCC, '-o', 'hello4.wasm', 'hello4.c', '-sSIDE_MODULE'] + args) self.run_process([EMCC, '--profiling-funcs', '-o', 'main.js', 'main.c', '-sMAIN_MODULE=2', '-sINITIAL_MEMORY=32Mb', '--embed-file', 'hello1.wasm@/lib/libhello1.wasm', '--embed-file', 'hello2.wasm@/usr/lib/libhello2.wasm', '--embed-file', 'hello3.wasm@/libhello3.wasm', '--embed-file', 'hello4.wasm@/usr/local/lib/libhello4.wasm', 'hello1.wasm', 'hello2.wasm', 'hello3.wasm', 'hello4.wasm', '-sNO_AUTOLOAD_DYLIBS', - '--pre-js', 'pre.js']) + '--pre-js', 'pre.js'] + args) out = self.run_js('main.js') self.assertContained('Hello1', out) self.assertContained('Hello2', out) @@ -13399,7 +13405,13 @@ def test_windows_batch_file_dp0_expansion_bug(self): create_file('build_with_quotes.bat', f'@"emcc" {test_file("hello_world.c")}') self.run_process(['build_with_quotes.bat']) - def test_preload_module(self): + @parameterized({ + '': ([],), + 'pthread': (['-g', '-pthread', '-Wno-experimental', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'],), + }) + def test_preload_module(self, args): + if args: + self.setup_node_pthreads() # TODO(sbc): This test is copyied from test_browser.py. Perhaps find a better way to # share code between them. create_file('library.c', r''' @@ -13408,17 +13420,20 @@ def test_preload_module(self): return 42; } ''') - self.run_process([EMCC, 'library.c', '-sSIDE_MODULE', '-o', 'library.so']) + self.run_process([EMCC, 'library.c', '-sSIDE_MODULE', '-o', 'library.so'] + args) create_file('main.c', r''' #include #include #include #include + #include int main() { - int found = EM_ASM_INT( - return preloadedWasm['/library.so'] !== undefined; - ); - assert(found); + if (emscripten_is_main_runtime_thread()) { + int found = EM_ASM_INT( + return preloadedWasm['/library.so'] !== undefined; + ); + assert(found); + } void *lib_handle = dlopen("/library.so", RTLD_NOW); assert(lib_handle); typedef int (*voidfunc)(); @@ -13429,4 +13444,4 @@ def test_preload_module(self): return 0; } ''') - self.do_runf('main.c', 'done\n', emcc_args=['-sMAIN_MODULE=2', '--preload-file', '.@/', '--use-preload-plugins']) + self.do_runf('main.c', 'done\n', emcc_args=['-sMAIN_MODULE=2', '--preload-file', '.@/', '--use-preload-plugins'] + args)