Skip to content

Commit

Permalink
[vm-rewrite] Fix tests (#20917)
Browse files Browse the repository at this point in the history
## Description 

Update how we handle deserialization errors so they maintain their
specificity. Also updates a bad merge.

## Test plan 

CI

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] gRPC:
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
  • Loading branch information
tzakian authored Jan 18, 2025
1 parent 3f841af commit 1a343dc
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ use crate::{
cache::identifier_interner::IdentifierKey,
jit::execution::ast::{Datatype, EnumType, Function, Module, Package, StructType, Type},
shared::{
constants::{MAX_TYPE_INSTANTIATION_NODES, MAX_TYPE_TO_LAYOUT_NODES, VALUE_DEPTH_MAX},
constants::{
HISTORICAL_MAX_TYPE_TO_LAYOUT_NODES, MAX_TYPE_INSTANTIATION_NODES, VALUE_DEPTH_MAX,
},
types::RuntimePackageId,
vm_pointer::VMPointer,
},
Expand All @@ -30,6 +32,7 @@ use move_core_types::{

use move_binary_format::file_format::DatatypeTyParameter;
use move_core_types::{annotated_value as A, identifier::Identifier, runtime_value as R};
use move_vm_config::runtime::VMConfig;
use parking_lot::RwLock;

use std::{collections::BTreeMap, sync::Arc};
Expand All @@ -52,6 +55,7 @@ use tracing::error;
/// vtable/cross-package function resolution but we will keep it simple for now.
#[derive(Debug, Clone)]
pub struct VMDispatchTables {
pub(crate) vm_config: Arc<VMConfig>,
pub(crate) loaded_packages: BTreeMap<RuntimePackageId, Arc<Package>>,
}

Expand Down Expand Up @@ -142,8 +146,14 @@ pub struct DepthFormula {
impl VMDispatchTables {
/// Create a new RuntimeVTables instance.
/// NOTE: This assumes linkage has already occured.
pub fn new(loaded_packages: BTreeMap<RuntimePackageId, Arc<Package>>) -> VMResult<Self> {
Ok(Self { loaded_packages })
pub fn new(
vm_config: Arc<VMConfig>,
loaded_packages: BTreeMap<RuntimePackageId, Arc<Package>>,
) -> VMResult<Self> {
Ok(Self {
vm_config,
loaded_packages,
})
}

pub fn get_package(&self, id: &RuntimePackageId) -> PartialVMResult<Arc<Package>> {
Expand Down Expand Up @@ -654,7 +664,12 @@ impl VMDispatchTables {
count: &mut u64,
depth: u64,
) -> PartialVMResult<runtime_value::MoveTypeLayout> {
if *count > MAX_TYPE_TO_LAYOUT_NODES {
if *count
> self
.vm_config
.max_type_to_layout_nodes
.unwrap_or(HISTORICAL_MAX_TYPE_TO_LAYOUT_NODES)
{
return Err(PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES));
}
if depth > VALUE_DEPTH_MAX {
Expand Down Expand Up @@ -795,7 +810,12 @@ impl VMDispatchTables {
count: &mut u64,
depth: u64,
) -> PartialVMResult<annotated_value::MoveTypeLayout> {
if *count > MAX_TYPE_TO_LAYOUT_NODES {
if *count
> self
.vm_config
.max_type_to_layout_nodes
.unwrap_or(HISTORICAL_MAX_TYPE_TO_LAYOUT_NODES)
{
return Err(PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES));
}
if depth > VALUE_DEPTH_MAX {
Expand Down
14 changes: 4 additions & 10 deletions external-crates/move/crates/move-vm-runtime/src/execution/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,6 @@ impl<'extensions> MoveVM<'extensions> {
// deserialization should be done outside of the VM calls.
let (ref_ids, deserialized_args) = deserialize_args(
&self.virtual_tables,
&self.vm_config,
&mut self.base_heap,
arg_types,
serialized_args,
Expand All @@ -348,22 +347,17 @@ impl<'extensions> MoveVM<'extensions> {
deserialized_args,
)?;

let serialized_return_values = serialize_return_values(
&self.virtual_tables,
&self.vm_config,
&return_types,
return_values,
)
.map_err(|e| e.finish(Location::Undefined))?;
let serialized_return_values =
serialize_return_values(&self.virtual_tables, &return_types, return_values)
.map_err(|e| e.finish(Location::Undefined))?;
let serialized_mut_ref_outputs = mut_ref_args
.into_iter()
.map(|(ndx, ty)| {
let heap_ref_id = ref_ids.get(&ndx).expect("No heap ref for ref argument");
// take the value of each reference; return values first in the case that a value
// points into this local
let local_val = self.base_heap.take_loc(*heap_ref_id)?;
let (bytes, layout) =
serialize_return_value(&self.virtual_tables, &self.vm_config, &ty, local_val)?;
let (bytes, layout) = serialize_return_value(&self.virtual_tables, &ty, local_val)?;
Ok((ndx as LocalIndex, bytes, layout))
})
.collect::<PartialVMResult<_>>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl MoveRuntime {
.map(|pkg| (pkg.runtime.runtime_id, Arc::clone(&pkg.runtime)))
.collect::<BTreeMap<RuntimePackageId, Arc<jit::execution::ast::Package>>>();

let virtual_tables = VMDispatchTables::new(runtime_packages)?;
let virtual_tables = VMDispatchTables::new(self.vm_config.clone(), runtime_packages)?;

let base_heap = BaseHeap::new();

Expand Down Expand Up @@ -200,7 +200,7 @@ impl MoveRuntime {
.map(|pkg| (pkg.runtime.runtime_id, Arc::clone(&pkg.runtime)))
.collect::<BTreeMap<RuntimePackageId, Arc<jit::execution::ast::Package>>>();

let virtual_tables = VMDispatchTables::new(runtime_packages)?;
let virtual_tables = VMDispatchTables::new(self.vm_config.clone(), runtime_packages)?;

let base_heap = BaseHeap::new();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub const VALUE_DEPTH_MAX: u64 = 128;
/// fields for struct types.
/// Maximal nodes which are allowed when converting to layout. This includes the types of
/// fields for datatypes.
pub const MAX_TYPE_TO_LAYOUT_NODES: u64 = 256;
pub const HISTORICAL_MAX_TYPE_TO_LAYOUT_NODES: u64 = 256;

/// Maximal nodes which are all allowed when instantiating a generic type. This does not include
/// field types of datatypes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use move_binary_format::{
file_format::LocalIndex,
};
use move_core_types::{runtime_value::MoveTypeLayout, vm_status::StatusCode};
use move_vm_config::runtime::VMConfig;

use tracing::warn;

Expand Down Expand Up @@ -68,7 +67,6 @@ pub fn deserialize_value(
/// Returns the list of mutable references plus the vector of values.
pub fn deserialize_args(
vtables: &VMDispatchTables,
_vm_config: &VMConfig,
heap: &mut BaseHeap,
arg_tys: Vec<Type>,
serialized_args: Vec<impl Borrow<[u8]>>,
Expand Down Expand Up @@ -106,7 +104,6 @@ pub fn deserialize_args(

pub fn serialize_return_value(
vtables: &VMDispatchTables,
vm_config: &VMConfig,
ty: &Type,
value: Value,
) -> PartialVMResult<(Vec<u8>, MoveTypeLayout)> {
Expand All @@ -123,7 +120,7 @@ pub fn serialize_return_value(
_ => (ty, value),
};

let layout = if vm_config.rethrow_serialization_type_layout_errors {
let layout = if vtables.vm_config.rethrow_serialization_type_layout_errors {
vtables.type_to_type_layout(ty)?
} else {
vtables.type_to_type_layout(ty).map_err(|_err| {
Expand All @@ -142,7 +139,6 @@ pub fn serialize_return_value(

pub fn serialize_return_values(
vtables: &VMDispatchTables,
vm_config: &VMConfig,
return_types: &[Type],
return_values: Vec<Value>,
) -> PartialVMResult<Vec<(Vec<u8>, MoveTypeLayout)>> {
Expand All @@ -161,6 +157,6 @@ pub fn serialize_return_values(
return_types
.iter()
.zip(return_values)
.map(|(ty, value)| serialize_return_value(vtables, vm_config, ty, value))
.map(|(ty, value)| serialize_return_value(vtables, ty, value))
.collect()
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,7 @@ pub(crate) fn package(vm_config: &VMConfig, pkg: SerializedPackage) -> VMResult<
let mut modules = BTreeMap::new();
for module in pkg.modules.iter() {
let module = CompiledModule::deserialize_with_config(module, &vm_config.binary_config)
.map_err(|err| -> move_binary_format::errors::VMError {
let msg = format!("Deserialization error: {:?}", err);
PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR)
.with_message(msg)
.finish(Location::Undefined) // TODO(tzakian): add Location::Package
})?;
.map_err(|err| err.finish(Location::Undefined))?; // TODO: add Location::Package
modules.insert(module.self_id(), module);
}

Expand Down

0 comments on commit 1a343dc

Please sign in to comment.