diff --git a/Cargo.lock b/Cargo.lock index 029fba529c85a..51f328bea6faf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11995,6 +11995,7 @@ dependencies = [ "itertools 0.13.0", "move-binary-format", "move-core-types", + "parking_lot 0.12.1", "proptest", "rand 0.7.3", "serde", diff --git a/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs b/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs index d6a96952a88e7..97ed268e84641 100644 --- a/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs +++ b/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs @@ -190,7 +190,10 @@ pub(crate) fn validate_combine_signer_and_txn_args( Ok(combined_args) } -// Return whether the argument is valid/allowed and whether it needs construction. +/// Returns true if the argument is valid (that is, it is a primitive type or a struct with a +/// known constructor function). Otherwise, (for structs without constructors, signers or +/// references) returns false. An error is returned in cases when a struct type is encountered and +/// its name cannot be queried for some reason. pub(crate) fn is_valid_txn_arg( session: &SessionExt, module_storage: &impl AptosModuleStorage, @@ -202,12 +205,21 @@ pub(crate) fn is_valid_txn_arg( match ty { Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address => true, Vector(inner) => is_valid_txn_arg(session, module_storage, inner, allowed_structs), - Struct { idx, .. } | StructInstantiation { idx, .. } => session - .fetch_struct_ty_by_idx(*idx, module_storage) - .is_some_and(|st| { - let full_name = format!("{}::{}", st.module.short_str_lossless(), st.name); - allowed_structs.contains_key(&full_name) - }), + Struct { .. } | StructInstantiation { .. } => { + // Note: Original behavior was to return false even if the module loading fails (e.g., + // if struct does not exist. This preserves it. + session + .get_struct_name(ty, module_storage) + .ok() + .flatten() + .is_some_and(|(module_id, identifier)| { + allowed_structs.contains_key(&format!( + "{}::{}", + module_id.short_str_lossless(), + identifier + )) + }) + }, Signer | Reference(_) | MutableReference(_) | TyParam(_) => false, } } @@ -340,12 +352,16 @@ pub(crate) fn recursively_construct_arg( len -= 1; } }, - Struct { idx, .. } | StructInstantiation { idx, .. } => { - let st = session - .fetch_struct_ty_by_idx(*idx, module_storage) + Struct { .. } | StructInstantiation { .. } => { + let (module_id, identifier) = session + .get_struct_name(ty, module_storage) + .map_err(|_| { + // Note: The original behaviour was to map all errors to an invalid signature + // error, here we want to preserve it for now. + invalid_signature() + })? .ok_or_else(invalid_signature)?; - - let full_name = format!("{}::{}", st.module.short_str_lossless(), st.name); + let full_name = format!("{}::{}", module_id.short_str_lossless(), identifier); let constructor = allowed_structs .get(&full_name) .ok_or_else(invalid_signature)?; diff --git a/third_party/move/move-vm/integration-tests/Cargo.toml b/third_party/move/move-vm/integration-tests/Cargo.toml index 9a592c082f8e3..e88fc6987a0c0 100644 --- a/third_party/move/move-vm/integration-tests/Cargo.toml +++ b/third_party/move/move-vm/integration-tests/Cargo.toml @@ -21,7 +21,7 @@ move-core-types = { workspace = true } move-stdlib = { path = "../../move-stdlib" } move-vm-runtime = { workspace = true, features = ["testing"] } move-vm-test-utils = { workspace = true } -move-vm-types = { workspace = true } +move-vm-types = { workspace = true, features = ["testing"] } smallvec = { workspace = true } tempfile = { workspace = true } diff --git a/third_party/move/move-vm/integration-tests/src/tests/module_storage_tests.rs b/third_party/move/move-vm/integration-tests/src/tests/module_storage_tests.rs index 7e17e4bb44af9..6b7471e835b5f 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/module_storage_tests.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/module_storage_tests.rs @@ -15,7 +15,10 @@ use move_vm_runtime::{ }; use move_vm_test_utils::InMemoryStorage; use move_vm_types::{ - loaded_data::runtime_types::{AbilityInfo, StructIdentifier, StructNameIndex, TypeBuilder}, + loaded_data::{ + runtime_types::{AbilityInfo, StructIdentifier, TypeBuilder}, + struct_name_indexing::StructNameIndex, + }, value_serde::FunctionValueExtension, }; use std::str::FromStr; @@ -74,7 +77,7 @@ fn test_function_value_extension() { let foo_ty = types.pop().unwrap(); let name = module_storage .runtime_environment() - .idx_to_struct_name_for_test(StructNameIndex(0)) + .idx_to_struct_name_for_test(StructNameIndex::new(0)) .unwrap(); assert_eq!(name, StructIdentifier { module: test_id.clone(), @@ -84,8 +87,10 @@ fn test_function_value_extension() { foo_ty, ty_builder .create_ref_ty( - &ty_builder - .create_struct_ty(StructNameIndex(0), AbilityInfo::struct_(AbilitySet::EMPTY)), + &ty_builder.create_struct_ty( + StructNameIndex::new(0), + AbilityInfo::struct_(AbilitySet::EMPTY) + ), false ) .unwrap() @@ -121,7 +126,7 @@ fn test_function_value_extension() { let bar_ty = types.pop().unwrap(); let name = module_storage .runtime_environment() - .idx_to_struct_name_for_test(StructNameIndex(1)) + .idx_to_struct_name_for_test(StructNameIndex::new(1)) .unwrap(); assert_eq!(name, StructIdentifier { module: other_test_id, @@ -130,7 +135,7 @@ fn test_function_value_extension() { assert_eq!( bar_ty, ty_builder.create_struct_ty( - StructNameIndex(1), + StructNameIndex::new(1), AbilityInfo::struct_(AbilitySet::from_u8(Ability::Drop as u8).unwrap()) ) ); diff --git a/third_party/move/move-vm/runtime/src/loader/mod.rs b/third_party/move/move-vm/runtime/src/loader/mod.rs index 2753b3775b850..fe5ec818db599 100644 --- a/third_party/move/move-vm/runtime/src/loader/mod.rs +++ b/third_party/move/move-vm/runtime/src/loader/mod.rs @@ -28,10 +28,15 @@ use move_core_types::{ value::MoveTypeLayout, vm_status::StatusCode, }; +use move_vm_metrics::{Timer, VM_TIMER}; use move_vm_types::{ gas::GasMeter, - loaded_data::runtime_types::{AbilityInfo, StructNameIndex, StructType, Type}, + loaded_data::{ + runtime_types::{AbilityInfo, StructType, Type}, + struct_name_indexing::StructNameIndex, + }, sha3_256, + value_serde::FunctionValueExtension, }; use parking_lot::{Mutex, RwLock}; use std::{ @@ -39,6 +44,7 @@ use std::{ hash::Hash, sync::Arc, }; +use type_loader::intern_type; use typed_arena::Arena; mod access_specifier_loader; @@ -51,8 +57,7 @@ use crate::{ loader::modules::{StructVariantInfo, VariantFieldInfo}, native_functions::NativeFunctions, storage::{ - loader::LoaderV2, module_storage::FunctionValueExtensionAdapter, - struct_name_index_map::StructNameIndexMap, ty_cache::StructInfoCache, + loader::LoaderV2, module_storage::FunctionValueExtensionAdapter, ty_cache::StructInfoCache, ty_layout_converter::LoaderLayoutConverter, ty_tag_converter::TypeTagConverter, }, }; @@ -64,14 +69,12 @@ use move_binary_format::file_format::{ StructVariantHandleIndex, StructVariantInstantiationIndex, TypeParameterIndex, VariantFieldHandleIndex, VariantFieldInstantiationIndex, VariantIndex, }; -use move_vm_metrics::{Timer, VM_TIMER}; -use move_vm_types::{ - loaded_data::runtime_types::{DepthFormula, StructLayout, TypeBuilder}, - value_serde::FunctionValueExtension, +use move_vm_types::loaded_data::{ + runtime_types::{DepthFormula, StructLayout, TypeBuilder}, + struct_name_indexing::StructNameIndexMap, }; pub use script::Script; pub(crate) use script::ScriptCache; -use type_loader::intern_type; type ScriptHash = [u8; 32]; diff --git a/third_party/move/move-vm/runtime/src/loader/modules.rs b/third_party/move/move-vm/runtime/src/loader/modules.rs index 388363f1ded0e..db0f27e24a5f3 100644 --- a/third_party/move/move-vm/runtime/src/loader/modules.rs +++ b/third_party/move/move-vm/runtime/src/loader/modules.rs @@ -9,7 +9,6 @@ use crate::{ BinaryCache, }, native_functions::NativeFunctions, - storage::struct_name_index_map::StructNameIndexMap, }; use move_binary_format::{ access::ModuleAccess, @@ -29,8 +28,9 @@ use move_core_types::{ vm_status::StatusCode, }; use move_vm_metrics::{Timer, VM_TIMER}; -use move_vm_types::loaded_data::runtime_types::{ - StructIdentifier, StructLayout, StructNameIndex, StructType, Type, +use move_vm_types::loaded_data::{ + runtime_types::{StructIdentifier, StructLayout, StructType, Type}, + struct_name_indexing::{StructNameIndex, StructNameIndexMap}, }; use parking_lot::RwLock; use std::{ @@ -575,7 +575,6 @@ impl Module { ) -> PartialVMResult { let struct_handle = module.struct_handle_at(struct_def.struct_handle); let abilities = struct_handle.abilities; - let name = module.identifier_at(struct_handle.name).to_owned(); let ty_params = struct_handle.type_parameters.clone(); let layout = match &struct_def.field_information { StructFieldInformation::Native => unreachable!("native structs have been removed"), @@ -613,8 +612,6 @@ impl Module { abilities, ty_params, idx: struct_name_table[struct_def.struct_handle.0 as usize], - module: module.self_id(), - name, }) } diff --git a/third_party/move/move-vm/runtime/src/loader/script.rs b/third_party/move/move-vm/runtime/src/loader/script.rs index cddfdea0f3548..7c3b9beac6c54 100644 --- a/third_party/move/move-vm/runtime/src/loader/script.rs +++ b/third_party/move/move-vm/runtime/src/loader/script.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use super::{intern_type, BinaryCache, Function, FunctionHandle, FunctionInstantiation}; -use crate::{loader::ScriptHash, storage::struct_name_index_map::StructNameIndexMap}; +use crate::loader::ScriptHash; use move_binary_format::{ access::ScriptAccess, binary_views::BinaryIndexedView, @@ -13,6 +13,7 @@ use move_core_types::{identifier::Identifier, language_storage::ModuleId, vm_sta use move_vm_types::loaded_data::{ runtime_access_specifier::AccessSpecifier, runtime_types::{StructIdentifier, Type}, + struct_name_indexing::StructNameIndexMap, }; use std::{collections::BTreeMap, ops::Deref, sync::Arc}; diff --git a/third_party/move/move-vm/runtime/src/loader/type_loader.rs b/third_party/move/move-vm/runtime/src/loader/type_loader.rs index c37a7513e4856..dbea652d3c703 100644 --- a/third_party/move/move-vm/runtime/src/loader/type_loader.rs +++ b/third_party/move/move-vm/runtime/src/loader/type_loader.rs @@ -7,7 +7,10 @@ use move_binary_format::{ file_format::SignatureToken, }; use move_core_types::vm_status::StatusCode; -use move_vm_types::loaded_data::runtime_types::{AbilityInfo, StructNameIndex, Type}; +use move_vm_types::loaded_data::{ + runtime_types::{AbilityInfo, Type}, + struct_name_indexing::StructNameIndex, +}; use triomphe::Arc as TriompheArc; /// Converts a signature token into the in memory type representation used by the MoveVM. diff --git a/third_party/move/move-vm/runtime/src/session.rs b/third_party/move/move-vm/runtime/src/session.rs index 7799a95b2953c..831c1e93e696d 100644 --- a/third_party/move/move-vm/runtime/src/session.rs +++ b/third_party/move/move-vm/runtime/src/session.rs @@ -18,17 +18,17 @@ use move_core_types::{ account_address::AccountAddress, effects::{ChangeSet, Changes}, gas_algebra::NumBytes, - identifier::IdentStr, + identifier::{IdentStr, Identifier}, language_storage::{ModuleId, TypeTag}, value::MoveTypeLayout, vm_status::StatusCode, }; use move_vm_types::{ gas::GasMeter, - loaded_data::runtime_types::{StructNameIndex, StructType, Type, TypeBuilder}, + loaded_data::runtime_types::{Type, TypeBuilder}, values::{GlobalValue, Value}, }; -use std::{borrow::Borrow, collections::BTreeSet, sync::Arc}; +use std::{borrow::Borrow, collections::BTreeSet}; pub struct Session<'r, 'l> { pub(crate) move_vm: &'l MoveVM, @@ -527,16 +527,28 @@ impl<'r, 'l> Session<'r, 'l> { self.move_vm.runtime.loader().ty_builder() } - pub fn fetch_struct_ty_by_idx( + /// If type is a (generic or non-generic) struct or enum, returns its name. Otherwise, returns + /// [None]. + pub fn get_struct_name( &self, - idx: StructNameIndex, - module_storage: &impl ModuleStorage, - ) -> Option> { - self.move_vm - .runtime - .loader() - .fetch_struct_ty_by_idx(idx, &self.module_store, module_storage) - .ok() + ty: &Type, + module_storage: &dyn ModuleStorage, + ) -> PartialVMResult> { + use Type::*; + + Ok(match ty { + Struct { idx, .. } | StructInstantiation { idx, .. } => { + let struct_identifier = self + .move_vm + .runtime + .loader() + .struct_name_index_map(module_storage) + .idx_to_struct_name(*idx)?; + Some((struct_identifier.module, struct_identifier.name)) + }, + Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | TyParam(_) + | Vector(_) | Reference(_) | MutableReference(_) => None, + }) } pub fn check_type_tag_dependencies_and_charge_gas( diff --git a/third_party/move/move-vm/runtime/src/storage/environment.rs b/third_party/move/move-vm/runtime/src/storage/environment.rs index 678c7ad35f935..057b1be157fb9 100644 --- a/third_party/move/move-vm/runtime/src/storage/environment.rs +++ b/third_party/move/move-vm/runtime/src/storage/environment.rs @@ -6,8 +6,8 @@ use crate::{ loader::check_natives, native_functions::{NativeFunction, NativeFunctions}, storage::{ - struct_name_index_map::StructNameIndexMap, ty_cache::StructInfoCache, - ty_tag_converter::TypeTagCache, verified_module_cache::VERIFIED_MODULES_V2, + ty_cache::StructInfoCache, ty_tag_converter::TypeTagCache, + verified_module_cache::VERIFIED_MODULES_V2, }, Module, Script, }; @@ -26,8 +26,11 @@ use move_core_types::{ vm_status::{sub_status::unknown_invariant_violation::EPARANOID_FAILURE, StatusCode}, }; use move_vm_metrics::{Timer, VM_TIMER}; +use move_vm_types::loaded_data::struct_name_indexing::StructNameIndexMap; #[cfg(any(test, feature = "testing"))] -use move_vm_types::loaded_data::runtime_types::{StructIdentifier, StructNameIndex}; +use move_vm_types::loaded_data::{ + runtime_types::StructIdentifier, struct_name_indexing::StructNameIndex, +}; use std::sync::Arc; /// [MoveVM] runtime environment encapsulating different configurations. Shared between the VM and diff --git a/third_party/move/move-vm/runtime/src/storage/mod.rs b/third_party/move/move-vm/runtime/src/storage/mod.rs index 134f241346d94..72e42aa2f7ff4 100644 --- a/third_party/move/move-vm/runtime/src/storage/mod.rs +++ b/third_party/move/move-vm/runtime/src/storage/mod.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 pub(crate) mod loader; -pub(crate) mod struct_name_index_map; pub(crate) mod ty_cache; pub(crate) mod ty_tag_converter; mod verified_module_cache; diff --git a/third_party/move/move-vm/runtime/src/storage/ty_cache.rs b/third_party/move/move-vm/runtime/src/storage/ty_cache.rs index 8ca1aae3361c4..53091a720cef9 100644 --- a/third_party/move/move-vm/runtime/src/storage/ty_cache.rs +++ b/third_party/move/move-vm/runtime/src/storage/ty_cache.rs @@ -1,10 +1,12 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use crate::storage::struct_name_index_map::StructNameIndexMap; use move_binary_format::errors::{PartialVMError, PartialVMResult}; use move_core_types::{language_storage::StructTag, value::MoveTypeLayout, vm_status::StatusCode}; -use move_vm_types::loaded_data::runtime_types::{DepthFormula, StructNameIndex, Type}; +use move_vm_types::loaded_data::{ + runtime_types::{DepthFormula, Type}, + struct_name_indexing::{StructNameIndex, StructNameIndexMap}, +}; use parking_lot::RwLock; /// Layout information of a single struct instantiation. diff --git a/third_party/move/move-vm/runtime/src/storage/ty_layout_converter.rs b/third_party/move/move-vm/runtime/src/storage/ty_layout_converter.rs index 22bdb610c005b..b65abc96e70ab 100644 --- a/third_party/move/move-vm/runtime/src/storage/ty_layout_converter.rs +++ b/third_party/move/move-vm/runtime/src/storage/ty_layout_converter.rs @@ -4,10 +4,7 @@ use crate::{ config::VMConfig, loader::{LegacyModuleStorageAdapter, Loader, PseudoGasContext, VALUE_DEPTH_MAX}, - storage::{ - struct_name_index_map::StructNameIndexMap, ty_cache::StructInfoCache, - ty_tag_converter::TypeTagConverter, - }, + storage::{ty_cache::StructInfoCache, ty_tag_converter::TypeTagConverter}, ModuleStorage, }; use move_binary_format::errors::{PartialVMError, PartialVMResult}; @@ -16,7 +13,10 @@ use move_core_types::{ value::{IdentifierMappingKind, MoveFieldLayout, MoveStructLayout, MoveTypeLayout}, vm_status::StatusCode, }; -use move_vm_types::loaded_data::runtime_types::{StructLayout, StructNameIndex, StructType, Type}; +use move_vm_types::loaded_data::{ + runtime_types::{StructLayout, StructType, Type}, + struct_name_indexing::{StructNameIndex, StructNameIndexMap}, +}; use std::sync::Arc; /// Maximal nodes which are allowed when converting to layout. This includes the types of diff --git a/third_party/move/move-vm/runtime/src/storage/ty_tag_converter.rs b/third_party/move/move-vm/runtime/src/storage/ty_tag_converter.rs index b082ccaa63d19..00d502aec39d3 100644 --- a/third_party/move/move-vm/runtime/src/storage/ty_tag_converter.rs +++ b/third_party/move/move-vm/runtime/src/storage/ty_tag_converter.rs @@ -7,7 +7,7 @@ use move_core_types::{ language_storage::{StructTag, TypeTag}, vm_status::StatusCode, }; -use move_vm_types::loaded_data::runtime_types::{StructNameIndex, Type}; +use move_vm_types::loaded_data::{runtime_types::Type, struct_name_indexing::StructNameIndex}; use parking_lot::RwLock; use std::collections::{hash_map::Entry, HashMap}; @@ -239,24 +239,26 @@ mod tests { fn test_type_tag_cache() { let cache = TypeTagCache::empty(); assert!(cache.cache.read().is_empty()); - assert!(cache.get_struct_tag(&StructNameIndex(0), &[]).is_none()); + assert!(cache + .get_struct_tag(&StructNameIndex::new(0), &[]) + .is_none()); let tag = PricedStructTag { struct_tag: StructTag::from_str("0x1::foo::Foo").unwrap(), pseudo_gas_cost: 10, }; - assert!(cache.insert_struct_tag(&StructNameIndex(0), &[], &tag)); + assert!(cache.insert_struct_tag(&StructNameIndex::new(0), &[], &tag)); let tag = PricedStructTag { struct_tag: StructTag::from_str("0x1::foo::Foo").unwrap(), // Set different cost to check. pseudo_gas_cost: 100, }; - assert!(!cache.insert_struct_tag(&StructNameIndex(0), &[], &tag)); + assert!(!cache.insert_struct_tag(&StructNameIndex::new(0), &[], &tag)); assert_eq!(cache.cache.read().len(), 1); let cost = cache - .get_struct_tag(&StructNameIndex(0), &[]) + .get_struct_tag(&StructNameIndex::new(0), &[]) .unwrap() .pseudo_gas_cost; assert_eq!(cost, 10); @@ -344,8 +346,6 @@ mod tests { constraints: AbilitySet::EMPTY, is_phantom: false, }], - name: Identifier::new("Foo").unwrap(), - module: ModuleId::new(AccountAddress::TWO, Identifier::new("foo").unwrap()), }; let generic_struct_ty = ty_builder .create_struct_instantiation_ty(&struct_ty, &[Type::TyParam(0)], &[bool_vec_ty]) diff --git a/third_party/move/move-vm/types/Cargo.toml b/third_party/move/move-vm/types/Cargo.toml index 858a924fc1641..7e70d23dccdae 100644 --- a/third_party/move/move-vm/types/Cargo.toml +++ b/third_party/move/move-vm/types/Cargo.toml @@ -20,6 +20,7 @@ hashbrown = { workspace = true } itertools = { workspace = true } move-binary-format = { workspace = true } move-core-types = { workspace = true } +parking_lot = { workspace = true } proptest = { workspace = true, optional = true } serde = { workspace = true, features = ["derive", "rc"] } sha3 = { workspace = true } diff --git a/third_party/move/move-vm/types/src/code/errors.rs b/third_party/move/move-vm/types/src/code/errors.rs index 54bfd5a8e5fa7..1a276e2f939ee 100644 --- a/third_party/move/move-vm/types/src/code/errors.rs +++ b/third_party/move/move-vm/types/src/code/errors.rs @@ -1,17 +1,6 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -#[macro_export] -macro_rules! panic_error { - ($msg:expr) => {{ - println!("[Error] panic detected: {}", $msg); - move_binary_format::errors::PartialVMError::new( - move_core_types::vm_status::StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR, - ) - .with_message(format!("Panic detected: {:?}", $msg)) - }}; -} - #[macro_export] macro_rules! module_storage_error { ($addr:expr, $name:expr, $err:ident) => { diff --git a/third_party/move/move-vm/types/src/loaded_data/mod.rs b/third_party/move/move-vm/types/src/loaded_data/mod.rs index 50ceaba95ca0e..d650a543423b9 100644 --- a/third_party/move/move-vm/types/src/loaded_data/mod.rs +++ b/third_party/move/move-vm/types/src/loaded_data/mod.rs @@ -9,3 +9,4 @@ pub mod runtime_access_specifier; #[cfg(test)] mod runtime_access_specifiers_prop_tests; pub mod runtime_types; +pub mod struct_name_indexing; diff --git a/third_party/move/move-vm/types/src/loaded_data/runtime_types.rs b/third_party/move/move-vm/types/src/loaded_data/runtime_types.rs index 60a648056d999..76f6dbce6d79f 100644 --- a/third_party/move/move-vm/types/src/loaded_data/runtime_types.rs +++ b/third_party/move/move-vm/types/src/loaded_data/runtime_types.rs @@ -4,6 +4,7 @@ #![allow(clippy::non_canonical_partial_ord_impl)] +use crate::loaded_data::struct_name_indexing::StructNameIndex; use derivative::Derivative; use itertools::Itertools; use move_binary_format::{ @@ -12,8 +13,6 @@ use move_binary_format::{ SignatureToken, StructHandle, StructTypeParameter, TypeParameterIndex, VariantIndex, }, }; -#[cfg(test)] -use move_core_types::account_address::AccountAddress; use move_core_types::{ ability::{Ability, AbilitySet}, identifier::Identifier, @@ -133,8 +132,6 @@ pub struct StructType { pub phantom_ty_params_mask: SmallBitVec, pub abilities: AbilitySet, pub ty_params: Vec, - pub name: Identifier, - pub module: ModuleId, } #[derive(Debug, Clone, Eq, Hash, PartialEq)] @@ -249,20 +246,15 @@ impl StructType { #[cfg(test)] pub fn for_test() -> StructType { Self { - idx: StructNameIndex(0), + idx: StructNameIndex::new(0), layout: StructLayout::Single(vec![]), phantom_ty_params_mask: SmallBitVec::new(), abilities: AbilitySet::EMPTY, ty_params: vec![], - name: Identifier::new("Foo").unwrap(), - module: ModuleId::new(AccountAddress::ONE, Identifier::new("foo").unwrap()), } } } -#[derive(Debug, Copy, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)] -pub struct StructNameIndex(pub usize); - #[derive(Debug, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)] pub struct StructIdentifier { pub module: ModuleId, @@ -751,7 +743,7 @@ impl fmt::Display for Type { Address => f.write_str("address"), Signer => f.write_str("signer"), Vector(et) => write!(f, "vector<{}>", et), - Struct { idx, ability: _ } => write!(f, "s#{}", idx.0), + Struct { idx, ability: _ } => write!(f, "s#{}", idx), StructInstantiation { idx, ty_args, @@ -759,7 +751,7 @@ impl fmt::Display for Type { } => write!( f, "s#{}<{}>", - idx.0, + idx, ty_args.iter().map(|t| t.to_string()).join(",") ), Reference(t) => write!(f, "&{}", t), @@ -1221,7 +1213,7 @@ mod unit_tests { fn struct_instantiation_ty_for_test(ty_args: Vec) -> Type { Type::StructInstantiation { - idx: StructNameIndex(0), + idx: StructNameIndex::new(0), ability: AbilityInfo::struct_(AbilitySet::EMPTY), ty_args: TriompheArc::new(ty_args), } @@ -1229,7 +1221,7 @@ mod unit_tests { fn struct_ty_for_test() -> Type { Type::Struct { - idx: StructNameIndex(0), + idx: StructNameIndex::new(0), ability: AbilityInfo::struct_(AbilitySet::EMPTY), } } @@ -1372,7 +1364,7 @@ mod unit_tests { #[test] fn test_create_struct_ty() { - let idx = StructNameIndex(0); + let idx = StructNameIndex::new(0); let ability_info = AbilityInfo::struct_(AbilitySet::EMPTY); // Limits are not relevant here. diff --git a/third_party/move/move-vm/runtime/src/storage/struct_name_index_map.rs b/third_party/move/move-vm/types/src/loaded_data/struct_name_indexing.rs similarity index 85% rename from third_party/move/move-vm/runtime/src/storage/struct_name_index_map.rs rename to third_party/move/move-vm/types/src/loaded_data/struct_name_indexing.rs index 98329a6845a73..0da5bf8936a07 100644 --- a/third_party/move/move-vm/runtime/src/storage/struct_name_index_map.rs +++ b/third_party/move/move-vm/types/src/loaded_data/struct_name_indexing.rs @@ -1,14 +1,41 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 +use crate::loaded_data::runtime_types::StructIdentifier; use move_binary_format::errors::PartialVMResult; use move_core_types::language_storage::{StructTag, TypeTag}; -use move_vm_types::{ - loaded_data::runtime_types::{StructIdentifier, StructNameIndex}, - panic_error, -}; use parking_lot::RwLock; -use std::{collections::BTreeMap, sync::Arc}; +use std::{collections::BTreeMap, fmt::Formatter, sync::Arc}; + +macro_rules! panic_error { + ($msg:expr) => {{ + println!("[Error] panic detected: {}", $msg); + move_binary_format::errors::PartialVMError::new( + move_core_types::vm_status::StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR, + ) + .with_message(format!("Panic detected: {:?}", $msg)) + }}; +} + +/// Represents a unique identifier for the struct name. Note that this index has no public +/// constructor - the only way to construct it is via [StructNameIndexMap]. +#[derive(Debug, Copy, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub struct StructNameIndex(usize); + +impl StructNameIndex { + /// Creates a new index for testing purposes only. For production, indices must always be + /// created by the data structure that uses them to intern struct names. + #[cfg(any(test, feature = "testing"))] + pub fn new(idx: usize) -> Self { + Self(idx) + } +} + +impl std::fmt::Display for StructNameIndex { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} #[derive(Clone)] struct IndexMap { @@ -19,11 +46,11 @@ struct IndexMap { /// A data structure to cache struct identifiers (address, module name, struct name) and use /// indices instead, to save on the memory consumption and avoid unnecessary cloning. It /// guarantees that the same struct name identifier always corresponds to a unique index. -pub(crate) struct StructNameIndexMap(RwLock>); +pub struct StructNameIndexMap(RwLock>); impl StructNameIndexMap { /// Returns an empty map with no entries. - pub(crate) fn empty() -> Self { + pub fn empty() -> Self { Self(RwLock::new(IndexMap { forward_map: BTreeMap::new(), backward_map: vec![], @@ -31,7 +58,7 @@ impl StructNameIndexMap { } /// Flushes the cached struct names and indices. - pub(crate) fn flush(&self) { + pub fn flush(&self) { let mut index_map = self.0.write(); index_map.backward_map.clear(); index_map.forward_map.clear(); @@ -40,7 +67,7 @@ impl StructNameIndexMap { /// Maps the struct identifier into an index. If the identifier already exists returns the /// corresponding index. This function guarantees that for any struct identifiers A and B, /// if A == B, they have the same indices. - pub(crate) fn struct_name_to_idx( + pub fn struct_name_to_idx( &self, struct_name: &StructIdentifier, ) -> PartialVMResult { @@ -88,7 +115,7 @@ impl StructNameIndexMap { /// Returns the reference of the struct name corresponding to the index. Here, we wrap the /// name into an [Arc] to ensure that the lock is released. - pub(crate) fn idx_to_struct_name_ref( + pub fn idx_to_struct_name_ref( &self, idx: StructNameIndex, ) -> PartialVMResult> { @@ -98,10 +125,7 @@ impl StructNameIndexMap { /// Returns the clone of the struct name corresponding to the index. The clone ensures that the /// lock is released before the control returns to the caller. - pub(crate) fn idx_to_struct_name( - &self, - idx: StructNameIndex, - ) -> PartialVMResult { + pub fn idx_to_struct_name(&self, idx: StructNameIndex) -> PartialVMResult { let index_map = self.0.read(); Ok(Self::idx_to_struct_name_helper(&index_map, idx)? .as_ref() @@ -109,7 +133,7 @@ impl StructNameIndexMap { } /// Returns the struct tag corresponding to the struct name and the provided type arguments. - pub(crate) fn idx_to_struct_tag( + pub fn idx_to_struct_tag( &self, idx: StructNameIndex, ty_args: Vec, @@ -126,7 +150,7 @@ impl StructNameIndexMap { /// Returns the number of cached entries. Asserts that the number of cached indices is equal to /// the number of cached struct names. - pub(crate) fn checked_len(&self) -> PartialVMResult { + pub fn checked_len(&self) -> PartialVMResult { let (forward_map_len, backward_map_len) = { let index_map = self.0.read(); (index_map.forward_map.len(), index_map.backward_map.len()) @@ -145,7 +169,6 @@ impl StructNameIndexMap { } } -// Only used by V1 loader. impl Clone for StructNameIndexMap { fn clone(&self) -> Self { Self(RwLock::new(self.0.read().clone())) @@ -171,7 +194,7 @@ mod test { #[test] fn test_index_map_must_contain_idx() { let struct_name_idx_map = StructNameIndexMap::empty(); - assert_err!(struct_name_idx_map.idx_to_struct_name_ref(StructNameIndex(0))); + assert_err!(struct_name_idx_map.idx_to_struct_name_ref(StructNameIndex::new(0))); } #[test]