Skip to content

Commit

Permalink
Misc int32 issues (#926)
Browse files Browse the repository at this point in the history
* Mop up a couple missed cases from PR #925

* Cap vector and map sizes to u32::max

* Cast usize to u64 before multiplying, for platform-consistency.
  • Loading branch information
graydon authored Jul 5, 2023
1 parent 1f7ca98 commit 10e631f
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 17 deletions.
2 changes: 1 addition & 1 deletion soroban-env-host/benches/common/cost_types/vec_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl HostCostMeasurement for VecEntryMeasure {
fn new_random_case(_host: &Host, rng: &mut StdRng, input: u64) -> VecEntrySample {
let input = 1 + input * Self::STEP_SIZE;
let ov = util::to_rawval_u32(0..(input as u32)).collect();
let vec: MeteredVector<_> = MeteredVector::from_vec(ov);
let vec: MeteredVector<_> = MeteredVector::from_vec(ov).unwrap();
let mut idxs: Vec<usize> = (0..input as usize).collect();
idxs.shuffle(rng);
VecEntrySample { vec, idxs }
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/benches/common/measure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ where
),
{
let mut recycled_samples = Vec::with_capacity(samples.len());
host.as_budget().reset_unlimited();
host.as_budget().reset_unlimited().unwrap();

// start the cpu and mem measurement
mem_tracker.0.store(0, Ordering::SeqCst);
Expand Down
4 changes: 2 additions & 2 deletions soroban-env-host/benches/common/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use soroban_env_host::{budget::AsBudget, Host, Val, I256};

pub(crate) fn test_host() -> Host {
let host = Host::default();
host.as_budget().reset_unlimited();
host.as_budget().reset_fuel_config();
host.as_budget().reset_unlimited().unwrap();
host.as_budget().reset_fuel_config().unwrap();
host
}

Expand Down
9 changes: 5 additions & 4 deletions soroban-env-host/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1724,12 +1724,13 @@ impl VmCallerEnv for Host {
}

fn vec_new(&self, _vmcaller: &mut VmCaller<Host>, c: Val) -> Result<VecObject, HostError> {
let capacity: usize = if c.is_void() {
// NB: we ignore capacity because vectors are immutable
// and there's no reuse, we always size them exactly.
let _capacity: usize = if c.is_void() {
0
} else {
self.usize_from_rawval_u32_input("c", c)?
};
// TODO: optimize the vector based on capacity
self.add_host_object(HostVec::new())
}

Expand Down Expand Up @@ -1941,7 +1942,7 @@ impl VmCallerEnv for Host {
vals.as_mut_slice(),
|buf| Val::from_payload(u64::from_le_bytes(*buf)),
)?;
self.add_host_object(HostVec::from_vec(vals))
self.add_host_object(HostVec::from_vec(vals)?)
}

fn vec_unpack_to_linear_memory(
Expand Down Expand Up @@ -2824,7 +2825,7 @@ impl VmCallerEnv for Host {
let inner = MeteredVector::from_array(&vals, self.as_budget())?;
outer.push(self.add_host_object(inner)?.into());
}
self.add_host_object(HostVec::from_vec(outer))
self.add_host_object(HostVec::from_vec(outer)?)
}

fn fail_with_error(
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/host/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ impl Host {
for e in v.iter() {
vv.push(self.to_host_val(e)?)
}
Ok(self.add_host_object(HostVec::from_vec(vv))?.into())
Ok(self.add_host_object(HostVec::from_vec(vv)?)?.into())
}
ScVal::Map(Some(m)) => {
metered_clone::charge_heap_alloc::<(Val, Val)>(m.len() as u64, self.as_budget())?;
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/host/mem_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl Host {
let mem_data = vm.get_memory(self)?.data(vmcaller.try_mut()?);
self.charge_budget(
ContractCostType::VmMemRead,
Some(num_slices.saturating_mul(8) as u64),
Some((num_slices as u64).saturating_mul(8)),
)?;

for i in 0..num_slices {
Expand Down
3 changes: 3 additions & 0 deletions soroban-env-host/src/host/metered_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ where
}

pub fn from_map(map: Vec<(K, V)>, ctx: &Ctx) -> Result<Self, HostError> {
if u32::try_from(map.len()).is_err() {
return Err((ScErrorType::Object, ScErrorCode::ExceededLimit).into());
}
// Allocation cost already paid for by caller, here just checks that input
// has sorted and unique keys.
let m = MeteredOrdMap {
Expand Down
17 changes: 10 additions & 7 deletions soroban-env-host/src/host/metered_vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ where
{
// Constructs a empty new `MeteredVector`.
pub fn new() -> Self {
Self::from_vec(Vec::new())
Self { vec: Vec::new() }
}

// Constructs a new, empty `MeteredVector` with at least the specified capacity.
Expand All @@ -60,19 +60,22 @@ where
#[cfg(any(test, feature = "testutils"))]
pub fn with_capacity(capacity: usize, budget: &Budget) -> Result<Self, HostError> {
super::metered_clone::charge_heap_alloc::<A>(capacity as u64, budget)?;
Ok(Self::from_vec(Vec::with_capacity(capacity)))
Self::from_vec(Vec::with_capacity(capacity))
}

pub fn from_array(buf: &[A], budget: &Budget) -> Result<Self, HostError> {
// we may temporarily go over budget here.
let vec: Vec<A> = buf.into();
vec.charge_deep_clone(budget)?;
Ok(Self::from_vec(vec))
Self::from_vec(vec)
}

// No meter charge, assuming allocation cost has been covered by the caller from the outside.
pub fn from_vec(vec: Vec<A>) -> Self {
Self { vec }
pub fn from_vec(vec: Vec<A>) -> Result<Self, HostError> {
if u32::try_from(vec.len()).is_err() {
return Err((ScErrorType::Object, ScErrorCode::ExceededLimit).into());
}
Ok(Self { vec })
}

pub fn as_slice(&self) -> &[A] {
Expand All @@ -98,7 +101,7 @@ where
// the clone into one (when A::IS_SHALLOW==true).
let vec: Vec<A> = iter.collect();
vec.charge_deep_clone(budget)?;
Ok(Self::from_vec(vec))
Self::from_vec(vec)
} else {
// This is a logic error, we should never get here.
Err((ScErrorType::Object, ScErrorCode::InternalError).into())
Expand Down Expand Up @@ -289,7 +292,7 @@ where
}
}
vec.charge_deep_clone(budget)?;
Ok(Self::from_vec(vec))
Self::from_vec(vec)
}

pub fn iter(&self) -> std::slice::Iter<'_, A> {
Expand Down

0 comments on commit 10e631f

Please sign in to comment.