From c5a4b9644d18645335416973104a72cbb7c2bd6e Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Thu, 13 Jul 2023 01:04:39 -0700 Subject: [PATCH] WIP relative objects --- soroban-env-common/src/env.rs | 7 +++ soroban-env-common/src/val.rs | 24 ++++---- soroban-env-common/src/wrapper_macros.rs | 18 ++++-- soroban-env-host/src/host.rs | 62 ++++++++++++++++++-- soroban-env-host/src/host/frame.rs | 19 +++++- soroban-env-host/src/host_object.rs | 74 +++++++++++++++++++++--- soroban-env-host/src/test/basic.rs | 4 +- soroban-env-host/src/test/vec.rs | 4 +- soroban-env-host/src/vm.rs | 6 +- soroban-env-host/src/vm/dispatch.rs | 4 +- 10 files changed, 181 insertions(+), 41 deletions(-) diff --git a/soroban-env-common/src/env.rs b/soroban-env-common/src/env.rs index dff4a438a..56db210d2 100644 --- a/soroban-env-common/src/env.rs +++ b/soroban-env-common/src/env.rs @@ -139,6 +139,13 @@ pub trait EnvBase: Sized + Clone { /// events are enabled. When running on host, logs directly; when running on /// guest, redirects through log_from_linear_memory. fn log_from_slice(&self, msg: &str, vals: &[Val]) -> Result; + + fn make_relative(&self, val: Val) -> Val { + val + } + fn make_absolute(&self, val: Val) -> Val { + val + } } /////////////////////////////////////////////////////////////////////////////// diff --git a/soroban-env-common/src/val.rs b/soroban-env-common/src/val.rs index 531ac0688..ebc055ed1 100644 --- a/soroban-env-common/src/val.rs +++ b/soroban-env-common/src/val.rs @@ -1,7 +1,7 @@ use crate::{ declare_tag_based_object_wrapper, declare_tag_based_wrapper, impl_rawval_wrapper_base, - impl_tryfroms_and_tryfromvals_delegating_to_rawvalconvertible, Compare, I32Val, SymbolSmall, - SymbolStr, U32Val, + impl_tryfroms_and_tryfromvals_delegating_to_rawvalconvertible, Compare, EnvBase, I32Val, + SymbolSmall, SymbolStr, U32Val, }; use stellar_xdr::{ScError, ScValType}; @@ -374,28 +374,28 @@ impl_tryfroms_and_tryfromvals_delegating_to_rawvalconvertible!(Error); #[cfg(feature = "wasmi")] pub trait WasmiMarshal: Sized { - fn try_marshal_from_value(v: wasmi::Value) -> Option; - fn marshal_from_self(self) -> wasmi::Value; + fn try_marshal_from_value(env: &E, v: wasmi::Value) -> Option; + fn marshal_from_self(self, env: &E) -> wasmi::Value; } #[cfg(feature = "wasmi")] impl WasmiMarshal for Val { - fn try_marshal_from_value(v: wasmi::Value) -> Option { + fn try_marshal_from_value(env: &E, v: wasmi::Value) -> Option { if let wasmi::Value::I64(i) = v { - Some(Val::from_payload(i as u64)) + Some(env.make_absolute(Val::from_payload(i as u64))) } else { None } } - fn marshal_from_self(self) -> wasmi::Value { - wasmi::Value::I64(self.get_payload() as i64) + fn marshal_from_self(self, env: &E) -> wasmi::Value { + wasmi::Value::I64(env.make_relative(self).get_payload() as i64) } } #[cfg(feature = "wasmi")] impl WasmiMarshal for u64 { - fn try_marshal_from_value(v: wasmi::Value) -> Option { + fn try_marshal_from_value(_env: &E, v: wasmi::Value) -> Option { if let wasmi::Value::I64(i) = v { Some(i as u64) } else { @@ -403,14 +403,14 @@ impl WasmiMarshal for u64 { } } - fn marshal_from_self(self) -> wasmi::Value { + fn marshal_from_self(self, _env: &E) -> wasmi::Value { wasmi::Value::I64(self as i64) } } #[cfg(feature = "wasmi")] impl WasmiMarshal for i64 { - fn try_marshal_from_value(v: wasmi::Value) -> Option { + fn try_marshal_from_value(_env: &E, v: wasmi::Value) -> Option { if let wasmi::Value::I64(i) = v { Some(i) } else { @@ -418,7 +418,7 @@ impl WasmiMarshal for i64 { } } - fn marshal_from_self(self) -> wasmi::Value { + fn marshal_from_self(self, _env: &E) -> wasmi::Value { wasmi::Value::I64(self) } } diff --git a/soroban-env-common/src/wrapper_macros.rs b/soroban-env-common/src/wrapper_macros.rs index be521826c..daef802d0 100644 --- a/soroban-env-common/src/wrapper_macros.rs +++ b/soroban-env-common/src/wrapper_macros.rs @@ -84,9 +84,12 @@ macro_rules! impl_wrapper_wasmi_conversions { // wasmi / VM argument support #[cfg(feature = "wasmi")] impl $crate::WasmiMarshal for $wrapper { - fn try_marshal_from_value(v: wasmi::Value) -> Option { + fn try_marshal_from_value( + env: &E, + v: wasmi::Value, + ) -> Option { if let wasmi::Value::I64(i) = v { - let val = $crate::Val::from_payload(i as u64); + let val = env.make_absolute($crate::Val::from_payload(i as u64)); if ::is_val_type(val) { return Some(unsafe { ::unchecked_from_val(val) @@ -96,8 +99,8 @@ macro_rules! impl_wrapper_wasmi_conversions { None } - fn marshal_from_self(self) -> wasmi::Value { - wasmi::Value::I64(self.as_val().get_payload() as i64) + fn marshal_from_self(self, env: &E) -> wasmi::Value { + wasmi::Value::I64(env.make_relative(self.to_val()).get_payload() as i64) } } }; @@ -189,7 +192,10 @@ macro_rules! declare_wasmi_marshal_for_enum { ($ENUM:ident) => { #[cfg(feature = "wasmi")] impl $crate::WasmiMarshal for $ENUM { - fn try_marshal_from_value(v: wasmi::Value) -> Option { + fn try_marshal_from_value( + _env: &E, + v: wasmi::Value, + ) -> Option { if let wasmi::Value::I64(i) = v { use num_traits::FromPrimitive; $ENUM::from_i64(i) @@ -198,7 +204,7 @@ macro_rules! declare_wasmi_marshal_for_enum { } } - fn marshal_from_self(self) -> wasmi::Value { + fn marshal_from_self(self, _env: &E) -> wasmi::Value { wasmi::Value::I64(self as i64) } } diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index d58f8ea6a..36d76060c 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -10,7 +10,10 @@ use crate::{ err, events::{diagnostic::DiagnosticLevel, Events, InternalEventsBuffer}, expiration_ledger_bumps::{ExpirationLedgerBumps, LedgerBump}, - host_object::{HostMap, HostObject, HostObjectType, HostVec}, + host_object::{ + handle_to_index, index_to_handle, is_relative_object_handle, HostMap, HostObject, + HostObjectType, HostVec, + }, impl_bignum_host_fns_rhs_u32, impl_wrapping_obj_from_num, impl_wrapping_obj_to_num, num::*, storage::{InstanceStorageMap, Storage}, @@ -1098,6 +1101,55 @@ impl EnvBase for Host { fn log_from_slice(&self, msg: &str, vals: &[Val]) -> Result { self.log_diagnostics(msg, vals).map(|_| Void::from(())) } + + fn make_absolute(&self, val: Val) -> Val { + if let Ok(obj) = Object::try_from(val) { + let handle = obj.get_handle(); + if is_relative_object_handle(handle) { + let index = handle_to_index(handle); + // FIXME: error handling + return self + .with_current_context_mut(|ctx| match ctx.relative_objects.get(index) { + Some(abs) => Ok(*abs), + None => Err(self.err( + ScErrorType::Context, + ScErrorCode::InvalidInput, + "unknown relative object reference", + &[val], + )), + }) + .unwrap() + .to_val(); + } else { + // FIXME: error handling + self.err( + ScErrorType::Context, + ScErrorCode::InvalidInput, + "make_absolute given an absolute reference", + &[val], + ); + } + } + val + } + + fn make_relative(&self, val: Val) -> Val { + if let Ok(obj) = Object::try_from(val) { + let handle = obj.get_handle(); + if !is_relative_object_handle(handle) { + // FIXME: error handling + return self + .with_current_context_mut(|ctx| { + let index = ctx.relative_objects.len(); + let handle = index_to_handle(self, index, true)?; + ctx.relative_objects.push(obj); + Ok(Object::from_handle_and_tag(handle, val.get_tag()).to_val()) + }) + .unwrap(); + } + } + val + } } impl VmCallerEnv for Host { @@ -1665,7 +1717,7 @@ impl VmCallerEnv for Host { &vm, vals_pos, vals.as_mut_slice(), - |buf| Val::from_payload(u64::from_le_bytes(*buf)), + |buf| self.make_absolute(Val::from_payload(u64::from_le_bytes(*buf))), )?; for v in vals.iter() { self.check_val_integrity(*v)?; @@ -1726,7 +1778,7 @@ impl VmCallerEnv for Host { &vm, vals_pos.into(), mapobj.map.as_slice(), - |pair| u64::to_le_bytes(pair.1.get_payload()), + |pair| u64::to_le_bytes(self.make_relative(pair.1).get_payload()), )?; Ok(()) })?; @@ -1955,7 +2007,7 @@ impl VmCallerEnv for Host { &vm, pos, vals.as_mut_slice(), - |buf| Val::from_payload(u64::from_le_bytes(*buf)), + |buf| self.make_absolute(Val::from_payload(u64::from_le_bytes(*buf))), )?; for v in vals.iter() { self.check_val_integrity(*v)?; @@ -1977,7 +2029,7 @@ impl VmCallerEnv for Host { &vm, vals_pos.into(), vecobj.as_slice(), - |x| u64::to_le_bytes(x.get_payload()), + |x| u64::to_le_bytes(self.make_relative(*x).get_payload()), ) })?; Ok(Val::VOID) diff --git a/soroban-env-host/src/host/frame.rs b/soroban-env-host/src/host/frame.rs index 81ad16c87..243335120 100644 --- a/soroban-env-host/src/host/frame.rs +++ b/soroban-env-host/src/host/frame.rs @@ -9,7 +9,7 @@ use crate::{ err, storage::{InstanceStorageMap, StorageMap}, xdr::{ContractCostType, ContractExecutable, Hash, HostFunction, HostFunctionType, ScVal}, - Error, Host, HostError, Symbol, SymbolStr, TryFromVal, TryIntoVal, Val, + Error, Host, HostError, Object, Symbol, SymbolStr, TryFromVal, TryIntoVal, Val, }; #[cfg(any(test, feature = "testutils"))] @@ -88,6 +88,7 @@ impl TestContractFrame { pub(crate) struct Context { pub(crate) frame: Frame, prng: Option, + pub(crate) relative_objects: Vec, pub(crate) storage: Option, } @@ -125,6 +126,7 @@ impl Host { frame, prng: None, storage: None, + relative_objects: Vec::new(), }; self.try_borrow_context_mut()?.push(ctx); Ok(RollbackPoint { @@ -246,6 +248,21 @@ impl Host { } } + pub(crate) fn with_current_context_mut_opt(&self, f: F) -> Result + where + F: FnOnce(Option<&mut Context>) -> Result, + { + let Ok(mut context_guard) = self.0.context.try_borrow_mut() else { + return Err(self.err(ScErrorType::Context, ScErrorCode::InternalError, "context is already borrowed", &[])); + }; + if let Some(context) = context_guard.last_mut() { + f(Some(context)) + } else { + drop(context_guard); + f(None) + } + } + pub(crate) fn with_current_prng(&self, f: F) -> Result where F: FnOnce(&mut Prng) -> Result, diff --git a/soroban-env-host/src/host_object.rs b/soroban-env-host/src/host_object.rs index 004a707fa..cd34d664d 100644 --- a/soroban-env-host/src/host_object.rs +++ b/soroban-env-host/src/host_object.rs @@ -1,6 +1,9 @@ +#![allow(dead_code)] + use soroban_env_common::{ - xdr::ContractCostType, Compare, DurationSmall, I128Small, I256Small, I64Small, SymbolSmall, - SymbolStr, Tag, TimepointSmall, U128Small, U256Small, U64Small, + xdr::{ContractCostType, ScErrorCode, ScErrorType}, + Compare, DurationSmall, I128Small, I256Small, I64Small, SymbolSmall, SymbolStr, Tag, + TimepointSmall, U128Small, U256Small, U64Small, }; use crate::{ @@ -169,6 +172,55 @@ declare_mem_host_object_type!(xdr::ScString, StringObject, String); declare_mem_host_object_type!(xdr::ScSymbol, SymbolObject, Symbol); declare_host_object_type!(xdr::ScAddress, AddressObject, Address); +// Objects come in two flavors: relative and absolute. Relative objects are the +// ones we pass to and from wasm code, and are looked up indirectly through a +// per-context table to find their absolute values. Absolute objects are the +// same in all contexts (and outside contexts, eg. in host objects themselves or +// while setting-up the host). Relative-to-absolute translation is done very +// close to the VM, when marshalling. Host code should never see relative object +// handles, and if you ever try to look one up in the host object table, it will +// fail. +// +// Also note: the relative/absolute object reference translation is _not_ done +// when running in native / local-testing mode, so you will not get identical +// object numbers in that case. Since there is no real isolation between +// contracts in that mode, there's no point bothering with the translation (and +// there's no really obvious place to perform it systematically, like in the +// wasm marshalling path). + +pub fn is_relative_object_handle(handle: u32) -> bool { + handle & 1 == 0 +} + +pub fn is_relative_object(obj: Object) -> bool { + is_relative_object_handle(obj.get_handle()) +} + +pub fn is_relative_object_value(val: Val) -> bool { + if let Ok(obj) = Object::try_from(val) { + is_relative_object(obj) + } else { + false + } +} + +pub fn handle_to_index(handle: u32) -> usize { + (handle as usize) >> 1 +} + +pub fn index_to_handle(host: &Host, index: usize, relative: bool) -> Result { + if let Ok(smaller) = u32::try_from(index) { + if let Some(shifted) = smaller.checked_shl(1) { + if relative { + return Ok(shifted | 0); + } else { + return Ok(shifted | 1); + } + } + } + Err(host.err_arith_overflow()) +} + impl Host { /// Moves a value of some type implementing [`HostObjectType`] into the host's /// object array, returning a [`HostObj`] containing the new object's array @@ -177,15 +229,12 @@ impl Host { &self, hot: HOT, ) -> Result { - let prev_len = self.try_borrow_objects()?.len(); - if prev_len > u32::MAX as usize { - return Err(self.err_arith_overflow()); - } + let index = self.try_borrow_objects()?.len(); + let handle = index_to_handle(self, index, false)?; // charge for the new host object, which is just the amortized cost of a single // `HostObject` allocation metered_clone::charge_heap_alloc::(1, self.as_budget())?; self.try_borrow_objects_mut()?.push(HOT::inject(hot)); - let handle = prev_len as u32; Ok(HOT::new_from_handle(handle)) } @@ -203,7 +252,16 @@ impl Host { let r = self.try_borrow_objects()?; let obj: Object = obj.into(); let handle: u32 = obj.get_handle(); - f(r.get(handle as usize)) + if is_relative_object_handle(handle) { + Err(self.err( + ScErrorType::Context, + ScErrorCode::InternalError, + "looking up relative object", + &[obj.to_val()], + )) + } else { + f(r.get(handle_to_index(handle))) + } } pub(crate) fn check_val_integrity(&self, val: Val) -> Result<(), HostError> { diff --git a/soroban-env-host/src/test/basic.rs b/soroban-env-host/src/test/basic.rs index 64352090d..b3e13dc81 100644 --- a/soroban-env-host/src/test/basic.rs +++ b/soroban-env-host/src/test/basic.rs @@ -16,7 +16,7 @@ fn u64_roundtrip() -> Result<(), HostError> { let v2: Val = u2.try_into_val(&host)?; assert_eq!(v2.get_tag(), Tag::U64Object); let obj: Object = v2.try_into()?; - assert_eq!(obj.get_handle(), 0); + assert_eq!(obj.get_handle(), 1); let k = u64::try_from_val(&host, &v2)?; assert_eq!(u2, k); Ok(()) @@ -35,7 +35,7 @@ fn i64_roundtrip() -> Result<(), HostError> { let v2: Val = i2.try_into_val(&host)?; assert_eq!(v2.get_tag(), Tag::I64Object); let obj: Object = v2.try_into()?; - assert_eq!(obj.get_handle(), 0); + assert_eq!(obj.get_handle(), 1); let k = i64::try_from_val(&host, &v2)?; assert_eq!(i2, k); Ok(()) diff --git a/soroban-env-host/src/test/vec.rs b/soroban-env-host/src/test/vec.rs index 480a858be..b9d87cd2b 100644 --- a/soroban-env-host/src/test/vec.rs +++ b/soroban-env-host/src/test/vec.rs @@ -16,8 +16,8 @@ fn vec_as_seen_by_host() -> Result<(), HostError> { assert_eq!(val1.get_tag(), Tag::VecObject); let obj0: Object = val0.try_into()?; let obj1: Object = val1.try_into()?; - assert_eq!(obj0.get_handle(), 0); - assert_eq!(obj1.get_handle(), 1); + assert_eq!(obj0.get_handle(), 1); + assert_eq!(obj1.get_handle(), 3); assert_eq!(obj0.as_val().get_tag(), Tag::VecObject); assert_eq!(obj1.as_val().get_tag(), Tag::VecObject); // Check that we got 2 distinct Vec objects diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index a0de31fae..be3ff7823 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -21,7 +21,7 @@ use func_info::HOST_FUNCTIONS; use soroban_env_common::{ meta::{self, get_ledger_protocol_version, get_pre_release_version}, xdr::{ReadXdr, ScEnvMetaEntry, ScErrorCode, ScErrorType}, - ConversionError, SymbolStr, TryIntoVal, WasmiMarshal, + ConversionError, EnvBase, SymbolStr, TryIntoVal, WasmiMarshal, }; use wasmi::{Engine, FuelConsumptionMode, Func, Instance, Linker, Memory, Module, Store, Value}; @@ -269,7 +269,7 @@ impl Vm { } } Ok( - <_ as WasmiMarshal>::try_marshal_from_value(wasm_ret[0].clone()) + <_ as WasmiMarshal>::try_marshal_from_value(host, wasm_ret[0].clone()) .ok_or(ConversionError)?, ) } @@ -283,7 +283,7 @@ impl Vm { host.charge_budget(ContractCostType::InvokeVmFunction, None)?; let wasm_args: Vec = args .iter() - .map(|i| Value::I64(i.get_payload() as i64)) + .map(|i| Value::I64(host.make_relative(*i).get_payload() as i64)) .collect(); let func_ss: SymbolStr = func_sym.try_into_val(host)?; let ext = match self diff --git a/soroban-env-host/src/vm/dispatch.rs b/soroban-env-host/src/vm/dispatch.rs index 505e97d67..58c6ce364 100644 --- a/soroban-env-host/src/vm/dispatch.rs +++ b/soroban-env-host/src/vm/dispatch.rs @@ -90,11 +90,11 @@ macro_rules! generate_dispatch_functions { // happens to be a natural switching point for that: we have // conversions to and from both Val and i64 / u64 for // wasmi::Value. - let res: Result<_, HostError> = host.$fn_id(&mut vmcaller, $(<$type>::try_marshal_from_value(Value::I64($arg)).ok_or(BadSignature)?),*); + let res: Result<_, HostError> = host.$fn_id(&mut vmcaller, $(<$type>::try_marshal_from_value(&host, Value::I64($arg)).ok_or(BadSignature)?),*); let res = match res { Ok(ok) => { - let val: Value = ok.marshal_from_self(); + let val: Value = ok.marshal_from_self(&host); if let Value::I64(v) = val { Ok((v,)) } else {