From 5287f74fbf3aececbbe1e717a7096a205b12a74a Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Tue, 7 May 2024 18:28:07 +0200 Subject: [PATCH 01/11] Revamp region system --- packages/std/src/exports.rs | 206 +++++++++++++++++++------------- packages/std/src/imports.rs | 180 +++++++++++++++------------- packages/std/src/memory.rs | 231 ++++++++++++++++++++---------------- packages/vm/src/memory.rs | 4 + 4 files changed, 354 insertions(+), 267 deletions(-) diff --git a/packages/std/src/exports.rs b/packages/std/src/exports.rs index 4a18ccf117..41b57c10f3 100644 --- a/packages/std/src/exports.rs +++ b/packages/std/src/exports.rs @@ -20,7 +20,7 @@ use crate::ibc::{ }; use crate::ibc::{IbcChannelOpenMsg, IbcChannelOpenResponse}; use crate::imports::{ExternalApi, ExternalQuerier, ExternalStorage}; -use crate::memory::{alloc, consume_region, release_buffer, Region}; +use crate::memory::{Owned, Region}; #[cfg(feature = "abort")] use crate::panic::install_panic_handler; use crate::query::CustomQuery; @@ -75,7 +75,7 @@ extern "C" fn interface_version_8() -> () {} /// and should be accompanied by a corresponding deallocate #[no_mangle] extern "C" fn allocate(size: usize) -> u32 { - alloc(size) as u32 + Region::with_capacity(size).to_heap_ptr() as u32 } /// deallocate expects a pointer to a Region created with allocate. @@ -83,7 +83,7 @@ extern "C" fn allocate(size: usize) -> u32 { #[no_mangle] extern "C" fn deallocate(pointer: u32) { // auto-drop Region on function end - let _ = unsafe { consume_region(pointer as *mut Region) }; + let _ = unsafe { Region::from_heap_ptr(pointer as *mut Region) }; } // TODO: replace with https://doc.rust-lang.org/std/ops/trait.Try.html once stabilized @@ -123,12 +123,12 @@ where install_panic_handler(); let res = _do_instantiate( instantiate_fn, - env_ptr as *mut Region, - info_ptr as *mut Region, - msg_ptr as *mut Region, + env_ptr as *mut Region, + info_ptr as *mut Region, + msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_execute should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -153,12 +153,12 @@ where install_panic_handler(); let res = _do_execute( execute_fn, - env_ptr as *mut Region, - info_ptr as *mut Region, - msg_ptr as *mut Region, + env_ptr as *mut Region, + info_ptr as *mut Region, + msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_migrate should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -180,9 +180,13 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_migrate(migrate_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_migrate( + migrate_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_sudo should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -204,9 +208,13 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_sudo(sudo_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_sudo( + sudo_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_reply should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -227,9 +235,13 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_reply(reply_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_reply( + reply_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_query should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -249,9 +261,13 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_query(query_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_query( + query_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_ibc_channel_open is designed for use with #[entry_point] to make a "C" extern @@ -272,9 +288,13 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_ibc_channel_open(contract_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_ibc_channel_open( + contract_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_ibc_channel_connect is designed for use with #[entry_point] to make a "C" extern @@ -297,9 +317,13 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_ibc_channel_connect(contract_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_ibc_channel_connect( + contract_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_ibc_channel_close is designed for use with #[entry_point] to make a "C" extern @@ -322,9 +346,13 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_ibc_channel_close(contract_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_ibc_channel_close( + contract_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_ibc_packet_receive is designed for use with #[entry_point] to make a "C" extern @@ -348,9 +376,13 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_ibc_packet_receive(contract_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_ibc_packet_receive( + contract_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_ibc_packet_ack is designed for use with #[entry_point] to make a "C" extern @@ -374,9 +406,13 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_ibc_packet_ack(contract_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_ibc_packet_ack( + contract_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } /// do_ibc_packet_timeout is designed for use with #[entry_point] to make a "C" extern @@ -401,16 +437,20 @@ where { #[cfg(feature = "abort")] install_panic_handler(); - let res = _do_ibc_packet_timeout(contract_fn, env_ptr as *mut Region, msg_ptr as *mut Region); + let res = _do_ibc_packet_timeout( + contract_fn, + env_ptr as *mut Region, + msg_ptr as *mut Region, + ); let v = to_json_vec(&res).unwrap(); - release_buffer(v) as u32 + Region::from_data(v).to_heap_ptr() as u32 } fn _do_instantiate( instantiate_fn: &dyn Fn(DepsMut, Env, MessageInfo, M) -> Result, E>, - env_ptr: *mut Region, - info_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + info_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, @@ -418,9 +458,9 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let info: Vec = unsafe { consume_region(info_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let info: Vec = unsafe { Region::from_heap_ptr(info_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let info: MessageInfo = try_into_contract_result!(from_json(info)); @@ -432,9 +472,9 @@ where fn _do_execute( execute_fn: &dyn Fn(DepsMut, Env, MessageInfo, M) -> Result, E>, - env_ptr: *mut Region, - info_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + info_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, @@ -442,9 +482,9 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let info: Vec = unsafe { consume_region(info_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let info: Vec = unsafe { Region::from_heap_ptr(info_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let info: MessageInfo = try_into_contract_result!(from_json(info)); @@ -456,8 +496,8 @@ where fn _do_migrate( migrate_fn: &dyn Fn(DepsMut, Env, M) -> Result, E>, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, @@ -465,8 +505,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -477,8 +517,8 @@ where fn _do_sudo( sudo_fn: &dyn Fn(DepsMut, Env, M) -> Result, E>, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, @@ -486,8 +526,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -498,16 +538,16 @@ where fn _do_reply( reply_fn: &dyn Fn(DepsMut, Env, Reply) -> Result, E>, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: Reply = try_into_contract_result!(from_json(msg)); @@ -518,16 +558,16 @@ where fn _do_query( query_fn: &dyn Fn(Deps, Env, M) -> Result, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult where Q: CustomQuery, M: DeserializeOwned, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -538,15 +578,15 @@ where fn _do_ibc_channel_open( contract_fn: &dyn Fn(DepsMut, Env, IbcChannelOpenMsg) -> Result, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult where Q: CustomQuery, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcChannelOpenMsg = try_into_contract_result!(from_json(msg)); @@ -558,16 +598,16 @@ where #[cfg(feature = "stargate")] fn _do_ibc_channel_connect( contract_fn: &dyn Fn(DepsMut, Env, IbcChannelConnectMsg) -> Result, E>, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcChannelConnectMsg = try_into_contract_result!(from_json(msg)); @@ -579,16 +619,16 @@ where #[cfg(feature = "stargate")] fn _do_ibc_channel_close( contract_fn: &dyn Fn(DepsMut, Env, IbcChannelCloseMsg) -> Result, E>, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcChannelCloseMsg = try_into_contract_result!(from_json(msg)); @@ -600,16 +640,16 @@ where #[cfg(feature = "stargate")] fn _do_ibc_packet_receive( contract_fn: &dyn Fn(DepsMut, Env, IbcPacketReceiveMsg) -> Result, E>, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcPacketReceiveMsg = try_into_contract_result!(from_json(msg)); @@ -621,16 +661,16 @@ where #[cfg(feature = "stargate")] fn _do_ibc_packet_ack( contract_fn: &dyn Fn(DepsMut, Env, IbcPacketAckMsg) -> Result, E>, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcPacketAckMsg = try_into_contract_result!(from_json(msg)); @@ -642,16 +682,16 @@ where #[cfg(feature = "stargate")] fn _do_ibc_packet_timeout( contract_fn: &dyn Fn(DepsMut, Env, IbcPacketTimeoutMsg) -> Result, E>, - env_ptr: *mut Region, - msg_ptr: *mut Region, + env_ptr: *mut Region, + msg_ptr: *mut Region, ) -> ContractResult> where Q: CustomQuery, C: CustomMsg, E: ToString, { - let env: Vec = unsafe { consume_region(env_ptr) }; - let msg: Vec = unsafe { consume_region(msg_ptr) }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcPacketTimeoutMsg = try_into_contract_result!(from_json(msg)); diff --git a/packages/std/src/imports.rs b/packages/std/src/imports.rs index ef970f2286..32a48ab87e 100644 --- a/packages/std/src/imports.rs +++ b/packages/std/src/imports.rs @@ -2,7 +2,7 @@ use alloc::vec::Vec; use cosmwasm_core::{Addr, CanonicalAddr}; use crate::import_helpers::{from_high_half, from_low_half}; -use crate::memory::{alloc, build_region, consume_region, Region}; +use crate::memory::{Owned, Region}; use crate::results::SystemResult; #[cfg(feature = "iterator")] use crate::sections::decode_sections2; @@ -106,8 +106,8 @@ impl ExternalStorage { impl Storage for ExternalStorage { fn get(&self, key: &[u8]) -> Option> { - let key = build_region(key); - let key_ptr = &*key as *const Region as u32; + let key = Region::from_data(key); + let key_ptr = key.as_ptr() as u32; let read = unsafe { db_read(key_ptr) }; if read == 0 { @@ -115,9 +115,10 @@ impl Storage for ExternalStorage { return None; } - let value_ptr = read as *mut Region; - let data = unsafe { consume_region(value_ptr) }; - Some(data) + let value_ptr = read as *mut Region; + let data = unsafe { Region::from_heap_ptr(value_ptr) }; + + Some(data.into_inner()) } fn set(&mut self, key: &[u8], value: &[u8]) { @@ -125,18 +126,19 @@ impl Storage for ExternalStorage { panic!("TL;DR: Value must not be empty in Storage::set but in most cases you can use Storage::remove instead. Long story: Getting empty values from storage is not well supported at the moment. Some of our internal interfaces cannot differentiate between a non-existent key and an empty value. Right now, you cannot rely on the behaviour of empty values. To protect you from trouble later on, we stop here. Sorry for the inconvenience! We highly welcome you to contribute to CosmWasm, making this more solid one way or the other."); } - // keep the boxes in scope, so we free it at the end (don't cast to pointers same line as build_region) - let key = build_region(key); - let key_ptr = &*key as *const Region as u32; - let mut value = build_region(value); - let value_ptr = &mut *value as *mut Region as u32; + let key = Region::from_data(key); + let key_ptr = key.as_ptr() as u32; + + let value = Region::from_data(value); + let value_ptr = value.as_ptr() as u32; + unsafe { db_write(key_ptr, value_ptr) }; } fn remove(&mut self, key: &[u8]) { - // keep the boxes in scope, so we free it at the end (don't cast to pointers same line as build_region) - let key = build_region(key); - let key_ptr = &*key as *const Region as u32; + let key = Region::from_data(key); + let key_ptr = key.as_ptr() as u32; + unsafe { db_remove(key_ptr) }; } @@ -187,8 +189,8 @@ impl Storage for ExternalStorage { fn create_iter(start: Option<&[u8]>, end: Option<&[u8]>, order: Order) -> u32 { // There is lots of gotchas on turning options into regions for FFI, thus this design // See: https://github.com/CosmWasm/cosmwasm/pull/509 - let start_region = start.map(build_region); - let end_region = end.map(build_region); + let start_region = start.map(Region::from_data); + let end_region = end.map(Region::from_data); let start_region_addr = get_optional_region_address(&start_region.as_ref()); let end_region_addr = get_optional_region_address(&end_region.as_ref()); unsafe { db_scan(start_region_addr, end_region_addr, order as i32) } @@ -235,9 +237,10 @@ impl Iterator for ExternalPartialIterator { return None; } - let data_region = next_result as *mut Region; - let data = unsafe { consume_region(data_region) }; - Some(data) + let data_region = next_result as *mut Region; + let data = unsafe { Region::from_heap_ptr(data_region) }; + + Some(data.into_inner()) } } @@ -263,9 +266,11 @@ impl Iterator for ExternalIterator { fn next(&mut self) -> Option { let next_result = unsafe { db_next(self.iterator_id) }; - let kv_region_ptr = next_result as *mut Region; - let kv = unsafe { consume_region(kv_region_ptr) }; - let (key, value) = decode_sections2(kv); + let kv_region_ptr = next_result as *mut Region; + let kv = unsafe { Region::from_heap_ptr(kv_region_ptr) }; + + let (key, value) = decode_sections2(kv.into_inner()); + if key.len() == 0 { None } else { @@ -283,8 +288,9 @@ fn skip_iter(iter_id: u32, count: usize) { // early return return; } + // just deallocate the region - unsafe { consume_region(region as *mut Region) }; + unsafe { Region::from_heap_ptr(region as *mut Region) }; } } @@ -307,12 +313,13 @@ impl Api for ExternalApi { // Stop here to allow handling the error in the contract. return Err(StdError::generic_err("input too long for addr_validate")); } - let source = build_region(input_bytes); - let source_ptr = &*source as *const Region as u32; + let source = Region::from_data(input_bytes); + let source_ptr = source.as_ptr() as u32; let result = unsafe { addr_validate(source_ptr) }; if result != 0 { - let error = unsafe { consume_string_region_written_by_vm(result as *mut Region) }; + let error = + unsafe { consume_string_region_written_by_vm(result as *mut Region) }; return Err(StdError::generic_err(format!( "addr_validate errored: {}", error @@ -332,38 +339,41 @@ impl Api for ExternalApi { "input too long for addr_canonicalize", )); } - let send = build_region(input_bytes); - let send_ptr = &*send as *const Region as u32; - let canon = alloc(CANONICAL_ADDRESS_BUFFER_LENGTH); + let send = Region::from_data(input_bytes); + let send_ptr = send.as_ptr() as u32; + let canon = Region::with_capacity(CANONICAL_ADDRESS_BUFFER_LENGTH); - let result = unsafe { addr_canonicalize(send_ptr, canon as u32) }; + let result = unsafe { addr_canonicalize(send_ptr, canon.as_ptr() as u32) }; if result != 0 { - let error = unsafe { consume_string_region_written_by_vm(result as *mut Region) }; + let error = + unsafe { consume_string_region_written_by_vm(result as *mut Region) }; return Err(StdError::generic_err(format!( "addr_canonicalize errored: {}", error ))); } - let out = unsafe { consume_region(canon) }; - Ok(CanonicalAddr::from(out)) + Ok(CanonicalAddr::from(canon.into_inner())) } fn addr_humanize(&self, canonical: &CanonicalAddr) -> StdResult { - let send = build_region(canonical.as_slice()); - let send_ptr = &*send as *const Region as u32; - let human = alloc(HUMAN_ADDRESS_BUFFER_LENGTH); + let send = Region::from_data(canonical.as_slice()); + let send_ptr = send.as_ptr() as u32; + let human = Region::with_capacity(HUMAN_ADDRESS_BUFFER_LENGTH); - let result = unsafe { addr_humanize(send_ptr, human as u32) }; + let result = unsafe { addr_humanize(send_ptr, human.as_ptr() as u32) }; if result != 0 { - let error = unsafe { consume_string_region_written_by_vm(result as *mut Region) }; + let error = + unsafe { consume_string_region_written_by_vm(result as *mut Region) }; return Err(StdError::generic_err(format!( "addr_humanize errored: {}", error ))); } - let address = unsafe { consume_string_region_written_by_vm(human) }; + // TODO: Optimize this heap allocation away? + // I mean technically we save a bunch of heap allocation now anyway, but we can optimize this, too! + let address = unsafe { consume_string_region_written_by_vm(human.to_heap_ptr()) }; Ok(Addr::unchecked(address)) } @@ -373,12 +383,12 @@ impl Api for ExternalApi { signature: &[u8], public_key: &[u8], ) -> Result { - let hash_send = build_region(message_hash); - let hash_send_ptr = &*hash_send as *const Region as u32; - let sig_send = build_region(signature); - let sig_send_ptr = &*sig_send as *const Region as u32; - let pubkey_send = build_region(public_key); - let pubkey_send_ptr = &*pubkey_send as *const Region as u32; + let hash_send = Region::from_data(message_hash); + let hash_send_ptr = hash_send.as_ptr() as u32; + let sig_send = Region::from_data(signature); + let sig_send_ptr = sig_send.as_ptr() as u32; + let pubkey_send = Region::from_data(public_key); + let pubkey_send_ptr = pubkey_send.as_ptr() as u32; let result = unsafe { secp256k1_verify(hash_send_ptr, sig_send_ptr, pubkey_send_ptr) }; match result { @@ -399,10 +409,10 @@ impl Api for ExternalApi { signature: &[u8], recover_param: u8, ) -> Result, RecoverPubkeyError> { - let hash_send = build_region(message_hash); - let hash_send_ptr = &*hash_send as *const Region as u32; - let sig_send = build_region(signature); - let sig_send_ptr = &*sig_send as *const Region as u32; + let hash_send = Region::from_data(message_hash); + let hash_send_ptr = hash_send.as_ptr() as u32; + let sig_send = Region::from_data(signature); + let sig_send_ptr = sig_send.as_ptr() as u32; let result = unsafe { secp256k1_recover_pubkey(hash_send_ptr, sig_send_ptr, recover_param.into()) }; @@ -410,7 +420,8 @@ impl Api for ExternalApi { let pubkey_ptr = from_low_half(result); match error_code { 0 => { - let pubkey = unsafe { consume_region(pubkey_ptr as *mut Region) }; + let pubkey = + unsafe { Region::from_heap_ptr(pubkey_ptr as *mut Region).into_inner() }; Ok(pubkey) } 2 => panic!("MessageTooLong must not happen. This is a bug in the VM."), @@ -428,12 +439,12 @@ impl Api for ExternalApi { signature: &[u8], public_key: &[u8], ) -> Result { - let hash_send = build_region(message_hash); - let hash_send_ptr = &*hash_send as *const Region as u32; - let sig_send = build_region(signature); - let sig_send_ptr = &*sig_send as *const Region as u32; - let pubkey_send = build_region(public_key); - let pubkey_send_ptr = &*pubkey_send as *const Region as u32; + let hash_send = Region::from_data(message_hash); + let hash_send_ptr = hash_send.as_ptr() as u32; + let sig_send = Region::from_data(signature); + let sig_send_ptr = sig_send.as_ptr() as u32; + let pubkey_send = Region::from_data(public_key); + let pubkey_send_ptr = pubkey_send.as_ptr() as u32; let result = unsafe { secp256r1_verify(hash_send_ptr, sig_send_ptr, pubkey_send_ptr) }; match result { @@ -455,10 +466,10 @@ impl Api for ExternalApi { signature: &[u8], recover_param: u8, ) -> Result, RecoverPubkeyError> { - let hash_send = build_region(message_hash); - let hash_send_ptr = &*hash_send as *const Region as u32; - let sig_send = build_region(signature); - let sig_send_ptr = &*sig_send as *const Region as u32; + let hash_send = Region::from_data(message_hash); + let hash_send_ptr = hash_send.as_ptr() as u32; + let sig_send = Region::from_data(signature); + let sig_send_ptr = sig_send.as_ptr() as u32; let result = unsafe { secp256r1_recover_pubkey(hash_send_ptr, sig_send_ptr, recover_param.into()) }; @@ -466,7 +477,8 @@ impl Api for ExternalApi { let pubkey_ptr = from_low_half(result); match error_code { 0 => { - let pubkey = unsafe { consume_region(pubkey_ptr as *mut Region) }; + let pubkey = + unsafe { Region::from_heap_ptr(pubkey_ptr as *mut Region).into_inner() }; Ok(pubkey) } 2 => panic!("MessageTooLong must not happen. This is a bug in the VM."), @@ -483,12 +495,12 @@ impl Api for ExternalApi { signature: &[u8], public_key: &[u8], ) -> Result { - let msg_send = build_region(message); - let msg_send_ptr = &*msg_send as *const Region as u32; - let sig_send = build_region(signature); - let sig_send_ptr = &*sig_send as *const Region as u32; - let pubkey_send = build_region(public_key); - let pubkey_send_ptr = &*pubkey_send as *const Region as u32; + let msg_send = Region::from_data(message); + let msg_send_ptr = msg_send.as_ptr() as u32; + let sig_send = Region::from_data(signature); + let sig_send_ptr = sig_send.as_ptr() as u32; + let pubkey_send = Region::from_data(public_key); + let pubkey_send_ptr = pubkey_send.as_ptr() as u32; let result = unsafe { ed25519_verify(msg_send_ptr, sig_send_ptr, pubkey_send_ptr) }; match result { @@ -510,16 +522,16 @@ impl Api for ExternalApi { public_keys: &[&[u8]], ) -> Result { let msgs_encoded = encode_sections(messages); - let msgs_send = build_region(&msgs_encoded); - let msgs_send_ptr = &*msgs_send as *const Region as u32; + let msgs_send = Region::from_data(msgs_encoded); + let msgs_send_ptr = msgs_send.as_ptr() as u32; let sigs_encoded = encode_sections(signatures); - let sig_sends = build_region(&sigs_encoded); - let sigs_send_ptr = &*sig_sends as *const Region as u32; + let sig_sends = Region::from_data(sigs_encoded); + let sigs_send_ptr = sig_sends.as_ptr() as u32; let pubkeys_encoded = encode_sections(public_keys); - let pubkeys_send = build_region(&pubkeys_encoded); - let pubkeys_send_ptr = &*pubkeys_send as *const Region as u32; + let pubkeys_send = Region::from_data(pubkeys_encoded); + let pubkeys_send_ptr = pubkeys_send.as_ptr() as u32; let result = unsafe { ed25519_batch_verify(msgs_send_ptr, sigs_send_ptr, pubkeys_send_ptr) }; @@ -536,17 +548,17 @@ impl Api for ExternalApi { } fn debug(&self, message: &str) { - // keep the boxes in scope, so we free it at the end (don't cast to pointers same line as build_region) - let region = build_region(message.as_bytes()); - let region_ptr = region.as_ref() as *const Region as u32; + // keep the boxes in scope, so we free it at the end (don't cast to pointers same line as Region::from_data) + let region = Region::from_data(message.as_bytes()); + let region_ptr = region.as_ptr() as u32; unsafe { debug(region_ptr) }; } } /// Takes a pointer to a Region and reads the data into a String. /// This is for trusted string sources only. -unsafe fn consume_string_region_written_by_vm(from: *mut Region) -> String { - let data = consume_region(from); +unsafe fn consume_string_region_written_by_vm(from: *mut Region) -> String { + let data = Region::from_heap_ptr(from).into_inner(); // We trust the VM/chain to return correct UTF-8, so let's save some gas String::from_utf8_unchecked(data) } @@ -562,11 +574,12 @@ impl ExternalQuerier { impl Querier for ExternalQuerier { fn raw_query(&self, bin_request: &[u8]) -> QuerierResult { - let req = build_region(bin_request); - let request_ptr = &*req as *const Region as u32; + let req = Region::from_data(bin_request); + let request_ptr = req.as_ptr() as u32; let response_ptr = unsafe { query_chain(request_ptr) }; - let response = unsafe { consume_region(response_ptr as *mut Region) }; + let response = + unsafe { Region::from_heap_ptr(response_ptr as *mut Region).into_inner() }; from_json(&response).unwrap_or_else(|parsing_err| { SystemResult::Err(SystemError::InvalidResponse { @@ -579,8 +592,7 @@ impl Querier for ExternalQuerier { #[cfg(feature = "abort")] pub fn handle_panic(message: &str) { - // keep the boxes in scope, so we free it at the end (don't cast to pointers same line as build_region) - let region = build_region(message.as_bytes()); - let region_ptr = region.as_ref() as *const Region as u32; + let region = Region::from_data(message.as_bytes()); + let region_ptr = region.as_ptr() as u32; unsafe { abort(region_ptr) }; } diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index eba092845e..e5f396b9d1 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -1,75 +1,7 @@ use alloc::vec::Vec; -use core::mem; +use core::{any::TypeId, marker::PhantomData, mem, ops::Deref, slice}; -/// Describes some data allocated in Wasm's linear memory. -/// A pointer to an instance of this can be returned over FFI boundaries. -/// -/// This struct is crate internal since the cosmwasm-vm defines the same type independently. -#[repr(C)] -pub struct Region { - /// The beginning of the region expressed as bytes from the beginning of the linear memory - pub offset: u32, - /// The number of bytes available in this region - pub capacity: u32, - /// The number of bytes used in this region - pub length: u32, -} - -/// Creates a memory region of capacity `size` and length 0. Returns a pointer to the Region. -/// This is the same as the `allocate` export, but designed to be called internally. -pub fn alloc(size: usize) -> *mut Region { - let data: Vec = Vec::with_capacity(size); - let data_ptr = data.as_ptr() as usize; - - let region = build_region_from_components( - u32::try_from(data_ptr).expect("pointer doesn't fit in u32"), - u32::try_from(data.capacity()).expect("capacity doesn't fit in u32"), - 0, - ); - mem::forget(data); - Box::into_raw(region) -} - -/// Similar to alloc, but instead of creating a new vector it consumes an existing one and returns -/// a pointer to the Region (preventing the memory from being freed until explicitly called later). -/// -/// The resulting Region spans the entire region allocated by the vector, preserving the length and capacity components. -pub fn release_buffer(buffer: Vec) -> *mut Region { - let region = build_region(&buffer); - mem::forget(buffer); - Box::into_raw(region) -} - -/// Return the data referenced by the Region and -/// deallocates the Region (and the vector when finished). -/// Warning: only use this when you are sure the caller will never use (or free) the Region later -/// -/// # Safety -/// -/// The ptr must refer to a valid Region, which was previously returned by alloc, -/// and not yet deallocated. This call will deallocate the Region and return an owner vector -/// to the caller containing the referenced data. -/// -/// Naturally, calling this function twice on the same pointer will double deallocate data -/// and lead to a crash. Make sure to call it exactly once (either consuming the input in -/// the wasm code OR deallocating the buffer from the caller). -pub unsafe fn consume_region(ptr: *mut Region) -> Vec { - assert!(!ptr.is_null(), "Region pointer is null"); - let region = Box::from_raw(ptr); - - let region_start = region.offset as *mut u8; - // This case is explicitely disallowed by Vec - // "The pointer will never be null, so this type is null-pointer-optimized." - assert!(!region_start.is_null(), "Region starts at null pointer"); - - Vec::from_raw_parts( - region_start, - region.length as usize, - region.capacity as usize, - ) -} - -/// Element that can be used to construct a new `Box` +/// Element that can be used to construct a new `Region` /// /// # Safety /// @@ -83,12 +15,16 @@ pub unsafe fn consume_region(ptr: *mut Region) -> Vec { /// /// See: pub unsafe trait RegionSource { + type Ownership: Ownership; + fn ptr(&self) -> *const u8; fn len(&self) -> usize; fn capacity(&self) -> usize; } unsafe impl RegionSource for &[u8] { + type Ownership = Borrowed; + fn ptr(&self) -> *const u8 { self.as_ptr() } @@ -103,6 +39,8 @@ unsafe impl RegionSource for &[u8] { } unsafe impl RegionSource for Vec { + type Ownership = Owned; + fn ptr(&self) -> *const u8 { self.as_ptr() } @@ -116,55 +54,148 @@ unsafe impl RegionSource for Vec { } } -unsafe impl RegionSource for &T +mod sealed { + pub trait Sealed: 'static {} + + impl Sealed for super::Owned {} + + impl Sealed for super::Borrowed {} +} + +pub trait Ownership: sealed::Sealed + 'static {} + +impl Ownership for T where T: sealed::Sealed {} + +pub struct Owned {} + +pub struct Borrowed {} + +/// Describes some data allocated in Wasm's linear memory. +/// A pointer to an instance of this can be returned over FFI boundaries. +/// +/// This struct is crate internal since the cosmwasm-vm defines the same type independently. +#[repr(C)] +pub struct Region { + /// The beginning of the region expressed as bytes from the beginning of the linear memory + pub offset: u32, + /// The number of bytes available in this region + pub capacity: u32, + /// The number of bytes used in this region + pub length: u32, + + _marker: PhantomData, +} + +impl Region { + pub unsafe fn from_heap_ptr(ptr: *mut Self) -> Box { + assert!(!ptr.is_null(), "Region pointer is null"); + Box::from_raw(ptr) + } + + pub fn with_capacity(cap: usize) -> Self { + let data = Vec::with_capacity(cap); + let region = Self::from_data(data); + region + } + + pub fn into_inner(self) -> Vec { + let vector = unsafe { + Vec::from_raw_parts( + self.offset as *mut u8, + self.length as usize, + self.capacity as usize, + ) + }; + mem::forget(self); + vector + } +} + +impl Region where - T: RegionSource, + O: Ownership, { - fn ptr(&self) -> *const u8 { - (**self).ptr() + pub fn from_data(data: S) -> Self + where + S: RegionSource, + { + // Well, this technically violates pointer provenance rules. + // But there isn't a stable API for it, so that's the best we can do, I guess. + let region = Region { + offset: u32::try_from(data.ptr() as usize).expect("pointer doesn't fit in u32"), + capacity: u32::try_from(data.capacity()).expect("capacity doesn't fit in u32"), + length: u32::try_from(data.len()).expect("length doesn't fit in u32"), + + _marker: PhantomData, + }; + + // We gonna forget this.. as a safety measure.. + // If we didn't do this and the `RegionSource` was a `Vec` we would deallocate it and that's BAD + mem::forget(data); + + region } +} - fn len(&self) -> usize { - (**self).len() +impl Region +where + T: Ownership, +{ + pub fn as_bytes(&self) -> &[u8] { + unsafe { slice::from_raw_parts(self.offset as *const u8, self.length as usize) } } - fn capacity(&self) -> usize { - (**self).capacity() + pub fn as_ptr(&self) -> *const Self { + self + } + + pub fn to_heap_ptr(self) -> *mut Self { + let boxed = Box::new(self); + Box::into_raw(boxed) } } -/// Returns a box of a Region, which can be sent over a call to extern -/// note that this DOES NOT take ownership of the data, and we MUST NOT consume_region -/// the resulting data. -/// The Box must be dropped (with scope), but not the data -pub fn build_region(data: S) -> Box +impl Deref for Region where - S: RegionSource, + T: Ownership, { - // Well, this technically violates pointer provenance rules. - // But there isn't a stable API for it, so that's the best we can do, I guess. - build_region_from_components( - u32::try_from(data.ptr() as usize).expect("pointer doesn't fit in u32"), - u32::try_from(data.capacity()).expect("capacity doesn't fit in u32"), - u32::try_from(data.len()).expect("length doesn't fit in u32"), - ) + type Target = [u8]; + + fn deref(&self) -> &Self::Target { + self.as_bytes() + } } -fn build_region_from_components(offset: u32, capacity: u32, length: u32) -> Box { - Box::new(Region { - offset, - capacity, - length, - }) +impl Drop for Region +where + T: Ownership, +{ + fn drop(&mut self) { + // Since we can't specialize the drop impl we need to perform a runtime check + if TypeId::of::() == TypeId::of::() { + let region_start = self.offset as *mut u8; + + // This case is explicitely disallowed by Vec + // "The pointer will never be null, so this type is null-pointer-optimized." + assert!(!region_start.is_null(), "Region starts at null pointer"); + + unsafe { + let data = + Vec::from_raw_parts(region_start, self.length as usize, self.capacity as usize); + + drop(data); + } + } + } } /// Returns the address of the optional Region as an offset in linear memory, /// or zero if not present #[cfg(feature = "iterator")] -pub fn get_optional_region_address(region: &Option<&Box>) -> u32 { +pub fn get_optional_region_address(region: &Option<&Region>) -> u32 { /// Returns the address of the Region as an offset in linear memory - fn get_region_address(region: &Box) -> u32 { - region.as_ref() as *const Region as u32 + fn get_region_address(region: &Region) -> u32 { + region.as_ptr() as u32 } region.map(get_region_address).unwrap_or(0) diff --git a/packages/vm/src/memory.rs b/packages/vm/src/memory.rs index 5870b23711..3d02951158 100644 --- a/packages/vm/src/memory.rs +++ b/packages/vm/src/memory.rs @@ -8,6 +8,10 @@ use crate::errors::{ VmResult, }; +const _: () = { + assert!(std::mem::size_of::() == 12); +}; + /****** read/write to wasm memory buffer ****/ /// Describes some data allocated in Wasm's linear memory. From 86abdfbd06a5ff802485643581833c32b944cf67 Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 12:36:21 +0200 Subject: [PATCH 02/11] Add some documentation --- packages/std/src/memory.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index e5f396b9d1..673f024611 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -87,11 +87,21 @@ pub struct Region { } impl Region { + /// Reconstruct a region from a raw pointer pointing to a `Box`. + /// You'll want to use this when you received a region from the VM and want to dereference its contents. + /// + /// # Safety + /// + /// - The pointer must not be null + /// - The pointer must be heap allocated + /// - This region must point to a valid memory region + /// - The memory region this region points to must be heap allocated as well pub unsafe fn from_heap_ptr(ptr: *mut Self) -> Box { assert!(!ptr.is_null(), "Region pointer is null"); Box::from_raw(ptr) } + /// Construct a new empty region with *at least* a capacity of what you passed in and a length of 0 pub fn with_capacity(cap: usize) -> Self { let data = Vec::with_capacity(cap); let region = Self::from_data(data); @@ -115,6 +125,7 @@ impl Region where O: Ownership, { + /// Construct a new region from any kind of data that can be turned into a region pub fn from_data(data: S) -> Self where S: RegionSource, @@ -141,14 +152,22 @@ impl Region where T: Ownership, { + /// Access the memory region this region points to in form of a byte slice pub fn as_bytes(&self) -> &[u8] { unsafe { slice::from_raw_parts(self.offset as *const u8, self.length as usize) } } + /// Obtain the pointer to the region + /// + /// This is nothing but `&self as *const Region` but makes sure the correct generic parameter is used pub fn as_ptr(&self) -> *const Self { self } + /// Transform the region into an unmanaged mutable pointer + /// + /// This means we move this regions onto the heap (note, only the *structure* of the region, not the *contents of the pointer* we manage internally). + /// To then deallocate this structure, you'll have to reconstruct the region via [`Region::from_heap_ptr`] and drop it. pub fn to_heap_ptr(self) -> *mut Self { let boxed = Box::new(self); Box::into_raw(boxed) From 7915453a2782add1cbaf72a6c2a96a2f4afa0925 Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 13:44:30 +0200 Subject: [PATCH 03/11] Rename to `into_vec` --- packages/std/src/exports.rs | 52 ++++++++++++++++++------------------- packages/std/src/imports.rs | 16 ++++++------ packages/std/src/memory.rs | 2 +- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/packages/std/src/exports.rs b/packages/std/src/exports.rs index 41b57c10f3..92c027dd61 100644 --- a/packages/std/src/exports.rs +++ b/packages/std/src/exports.rs @@ -458,9 +458,9 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let info: Vec = unsafe { Region::from_heap_ptr(info_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let info: Vec = unsafe { Region::from_heap_ptr(info_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let info: MessageInfo = try_into_contract_result!(from_json(info)); @@ -482,9 +482,9 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let info: Vec = unsafe { Region::from_heap_ptr(info_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let info: Vec = unsafe { Region::from_heap_ptr(info_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let info: MessageInfo = try_into_contract_result!(from_json(info)); @@ -505,8 +505,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -526,8 +526,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -546,8 +546,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: Reply = try_into_contract_result!(from_json(msg)); @@ -566,8 +566,8 @@ where M: DeserializeOwned, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -585,8 +585,8 @@ where Q: CustomQuery, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcChannelOpenMsg = try_into_contract_result!(from_json(msg)); @@ -606,8 +606,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcChannelConnectMsg = try_into_contract_result!(from_json(msg)); @@ -627,8 +627,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcChannelCloseMsg = try_into_contract_result!(from_json(msg)); @@ -648,8 +648,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcPacketReceiveMsg = try_into_contract_result!(from_json(msg)); @@ -669,8 +669,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcPacketAckMsg = try_into_contract_result!(from_json(msg)); @@ -690,8 +690,8 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_inner() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_inner() }; + let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; + let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcPacketTimeoutMsg = try_into_contract_result!(from_json(msg)); diff --git a/packages/std/src/imports.rs b/packages/std/src/imports.rs index 32a48ab87e..18c52e699c 100644 --- a/packages/std/src/imports.rs +++ b/packages/std/src/imports.rs @@ -118,7 +118,7 @@ impl Storage for ExternalStorage { let value_ptr = read as *mut Region; let data = unsafe { Region::from_heap_ptr(value_ptr) }; - Some(data.into_inner()) + Some(data.into_vec()) } fn set(&mut self, key: &[u8], value: &[u8]) { @@ -240,7 +240,7 @@ impl Iterator for ExternalPartialIterator { let data_region = next_result as *mut Region; let data = unsafe { Region::from_heap_ptr(data_region) }; - Some(data.into_inner()) + Some(data.into_vec()) } } @@ -269,7 +269,7 @@ impl Iterator for ExternalIterator { let kv_region_ptr = next_result as *mut Region; let kv = unsafe { Region::from_heap_ptr(kv_region_ptr) }; - let (key, value) = decode_sections2(kv.into_inner()); + let (key, value) = decode_sections2(kv.into_vec()); if key.len() == 0 { None @@ -353,7 +353,7 @@ impl Api for ExternalApi { ))); } - Ok(CanonicalAddr::from(canon.into_inner())) + Ok(CanonicalAddr::from(canon.into_vec())) } fn addr_humanize(&self, canonical: &CanonicalAddr) -> StdResult { @@ -421,7 +421,7 @@ impl Api for ExternalApi { match error_code { 0 => { let pubkey = - unsafe { Region::from_heap_ptr(pubkey_ptr as *mut Region).into_inner() }; + unsafe { Region::from_heap_ptr(pubkey_ptr as *mut Region).into_vec() }; Ok(pubkey) } 2 => panic!("MessageTooLong must not happen. This is a bug in the VM."), @@ -478,7 +478,7 @@ impl Api for ExternalApi { match error_code { 0 => { let pubkey = - unsafe { Region::from_heap_ptr(pubkey_ptr as *mut Region).into_inner() }; + unsafe { Region::from_heap_ptr(pubkey_ptr as *mut Region).into_vec() }; Ok(pubkey) } 2 => panic!("MessageTooLong must not happen. This is a bug in the VM."), @@ -558,7 +558,7 @@ impl Api for ExternalApi { /// Takes a pointer to a Region and reads the data into a String. /// This is for trusted string sources only. unsafe fn consume_string_region_written_by_vm(from: *mut Region) -> String { - let data = Region::from_heap_ptr(from).into_inner(); + let data = Region::from_heap_ptr(from).into_vec(); // We trust the VM/chain to return correct UTF-8, so let's save some gas String::from_utf8_unchecked(data) } @@ -579,7 +579,7 @@ impl Querier for ExternalQuerier { let response_ptr = unsafe { query_chain(request_ptr) }; let response = - unsafe { Region::from_heap_ptr(response_ptr as *mut Region).into_inner() }; + unsafe { Region::from_heap_ptr(response_ptr as *mut Region).into_vec() }; from_json(&response).unwrap_or_else(|parsing_err| { SystemResult::Err(SystemError::InvalidResponse { diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index 673f024611..cbcd259876 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -108,7 +108,7 @@ impl Region { region } - pub fn into_inner(self) -> Vec { + pub fn into_vec(self) -> Vec { let vector = unsafe { Vec::from_raw_parts( self.offset as *mut u8, From a0e0356dc71832a1e61ec233c9efc122c6f8b776 Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 13:45:46 +0200 Subject: [PATCH 04/11] Make the ownership markers unit structs --- packages/std/src/memory.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index cbcd259876..c8f6fa6134 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -66,9 +66,9 @@ pub trait Ownership: sealed::Sealed + 'static {} impl Ownership for T where T: sealed::Sealed {} -pub struct Owned {} +pub struct Owned; -pub struct Borrowed {} +pub struct Borrowed; /// Describes some data allocated in Wasm's linear memory. /// A pointer to an instance of this can be returned over FFI boundaries. From bdddee45469a0f62462d8f3ca5abc654ce9cfdee Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 13:46:51 +0200 Subject: [PATCH 05/11] Remove redundant assertion --- packages/vm/src/memory.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/vm/src/memory.rs b/packages/vm/src/memory.rs index 3d02951158..5870b23711 100644 --- a/packages/vm/src/memory.rs +++ b/packages/vm/src/memory.rs @@ -8,10 +8,6 @@ use crate::errors::{ VmResult, }; -const _: () = { - assert!(std::mem::size_of::() == 12); -}; - /****** read/write to wasm memory buffer ****/ /// Describes some data allocated in Wasm's linear memory. From 50349ef4dd1f7aeef8fe915f8c41fa3d0a5749f8 Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 13:55:32 +0200 Subject: [PATCH 06/11] Split up functions for slices and vectors --- packages/std/src/exports.rs | 24 ++++----- packages/std/src/imports.rs | 58 +++++++++++----------- packages/std/src/memory.rs | 99 +++++++++---------------------------- 3 files changed, 63 insertions(+), 118 deletions(-) diff --git a/packages/std/src/exports.rs b/packages/std/src/exports.rs index 92c027dd61..3d9e445c3a 100644 --- a/packages/std/src/exports.rs +++ b/packages/std/src/exports.rs @@ -128,7 +128,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_execute should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -158,7 +158,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_migrate should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -186,7 +186,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_sudo should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -214,7 +214,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_reply should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -241,7 +241,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_query should be wrapped in an external "C" export, containing a contract-specific function as arg @@ -267,7 +267,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_ibc_channel_open is designed for use with #[entry_point] to make a "C" extern @@ -294,7 +294,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_ibc_channel_connect is designed for use with #[entry_point] to make a "C" extern @@ -323,7 +323,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_ibc_channel_close is designed for use with #[entry_point] to make a "C" extern @@ -352,7 +352,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_ibc_packet_receive is designed for use with #[entry_point] to make a "C" extern @@ -382,7 +382,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_ibc_packet_ack is designed for use with #[entry_point] to make a "C" extern @@ -412,7 +412,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } /// do_ibc_packet_timeout is designed for use with #[entry_point] to make a "C" extern @@ -443,7 +443,7 @@ where msg_ptr as *mut Region, ); let v = to_json_vec(&res).unwrap(); - Region::from_data(v).to_heap_ptr() as u32 + Region::from_vec(v).to_heap_ptr() as u32 } fn _do_instantiate( diff --git a/packages/std/src/imports.rs b/packages/std/src/imports.rs index 18c52e699c..c819c97a5b 100644 --- a/packages/std/src/imports.rs +++ b/packages/std/src/imports.rs @@ -106,7 +106,7 @@ impl ExternalStorage { impl Storage for ExternalStorage { fn get(&self, key: &[u8]) -> Option> { - let key = Region::from_data(key); + let key = Region::from_slice(key); let key_ptr = key.as_ptr() as u32; let read = unsafe { db_read(key_ptr) }; @@ -126,17 +126,17 @@ impl Storage for ExternalStorage { panic!("TL;DR: Value must not be empty in Storage::set but in most cases you can use Storage::remove instead. Long story: Getting empty values from storage is not well supported at the moment. Some of our internal interfaces cannot differentiate between a non-existent key and an empty value. Right now, you cannot rely on the behaviour of empty values. To protect you from trouble later on, we stop here. Sorry for the inconvenience! We highly welcome you to contribute to CosmWasm, making this more solid one way or the other."); } - let key = Region::from_data(key); + let key = Region::from_slice(key); let key_ptr = key.as_ptr() as u32; - let value = Region::from_data(value); + let value = Region::from_slice(value); let value_ptr = value.as_ptr() as u32; unsafe { db_write(key_ptr, value_ptr) }; } fn remove(&mut self, key: &[u8]) { - let key = Region::from_data(key); + let key = Region::from_slice(key); let key_ptr = key.as_ptr() as u32; unsafe { db_remove(key_ptr) }; @@ -189,8 +189,8 @@ impl Storage for ExternalStorage { fn create_iter(start: Option<&[u8]>, end: Option<&[u8]>, order: Order) -> u32 { // There is lots of gotchas on turning options into regions for FFI, thus this design // See: https://github.com/CosmWasm/cosmwasm/pull/509 - let start_region = start.map(Region::from_data); - let end_region = end.map(Region::from_data); + let start_region = start.map(Region::from_slice); + let end_region = end.map(Region::from_slice); let start_region_addr = get_optional_region_address(&start_region.as_ref()); let end_region_addr = get_optional_region_address(&end_region.as_ref()); unsafe { db_scan(start_region_addr, end_region_addr, order as i32) } @@ -313,7 +313,7 @@ impl Api for ExternalApi { // Stop here to allow handling the error in the contract. return Err(StdError::generic_err("input too long for addr_validate")); } - let source = Region::from_data(input_bytes); + let source = Region::from_slice(input_bytes); let source_ptr = source.as_ptr() as u32; let result = unsafe { addr_validate(source_ptr) }; @@ -339,7 +339,7 @@ impl Api for ExternalApi { "input too long for addr_canonicalize", )); } - let send = Region::from_data(input_bytes); + let send = Region::from_slice(input_bytes); let send_ptr = send.as_ptr() as u32; let canon = Region::with_capacity(CANONICAL_ADDRESS_BUFFER_LENGTH); @@ -357,7 +357,7 @@ impl Api for ExternalApi { } fn addr_humanize(&self, canonical: &CanonicalAddr) -> StdResult { - let send = Region::from_data(canonical.as_slice()); + let send = Region::from_slice(canonical.as_slice()); let send_ptr = send.as_ptr() as u32; let human = Region::with_capacity(HUMAN_ADDRESS_BUFFER_LENGTH); @@ -383,11 +383,11 @@ impl Api for ExternalApi { signature: &[u8], public_key: &[u8], ) -> Result { - let hash_send = Region::from_data(message_hash); + let hash_send = Region::from_slice(message_hash); let hash_send_ptr = hash_send.as_ptr() as u32; - let sig_send = Region::from_data(signature); + let sig_send = Region::from_slice(signature); let sig_send_ptr = sig_send.as_ptr() as u32; - let pubkey_send = Region::from_data(public_key); + let pubkey_send = Region::from_slice(public_key); let pubkey_send_ptr = pubkey_send.as_ptr() as u32; let result = unsafe { secp256k1_verify(hash_send_ptr, sig_send_ptr, pubkey_send_ptr) }; @@ -409,9 +409,9 @@ impl Api for ExternalApi { signature: &[u8], recover_param: u8, ) -> Result, RecoverPubkeyError> { - let hash_send = Region::from_data(message_hash); + let hash_send = Region::from_slice(message_hash); let hash_send_ptr = hash_send.as_ptr() as u32; - let sig_send = Region::from_data(signature); + let sig_send = Region::from_slice(signature); let sig_send_ptr = sig_send.as_ptr() as u32; let result = @@ -439,11 +439,11 @@ impl Api for ExternalApi { signature: &[u8], public_key: &[u8], ) -> Result { - let hash_send = Region::from_data(message_hash); + let hash_send = Region::from_slice(message_hash); let hash_send_ptr = hash_send.as_ptr() as u32; - let sig_send = Region::from_data(signature); + let sig_send = Region::from_slice(signature); let sig_send_ptr = sig_send.as_ptr() as u32; - let pubkey_send = Region::from_data(public_key); + let pubkey_send = Region::from_slice(public_key); let pubkey_send_ptr = pubkey_send.as_ptr() as u32; let result = unsafe { secp256r1_verify(hash_send_ptr, sig_send_ptr, pubkey_send_ptr) }; @@ -466,9 +466,9 @@ impl Api for ExternalApi { signature: &[u8], recover_param: u8, ) -> Result, RecoverPubkeyError> { - let hash_send = Region::from_data(message_hash); + let hash_send = Region::from_slice(message_hash); let hash_send_ptr = hash_send.as_ptr() as u32; - let sig_send = Region::from_data(signature); + let sig_send = Region::from_slice(signature); let sig_send_ptr = sig_send.as_ptr() as u32; let result = @@ -495,11 +495,11 @@ impl Api for ExternalApi { signature: &[u8], public_key: &[u8], ) -> Result { - let msg_send = Region::from_data(message); + let msg_send = Region::from_slice(message); let msg_send_ptr = msg_send.as_ptr() as u32; - let sig_send = Region::from_data(signature); + let sig_send = Region::from_slice(signature); let sig_send_ptr = sig_send.as_ptr() as u32; - let pubkey_send = Region::from_data(public_key); + let pubkey_send = Region::from_slice(public_key); let pubkey_send_ptr = pubkey_send.as_ptr() as u32; let result = unsafe { ed25519_verify(msg_send_ptr, sig_send_ptr, pubkey_send_ptr) }; @@ -522,15 +522,15 @@ impl Api for ExternalApi { public_keys: &[&[u8]], ) -> Result { let msgs_encoded = encode_sections(messages); - let msgs_send = Region::from_data(msgs_encoded); + let msgs_send = Region::from_vec(msgs_encoded); let msgs_send_ptr = msgs_send.as_ptr() as u32; let sigs_encoded = encode_sections(signatures); - let sig_sends = Region::from_data(sigs_encoded); + let sig_sends = Region::from_vec(sigs_encoded); let sigs_send_ptr = sig_sends.as_ptr() as u32; let pubkeys_encoded = encode_sections(public_keys); - let pubkeys_send = Region::from_data(pubkeys_encoded); + let pubkeys_send = Region::from_vec(pubkeys_encoded); let pubkeys_send_ptr = pubkeys_send.as_ptr() as u32; let result = @@ -548,8 +548,8 @@ impl Api for ExternalApi { } fn debug(&self, message: &str) { - // keep the boxes in scope, so we free it at the end (don't cast to pointers same line as Region::from_data) - let region = Region::from_data(message.as_bytes()); + // keep the boxes in scope, so we free it at the end (don't cast to pointers same line as Region::from_slice) + let region = Region::from_slice(message.as_bytes()); let region_ptr = region.as_ptr() as u32; unsafe { debug(region_ptr) }; } @@ -574,7 +574,7 @@ impl ExternalQuerier { impl Querier for ExternalQuerier { fn raw_query(&self, bin_request: &[u8]) -> QuerierResult { - let req = Region::from_data(bin_request); + let req = Region::from_slice(bin_request); let request_ptr = req.as_ptr() as u32; let response_ptr = unsafe { query_chain(request_ptr) }; @@ -592,7 +592,7 @@ impl Querier for ExternalQuerier { #[cfg(feature = "abort")] pub fn handle_panic(message: &str) { - let region = Region::from_data(message.as_bytes()); + let region = Region::from_slice(message.as_bytes()); let region_ptr = region.as_ptr() as u32; unsafe { abort(region_ptr) }; } diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index c8f6fa6134..9f9ce02513 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -1,59 +1,6 @@ use alloc::vec::Vec; use core::{any::TypeId, marker::PhantomData, mem, ops::Deref, slice}; -/// Element that can be used to construct a new `Region` -/// -/// # Safety -/// -/// The following invariant must be upheld: -/// -/// - full allocated capacity == value returned by capacity -/// -/// This is important to uphold the safety invariant of the `dealloc` method, which requires us to pass the same Layout -/// into it as was used to allocate a memory region. -/// And since `size` is one of the parameters, it is important to pass in the exact same capacity. -/// -/// See: -pub unsafe trait RegionSource { - type Ownership: Ownership; - - fn ptr(&self) -> *const u8; - fn len(&self) -> usize; - fn capacity(&self) -> usize; -} - -unsafe impl RegionSource for &[u8] { - type Ownership = Borrowed; - - fn ptr(&self) -> *const u8 { - self.as_ptr() - } - - fn len(&self) -> usize { - (*self).len() - } - - fn capacity(&self) -> usize { - self.len() - } -} - -unsafe impl RegionSource for Vec { - type Ownership = Owned; - - fn ptr(&self) -> *const u8 { - self.as_ptr() - } - - fn len(&self) -> usize { - self.len() - } - - fn capacity(&self) -> usize { - self.capacity() - } -} - mod sealed { pub trait Sealed: 'static {} @@ -86,7 +33,20 @@ pub struct Region { _marker: PhantomData, } +impl Region { + pub fn from_slice(slice: &[u8]) -> Self { + unsafe { Self::from_parts(slice.as_ptr(), slice.len(), slice.len()) } + } +} + impl Region { + /// Construct a region from an existing vector + pub fn from_vec(vec: Vec) -> Self { + let region = unsafe { Self::from_parts(vec.as_ptr(), vec.capacity(), vec.len()) }; + mem::forget(vec); + region + } + /// Reconstruct a region from a raw pointer pointing to a `Box`. /// You'll want to use this when you received a region from the VM and want to dereference its contents. /// @@ -104,7 +64,7 @@ impl Region { /// Construct a new empty region with *at least* a capacity of what you passed in and a length of 0 pub fn with_capacity(cap: usize) -> Self { let data = Vec::with_capacity(cap); - let region = Self::from_data(data); + let region = Self::from_vec(data); region } @@ -121,37 +81,22 @@ impl Region { } } -impl Region +impl Region where - O: Ownership, + T: Ownership, { - /// Construct a new region from any kind of data that can be turned into a region - pub fn from_data(data: S) -> Self - where - S: RegionSource, - { + unsafe fn from_parts(ptr: *const u8, capacity: usize, length: usize) -> Self { // Well, this technically violates pointer provenance rules. // But there isn't a stable API for it, so that's the best we can do, I guess. - let region = Region { - offset: u32::try_from(data.ptr() as usize).expect("pointer doesn't fit in u32"), - capacity: u32::try_from(data.capacity()).expect("capacity doesn't fit in u32"), - length: u32::try_from(data.len()).expect("length doesn't fit in u32"), + Region { + offset: u32::try_from(ptr as usize).expect("pointer doesn't fit in u32"), + capacity: u32::try_from(capacity).expect("capacity doesn't fit in u32"), + length: u32::try_from(length).expect("length doesn't fit in u32"), _marker: PhantomData, - }; - - // We gonna forget this.. as a safety measure.. - // If we didn't do this and the `RegionSource` was a `Vec` we would deallocate it and that's BAD - mem::forget(data); - - region + } } -} -impl Region -where - T: Ownership, -{ /// Access the memory region this region points to in form of a byte slice pub fn as_bytes(&self) -> &[u8] { unsafe { slice::from_raw_parts(self.offset as *const u8, self.length as usize) } From 305b42cd59d93089d08eca94b8d4bdb9e278f745 Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 13:58:06 +0200 Subject: [PATCH 07/11] `T` -> `O` --- packages/std/src/memory.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index 9f9ce02513..38d8fd2378 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -22,7 +22,7 @@ pub struct Borrowed; /// /// This struct is crate internal since the cosmwasm-vm defines the same type independently. #[repr(C)] -pub struct Region { +pub struct Region { /// The beginning of the region expressed as bytes from the beginning of the linear memory pub offset: u32, /// The number of bytes available in this region @@ -30,7 +30,7 @@ pub struct Region { /// The number of bytes used in this region pub length: u32, - _marker: PhantomData, + _marker: PhantomData, } impl Region { @@ -81,9 +81,9 @@ impl Region { } } -impl Region +impl Region where - T: Ownership, + O: Ownership, { unsafe fn from_parts(ptr: *const u8, capacity: usize, length: usize) -> Self { // Well, this technically violates pointer provenance rules. @@ -119,9 +119,9 @@ where } } -impl Deref for Region +impl Deref for Region where - T: Ownership, + O: Ownership, { type Target = [u8]; @@ -130,13 +130,13 @@ where } } -impl Drop for Region +impl Drop for Region where - T: Ownership, + O: Ownership, { fn drop(&mut self) { // Since we can't specialize the drop impl we need to perform a runtime check - if TypeId::of::() == TypeId::of::() { + if TypeId::of::() == TypeId::of::() { let region_start = self.offset as *mut u8; // This case is explicitely disallowed by Vec From 383092d1dd9926755ff8b7e017d31ad0961c8317 Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 14:01:41 +0200 Subject: [PATCH 08/11] Add constant assertions to check size of a region --- packages/std/src/memory.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index 38d8fd2378..d45ae8c44b 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -33,6 +33,11 @@ pub struct Region { _marker: PhantomData, } +const _: () = { + assert!(mem::size_of::>() == 12); + assert!(mem::size_of::>() == 12); +}; + impl Region { pub fn from_slice(slice: &[u8]) -> Self { unsafe { Self::from_parts(slice.as_ptr(), slice.len(), slice.len()) } From 98861f18a9175d39c402d1a45f40801a4c3db3e6 Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 23:13:50 +0200 Subject: [PATCH 09/11] Optimize string decoding, simplify function --- packages/std/src/imports.rs | 4 +--- packages/std/src/memory.rs | 7 +------ 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/std/src/imports.rs b/packages/std/src/imports.rs index c819c97a5b..e1a7b500ca 100644 --- a/packages/std/src/imports.rs +++ b/packages/std/src/imports.rs @@ -371,9 +371,7 @@ impl Api for ExternalApi { ))); } - // TODO: Optimize this heap allocation away? - // I mean technically we save a bunch of heap allocation now anyway, but we can optimize this, too! - let address = unsafe { consume_string_region_written_by_vm(human.to_heap_ptr()) }; + let address = unsafe { String::from_utf8_unchecked(human.into_vec()) }; Ok(Addr::unchecked(address)) } diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index d45ae8c44b..2999f832c1 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -162,10 +162,5 @@ where /// or zero if not present #[cfg(feature = "iterator")] pub fn get_optional_region_address(region: &Option<&Region>) -> u32 { - /// Returns the address of the Region as an offset in linear memory - fn get_region_address(region: &Region) -> u32 { - region.as_ptr() as u32 - } - - region.map(get_region_address).unwrap_or(0) + region.map(|r| r.as_ptr() as u32).unwrap_or(0) } From 72d8e921ac16b079e6ec1e6977b9c34edd0e984c Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 23:15:07 +0200 Subject: [PATCH 10/11] Remove sealed pattern --- packages/std/src/memory.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index 2999f832c1..a4e24cfa1b 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -1,17 +1,11 @@ use alloc::vec::Vec; use core::{any::TypeId, marker::PhantomData, mem, ops::Deref, slice}; -mod sealed { - pub trait Sealed: 'static {} +pub trait Ownership: 'static {} - impl Sealed for super::Owned {} +impl Ownership for Borrowed {} - impl Sealed for super::Borrowed {} -} - -pub trait Ownership: sealed::Sealed + 'static {} - -impl Ownership for T where T: sealed::Sealed {} +impl Ownership for Owned {} pub struct Owned; From b588ce0a25476b4862c22aab7334a18c56def7ed Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 8 May 2024 23:25:10 +0200 Subject: [PATCH 11/11] Add some more docs --- packages/std/src/memory.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index a4e24cfa1b..15dd0af828 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -1,16 +1,19 @@ use alloc::vec::Vec; use core::{any::TypeId, marker::PhantomData, mem, ops::Deref, slice}; +/// This trait is used to indicate whether a region is borrowed or owned pub trait Ownership: 'static {} impl Ownership for Borrowed {} impl Ownership for Owned {} -pub struct Owned; - +/// This type is used to indicate that the region is borrowed and must not be deallocated pub struct Borrowed; +/// This type is used to indicate that the region is owned by the region and must be deallocated +pub struct Owned; + /// Describes some data allocated in Wasm's linear memory. /// A pointer to an instance of this can be returned over FFI boundaries. /// @@ -67,6 +70,7 @@ impl Region { region } + /// Transform the region into a vector pub fn into_vec(self) -> Vec { let vector = unsafe { Vec::from_raw_parts(