Skip to content

Commit

Permalink
Turn dynamic borrow panics into HostErrors (#925)
Browse files Browse the repository at this point in the history
  • Loading branch information
graydon authored Jul 4, 2023
1 parent 7ae4467 commit 8b423d0
Show file tree
Hide file tree
Showing 25 changed files with 384 additions and 253 deletions.
86 changes: 43 additions & 43 deletions soroban-env-host/src/budget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::{
use soroban_env_common::xdr::{ScErrorCode, ScErrorType};

use crate::{
host::error::TryBorrowOrErr,
xdr::{ContractCostParamEntry, ContractCostParams, ContractCostType, ExtensionPoint},
Host, HostError,
};
Expand Down Expand Up @@ -412,13 +413,13 @@ pub struct Budget(pub(crate) Rc<RefCell<BudgetImpl>>);

impl Debug for Budget {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "{:?}", self.0.borrow())
writeln!(f, "{:?}", self.0.try_borrow().map_err(|_| std::fmt::Error)?)
}
}

impl Display for Budget {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "{}", self.0.borrow())
writeln!(f, "{}", self.0.try_borrow().map_err(|_| std::fmt::Error)?)
}
}

Expand Down Expand Up @@ -465,7 +466,7 @@ impl Budget {
where
F: FnOnce(RefMut<BudgetImpl>) -> Result<T, HostError>,
{
f(self.0.borrow_mut())
f(self.0.try_borrow_mut_or_err()?)
}

fn charge_in_bulk(
Expand All @@ -474,7 +475,7 @@ impl Budget {
iterations: u64,
input: Option<u64>,
) -> Result<(), HostError> {
if !self.0.borrow().enabled {
if !self.0.try_borrow_or_err()?.enabled {
return Ok(());
}

Expand Down Expand Up @@ -562,98 +563,97 @@ impl Budget {
res
}

pub fn get_tracker(&self, ty: ContractCostType) -> (u64, Option<u64>) {
self.0.borrow().tracker[ty as usize]
pub fn get_tracker(&self, ty: ContractCostType) -> Result<(u64, Option<u64>), HostError> {
Ok(self.0.try_borrow_or_err()?.tracker[ty as usize])
}

pub(crate) fn get_tracker_mut<F>(&self, ty: ContractCostType, f: F) -> Result<(), HostError>
where
F: FnOnce(&mut (u64, Option<u64>)) -> Result<(), HostError>,
{
f(&mut self.0.borrow_mut().tracker[ty as usize])
f(&mut self.0.try_borrow_mut_or_err()?.tracker[ty as usize])
}

pub fn get_cpu_insns_consumed(&self) -> u64 {
self.0.borrow().cpu_insns.get_total_count()
pub fn get_cpu_insns_consumed(&self) -> Result<u64, HostError> {
Ok(self.0.try_borrow_or_err()?.cpu_insns.get_total_count())
}

pub fn get_mem_bytes_consumed(&self) -> u64 {
self.0.borrow().mem_bytes.get_total_count()
pub fn get_mem_bytes_consumed(&self) -> Result<u64, HostError> {
Ok(self.0.try_borrow_or_err()?.mem_bytes.get_total_count())
}

pub fn get_cpu_insns_remaining(&self) -> u64 {
self.0.borrow().cpu_insns.get_remaining()
pub fn get_cpu_insns_remaining(&self) -> Result<u64, HostError> {
Ok(self.0.try_borrow_or_err()?.cpu_insns.get_remaining())
}

pub fn get_mem_bytes_remaining(&self) -> u64 {
self.0.borrow().mem_bytes.get_remaining()
pub fn get_mem_bytes_remaining(&self) -> Result<u64, HostError> {
Ok(self.0.try_borrow_or_err()?.mem_bytes.get_remaining())
}

pub fn reset_default(&self) {
*self.0.borrow_mut() = BudgetImpl::default()
pub fn reset_default(&self) -> Result<(), HostError> {
*self.0.try_borrow_mut_or_err()? = BudgetImpl::default();
Ok(())
}

pub fn reset_unlimited(&self) {
self.reset_unlimited_cpu();
self.reset_unlimited_mem();
pub fn reset_unlimited(&self) -> Result<(), HostError> {
self.reset_unlimited_cpu()?;
self.reset_unlimited_mem()?;
Ok(())
}

pub fn reset_unlimited_cpu(&self) {
pub fn reset_unlimited_cpu(&self) -> Result<(), HostError> {
self.mut_budget(|mut b| {
b.cpu_insns.reset(u64::MAX);
Ok(())
})
.unwrap(); // panic means multiple-mut-borrow bug
})?; // panic means multiple-mut-borrow bug
self.reset_tracker()
}

pub fn reset_unlimited_mem(&self) {
pub fn reset_unlimited_mem(&self) -> Result<(), HostError> {
self.mut_budget(|mut b| {
b.mem_bytes.reset(u64::MAX);
Ok(())
})
.unwrap(); // panic means multiple-mut-borrow bug
})?;
self.reset_tracker()
}

pub fn reset_tracker(&self) {
for tracker in self.0.borrow_mut().tracker.iter_mut() {
pub fn reset_tracker(&self) -> Result<(), HostError> {
for tracker in self.0.try_borrow_mut_or_err()?.tracker.iter_mut() {
tracker.0 = 0;
tracker.1 = tracker.1.map(|_| 0);
}
Ok(())
}

pub fn reset_limits(&self, cpu: u64, mem: u64) {
pub fn reset_limits(&self, cpu: u64, mem: u64) -> Result<(), HostError> {
self.mut_budget(|mut b| {
b.cpu_insns.reset(cpu);
b.mem_bytes.reset(mem);
Ok(())
})
.unwrap(); // impossible to panic

})?;
self.reset_tracker()
}

#[cfg(test)]
pub fn reset_models(&self) {
pub fn reset_models(&self) -> Result<(), HostError> {
self.mut_budget(|mut b| {
b.cpu_insns.reset_models();
b.mem_bytes.reset_models();
Ok(())
})
.unwrap(); // impossible to panic
}

#[cfg(any(test, feature = "testutils"))]
pub fn reset_fuel_config(&self) {
self.0.borrow_mut().fuel_config.reset()
pub fn reset_fuel_config(&self) -> Result<(), HostError> {
self.0.try_borrow_mut_or_err()?.fuel_config.reset();
Ok(())
}

fn get_cpu_insns_remaining_as_fuel(&self) -> Result<u64, HostError> {
let cpu_remaining = self.get_cpu_insns_remaining();
let cpu_remaining = self.get_cpu_insns_remaining()?;
let cpu_per_fuel = self
.0
.borrow()
.try_borrow_or_err()?
.cpu_insns
.get_cost_model(ContractCostType::WasmInsnExec)
.linear_term;
Expand All @@ -676,10 +676,10 @@ impl Budget {
}

fn get_mem_bytes_remaining_as_fuel(&self) -> Result<u64, HostError> {
let bytes_remaining = self.get_mem_bytes_remaining();
let bytes_remaining = self.get_mem_bytes_remaining()?;
let bytes_per_fuel = self
.0
.borrow()
.try_borrow_or_err()?
.mem_bytes
.get_cost_model(ContractCostType::WasmMemAlloc)
.linear_term;
Expand All @@ -699,15 +699,15 @@ impl Budget {
}

// generate a wasmi fuel cost schedule based on our calibration
pub fn wasmi_fuel_costs(&self) -> FuelCosts {
let config = &self.0.borrow().fuel_config;
pub fn wasmi_fuel_costs(&self) -> Result<FuelCosts, HostError> {
let config = &self.0.try_borrow_or_err()?.fuel_config;
let mut costs = FuelCosts::default();
costs.base = config.base;
costs.entity = config.entity;
costs.load = config.load;
costs.store = config.store;
costs.call = config.call;
costs
Ok(costs)
}
}

Expand Down
34 changes: 26 additions & 8 deletions soroban-env-host/src/cost_runner/cost_types/vm_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,19 @@ impl CostRunner for VmMemReadRun {
_iter: u64,
mut sample: Self::SampleType,
) -> Self::RecycledType {
black_box(sample.vm.with_vmcaller(|caller| {
host.metered_vm_read_bytes_from_linear_memory(caller, &sample.vm, 0, &mut sample.buf)
.unwrap()
}));
black_box(
sample
.vm
.with_vmcaller(|caller| {
host.metered_vm_read_bytes_from_linear_memory(
caller,
&sample.vm,
0,
&mut sample.buf,
)
})
.unwrap(),
);
sample
}

Expand All @@ -82,10 +91,19 @@ impl CostRunner for VmMemWriteRun {
_iter: u64,
mut sample: Self::SampleType,
) -> Self::RecycledType {
black_box(sample.vm.with_vmcaller(|caller| {
host.metered_vm_write_bytes_to_linear_memory(caller, &sample.vm, 0, &mut sample.buf)
.unwrap()
}));
black_box(
sample
.vm
.with_vmcaller(|caller| {
host.metered_vm_write_bytes_to_linear_memory(
caller,
&sample.vm,
0,
&mut sample.buf,
)
})
.unwrap(),
);
sample
}

Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/cost_runner/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,6 @@ pub trait CostRunner: Sized {
/// actual input from the host's perspective. So use it carefully. This should be
/// after the `run`, outside of the CPU-and-memory tracking machineary.
fn get_tracker(host: &Host) -> (u64, Option<u64>) {
host.0.budget.get_tracker(Self::COST_TYPE)
host.0.budget.get_tracker(Self::COST_TYPE).unwrap()
}
}
22 changes: 13 additions & 9 deletions soroban-env-host/src/events/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,21 @@ pub enum DiagnosticLevel {

/// None of these functions are metered, which is why they're behind the is_debug check
impl Host {
pub fn set_diagnostic_level(&self, diagnostic_level: DiagnosticLevel) {
*self.0.diagnostic_level.borrow_mut() = diagnostic_level;
pub fn set_diagnostic_level(&self, diagnostic_level: DiagnosticLevel) -> Result<(), HostError> {
*self.try_borrow_diagnostic_level_mut()? = diagnostic_level;
Ok(())
}

// As above, avoids having to import DiagnosticLevel.
pub fn enable_debug(&self) {
pub fn enable_debug(&self) -> Result<(), HostError> {
self.set_diagnostic_level(DiagnosticLevel::Debug)
}

pub fn is_debug(&self) -> bool {
matches!(*self.0.diagnostic_level.borrow(), DiagnosticLevel::Debug)
pub fn is_debug(&self) -> Result<bool, HostError> {
Ok(matches!(
*self.try_borrow_diagnostic_level()?,
DiagnosticLevel::Debug
))
}

/// Records a `System` contract event. `topics` is expected to be a `SCVec`
Expand Down Expand Up @@ -70,7 +74,7 @@ impl Host {
}

pub fn log_diagnostics(&self, msg: &str, args: &[Val]) -> Result<(), HostError> {
if !self.is_debug() {
if !self.is_debug()? {
return Ok(());
}
let calling_contract = self.get_current_contract_id_unmetered()?;
Expand All @@ -92,7 +96,7 @@ impl Host {
msg: &str,
args: &[Val],
) -> Result<(), HostError> {
if !self.is_debug() {
if !self.is_debug()? {
return Ok(());
}

Expand Down Expand Up @@ -130,7 +134,7 @@ impl Host {
func: &Symbol,
args: &[Val],
) -> Result<(), HostError> {
if !self.is_debug() {
if !self.is_debug()? {
return Ok(());
}

Expand Down Expand Up @@ -162,7 +166,7 @@ impl Host {
func: &Symbol,
res: &Val,
) -> Result<(), HostError> {
if !self.is_debug() {
if !self.is_debug()? {
return Ok(());
}

Expand Down
4 changes: 2 additions & 2 deletions soroban-env-host/src/events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,11 @@ impl Host {
where
F: FnOnce(&mut InternalEventsBuffer) -> Result<U, HostError>,
{
f(&mut self.0.events.borrow_mut())
f(&mut *self.try_borrow_events_mut()?)
}

pub fn get_events(&self) -> Result<Events, HostError> {
self.0.events.borrow().externalize(self)
self.try_borrow_events()?.externalize(self)
}

// Records a contract event.
Expand Down
Loading

0 comments on commit 8b423d0

Please sign in to comment.