Skip to content

Commit

Permalink
fix(wasmtime):Config methods should be idempotent
Browse files Browse the repository at this point in the history
This commit refactored `Config` to use a seperate `CompilerConfig` field instead
of operating on `CompilerBuilder` directly to make all its methods idempotent.

Fixes bytecodealliance#4189
  • Loading branch information
PureWhiteWu committed Jun 9, 2022
1 parent 8238175 commit fb584d1
Show file tree
Hide file tree
Showing 9 changed files with 181 additions and 97 deletions.
10 changes: 10 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ Unreleased.

### Changed

* The `Config::strategy`, `Config::cranelift_flag_enable` and `Config::cranelift_flag_set` APIs now return `&mut Self`
instead of `Result<&mut Self>` since the validation is deferred until `Engine::new`.
[#4252](https://github.com/bytecodealliance/wasmtime/pull/4252)

### Fixed

* A refactor of `Config` was made to fix an issue that the order of calls to `Config`
matters now, which may lead to unexpected behavior.
[#4252](https://github.com/bytecodealliance/wasmtime/pull/4252)

--------------------------------------------------------------------------------

## 0.38.0
Expand Down
2 changes: 1 addition & 1 deletion crates/c-api/include/wasmtime/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ WASMTIME_CONFIG_PROP(void, wasm_memory64, bool)
* If the compilation strategy selected could not be enabled then an error is
* returned.
*/
WASMTIME_CONFIG_PROP(wasmtime_error_t*, strategy, wasmtime_strategy_t)
WASMTIME_CONFIG_PROP(void, strategy, wasmtime_strategy_t)

/**
* \brief Configures whether Cranelift's debug verifier is enabled.
Expand Down
5 changes: 2 additions & 3 deletions crates/c-api/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,12 @@ pub extern "C" fn wasmtime_config_wasm_memory64_set(c: &mut wasm_config_t, enabl
pub extern "C" fn wasmtime_config_strategy_set(
c: &mut wasm_config_t,
strategy: wasmtime_strategy_t,
) -> Option<Box<wasmtime_error_t>> {
) {
use wasmtime_strategy_t::*;
let result = c.config.strategy(match strategy {
c.config.strategy(match strategy {
WASMTIME_STRATEGY_AUTO => Strategy::Auto,
WASMTIME_STRATEGY_CRANELIFT => Strategy::Cranelift,
});
handle_result(result, |_cfg| {})
}

#[no_mangle]
Expand Down
4 changes: 2 additions & 2 deletions crates/cli-flags/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,13 @@ impl CommonOptions {

for name in &self.cranelift_enable {
unsafe {
config.cranelift_flag_enable(name)?;
config.cranelift_flag_enable(name);
}
}

for (name, value) in &self.cranelift_set {
unsafe {
config.cranelift_flag_set(name, value)?;
config.cranelift_flag_set(name, value);
}
}

Expand Down
8 changes: 3 additions & 5 deletions crates/fuzzing/src/generators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,7 @@ impl Config {

if self.wasmtime.force_jump_veneers {
unsafe {
cfg.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true")
.unwrap();
cfg.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true");
}
}

Expand All @@ -431,8 +430,7 @@ impl Config {
cfg.cranelift_flag_set(
"wasmtime_linkopt_padding_between_functions",
&pad.to_string(),
)
.unwrap();
);
}
}

Expand Down Expand Up @@ -658,7 +656,7 @@ impl CodegenSettings {
config.target(target).unwrap();
for (key, value) in flags {
unsafe {
config.cranelift_flag_set(key, value).unwrap();
config.cranelift_flag_set(key, value);
}
}
}
Expand Down
Loading

0 comments on commit fb584d1

Please sign in to comment.