Skip to content

Fix late-binding symbols with JSPI (implementation 2) #24161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
14 changes: 14 additions & 0 deletions src/lib/libdylink.js
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,9 @@ var LibraryDylink = {
if (!resolved) {
resolved = moduleExports[sym];
}
if (resolved.orig) {
resolved = resolved.orig;
}
#if ASSERTIONS
assert(resolved, `undefined symbol '${sym}'. perhaps a side module was not linked in? if this global was expected to arrive from a system library, try to build the MAIN_MODULE with EMCC_FORCE_STDLIBS=1 in the environment`);
#endif
Expand Down Expand Up @@ -734,10 +737,21 @@ var LibraryDylink = {
// when first called.
if (!(prop in stubs)) {
var resolved;
#if JSPI
var func = (...args) => {
resolved ||= resolveSymbol(prop);
try {
resolved = WebAssembly.promising(resolved);
} catch(e) {}
return resolved(...args);
}
stubs[prop] = new WebAssembly.Suspending(func);
#else
stubs[prop] = (...args) => {
resolved ||= resolveSymbol(prop);
return resolved(...args);
};
#endif
}
return stubs[prop];
}
Expand Down
46 changes: 39 additions & 7 deletions test/core/test_dlfcn_jspi.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,59 @@
// University of Illinois/NCSA Open Source License. Both these licenses can be
// found in the LICENSE file.

#include <assert.h>
#include <emscripten.h>
#include <stdio.h>
#include <dlfcn.h>

EM_ASYNC_JS(int, test, (), {
EM_ASYNC_JS(int, test_suspending, (), {
console.log("sleeping");
await new Promise(res => setTimeout(res, 0));
console.log("slept");
return 77;
});

int test_wrapper() {
return test();
int test_suspending_wrapper() {
return test_suspending();
}

EM_JS(int, test_sync, (), {
console.log("sync");
return 77;
})

int test_sync_wrapper() {
return test_sync();
}


typedef int (*F)();
typedef int (*G)(F f);

void helper(F f) {
void* handle = dlopen("side_a.so", RTLD_NOW|RTLD_GLOBAL);
assert(handle != NULL);
G side_module_trampolinea = dlsym(handle, "side_module_trampoline_a");
assert(side_module_trampolinea != NULL);
int res = side_module_trampolinea(f);
printf("okay %d\n", res);
}


EMSCRIPTEN_KEEPALIVE void not_promising() {
helper(test_sync_wrapper);
}

EM_JS(void, js_trampoline, (), {
_not_promising();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kripken this part of the test would work in #23619 but doesn't work here. In particular, if we call into C from JS via an export that isn't in JSPI_EXPORTS then we fail when calling into the stub trampoline. In my "make the trampoline WebAssembly" version, it works fine.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting, thanks... sounds like the JSPI spec changed and I haven't followed that closely.

@brendandahl is there some workaround for this issue?

If not, it sounds like this approach would need to add all late-binding symbols to JSPI_EXPORTS..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think what would be needed would be to add all C functions that might call late-binding symbols to JSPI_EXPORTS? In this case the problem is fixed by adding -sJSPI_EXPORTS=not_promising.

If we wanted to go this way, we should probably make all exports JSPI_EXPORTS.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One thing we could do is make a selector function with body like:

try (result restype)
   local.get 0
   local.get 1
   call suspendingTrampoline
catch $SuspendError
   local.get 0
   local.get 1
   call nonsuspendingTrampoline
end

and then make both a suspending and nonsuspending version of the stub. But this still forces us to do most of the painful stuff in #23619 where we calculate the type and generate a dynamic wasm module.

})

int main() {
void* handle = dlopen("side.so", RTLD_NOW|RTLD_GLOBAL);
F side_module_trampoline = dlsym(handle, "side_module_trampoline");
int res = side_module_trampoline();
printf("done %d\n", res);
printf("Suspending test\n");
helper(test_suspending_wrapper);
printf("Non suspending test\n");
js_trampoline();
printf("done\n");
return 0;
}

12 changes: 10 additions & 2 deletions test/core/test_dlfcn_jspi.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
side_module_trampoline
Suspending test
side_module_trampoline_a
side_module_trampoline_b
sleeping
slept
done 77
okay 77
Non suspending test
side_module_trampoline_a
side_module_trampoline_b
sync
okay 77
done
10 changes: 10 additions & 0 deletions test/core/test_dlfcn_jspi_side_a.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#include <stdio.h>

typedef int (*F)();

int side_module_trampoline_b(F f);

int side_module_trampoline_a(F f) {
printf("side_module_trampoline_a\n");
return side_module_trampoline_b(f);
}
8 changes: 8 additions & 0 deletions test/core/test_dlfcn_jspi_side_b.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#include <stdio.h>

typedef int (*F)();

int side_module_trampoline_b(F f) {
printf("side_module_trampoline_b\n");
return f();
}
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_hello_dylink.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
11735
11736
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_hello_dylink.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
27774
27776
23 changes: 17 additions & 6 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -3789,13 +3789,24 @@ def test_dlfcn_jspi(self):
self.run_process(
[
EMCC,
"-o",
"side.so",
test_file("core/test_dlfcn_jspi_side.c"),
"-sSIDE_MODULE",
] + self.get_emcc_args()
'-o',
'side_b.so',
test_file('core/test_dlfcn_jspi_side_b.c'),
'-sSIDE_MODULE',
]
+ self.get_emcc_args()
)
self.run_process(
[
EMCC,
'-o',
'side_a.so',
test_file('core/test_dlfcn_jspi_side_a.c'),
'-sSIDE_MODULE',
]
+ self.get_emcc_args()
)
self.do_run_in_out_file_test("core/test_dlfcn_jspi.c", emcc_args=["side.so", "-sMAIN_MODULE=2"])
self.do_run_in_out_file_test('core/test_dlfcn_jspi.c', emcc_args=['side_a.so', 'side_b.so', '-sMAIN_MODULE=2', '-g'])

@needs_dylink
def test_dlfcn_rtld_local(self):
Expand Down
Loading