Skip to content

Comments

Enable multiple backends supported by the api crate to be used in the c-api as well#5443

Merged
xdoardo merged 42 commits intomainfrom
run-500-enable-c_api-backends-to-be-used-in-wasmers-implementation
Mar 12, 2025
Merged

Enable multiple backends supported by the api crate to be used in the c-api as well#5443
xdoardo merged 42 commits intomainfrom
run-500-enable-c_api-backends-to-be-used-in-wasmers-implementation

Conversation

@xdoardo
Copy link
Contributor

@xdoardo xdoardo commented Feb 28, 2025

(fixes #5224)

This PR adds to the wasm_engine_t enum the kinds V8, WAMR, WASMI and JSC to reflect the different possible kinds of engine that can actually be used in the underlying implementation. These enum variants can be by users used as a means to set the wanted engine in the underlying implementation.
Together with this addition, this PR changes the wasm_config_t type to (internally) hold an engine-specific configuration:

pub struct wasm_config_t {
    pub(super) engine: wasmer_engine_t,
    pub(super) engine_config: wasmer_engine_config_t, <------
    ...
}

where wasmer_engine_config_t is an enumeration of all the configurations for all the available engines:

enum wasmer_engine_config_t {
    #[cfg(feature = "sys")]
    Sys(sys::wasmer_sys_engine_config_t),

    #[cfg(feature = "jsc")]
    Jsc(jsc::wasmer_jsc_engine_config_t),
    
    ...
}

The two separate fields are needed because one, wasmer_engine_t, is public and users can set it; the other is definitely private and users cannot interact directly with it. This PR also introduces the relevant features to enable each of the backends cited above, with the matching X-default features to chose one as default.

Note: this PR produces a number of breaking changes in the API. Most notably, functions that were implicitly tied to the sys engine only, are now scoped with the sys name: wasm_config_canonicalize_nans is now wasm_config_sys_canonicalize_nans. Same goes for wasm_config_set_sys_target (previously wasm_config_set_target), wasm_config_sys_push_middleware (previously wasm_config_push_middleware), and wasm_config_set_sys_compiler (previously wasm_config_set_compiler).

Furthermore, a chage in the semantics of these functions is (debatably) introduced: what does it mean to call wasm_config_sys_push_middleware on a config whose selected wasmer_engine_t is not sys? In this PR the introduced behaviour is that of regardlessly change the selected engine to sys:

pub extern "C" fn wasm_config_set_sys_compiler(
    config: &mut wasm_config_t,
    compiler: wasmer_compiler_t,
) {
    ... 
    config.engine = wasmer_engine_t::UNIVERSAL;
    ... 
}

On one hand, this behaviour is transparent to the user - which should nonetheless expect the underlying engine to be the sys one, while on the other this function does not return a value, so we can't signal the user that the an error occurred in the case the current config.engine != sys.

Notice, also, that to maintain a bit of backwards compatibility, the enum constructor in the wasmer_engine_t corresponding to sys is still UNIVERSAL.

-- edit

In the process of testing these additions, I discovered the following bugs which are now fixed:

  • An error in the code that performs deserialization of V8 modules
  • Multiple errors in V8, WAMR, WASMI upon memory instantiation
  • Multiple errors in V8, WAMR, WASMI in Instance::exports()

--

Missing bits:

  • More tests with all the backends enabled
  • CI jobs to target those tests

@xdoardo xdoardo requested a review from syrusakbary as a code owner February 28, 2025 13:54
@promptless
Copy link

promptless bot commented Feb 28, 2025

📝 Documentation updates detected! A separate PR for documentation updates has been made here: wasmerio/docs.wasmer.io#118

@xdoardo xdoardo force-pushed the run-500-enable-c_api-backends-to-be-used-in-wasmers-implementation branch from 57cdaef to 6fe4229 Compare March 10, 2025 15:26
@xdoardo xdoardo enabled auto-merge March 12, 2025 10:30
@xdoardo xdoardo merged commit 2718334 into main Mar 12, 2025
82 of 84 checks passed
@xdoardo xdoardo deleted the run-500-enable-c_api-backends-to-be-used-in-wasmers-implementation branch March 12, 2025 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable c_api-backends to be used in Wasmer's implementation of the wasm_c_api

2 participants