From d00ea87947797b1b6f990b62152def5def326109 Mon Sep 17 00:00:00 2001 From: Arshia Ghafoori Date: Tue, 26 Aug 2025 10:24:21 +0400 Subject: [PATCH] Fix caching dlsym results in the linker --- lib/wasix/src/state/linker.rs | 37 ++++++++++++------ tests/wasix/dl-cache/.no-build | 0 tests/wasix/dl-cache/main.c | 68 ++++++++++++++++++++++++++++++++++ tests/wasix/dl-cache/run.sh | 12 ++++++ tests/wasix/dl-cache/side1.c | 6 +++ tests/wasix/dl-cache/side2.c | 6 +++ 6 files changed, 117 insertions(+), 12 deletions(-) create mode 100644 tests/wasix/dl-cache/.no-build create mode 100644 tests/wasix/dl-cache/main.c create mode 100755 tests/wasix/dl-cache/run.sh create mode 100644 tests/wasix/dl-cache/side1.c create mode 100644 tests/wasix/dl-cache/side2.c diff --git a/lib/wasix/src/state/linker.rs b/lib/wasix/src/state/linker.rs index d8c9f99cb6..1b0e0d4ddd 100644 --- a/lib/wasix/src/state/linker.rs +++ b/lib/wasix/src/state/linker.rs @@ -717,20 +717,24 @@ struct InProgressLinkState { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum SymbolResolutionKey { Needed(NeededSymbolResolutionKey), - Requested(String), + Requested { + // Note: since we don't support module unloading, resolving the same symbol + // from *every* module will find the same symbol every time, so we can cache + // the None case as well. + // TODO: once we implement RTLD_NEXT, that flag should also be taken into + // account here. + resolve_from: Option, + name: String, + }, } #[derive(Debug)] pub enum SymbolResolutionResult { // The symbol was resolved to a global address. We don't resolve again because // the value of globals and the memory_base for each module and all of its instances - // is fixed, and we can't nuke globals in the same way we do with functions. The end - // goal is to have new instance groups behave exactly the same as existing instance - // groups; since existing instance groups will have a (possibly invalid) pointer - // into memory from when this global still existed, we do the same for new instance - // groups. - // The case of unresolved globals is not mentioned here, since it can't exist once - // a link operation is complete. + // is fixed. + // The case of unresolved globals is not mentioned in this enum, since it can't exist + // once a link operation is finalized. Memory(u64), // The symbol was resolved to a global address, but the global is a TLS variable. // Each instance of each module has a different TLS area, and TLS symbols must be @@ -1708,7 +1712,10 @@ impl Linker { ) -> Result { trace!(?module_handle, symbol, "Resolving symbol"); - let resolution_key = SymbolResolutionKey::Requested(symbol.to_string()); + let resolution_key = SymbolResolutionKey::Requested { + resolve_from: module_handle, + name: symbol.to_string(), + }; lock_instance_group_state!(guard, group_state, self, ResolveError::InstanceGroupIsDead); @@ -2852,7 +2859,7 @@ impl InstanceGroupState { linker_state: &LinkerState, ) -> Result<(), LinkError> { for (key, val) in &linker_state.symbol_resolution_records { - if let SymbolResolutionKey::Requested(name) = key { + if let SymbolResolutionKey::Requested { name, .. } = key { if let SymbolResolutionResult::FunctionPointer { resolved_from, function_table_index, @@ -3502,13 +3509,19 @@ impl InstanceGroupState { } } - for (handle, instance) in &self.side_instances { + // Iterate over linker.side_modules to ensure we're going over the + // modules in increasing order of module_handle, A.K.A. the order + // in which modules were loaded. linker.side_modules is a BTreeMap + // whereas self.side_instances is a HashMap with undetermined + // iteration order. + for (handle, module) in &linker_state.side_modules { + let instance = &self.side_instances[handle]; match self.resolve_export_from( store, *handle, symbol, &instance.instance, - &linker_state.side_modules[handle].dylink_info, + &module.dylink_info, linker_state.memory_base(*handle), instance.tls_base, allow_hidden, diff --git a/tests/wasix/dl-cache/.no-build b/tests/wasix/dl-cache/.no-build new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/wasix/dl-cache/main.c b/tests/wasix/dl-cache/main.c new file mode 100644 index 0000000000..dd653a12ef --- /dev/null +++ b/tests/wasix/dl-cache/main.c @@ -0,0 +1,68 @@ +#include +#include + +int main() +{ + void *handle1 = dlopen("./libside1.so", RTLD_NOW | RTLD_GLOBAL); + if (!handle1) + { + fprintf(stderr, "dlopen failed: %s\n", dlerror()); + return 1; + } + + int (*side_func1)(int) = dlsym(handle1, "side_func"); + if (!side_func1) + { + fprintf(stderr, "dlsym failed: %s\n", dlerror()); + dlclose(handle1); + return 1; + } + + // side_func1 returns x + 42 + int res = side_func1(2); + if (res != 44) + { + fprintf(stderr, "side_func returned unexpected value: %d\n", res); + dlclose(handle1); + return 1; + } + + void *handle2 = dlopen("./libside2.so", RTLD_NOW | RTLD_GLOBAL); + if (!handle2) + { + fprintf(stderr, "dlopen failed: %s\n", dlerror()); + return 1; + } + + int (*side_func2)(int) = dlsym(handle2, "side_func"); + if (!side_func2) + { + fprintf(stderr, "dlsym failed: %s\n", dlerror()); + dlclose(handle1); + dlclose(handle2); + return 1; + } + + if (side_func1 == side_func2) + { + fprintf(stderr, "side_func1 and side_func2 should be different\n"); + dlclose(handle1); + dlclose(handle2); + return 1; + } + + // side_func2 returns x * 2 + res = side_func2(2); + if (res != 4) + { + fprintf(stderr, "side_func returned unexpected value: %d\n", res); + dlclose(handle1); + dlclose(handle2); + return 1; + } + + dlclose(handle1); + dlclose(handle2); + + return 0; +} \ No newline at end of file diff --git a/tests/wasix/dl-cache/run.sh b/tests/wasix/dl-cache/run.sh new file mode 100755 index 0000000000..10620b3e37 --- /dev/null +++ b/tests/wasix/dl-cache/run.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +set -e + +export WASIXCC_WASM_EXCEPTIONS=yes +export WASIXCC_PIC=yes + +wasixcc main.c -o main.wasm -Wl,-pie +wasixcc side1.c -o libside1.so -Wl,-shared +wasixcc side2.c -o libside2.so -Wl,-shared + +$WASMER -q run main.wasm --dir=. \ No newline at end of file diff --git a/tests/wasix/dl-cache/side1.c b/tests/wasix/dl-cache/side1.c new file mode 100644 index 0000000000..c3d8896293 --- /dev/null +++ b/tests/wasix/dl-cache/side1.c @@ -0,0 +1,6 @@ +extern int side_needed_func(int); + +int side_func(int x) +{ + return x + 42; +} \ No newline at end of file diff --git a/tests/wasix/dl-cache/side2.c b/tests/wasix/dl-cache/side2.c new file mode 100644 index 0000000000..bcab02e41c --- /dev/null +++ b/tests/wasix/dl-cache/side2.c @@ -0,0 +1,6 @@ +extern int side_needed_func(int); + +int side_func(int x) +{ + return x * 2; +} \ No newline at end of file