Skip to content

Commit

Permalink
addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@agryaznov
agryaznov committed Nov 18, 2022
1 parent 832eeb8 commit db2d4e5
Show file tree
Hide file tree
Showing 13 changed files with 112 additions and 268 deletions.
29 changes: 10 additions & 19 deletions src/gas_metering/backend.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
//! Provides backends for the gas metering instrumentation
use parity_wasm::{builder::FunctionDefinition, elements};
use parity_wasm::elements;

/// Implementation details of the specific method of the gas metering.
#[derive(Clone)]
pub enum GasMeter {
/// Gas metering with an external function.
External {
Expand All @@ -14,8 +15,8 @@ pub enum GasMeter {
Internal {
/// Name of the mutable global to be exported.
global: &'static str,
/// Definition of the local gas counting function to be injected.
function: FunctionDefinition,
/// Body of the local gas counting function to be injected.
func_instructions: elements::Instructions,
/// Cost of the gas function execution.
cost: u64,
},
Expand Down Expand Up @@ -69,10 +70,7 @@ pub mod host_function {
pub mod mutable_global {
use super::{Backend, GasMeter, Rules};
use alloc::vec;
use parity_wasm::{
builder,
elements::{self, Instruction, Module, ValueType},
};
use parity_wasm::elements::{self, Instruction, Module};
/// Injects a mutable global variable and a local function to the module to track
/// current gas left.
///
Expand All @@ -93,10 +91,6 @@ pub mod mutable_global {

impl Backend for Injector {
fn gas_meter<R: Rules>(self, module: &Module, rules: &R) -> GasMeter {
// Build local gas function
let fbuilder = builder::FunctionBuilder::new();
let gas_func_sig =
builder::SignatureBuilder::new().with_param(ValueType::I64).build_sig();
let gas_global_idx = module.globals_space() as u32;

let func_instructions = vec![
Expand Down Expand Up @@ -134,14 +128,11 @@ pub mod mutable_global {

gas_fn_cost -= fail_cost;

let func = fbuilder
.with_signature(gas_func_sig)
.body()
.with_instructions(elements::Instructions::new(func_instructions))
.build()
.build();

GasMeter::Internal { global: self.global_name, function: func, cost: gas_fn_cost }
GasMeter::Internal {
global: self.global_name,
func_instructions: elements::Instructions::new(func_instructions),
cost: gas_fn_cost,
}
}
}
}
71 changes: 42 additions & 29 deletions src/gas_metering/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,9 @@ pub fn inject<R: Rules, B: Backend>(

let mut mbuilder = builder::from_module(module);

// Calcutate the indexes
let (gas_func_idx, total_func) = match gas_meter {
// Calculate the indexes and gas function cost,
// for external gas function the cost is counted on the host side
let (gas_func_idx, total_func, gas_fn_cost) = match gas_meter {
GasMeter::External { module: gas_module, function } => {
// Inject the import of the gas function
let import_sig = mbuilder
Expand All @@ -175,9 +176,9 @@ pub fn inject<R: Rules, B: Backend>(
.build(),
);

(import_count, functions_space + 1)
(import_count, functions_space + 1, 0)
},
GasMeter::Internal { global, .. } => {
GasMeter::Internal { global, ref func_instructions, cost } => {
// Inject the gas counting global
mbuilder.push_global(
builder::global()
Expand All @@ -195,28 +196,51 @@ pub fn inject<R: Rules, B: Backend>(
mbuilder.push_export(global_export);

let func_idx = functions_space;
(func_idx, func_idx + 1)

// Build local gas function
let gas_func_sig =
builder::SignatureBuilder::new().with_param(ValueType::I64).build_sig();

let function = builder::FunctionBuilder::new()
.with_signature(gas_func_sig)
.body()
.with_instructions(func_instructions.clone())
.build()
.build();

// Inject local gas function
mbuilder.push_function(function);

(func_idx, func_idx + 1, cost)
},
};

// Build the module
// We need the built the module for making injections to its blocks
let mut module = mbuilder.build();

let mut need_grow_counter = false;
let mut error = false;

let mut gas_fn_cost = 0u64;
if let GasMeter::Internal { global: _, function: _, cost, .. } = &gas_meter {
gas_fn_cost = *cost;
};

// Iterate over module sections and perform needed transformations
// Iterate over module sections and perform needed transformations.
// Indexes are needed to be fixed up in `GasMeter::External` case, as it adds an imported
// function, which goes to the beginning of the module's functions space.
for section in module.sections_mut() {
match section {
elements::Section::Code(code_section) => {
for func_body in code_section.bodies_mut() {
let injection_targets = match gas_meter {
GasMeter::External { .. } => code_section.bodies_mut().as_mut_slice(),
// Don't inject counters to the local gas function, which is the last one as
// it's just added. Cost for its execution is added statically before each
// invocation (see `inject_counter()`).
GasMeter::Internal { .. } => {
let len = code_section.bodies().len();
&mut code_section.bodies_mut()[..len - 1]
},
};

for func_body in injection_targets.as_mut() {
// Increment calling addresses if needed
if let GasMeter::External { module: _, function: _ } = gas_meter {
if let GasMeter::External { .. } = gas_meter {
for instruction in func_body.code_mut().elements_mut().iter_mut() {
if let Instruction::Call(call_index) = instruction {
if *call_index >= gas_func_idx {
Expand Down Expand Up @@ -251,7 +275,7 @@ pub fn inject<R: Rules, B: Backend>(
elements::Section::Element(elements_section) => {
// Note that we do not need to check the element type referenced because in the
// WebAssembly 1.0 spec, the only allowed element type is funcref.
if let GasMeter::External { module: _, function: _ } = gas_meter {
if let GasMeter::External { .. } = gas_meter {
for segment in elements_section.entries_mut() {
// update all indirect call addresses initial values
for func_index in segment.members_mut() {
Expand All @@ -263,13 +287,13 @@ pub fn inject<R: Rules, B: Backend>(
}
},
elements::Section::Start(start_idx) =>
if let GasMeter::External { module: _, function: _ } = gas_meter {
if let GasMeter::External { .. } = gas_meter {
if *start_idx >= gas_func_idx {
*start_idx += 1
}
},
elements::Section::Name(s) =>
if let GasMeter::External { module: _, function: _ } = gas_meter {
if let GasMeter::External { .. } = gas_meter {
for functions in s.functions_mut() {
*functions.names_mut() =
IndexMap::from_iter(functions.names().iter().map(|(mut idx, name)| {
Expand All @@ -289,11 +313,6 @@ pub fn inject<R: Rules, B: Backend>(
return Err(module)
}

// Add local function if needed
if let GasMeter::Internal { global: _, function, .. } = gas_meter {
module = add_local_func(module, function);
}

if need_grow_counter {
Ok(add_grow_counter(module, rules, gas_func_idx))
} else {
Expand Down Expand Up @@ -544,12 +563,6 @@ fn add_grow_counter<R: Rules>(
b.build()
}

fn add_local_func(module: elements::Module, func: builder::FunctionDefinition) -> elements::Module {
let mut mbuilder = builder::from_module(module);
mbuilder.push_function(func);
mbuilder.build()
}

fn determine_metered_blocks<R: Rules>(
instructions: &elements::Instructions,
rules: &R,
Expand Down Expand Up @@ -732,7 +745,7 @@ mod tests {
let binary = serialize(injected_module).expect("serialization failed");
wasmparser::validate(&binary).unwrap();
}
// TBD make it macros too

#[test]
fn simple_grow_mut_global() {
let module = parse_wat(
Expand Down
100 changes: 60 additions & 40 deletions tests/diff.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,23 @@ fn dump<P: AsRef<Path>>(path: P, buf: &[u8]) -> io::Result<()> {
Ok(())
}

fn run_diff_test<F: FnOnce(&[u8]) -> Vec<u8>>(test_dir: &str, name: &str, test: F) {
fn run_diff_test<F: FnOnce(&[u8]) -> Vec<u8>>(
test_dir: &str,
in_name: &str,
out_name: &str,
test: F,
) {
let mut fixture_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
fixture_path.push("tests");
fixture_path.push("fixtures");
fixture_path.push(test_dir);
fixture_path.push(name);
fixture_path.push(in_name);

let mut expected_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
expected_path.push("tests");
expected_path.push("expectations");
expected_path.push(test_dir);
expected_path.push(name);
expected_path.push(out_name);

let fixture_wasm = wat::parse_file(&fixture_path).expect("Failed to read fixture");
validate(&fixture_wasm).expect("Fixture is invalid");
Expand All @@ -47,7 +52,7 @@ fn run_diff_test<F: FnOnce(&[u8]) -> Vec<u8>>(test_dir: &str, name: &str, test:
if actual_wat != expected_wat {
println!("difference!");
println!("--- {}", expected_path.display());
println!("+++ {} test {}", test_dir, name);
println!("+++ {} test {}", test_dir, out_name);
for diff in diff::lines(expected_wat, &actual_wat) {
match diff {
diff::Result::Left(l) => println!("-{}", l),
Expand All @@ -71,13 +76,18 @@ mod stack_height {
( $name:ident ) => {
#[test]
fn $name() {
run_diff_test("stack-height", concat!(stringify!($name), ".wat"), |input| {
let module =
elements::deserialize_buffer(input).expect("Failed to deserialize");
let instrumented = instrument::inject_stack_limiter(module, 1024)
.expect("Failed to instrument with stack counter");
elements::serialize(instrumented).expect("Failed to serialize")
});
run_diff_test(
"stack-height",
concat!(stringify!($name), ".wat"),
concat!(stringify!($name), ".wat"),
|input| {
let module =
elements::deserialize_buffer(input).expect("Failed to deserialize");
let instrumented = instrument::inject_stack_limiter(module, 1024)
.expect("Failed to instrument with stack counter");
elements::serialize(instrumented).expect("Failed to serialize")
},
);
}
};
}
Expand All @@ -95,43 +105,53 @@ mod gas {
use super::*;

macro_rules! def_gas_test {
( ($name1:ident, $name2:ident) ) => {
( ($input:ident, $name1:ident, $name2:ident) ) => {
#[test]
fn $name1() {
run_diff_test("gas", concat!(stringify!($name1), ".wat"), |input| {
let rules = gas_metering::ConstantCostRules::default();

let module: elements::Module =
elements::deserialize_buffer(input).expect("Failed to deserialize");
let module = module.parse_names().expect("Failed to parse names");
let backend = gas_metering::host_function::Injector::new("env", "gas");

let instrumented = gas_metering::inject(module, backend, &rules)
.expect("Failed to instrument with gas metering");
elements::serialize(instrumented).expect("Failed to serialize")
});
run_diff_test(
"gas",
concat!(stringify!($input), ".wat"),
concat!(stringify!($name1), ".wat"),
|input| {
let rules = gas_metering::ConstantCostRules::default();

let module: elements::Module =
elements::deserialize_buffer(input).expect("Failed to deserialize");
let module = module.parse_names().expect("Failed to parse names");
let backend = gas_metering::host_function::Injector::new("env", "gas");

let instrumented = gas_metering::inject(module, backend, &rules)
.expect("Failed to instrument with gas metering");
elements::serialize(instrumented).expect("Failed to serialize")
},
);
}

#[test]
fn $name2() {
run_diff_test("gas", concat!(stringify!($name2), ".wat"), |input| {
let rules = gas_metering::ConstantCostRules::default();

let module: elements::Module =
elements::deserialize_buffer(input).expect("Failed to deserialize");
let module = module.parse_names().expect("Failed to parse names");
let backend = gas_metering::mutable_global::Injector::new("gas_left");
let instrumented = gas_metering::inject(module, backend, &rules)
.expect("Failed to instrument with gas metering");
elements::serialize(instrumented).expect("Failed to serialize")
});
run_diff_test(
"gas",
concat!(stringify!($input), ".wat"),
concat!(stringify!($name2), ".wat"),
|input| {
let rules = gas_metering::ConstantCostRules::default();

let module: elements::Module =
elements::deserialize_buffer(input).expect("Failed to deserialize");
let module = module.parse_names().expect("Failed to parse names");
let backend = gas_metering::mutable_global::Injector::new("gas_left");
let instrumented = gas_metering::inject(module, backend, &rules)
.expect("Failed to instrument with gas metering");
elements::serialize(instrumented).expect("Failed to serialize")
},
);
}
};
}

def_gas_test!((ifs_host_fn, ifs_mut_global));
def_gas_test!((simple_host_fn, simple_mut_global));
def_gas_test!((start_host_fn, start_mut_global));
def_gas_test!((call_host_fn, call_mut_global));
def_gas_test!((branch_host_fn, branch_mut_global));
def_gas_test!((ifs, ifs_host_fn, ifs_mut_global));
def_gas_test!((simple, simple_host_fn, simple_mut_global));
def_gas_test!((start, start_host_fn, start_mut_global));
def_gas_test!((call, call_host_fn, call_mut_global));
def_gas_test!((branch, branch_host_fn, branch_mut_global));
}
27 changes: 0 additions & 27 deletions tests/fixtures/gas/branch_host_fn.wat
View file

This file was deleted.

0 comments on commit db2d4e5

Please sign in to comment.