From b96202fafa65d5938b34c9b7ad6e6fe573dd6a95 Mon Sep 17 00:00:00 2001 From: mqxf Date: Tue, 9 Jan 2024 17:38:56 +0100 Subject: [PATCH 01/13] Started writing checks --- utils/wasm-builder/src/checks.rs | 48 ++++++++++++++++++++++++++ utils/wasm-builder/src/lib.rs | 1 + utils/wasm-builder/src/wasm_project.rs | 6 ++++ 3 files changed, 55 insertions(+) create mode 100644 utils/wasm-builder/src/checks.rs diff --git a/utils/wasm-builder/src/checks.rs b/utils/wasm-builder/src/checks.rs new file mode 100644 index 00000000000..4a78662d017 --- /dev/null +++ b/utils/wasm-builder/src/checks.rs @@ -0,0 +1,48 @@ +use gear_wasm_instrument::parity_wasm::elements::{Internal, Module, Type}; + +#[derive(Debug)] +pub enum Error { + ExportSectionMissing, + ImportSectionMissing, + FunctionSectionMissing, + TypeSectionMissing, + CodeSectionMissing, + UnexpectedFunctionExport(String), + MissingEntryFunction, + InvalidExportFunctionSignature, +} + +pub fn do_checks(module: Module) -> Result<(), Error> { + + let exports = module.export_section().ok_or(Error::ExportSectionMissing)?; + let imports = module.import_section().ok_or(Error::ImportSectionMissing)?; + let functions = module.function_section().ok_or(Error::FunctionSectionMissing)?; + let types = module.type_section().ok_or(Error::TypeSectionMissing)?; + let code = module.code_section().ok_or(Error::CodeSectionMissing)?; + + let imported_functions = imports.functions(); + + // While this is checked in Code::new, we might as well check it when building wasm program + let mut entry = false; + for e in exports.entries() { + if let Internal::Function(i) = e.internal() { + match e.field() { + "init" | "handle" | "handle_reply" | "handle_signal" | "state" | "meta" => { + entry = true; + let Type::Function(ref t) = types.types()[functions.entries()[*i as usize - imported_functions]]; + if !t.params().is_empty() || !t.results().is_empty() { + return Err(Error::InvalidExportFunctionSignature); + } + }, + _ => return Err(Error::UnexpectedFunctionExport(e.field().to_string())) + } + } + } + if !entry { + return Err(Error::MissingEntryFunction) + } + + + + Ok(()) +} \ No newline at end of file diff --git a/utils/wasm-builder/src/lib.rs b/utils/wasm-builder/src/lib.rs index 6562f1cc155..a92cb89af6a 100644 --- a/utils/wasm-builder/src/lib.rs +++ b/utils/wasm-builder/src/lib.rs @@ -36,6 +36,7 @@ pub mod optimize; mod smart_fs; mod stack_end; mod wasm_project; +mod checks; pub const TARGET: &str = env!("TARGET"); diff --git a/utils/wasm-builder/src/wasm_project.rs b/utils/wasm-builder/src/wasm_project.rs index 9fb4a339358..0b0e812e1f2 100644 --- a/utils/wasm-builder/src/wasm_project.rs +++ b/utils/wasm-builder/src/wasm_project.rs @@ -30,6 +30,8 @@ use std::{ path::{Path, PathBuf}, }; use toml::value::Table; +use crate::checks::do_checks; +use crate::optimize::do_optimization; const OPT_LEVEL: &str = "z"; @@ -371,6 +373,10 @@ extern "C" fn metahash() {{ .context("Failed to write optimized WASM binary")?; } + // Generate module from optimized wasm file to run gear checks + let module = parity_wasm::elements::Module::from_bytes(fs::read(opt_wasm_path.clone()).expect("Failed to read optimized wasm binary")).expect("Invalid wasm generated by wasm-opt"); + do_checks(module).expect("Wasm code did not pass gear checks"); + // Create path string in `.binpath` file. let relative_path_to_wasm = pathdiff::diff_paths(&self.wasm_target_dir, &self.original_dir) .with_context(|| { From 331f3f000ca7758d1c8cc997d30517a34ab8a379 Mon Sep 17 00:00:00 2001 From: mqxf Date: Wed, 10 Jan 2024 16:28:28 +0100 Subject: [PATCH 02/13] Move checks from wasm-builder into core/code --- core/src/code.rs | 74 +++++++++++++++++--------- utils/wasm-builder/src/checks.rs | 48 ----------------- utils/wasm-builder/src/lib.rs | 1 - utils/wasm-builder/src/wasm_project.rs | 12 +---- 4 files changed, 50 insertions(+), 85 deletions(-) delete mode 100644 utils/wasm-builder/src/checks.rs diff --git a/core/src/code.rs b/core/src/code.rs index e9eb03b0468..ec43bd32420 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -38,20 +38,39 @@ use scale_info::{ scale::{Decode, Encode}, TypeInfo, }; +use gear_wasm_instrument::parity_wasm::elements::ImportCountType; +use crate::costs::RuntimeCosts::PayProgramRent; /// Defines maximal permitted count of memory pages. pub const MAX_WASM_PAGE_COUNT: u16 = 512; -/// Name of exports allowed on chain except execution kinds. -pub const STATE_EXPORTS: [&str; 2] = ["state", "metahash"]; +/// Name of exports allowed on chain. +pub const ALLOWED_EXPORTS: [&str; 6] = ["init", "handle", "handle_reply", "handle_signal", "state", "metahash"]; + +/// Name of exports required on chain (only 1 of these is required). +pub const REQUIRED_EXPORTS: [&str; 2] = ["init", "handle"]; -/// Parse function exports from wasm module into [`DispatchKind`]. fn get_exports( - module: &Module, - reject_unnecessary: bool, -) -> Result, CodeError> { - let mut exports = BTreeSet::::new(); + module: &Module +) -> BTreeSet { + let mut entries = BTreeSet::new(); + + for entry in module.export_section().expect("Exports section has been checked for already").entries().iter() { + if let Internal::Function(_) = entry.internal() { + if let Some(entry) = DispatchKind::try_from_entry(entry.field()) { + entries.insert(entry); + } + } + } + entries +} + +/// Parse function exports from wasm module into [`DispatchKind`]. +fn check_code( + module: &Module, + config: &TryNewCodeConfig, +) -> Result<(), CodeError> { let funcs = module .function_section() .ok_or(CodeError::FunctionSectionNotFound)? @@ -62,19 +81,22 @@ fn get_exports( .ok_or(CodeError::TypeSectionNotFound)? .types(); - let import_count = module + let import_count = module.import_count(ImportCountType::Function); + + let imports = module .import_section() .ok_or(CodeError::ImportSectionNotFound)? - .functions(); + .entries(); - for entry in module + let exports = module .export_section() .ok_or(CodeError::ExportSectionNotFound)? - .entries() - .iter() - { - if let Internal::Function(i) = entry.internal() { - if reject_unnecessary { + .entries(); + + let mut entry = false; + for export in exports { + if let Internal::Function(i) = export.internal() { + if config.check_exports { // Index access into arrays cannot panic unless the Module structure is invalid let type_id = funcs[*i as usize - import_count].type_ref(); let Type::Function(ref f) = types[type_id as usize]; @@ -82,15 +104,20 @@ fn get_exports( return Err(CodeError::InvalidExportFnSignature); } } - if let Some(kind) = DispatchKind::try_from_entry(entry.field()) { - exports.insert(kind); - } else if !STATE_EXPORTS.contains(&entry.field()) && reject_unnecessary { + if !ALLOWED_EXPORTS.contains(&export.field()) && config.check_exports { return Err(CodeError::NonGearExportFnFound); } + if REQUIRED_EXPORTS.contains(&export.field()) { + entry = true; + } } } - Ok(exports) + if !entry { + return Err(CodeError::RequiredExportFnNotFound); + } + + Ok(()) } fn get_export_entry<'a>(module: &'a Module, name: &str) -> Option<&'a ExportEntry> { @@ -417,12 +444,9 @@ impl Code { return Err(CodeError::InvalidStaticPageCount); } - let exports = get_exports(&module, config.check_exports)?; - if config.check_exports - && !(exports.contains(&DispatchKind::Init) || exports.contains(&DispatchKind::Handle)) - { - return Err(CodeError::RequiredExportFnNotFound); - } + check_code(&module, &config)?; + + let exports = get_exports(&module); let mut instrumentation_builder = InstrumentationBuilder::new("env"); diff --git a/utils/wasm-builder/src/checks.rs b/utils/wasm-builder/src/checks.rs deleted file mode 100644 index 4a78662d017..00000000000 --- a/utils/wasm-builder/src/checks.rs +++ /dev/null @@ -1,48 +0,0 @@ -use gear_wasm_instrument::parity_wasm::elements::{Internal, Module, Type}; - -#[derive(Debug)] -pub enum Error { - ExportSectionMissing, - ImportSectionMissing, - FunctionSectionMissing, - TypeSectionMissing, - CodeSectionMissing, - UnexpectedFunctionExport(String), - MissingEntryFunction, - InvalidExportFunctionSignature, -} - -pub fn do_checks(module: Module) -> Result<(), Error> { - - let exports = module.export_section().ok_or(Error::ExportSectionMissing)?; - let imports = module.import_section().ok_or(Error::ImportSectionMissing)?; - let functions = module.function_section().ok_or(Error::FunctionSectionMissing)?; - let types = module.type_section().ok_or(Error::TypeSectionMissing)?; - let code = module.code_section().ok_or(Error::CodeSectionMissing)?; - - let imported_functions = imports.functions(); - - // While this is checked in Code::new, we might as well check it when building wasm program - let mut entry = false; - for e in exports.entries() { - if let Internal::Function(i) = e.internal() { - match e.field() { - "init" | "handle" | "handle_reply" | "handle_signal" | "state" | "meta" => { - entry = true; - let Type::Function(ref t) = types.types()[functions.entries()[*i as usize - imported_functions]]; - if !t.params().is_empty() || !t.results().is_empty() { - return Err(Error::InvalidExportFunctionSignature); - } - }, - _ => return Err(Error::UnexpectedFunctionExport(e.field().to_string())) - } - } - } - if !entry { - return Err(Error::MissingEntryFunction) - } - - - - Ok(()) -} \ No newline at end of file diff --git a/utils/wasm-builder/src/lib.rs b/utils/wasm-builder/src/lib.rs index a92cb89af6a..6562f1cc155 100644 --- a/utils/wasm-builder/src/lib.rs +++ b/utils/wasm-builder/src/lib.rs @@ -36,7 +36,6 @@ pub mod optimize; mod smart_fs; mod stack_end; mod wasm_project; -mod checks; pub const TARGET: &str = env!("TARGET"); diff --git a/utils/wasm-builder/src/wasm_project.rs b/utils/wasm-builder/src/wasm_project.rs index 0b0e812e1f2..6a726afc735 100644 --- a/utils/wasm-builder/src/wasm_project.rs +++ b/utils/wasm-builder/src/wasm_project.rs @@ -16,11 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::{ - crate_info::CrateInfo, - optimize::{self, OptType, Optimizer}, - smart_fs, -}; +use crate::{crate_info::CrateInfo, optimize::{self, OptType, Optimizer}, smart_fs}; use anyhow::{Context, Result}; use chrono::offset::Local as ChronoLocal; use gmeta::MetadataRepr; @@ -30,8 +26,6 @@ use std::{ path::{Path, PathBuf}, }; use toml::value::Table; -use crate::checks::do_checks; -use crate::optimize::do_optimization; const OPT_LEVEL: &str = "z"; @@ -373,10 +367,6 @@ extern "C" fn metahash() {{ .context("Failed to write optimized WASM binary")?; } - // Generate module from optimized wasm file to run gear checks - let module = parity_wasm::elements::Module::from_bytes(fs::read(opt_wasm_path.clone()).expect("Failed to read optimized wasm binary")).expect("Invalid wasm generated by wasm-opt"); - do_checks(module).expect("Wasm code did not pass gear checks"); - // Create path string in `.binpath` file. let relative_path_to_wasm = pathdiff::diff_paths(&self.wasm_target_dir, &self.original_dir) .with_context(|| { From 882a7dafadc8659ee70e6281029c0916ed6b5d60 Mon Sep 17 00:00:00 2001 From: mqxf Date: Fri, 12 Jan 2024 14:43:51 +0100 Subject: [PATCH 03/13] Added import checks --- core/src/code.rs | 73 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/core/src/code.rs b/core/src/code.rs index ec43bd32420..d5fb3845874 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -24,21 +24,17 @@ use crate::{ pages::{PageNumber, PageU32Size, WasmPage}, }; use alloc::{collections::BTreeSet, vec, vec::Vec}; -use gear_wasm_instrument::{ - parity_wasm::{ - self, - elements::{ - ExportEntry, GlobalEntry, GlobalType, InitExpr, Instruction, Internal, Module, Type, - }, +use gear_wasm_instrument::{parity_wasm::{ + self, + elements::{ + ExportEntry, GlobalEntry, GlobalType, InitExpr, Instruction, Internal, Module, Type, }, - wasm_instrument::gas_metering::{ConstantCostRules, Rules}, - InstrumentationBuilder, STACK_END_EXPORT_NAME, -}; +}, wasm_instrument::gas_metering::{ConstantCostRules, Rules}, InstrumentationBuilder, STACK_END_EXPORT_NAME, SyscallName}; use scale_info::{ scale::{Decode, Encode}, TypeInfo, }; -use gear_wasm_instrument::parity_wasm::elements::ImportCountType; +use gear_wasm_instrument::parity_wasm::elements::{External, ImportCountType, ValueType}; use crate::costs::RuntimeCosts::PayProgramRent; /// Defines maximal permitted count of memory pages. @@ -71,6 +67,8 @@ fn check_code( module: &Module, config: &TryNewCodeConfig, ) -> Result<(), CodeError> { + //return Ok(()); + let funcs = module .function_section() .ok_or(CodeError::FunctionSectionNotFound)? @@ -93,28 +91,50 @@ fn check_code( .ok_or(CodeError::ExportSectionNotFound)? .entries(); - let mut entry = false; - for export in exports { - if let Internal::Function(i) = export.internal() { - if config.check_exports { + if config.check_exports { + let mut entry = false; + for export in exports { + if let Internal::Function(i) = export.internal() { // Index access into arrays cannot panic unless the Module structure is invalid let type_id = funcs[*i as usize - import_count].type_ref(); let Type::Function(ref f) = types[type_id as usize]; if !f.params().is_empty() || !f.results().is_empty() { return Err(CodeError::InvalidExportFnSignature); } + if !ALLOWED_EXPORTS.contains(&export.field()) && config.check_exports { + return Err(CodeError::NonGearExportFnFound); + } + if REQUIRED_EXPORTS.contains(&export.field()) { + entry = true; + } } - if !ALLOWED_EXPORTS.contains(&export.field()) && config.check_exports { - return Err(CodeError::NonGearExportFnFound); - } - if REQUIRED_EXPORTS.contains(&export.field()) { - entry = true; - } + } + + if !entry { + return Err(CodeError::RequiredExportFnNotFound); } } - if !entry { - return Err(CodeError::RequiredExportFnNotFound); + if config.check_imports { + let syscalls = SyscallName::all().collect::>(); + for import in imports { + if let External::Function(i) = import.external() { + let Type::Function(types) = &types[*i as usize]; + // We can likely improve this by adding some helper function in SyscallName + let syscall = syscalls.iter().find(|s| s.to_str() == import.field()).ok_or(CodeError::UnknownImport)?; + let signature = syscall.signature(); + + let params = signature.params().iter().copied().map(Into::::into).collect::>(); + if ¶ms != types.params() { + return Err(CodeError::InvalidImportFnSignature); + } + + let results = signature.results().unwrap_or(&[]); + if results != types.results() { + return Err(CodeError::InvalidImportFnSignature); + } + } + } } Ok(()) @@ -288,6 +308,12 @@ pub enum CodeError { /// The signature of an exported function is invalid. #[display(fmt = "Invalid function signature for exported function")] InvalidExportFnSignature, + /// An imported function was not recognized. + #[display(fmt = "Unknown function name in import section")] + UnknownImport, + /// The signature of an imported function is invalid. + #[display(fmt = "Invalid function signature for imported function")] + InvalidImportFnSignature, } /// Contains instrumented binary code of a program and initial memory size from memory import. @@ -357,6 +383,8 @@ pub struct TryNewCodeConfig { pub export_stack_height: bool, /// Check exports (wasm contains init or handle exports) pub check_exports: bool, + /// Check imports (check that all imports are valid syscalls with correct signature) + pub check_imports: bool, /// Check and canonize stack end pub check_and_canonize_stack_end: bool, /// Check mutable global exports @@ -374,6 +402,7 @@ impl Default for TryNewCodeConfig { stack_height: None, export_stack_height: false, check_exports: true, + check_imports: true, check_and_canonize_stack_end: true, check_mut_global_exports: true, check_start_section: true, From 5466698210ace253b36d9575fe989f04c9cf6407 Mon Sep 17 00:00:00 2001 From: mqxf Date: Wed, 17 Jan 2024 14:49:49 +0100 Subject: [PATCH 04/13] Fmt + fix merge issue --- core/src/code.rs | 54 +++++++++++++++++--------- utils/wasm-builder/src/wasm_project.rs | 6 ++- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/core/src/code.rs b/core/src/code.rs index c9ed77db9e2..31dc07d8b57 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -24,34 +24,47 @@ use crate::{ pages::{PageNumber, PageU32Size, WasmPage}, }; use alloc::{collections::BTreeSet, vec, vec::Vec}; -use gear_wasm_instrument::{parity_wasm::{ - self, - elements::{ - ExportEntry, GlobalEntry, GlobalType, InitExpr, Instruction, Internal, Module, Type, +use gear_wasm_instrument::{ + parity_wasm::{ + self, + elements::{ + ExportEntry, External, GlobalEntry, GlobalType, ImportCountType, InitExpr, Instruction, + Internal, Module, Type, ValueType, + }, }, -}, wasm_instrument::gas_metering::{ConstantCostRules, Rules}, InstrumentationBuilder, STACK_END_EXPORT_NAME, SyscallName}; + wasm_instrument::gas_metering::{ConstantCostRules, Rules}, + InstrumentationBuilder, SyscallName, STACK_END_EXPORT_NAME, +}; use scale_info::{ scale::{Decode, Encode}, TypeInfo, }; -use gear_wasm_instrument::parity_wasm::elements::{External, ImportCountType, ValueType}; -use crate::costs::RuntimeCosts::PayProgramRent; /// Defines maximal permitted count of memory pages. pub const MAX_WASM_PAGE_COUNT: u16 = 512; /// Name of exports allowed on chain. -pub const ALLOWED_EXPORTS: [&str; 6] = ["init", "handle", "handle_reply", "handle_signal", "state", "metahash"]; +pub const ALLOWED_EXPORTS: [&str; 6] = [ + "init", + "handle", + "handle_reply", + "handle_signal", + "state", + "metahash", +]; /// Name of exports required on chain (only 1 of these is required). pub const REQUIRED_EXPORTS: [&str; 2] = ["init", "handle"]; -fn get_exports( - module: &Module -) -> BTreeSet { +fn get_exports(module: &Module) -> BTreeSet { let mut entries = BTreeSet::new(); - for entry in module.export_section().expect("Exports section has been checked for already").entries().iter() { + for entry in module + .export_section() + .expect("Exports section has been checked for already") + .entries() + .iter() + { if let Internal::Function(_) = entry.internal() { if let Some(entry) = DispatchKind::try_from_entry(entry.field()) { entries.insert(entry); @@ -63,10 +76,7 @@ fn get_exports( } /// Parse function exports from wasm module into [`DispatchKind`]. -fn check_code( - module: &Module, - config: &TryNewCodeConfig, -) -> Result<(), CodeError> { +fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeError> { //return Ok(()); let funcs = module @@ -116,15 +126,21 @@ fn check_code( } if config.check_imports { - let syscalls = SyscallName::all().collect::>(); for import in imports { if let External::Function(i) = import.external() { let Type::Function(types) = &types[*i as usize]; // We can likely improve this by adding some helper function in SyscallName - let syscall = syscalls.iter().find(|s| s.to_str() == import.field()).ok_or(CodeError::UnknownImport)?; + let syscall = SyscallName::all() + .find(|s| s.to_str() == import.field()) + .ok_or(CodeError::UnknownImport)?; let signature = syscall.signature(); - let params = signature.params().iter().copied().map(Into::::into).collect::>(); + let params = signature + .params() + .iter() + .copied() + .map(Into::::into) + .collect::>(); if ¶ms != types.params() { return Err(CodeError::InvalidImportFnSignature); } diff --git a/utils/wasm-builder/src/wasm_project.rs b/utils/wasm-builder/src/wasm_project.rs index e777913fbd7..8a2ffa19a79 100644 --- a/utils/wasm-builder/src/wasm_project.rs +++ b/utils/wasm-builder/src/wasm_project.rs @@ -16,7 +16,11 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::{crate_info::CrateInfo, optimize::{self, OptType, Optimizer}, smart_fs}; +use crate::{ + crate_info::CrateInfo, + optimize::{self, OptType, Optimizer}, + smart_fs, +}; use anyhow::{Context, Result}; use chrono::offset::Local as ChronoLocal; use gmeta::MetadataRepr; From bd9d34910df554d6cb9609874770dc10a9dc7192 Mon Sep 17 00:00:00 2001 From: mqxf Date: Wed, 17 Jan 2024 15:05:50 +0100 Subject: [PATCH 05/13] Fix clippy --- core/src/code.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/code.rs b/core/src/code.rs index 31dc07d8b57..43834dbb058 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -141,7 +141,7 @@ fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeErro .copied() .map(Into::::into) .collect::>(); - if ¶ms != types.params() { + if params != types.params() { return Err(CodeError::InvalidImportFnSignature); } From f89050b8bb30a7dca4b23ae80656c3c2957bea6c Mon Sep 17 00:00:00 2001 From: mqxf Date: Wed, 17 Jan 2024 15:22:12 +0100 Subject: [PATCH 06/13] Cleanup old comment --- core/src/code.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/src/code.rs b/core/src/code.rs index 43834dbb058..ce6adc22d75 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -77,8 +77,6 @@ fn get_exports(module: &Module) -> BTreeSet { /// Parse function exports from wasm module into [`DispatchKind`]. fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeError> { - //return Ok(()); - let funcs = module .function_section() .ok_or(CodeError::FunctionSectionNotFound)? From 9eb8746955d4a9c2e5e95afc7cab8d3065ab6064 Mon Sep 17 00:00:00 2001 From: mqxf Date: Thu, 18 Jan 2024 11:17:53 +0100 Subject: [PATCH 07/13] Update core/src/code.rs Co-authored-by: StackOverflowExcept1on <109800286+StackOverflowExcept1on@users.noreply.github.com> --- core/src/code.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/code.rs b/core/src/code.rs index ce6adc22d75..3755b04f8a3 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -109,7 +109,7 @@ fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeErro if !f.params().is_empty() || !f.results().is_empty() { return Err(CodeError::InvalidExportFnSignature); } - if !ALLOWED_EXPORTS.contains(&export.field()) && config.check_exports { + if !ALLOWED_EXPORTS.contains(&export.field()) { return Err(CodeError::NonGearExportFnFound); } if REQUIRED_EXPORTS.contains(&export.field()) { From 1522e34a2e0501f8149a31914068f03c525c162d Mon Sep 17 00:00:00 2001 From: mqxf Date: Thu, 18 Jan 2024 11:39:10 +0100 Subject: [PATCH 08/13] Made code more efficient --- core/src/code.rs | 10 +++++----- utils/wasm-instrument/src/syscalls.rs | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/core/src/code.rs b/core/src/code.rs index 3755b04f8a3..f31a753ecfd 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -124,12 +124,13 @@ fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeErro } if config.check_imports { + let syscalls = SyscallName::instrumentable_map(); for import in imports { if let External::Function(i) = import.external() { let Type::Function(types) = &types[*i as usize]; // We can likely improve this by adding some helper function in SyscallName - let syscall = SyscallName::all() - .find(|s| s.to_str() == import.field()) + let syscall = syscalls + .get(import.field()) .ok_or(CodeError::UnknownImport)?; let signature = syscall.signature(); @@ -137,9 +138,8 @@ fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeErro .params() .iter() .copied() - .map(Into::::into) - .collect::>(); - if params != types.params() { + .map(Into::::into); + if !params.eq(types.params().iter().copied()) { return Err(CodeError::InvalidImportFnSignature); } diff --git a/utils/wasm-instrument/src/syscalls.rs b/utils/wasm-instrument/src/syscalls.rs index ff891d7897b..01388199af5 100644 --- a/utils/wasm-instrument/src/syscalls.rs +++ b/utils/wasm-instrument/src/syscalls.rs @@ -19,7 +19,12 @@ //! Gear syscalls for smart contracts execution signatures. use crate::parity_wasm::elements::{FunctionType, ValueType}; -use alloc::{borrow::ToOwned, collections::BTreeSet, vec::Vec}; +use alloc::{ + borrow::ToOwned, + collections::{BTreeMap, BTreeSet}, + string::{String, ToString}, + vec::Vec, +}; use core::iter; use enum_iterator::{self, Sequence}; pub use pointers::*; @@ -237,6 +242,14 @@ impl SyscallName { .into() } + /// Returns map of all syscall string values to syscall names (actually supported by this module syscalls). + pub fn instrumentable_map() -> BTreeMap { + Self::instrumentable() + .into_iter() + .map(|n| (n.to_str().to_string(), n)) + .collect() + } + /// Returns signature for syscall by name. pub fn signature(self) -> SyscallSignature { use RegularParamType::*; From 945cba5321370b3e067944d385dc07e51662166d Mon Sep 17 00:00:00 2001 From: mqxf Date: Thu, 18 Jan 2024 12:32:44 +0100 Subject: [PATCH 09/13] Trigger CI From 947bd0920ad8a1cfa1a5d35e5a54231d61af3bb7 Mon Sep 17 00:00:00 2001 From: mqxf Date: Thu, 18 Jan 2024 14:28:08 +0100 Subject: [PATCH 10/13] import checks only called from wasm-builder --- core/src/code.rs | 9 +++++++-- utils/wasm-builder/src/optimize.rs | 9 ++++++--- utils/wasm-instrument/src/syscalls.rs | 16 ++++++++-------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/core/src/code.rs b/core/src/code.rs index f31a753ecfd..eabc358c89d 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -124,7 +124,7 @@ fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeErro } if config.check_imports { - let syscalls = SyscallName::instrumentable_map(); + let syscalls = SyscallName::all_map(); for import in imports { if let External::Function(i) = import.external() { let Type::Function(types) = &types[*i as usize]; @@ -132,6 +132,11 @@ fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeErro let syscall = syscalls .get(import.field()) .ok_or(CodeError::UnknownImport)?; + + if syscall == SyscallName::SystemBreak { + continue; + } + let signature = syscall.signature(); let params = signature @@ -416,7 +421,7 @@ impl Default for TryNewCodeConfig { stack_height: None, export_stack_height: false, check_exports: true, - check_imports: true, + check_imports: false, check_and_canonize_stack_end: true, check_mut_global_exports: true, check_start_section: true, diff --git a/utils/wasm-builder/src/optimize.rs b/utils/wasm-builder/src/optimize.rs index 376636dd64d..cd6e1c3ec26 100644 --- a/utils/wasm-builder/src/optimize.rs +++ b/utils/wasm-builder/src/optimize.rs @@ -131,11 +131,14 @@ impl Optimizer { .map_err(BuilderError::CodeCheckFailed)?, // validate wasm code // see `pallet_gear::pallet::Pallet::upload_program(...)` - OptType::Opt => Code::try_new( + OptType::Opt => Code::try_new_mock_with_rules( original_code, - 1, |_| CustomConstantCostRules::default(), - None, + TryNewCodeConfig { + version: 1, + check_imports: true, + ..Default::default() + }, ) .map(|_| ()) .map_err(BuilderError::CodeCheckFailed)?, diff --git a/utils/wasm-instrument/src/syscalls.rs b/utils/wasm-instrument/src/syscalls.rs index 01388199af5..d9590d199fb 100644 --- a/utils/wasm-instrument/src/syscalls.rs +++ b/utils/wasm-instrument/src/syscalls.rs @@ -182,6 +182,14 @@ impl SyscallName { Self::all().count() } + /// Returns map of all syscall string values to syscall names. + pub fn all_map() -> BTreeMap { + Self::all() + .into_iter() + .map(|n| (n.to_str().to_string(), n)) + .collect() + } + /// Returns list of all syscall names (actually supported by this module syscalls). pub fn instrumentable() -> BTreeSet { [ @@ -242,14 +250,6 @@ impl SyscallName { .into() } - /// Returns map of all syscall string values to syscall names (actually supported by this module syscalls). - pub fn instrumentable_map() -> BTreeMap { - Self::instrumentable() - .into_iter() - .map(|n| (n.to_str().to_string(), n)) - .collect() - } - /// Returns signature for syscall by name. pub fn signature(self) -> SyscallSignature { use RegularParamType::*; From 626b59a6c184de6c5758994482362646b11577ef Mon Sep 17 00:00:00 2001 From: mqxf Date: Thu, 18 Jan 2024 14:46:33 +0100 Subject: [PATCH 11/13] Fix clippy --- utils/wasm-instrument/src/syscalls.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/wasm-instrument/src/syscalls.rs b/utils/wasm-instrument/src/syscalls.rs index d9590d199fb..03c519fc36f 100644 --- a/utils/wasm-instrument/src/syscalls.rs +++ b/utils/wasm-instrument/src/syscalls.rs @@ -185,7 +185,6 @@ impl SyscallName { /// Returns map of all syscall string values to syscall names. pub fn all_map() -> BTreeMap { Self::all() - .into_iter() .map(|n| (n.to_str().to_string(), n)) .collect() } From 3ae0f74825df955e13fcf9cca78ff6dd01d33e57 Mon Sep 17 00:00:00 2001 From: mqxf Date: Thu, 18 Jan 2024 15:45:18 +0100 Subject: [PATCH 12/13] Implemented changes --- core/src/code.rs | 11 +++++++---- utils/wasm-instrument/src/syscalls.rs | 17 ++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/core/src/code.rs b/core/src/code.rs index eabc358c89d..2ee03fda8dc 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -124,17 +124,17 @@ fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeErro } if config.check_imports { - let syscalls = SyscallName::all_map(); + let syscalls = SyscallName::instrumentable_map(); + let mut seen = BTreeSet::new(); for import in imports { if let External::Function(i) = import.external() { let Type::Function(types) = &types[*i as usize]; - // We can likely improve this by adding some helper function in SyscallName let syscall = syscalls .get(import.field()) .ok_or(CodeError::UnknownImport)?; - if syscall == SyscallName::SystemBreak { - continue; + if !seen.insert(syscall.clone()) { + return Err(CodeError::DuplicateImport); } let signature = syscall.signature(); @@ -333,6 +333,9 @@ pub enum CodeError { /// The signature of an imported function is invalid. #[display(fmt = "Invalid function signature for imported function")] InvalidImportFnSignature, + /// An imported was declared multiple times. + #[display(fmt = "An import was declared multiple times")] + DuplicateImport, } /// Contains instrumented binary code of a program and initial memory size from memory import. diff --git a/utils/wasm-instrument/src/syscalls.rs b/utils/wasm-instrument/src/syscalls.rs index 03c519fc36f..a8672228a96 100644 --- a/utils/wasm-instrument/src/syscalls.rs +++ b/utils/wasm-instrument/src/syscalls.rs @@ -182,13 +182,6 @@ impl SyscallName { Self::all().count() } - /// Returns map of all syscall string values to syscall names. - pub fn all_map() -> BTreeMap { - Self::all() - .map(|n| (n.to_str().to_string(), n)) - .collect() - } - /// Returns list of all syscall names (actually supported by this module syscalls). pub fn instrumentable() -> BTreeSet { [ @@ -245,10 +238,20 @@ impl SyscallName { Self::ReserveGas, Self::UnreserveGas, Self::Random, + Self::SystemReserveGas, + Self::EnvVars, ] .into() } + /// Returns map of all syscall string values to syscall names. + pub fn instrumentable_map() -> BTreeMap { + Self::instrumentable() + .into_iter() + .map(|n| (n.to_str().to_string(), n)) + .collect() + } + /// Returns signature for syscall by name. pub fn signature(self) -> SyscallSignature { use RegularParamType::*; From d549d1a95687dd377f925017e25fd14d324aa105 Mon Sep 17 00:00:00 2001 From: mqxf Date: Thu, 18 Jan 2024 16:12:51 +0100 Subject: [PATCH 13/13] Fix clippy --- core/src/code.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/code.rs b/core/src/code.rs index 2ee03fda8dc..a0dd4504b46 100644 --- a/core/src/code.rs +++ b/core/src/code.rs @@ -133,7 +133,7 @@ fn check_code(module: &Module, config: &TryNewCodeConfig) -> Result<(), CodeErro .get(import.field()) .ok_or(CodeError::UnknownImport)?; - if !seen.insert(syscall.clone()) { + if !seen.insert(*syscall) { return Err(CodeError::DuplicateImport); }