-
Notifications
You must be signed in to change notification settings - Fork 2
add metering for bulk memory opcodes #79
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
base: rc/after-supernova
Are you sure you want to change the base?
Changes from 1 commit
d433c59
ad1c798
b82a9a6
642fa74
265657f
00162a7
876a457
d434536
6aa525e
d655288
e0fbc4e
3b1185f
ed0ff7d
61697d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,8 +1,7 @@ | ||||||
| use crate::executor_interface::OpcodeCost; | ||||||
| use crate::wasmer_breakpoints::{Breakpoints, BREAKPOINT_VALUE_OUT_OF_GAS}; | ||||||
| use crate::wasmer_helpers::{ | ||||||
| create_global_index, get_global_value_u64, is_control_flow_operator, set_global_value_u64, | ||||||
| MiddlewareWithProtectedGlobals, | ||||||
| create_global_index, get_global_value_u64, is_control_flow_operator, is_supported_bulk_memory_operator, set_global_value_u64, MiddlewareWithProtectedGlobals | ||||||
| }; | ||||||
| use crate::{get_local_cost, get_opcode_cost}; | ||||||
| use loupe::{MemoryUsage, MemoryUsageTracker}; | ||||||
|
|
@@ -17,12 +16,14 @@ use wasmer_types::{GlobalIndex, ModuleInfo}; | |||||
|
|
||||||
| const METERING_POINTS_LIMIT: &str = "metering_points_limit"; | ||||||
| const METERING_POINTS_USED: &str = "metering_points_used"; | ||||||
| const METERING_BULK_MEMORY_SIZE_OPERAND_BACKUP: &str = "metering_bulk_memory_size_operand_backup"; | ||||||
| const MAX_LOCAL_COUNT: u32 = 4000; | ||||||
|
|
||||||
| #[derive(Clone, Debug, MemoryUsage)] | ||||||
| struct MeteringGlobalIndexes { | ||||||
| points_limit_global_index: GlobalIndex, | ||||||
| points_used_global_index: GlobalIndex, | ||||||
| bulk_memory_size_operand_backup_global_index: GlobalIndex, | ||||||
| } | ||||||
|
|
||||||
| #[derive(Debug)] | ||||||
|
|
@@ -67,6 +68,15 @@ impl Metering { | |||||
| .unwrap() | ||||||
| .points_used_global_index | ||||||
| } | ||||||
|
|
||||||
| fn get_bulk_memory_size_operand_backup_global_index(&self) -> GlobalIndex { | ||||||
| self.global_indexes | ||||||
| .lock() | ||||||
| .unwrap() | ||||||
| .as_ref() | ||||||
| .unwrap() | ||||||
| .bulk_memory_size_operand_backup_global_index | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| unsafe impl Send for Metering {} | ||||||
|
|
@@ -105,6 +115,11 @@ impl ModuleMiddleware for Metering { | |||||
| points_limit, | ||||||
| ), | ||||||
| points_used_global_index: create_global_index(module_info, METERING_POINTS_USED, 0), | ||||||
| bulk_memory_size_operand_backup_global_index: create_global_index( | ||||||
| module_info, | ||||||
| METERING_BULK_MEMORY_SIZE_OPERAND_BACKUP, | ||||||
| 0, | ||||||
| ), | ||||||
| }); | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -114,6 +129,7 @@ impl MiddlewareWithProtectedGlobals for Metering { | |||||
| vec![ | ||||||
| self.get_points_limit_global_index().as_u32(), | ||||||
| self.get_points_used_global_index().as_u32(), | ||||||
| self.get_bulk_memory_size_operand_backup_global_index().as_u32(), | ||||||
| ] | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -156,6 +172,39 @@ impl FunctionMetering { | |||||
| self.breakpoints_middleware | ||||||
| .inject_breakpoint_condition(state, BREAKPOINT_VALUE_OUT_OF_GAS); | ||||||
| } | ||||||
|
|
||||||
| fn inject_bulk_memory_cost(&self, state: &mut MiddlewareReaderState, cost_per_byte: u32) { | ||||||
| // backup the bulk memory size | ||||||
| state.extend(&[Operator::GlobalSet { | ||||||
| global_index: self.global_indexes.bulk_memory_size_operand_backup_global_index.as_u32(), | ||||||
| }]); | ||||||
|
|
||||||
| // inject bulk memory cost | ||||||
| state.extend(&[ | ||||||
| // memory size * price | ||||||
| Operator::GlobalGet { | ||||||
| global_index: self.global_indexes.bulk_memory_size_operand_backup_global_index.as_u32(), | ||||||
| }, | ||||||
| Operator::I64Const { | ||||||
| value: cost_per_byte as i64, | ||||||
| }, | ||||||
| Operator::I64Mul, | ||||||
|
|
||||||
| // points user += memory size * price | ||||||
|
||||||
| Operator::GlobalGet { | ||||||
| global_index: self.global_indexes.points_used_global_index.as_u32(), | ||||||
| }, | ||||||
| Operator::I64Add, | ||||||
| Operator::GlobalSet { | ||||||
| global_index: self.global_indexes.points_used_global_index.as_u32(), | ||||||
| }, | ||||||
| ]); | ||||||
|
|
||||||
| // bring back the bulk memory size | ||||||
| state.extend(&[Operator::GlobalGet { | ||||||
| global_index: self.global_indexes.bulk_memory_size_operand_backup_global_index.as_u32(), | ||||||
| }]); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| impl FunctionMiddleware for FunctionMetering { | ||||||
|
|
@@ -169,7 +218,14 @@ impl FunctionMiddleware for FunctionMetering { | |||||
| // corner cases. | ||||||
| let option = get_opcode_cost(&operator, &self.opcode_cost.lock().unwrap()); | ||||||
| match option { | ||||||
| Some(cost) => self.accumulated_cost += cost as u64, | ||||||
| Some(cost) if is_supported_bulk_memory_operator(&operator) => { | ||||||
| self.inject_bulk_memory_cost(state, cost); | ||||||
| // immediatly insert out of gas check as this operation might be expensive | ||||||
|
||||||
| // immediatly insert out of gas check as this operation might be expensive | |
| // immediately insert out of gas check as this operation might be expensive |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,6 +121,8 @@ pub fn get_opcode_cost(op: &Operator, opcode_cost: &OpcodeCost) -> Option<u32> { | |
| Operator::Loop { .. } => Some(opcode_cost.opcode_loop), | ||
| Operator::MemoryGrow { .. } => Some(opcode_cost.opcode_memorygrow), | ||
| Operator::MemorySize { .. } => Some(opcode_cost.opcode_memorysize), | ||
| Operator::MemoryCopy { .. } => Some(opcode_cost.opcode_memorycopy), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need proper opcode check versioning. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not think we need. Like we might add some opcodes once per year, there is no need for that. SpaceVM can treat activations, there is no need to complicated the executor. |
||
| Operator::MemoryFill { .. } => Some(opcode_cost.opcode_memoryfill), | ||
| Operator::Nop { .. } => Some(opcode_cost.opcode_nop), | ||
| Operator::RefFunc { .. } => Some(opcode_cost.opcode_reffunc), | ||
| Operator::RefIsNull { .. } => Some(opcode_cost.opcode_refisnull), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bulk memory cost calculation multiplies the memory size (i64) by the cost_per_byte (u32 cast to i64) without overflow checking. For large memory operations, this could potentially overflow an i64. While WASM memory is typically limited, it would be safer to either validate the size before multiplication or use checked arithmetic to prevent potential overflow issues.