Skip to content
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

wasmtime: Consume fuel on all return paths #8837

Open
wants to merge 1 commit 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
7 changes: 2 additions & 5 deletions cranelift/wasm/src/code_translator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,13 +577,10 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
state.reachable = false;
}
Operator::Return => {
let return_count = {
let frame = &mut state.control_stack[0];
frame.num_return_values()
};
let return_count = state.control_stack[0].num_return_values();
{
let return_args = state.peekn_mut(return_count);
environ.handle_before_return(&return_args, builder);
environ.handle_before_return(return_args, builder);
bitcast_wasm_returns(environ, return_args, builder);
builder.ins().return_(return_args);
}
Expand Down
10 changes: 0 additions & 10 deletions cranelift/wasm/src/environ/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,16 +584,6 @@ pub trait FuncEnvironment: TargetEnvironment {
Ok(())
}

/// Optional callback for the `FunctionEnvironment` performing this translation to perform work
/// after the function body is translated.
fn after_translate_function(
&mut self,
_builder: &mut FunctionBuilder,
_state: &FuncTranslationState,
) -> WasmResult<()> {
Ok(())
}

/// Whether or not to force relaxed simd instructions to have deterministic
/// lowerings meaning they will produce the same results across all hosts,
/// regardless of the cost to performance.
Expand Down
1 change: 0 additions & 1 deletion cranelift/wasm/src/func_translator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ fn parse_function_body<FE: FuncEnvironment + ?Sized>(
translate_operator(validator, &op, builder, state, environ)?;
environ.after_translate_operator(&op, builder, state)?;
}
environ.after_translate_function(builder, state)?;
let pos = reader.original_position();
validator.finish(pos)?;

Expand Down
34 changes: 15 additions & 19 deletions crates/cranelift/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2810,15 +2810,23 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
Ok(())
}

fn after_translate_function(
&mut self,
builder: &mut FunctionBuilder,
state: &FuncTranslationState,
) -> WasmResult<()> {
if self.tunables.consume_fuel && state.reachable() {
fn handle_before_return(&mut self, retvals: &[ir::Value], builder: &mut FunctionBuilder) {
// Ignore unused-argument warnings
let _ = retvals;

if self.tunables.consume_fuel {
self.fuel_function_exit(builder);
}
Ok(())

#[cfg(feature = "wmemcheck")]
if self.wmemcheck {
let func_name = self.current_func_name(builder);
if func_name == Some("malloc") {
self.hook_malloc_exit(builder, retvals);
} else if func_name == Some("free") {
self.hook_free_exit(builder);
}
}
}

fn relaxed_simd_deterministic(&self) -> bool {
Expand Down Expand Up @@ -2849,18 +2857,6 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
self.isa.has_x86_pmaddubsw_lowering()
}

#[cfg(feature = "wmemcheck")]
fn handle_before_return(&mut self, retvals: &[ir::Value], builder: &mut FunctionBuilder) {
if self.wmemcheck {
let func_name = self.current_func_name(builder);
if func_name == Some("malloc") {
self.hook_malloc_exit(builder, retvals);
} else if func_name == Some("free") {
self.hook_free_exit(builder);
}
}
}

#[cfg(feature = "wmemcheck")]
fn before_load(
&mut self,
Expand Down