From 876fb163d557d2a269db79af275530421cd680f0 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Wed, 2 Aug 2023 20:48:59 -0700 Subject: [PATCH] Alloc example (#972) * Add EnvBase::augment_err_result to help gather debug info. * Charge linear memory growth delta to budget, not the total new size. * Add an example that does allocation using the SDK's alloc function. --------- Co-authored-by: Jay Geng --- soroban-env-common/src/env.rs | 12 +++++++ soroban-env-common/src/vmcaller_env.rs | 2 +- soroban-env-host/src/budget.rs | 5 +-- soroban-env-host/src/host.rs | 9 ++++++ soroban-env-host/src/host/error.rs | 21 ++++++++---- soroban-env-host/src/test/invocation.rs | 30 +++++++++++++++++- soroban-env-host/src/vm/dispatch.rs | 8 ++++- soroban-test-wasms/src/lib.rs | 1 + soroban-test-wasms/wasm-workspace/Cargo.lock | 7 ++++ soroban-test-wasms/wasm-workspace/Cargo.toml | 1 + .../wasm-workspace/alloc/Cargo.toml | 15 +++++++++ .../wasm-workspace/alloc/src/lib.rs | 27 ++++++++++++++++ .../wasm-workspace/opt/example_alloc.wasm | Bin 0 -> 2412 bytes 13 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 soroban-test-wasms/wasm-workspace/alloc/Cargo.toml create mode 100644 soroban-test-wasms/wasm-workspace/alloc/src/lib.rs create mode 100644 soroban-test-wasms/wasm-workspace/opt/example_alloc.wasm diff --git a/soroban-env-common/src/env.rs b/soroban-env-common/src/env.rs index dff4a438a..4fb35b78f 100644 --- a/soroban-env-common/src/env.rs +++ b/soroban-env-common/src/env.rs @@ -41,6 +41,18 @@ pub trait EnvBase: Sized + Clone { #[cfg(feature = "testutils")] fn escalate_error_to_panic(&self, e: Self::Error) -> !; + /// If `x` is `Err(...)`, ensure as much debug information as possible is + /// attached to that error; in any case return "essentially the same" `x` -- + /// either `Ok(...)` or `Err(...)` -- just with extra error context. + /// + /// This is called on a best-effort basis while propagating errors in the + /// host, to attach context "as soon as possible", and is necessary because + /// some errors are generated in contexts that do not have access to a Host, + /// and so cannot attach error context at the site of error generation. + fn augment_err_result(&self, x: Result) -> Result { + x + } + /// Used for recovering the concrete type of the Host. fn as_mut_any(&mut self) -> &mut dyn any::Any; diff --git a/soroban-env-common/src/vmcaller_env.rs b/soroban-env-common/src/vmcaller_env.rs index 9fa4bcafd..0c5e33098 100644 --- a/soroban-env-common/src/vmcaller_env.rs +++ b/soroban-env-common/src/vmcaller_env.rs @@ -173,7 +173,7 @@ macro_rules! vmcaller_none_function_helper { => { fn $fn_id(&self, $($arg:$type),*) -> Result<$ret, Self::Error> { - ::$fn_id(self, &mut VmCaller::none(), $($arg),*) + self.augment_err_result(::$fn_id(self, &mut VmCaller::none(), $($arg),*)) } }; } diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs index be4744b6d..3105450c7 100644 --- a/soroban-env-host/src/budget.rs +++ b/soroban-env-host/src/budget.rs @@ -1044,7 +1044,7 @@ impl Default for BudgetImpl { impl ResourceLimiter for Host { fn memory_growing( &mut self, - _current: usize, + current: usize, desired: usize, maximum: Option, ) -> Result { @@ -1063,8 +1063,9 @@ impl ResourceLimiter for Host { }; if allow { + let delta = (desired as u64).saturating_sub(current as u64); self.as_budget() - .bulk_charge(ContractCostType::WasmMemAlloc, desired as u64, None) + .bulk_charge(ContractCostType::WasmMemAlloc, delta, None) .map(|_| true) .map_err(|_| errors::MemoryError::OutOfBoundsGrowth) } else { diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 7375c3f70..2773c4dae 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -966,6 +966,15 @@ impl EnvBase for Host { panic!("{:?}", escalation) } + fn augment_err_result(&self, mut x: Result) -> Result { + if let Err(e) = &mut x { + if e.info.is_none() { + e.info = self.maybe_get_debug_info() + } + } + x + } + fn as_mut_any(&mut self) -> &mut dyn std::any::Any { todo!() } diff --git a/soroban-env-host/src/host/error.rs b/soroban-env-host/src/host/error.rs index b1640e432..4aa5d6394 100644 --- a/soroban-env-host/src/host/error.rs +++ b/soroban-env-host/src/host/error.rs @@ -57,7 +57,7 @@ impl Debug for HostError { || frame_name_matches(frame, "host::err") || frame_name_matches(frame, "Host::err") || frame_name_matches(frame, "Host>::err") - || frame_name_matches(frame, "::map_err") + || frame_name_matches(frame, "::augment_err_result") } writeln!(f, "HostError: {:?}", self.error)?; @@ -236,19 +236,28 @@ impl Host { if let Err(e) = self.err_diagnostics(events_refmut.deref_mut(), error, msg, args) { return e; } + } + let info = self.maybe_get_debug_info(); + return HostError { error, info }; + } + error.into() + } + + pub(crate) fn maybe_get_debug_info(&self) -> Option> { + if let Ok(true) = self.is_debug() { + if let Ok(events_ref) = self.0.events.try_borrow() { let events = match self .as_budget() - .with_free_budget(|| events_refmut.externalize(self)) + .with_free_budget(|| events_ref.externalize(self)) { Ok(events) => events, - Err(e) => return e, + Err(e) => return None, }; let backtrace = Backtrace::new_unresolved(); - let info = Some(Box::new(DebugInfo { backtrace, events })); - return HostError { error, info }; + return Some(Box::new(DebugInfo { backtrace, events })); } } - error.into() + None } // Some common error patterns here. diff --git a/soroban-env-host/src/test/invocation.rs b/soroban-env-host/src/test/invocation.rs index 291d27516..00f0c3ba3 100644 --- a/soroban-env-host/src/test/invocation.rs +++ b/soroban-env-host/src/test/invocation.rs @@ -9,7 +9,7 @@ use soroban_env_common::{ use crate::{ events::HostEvent, xdr::ScErrorType, ContractFunctionSet, Error, Host, HostError, Symbol, Tag, }; -use soroban_test_wasms::{ADD_I32, ERR, INVOKE_CONTRACT, VEC}; +use soroban_test_wasms::{ADD_I32, ALLOC, ERR, INVOKE_CONTRACT, VEC}; #[test] fn invoke_single_contract_function() -> Result<(), HostError> { @@ -36,6 +36,34 @@ fn invoke_single_contract_function() -> Result<(), HostError> { Ok(()) } +#[test] +fn invoke_alloc() -> Result<(), HostError> { + let host = Host::test_host_with_recording_footprint(); + host.enable_debug()?; + let contract_id_obj = host.register_test_contract_wasm(ALLOC); + let res = host.call( + contract_id_obj, + Symbol::try_from_small_str("sum")?, + host.test_vec_obj::(&[128])?, + )?; + assert!(res.shallow_eq(&8128_u32.into())); + let used_bytes = host.budget_cloned().get_mem_bytes_consumed()?; + // The general pattern of memory growth in this contract will be a sequence + // of vector-doublings, but these are masked by the fact that we only see + // the calls that cause the backing vector of wasm linear memory to grow, + // which happens as the guest vector crosses 64k boundaries (and eventually + // starts growing in steps larger than 64k itself). + // + // So we wind up with a growth-sequence that's a bit irregular: +0x10000, + // +0x20000, +0x30000, +0x50000, +0x90000. Total is 1 + 2 + 3 + 5 + 9 = 20 + // pages or about 1.3 MiB, plus the initial 17 pages (1.1MiB) plus some more + // slop from general host machinery allocations, we get around 2.5MiB. Call + // is "less than 3MiB". + assert!(used_bytes > (128 * 4096)); + assert!(used_bytes < 0x30_0000); + Ok(()) +} + fn invoke_cross_contract(diagnostics: bool) -> Result<(), HostError> { let host = Host::test_host_with_recording_footprint(); if diagnostics { diff --git a/soroban-env-host/src/vm/dispatch.rs b/soroban-env-host/src/vm/dispatch.rs index fe7eb308e..728404777 100644 --- a/soroban-env-host/src/vm/dispatch.rs +++ b/soroban-env-host/src/vm/dispatch.rs @@ -1,5 +1,5 @@ use super::FuelRefillable; -use crate::{xdr::ContractCostType, Host, HostError, VmCaller, VmCallerEnv}; +use crate::{xdr::ContractCostType, EnvBase, Host, HostError, VmCaller, VmCallerEnv}; use crate::{ AddressObject, Bool, BytesObject, DurationObject, Error, I128Object, I256Object, I256Val, I32Val, I64Object, MapObject, StorageType, StringObject, Symbol, SymbolObject, TimepointObject, @@ -164,6 +164,12 @@ macro_rules! generate_dispatch_functions { // wasmi::Value. let res: Result<_, HostError> = host.$fn_id(&mut vmcaller, $(<$type>::try_marshal_from_relative_value(Value::I64($arg), &host)?),*); + // On the off chance we got an error with no context, we can + // at least attach some here "at each host function call", + // fairly systematically. This will cause the context to + // propagate back through wasmi to its caller. + let res = host.augment_err_result(res); + let res = match res { Ok(ok) => { let val: Value = ok.marshal_relative_from_self(&host)?; diff --git a/soroban-test-wasms/src/lib.rs b/soroban-test-wasms/src/lib.rs index b428cb1fd..55f6edcce 100644 --- a/soroban-test-wasms/src/lib.rs +++ b/soroban-test-wasms/src/lib.rs @@ -44,6 +44,7 @@ pub const ADD_I32: &[u8] = include_bytes!("../wasm-workspace/opt/example_add_i32.wasm").as_slice(); pub const ADD_F32: &[u8] = include_bytes!("../wasm-workspace/opt/example_add_f32.wasm").as_slice(); +pub const ALLOC: &[u8] = include_bytes!("../wasm-workspace/opt/example_alloc.wasm").as_slice(); pub const CREATE_CONTRACT: &[u8] = include_bytes!("../wasm-workspace/opt/example_create_contract.wasm").as_slice(); pub const CONTRACT_STORAGE: &[u8] = diff --git a/soroban-test-wasms/wasm-workspace/Cargo.lock b/soroban-test-wasms/wasm-workspace/Cargo.lock index a356197f2..0587451fa 100644 --- a/soroban-test-wasms/wasm-workspace/Cargo.lock +++ b/soroban-test-wasms/wasm-workspace/Cargo.lock @@ -408,6 +408,13 @@ dependencies = [ "soroban-sdk", ] +[[package]] +name = "example_alloc" +version = "0.0.0" +dependencies = [ + "soroban-sdk", +] + [[package]] name = "example_complex" version = "0.0.0" diff --git a/soroban-test-wasms/wasm-workspace/Cargo.toml b/soroban-test-wasms/wasm-workspace/Cargo.toml index 266f4c74b..b0e8d683c 100644 --- a/soroban-test-wasms/wasm-workspace/Cargo.toml +++ b/soroban-test-wasms/wasm-workspace/Cargo.toml @@ -19,6 +19,7 @@ resolver = "2" members = [ "add_i32", "add_f32", + "alloc", "auth", "fib", "contract_data", diff --git a/soroban-test-wasms/wasm-workspace/alloc/Cargo.toml b/soroban-test-wasms/wasm-workspace/alloc/Cargo.toml new file mode 100644 index 000000000..1b50386fe --- /dev/null +++ b/soroban-test-wasms/wasm-workspace/alloc/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "example_alloc" +version = "0.0.0" +authors = ["Stellar Development Foundation "] +license = "Apache-2.0" +edition = "2021" +publish = false +rust-version = "1.65" + +[lib] +crate-type = ["cdylib", "rlib"] +doctest = false + +[dependencies] +soroban-sdk = { workspace = true, features = ["alloc"] } diff --git a/soroban-test-wasms/wasm-workspace/alloc/src/lib.rs b/soroban-test-wasms/wasm-workspace/alloc/src/lib.rs new file mode 100644 index 000000000..89bc1e972 --- /dev/null +++ b/soroban-test-wasms/wasm-workspace/alloc/src/lib.rs @@ -0,0 +1,27 @@ +#![no_std] +use soroban_sdk::{contract, contractimpl, Env}; + +extern crate alloc; + +#[contract] +pub struct AllocContract; + +struct Large { + x: u32, + space: [u8;4096] +} + +#[contractimpl] +impl AllocContract { + /// Allocates a temporary vector holding values (0..count), then computes + /// and returns their sum. Also allocates these values in a "large" + /// structure (with a bunch of pointless padding) to ensure the contract + /// allocates lots of memory. + pub fn sum(_env: Env, count: u32) -> u32 { + let mut v1 = alloc::vec![]; + for i in 0..count { + v1.push(Large{x: i, space: [0u8; 4096]}) + } + v1.iter().map(|l| l.x + l.space[0] as u32).sum() + } +} diff --git a/soroban-test-wasms/wasm-workspace/opt/example_alloc.wasm b/soroban-test-wasms/wasm-workspace/opt/example_alloc.wasm new file mode 100644 index 0000000000000000000000000000000000000000..f12e30fc0ea885295573ad47fdfb50520901c9a2 GIT binary patch literal 2412 zcmZuzO>f&q5Zzg>Bub=Y*>Qr@0ooyxg99|G9DgOfR5XyZh7lC#AFvsjj!nsuMbb_K z1d)OUMGigmo?{RF8v*)P`X72JkiJ=3PKwrmvCG}rd2il)Kq}5pNhw7ul>vNwUTEdy z=cjPLubjsI{$X6i`^o6gA#Z>GIEl~ppT>FO=z5J*L>`&OyH@S5I^CGwR#AHpF5FS9 zc0SNgJLU(|QywqqDGvDU@4<3uQGrEe$80;w>2ZZ=OS`(Kd)(&bZ%vPhE34W(Zj!G( zJ&jN`N;xBzD#to^-Pp^gNdu=6E`pvr#q`tL7xBV7b_gib(rBD1BJ*(t-^(l@ac zMq9cKmbk`lS=M$MP)V%;YL1=?9j?Pg)q+EBDgz?W7ER0}TsH8*b*{E6+*s9aK(hE& zWU*HPwILfU${ib|@N1>mK;6KIbjTZAdt76W zn}hIr^l}p3z#%vhH<50*k|<3;rK2ZJc(Z|l2sAZNjcc;hd?iUY;=(J>@wy0?qMMVT z30_MB*)6drs(5pEfPsIDe_{QG9xoNyf4zey++PQw04NNWNS|g8kyu}*jFB#5lnMg1 zk1;|uTb4MiVpFD?kZMWiWd@NjAF?)th$|EtBDjHx)UIR}`INXFN2#1uIb;por#JAv;>xmv^?KRrGo`oBas-7E$Q< z*3N_BwbJmKFhT)GEbS(ZXbZW8h|os%y`2Y-q@Nhae)P*_RYEhSTZ>9T=vLXT^F~Ht z+lCgoWL#h_w5#0S(leHJa;0BIkG9AzodsOb{zPS7Kv8UJ%&ebx%$se;W!h1gR!nzg zU&g**7w;hv+Hc+m5aSyas_4T92lK|BGKOLiLQqwqr%1R#F}5|TtW={!w7HkGVI)WeK@?Hx4jEG#~HSy~&7f-GPHSs(g;!5M?*^TqynRAh0#NJbZ@NfBRkRr$A!WkWr0&F}wp zS;_K?Br6SDYn|=&PWRsE2@A=?%(bv@zMQ_jqdtkck}7d z`o@E`wRm%Td$a%We*dW3f6(9BTu(OFw&CM9eTAH#CH*T}e@4UMxE~iu&M_Cs>Df4o zvlo1k^ow!E$K&DQ$>R~=46zMH9Ueusd(cf&~RMj^0xeq(?R^@uN*f