Skip to content

Commit

Permalink
Rebasing: adjusting to upstream changes
Browse files Browse the repository at this point in the history
  • Loading branch information
wrwg committed Feb 5, 2024
1 parent 88d91fb commit 26bd182
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -527,9 +527,7 @@ pub(crate) mod versioned_data {
return Err(PartialVMError::new(StatusCode::UNKNOWN_VERSION)
.with_message(format!("bytecode version {} unsupported", version)));
} else if version == VERSION_NEXT
&& !cfg!(test)
&& !cfg!(feature = "testing")
&& !cfg!(feature = "fuzzing")
&& !cfg!(any(test, feature = "testing", feature = "fuzzing"))
{
return Err(
PartialVMError::new(StatusCode::UNKNOWN_VERSION).with_message(format!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ Error: Function execution failed with VMError: {
sub_status: None,
location: 0x42::test,
indices: [],
offsets: [(FunctionDefinitionIndex(1), 1)],
offsets: [(FunctionDefinitionIndex(1), 3)],
exec_state: Some(ExecutionState { stack_trace: [] }),
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ module 0x42::test {
borrow_global<R>(signer::address_of(s)).value
}

fun fail1(a: address): bool reads R(a) {
fun fail1(_a: address): bool reads R(_a) {
borrow_global<R>(@0x2).value
}

fun fail2(s: &signer): bool reads R(signer::address_of(s)) {
fun fail2(_s: &signer): bool reads R(signer::address_of(_s)) {
borrow_global<R>(@0x2).value
}
}
Expand Down
57 changes: 39 additions & 18 deletions third_party/move/move-vm/runtime/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ use fail::fail_point;
use move_binary_format::{
errors::*,
file_format::{
Ability, AbilitySet, Bytecode, FunctionHandleIndex, FunctionInstantiationIndex, LocalIndex,
Ability, AbilitySet, AccessKind, Bytecode, FunctionHandleIndex, FunctionInstantiationIndex,
LocalIndex,
},
};
use move_core_types::{
Expand Down Expand Up @@ -616,20 +617,46 @@ impl Interpreter {
TypeWithLoader { ty, loader },
res.is_ok(),
)?;
let access_opt = if is_mut {
AccessInstance::write(ty, addr)
} else {
AccessInstance::read(ty, addr)
};
if let Some(access) = access_opt {
self.access_control.check_access(access)?;
}
self.check_access(
loader,
if is_mut {
AccessKind::Writes
} else {
AccessKind::Reads
},
ty,
addr,
)?;
self.operand_stack.push(res.map_err(|err| {
err.with_message(format!("Failed to borrow global resource from {:?}", addr))
})?)?;
Ok(())
}

fn check_access(
&self,
loader: &Loader,
kind: AccessKind,
ty: &Type,
addr: AccountAddress,
) -> PartialVMResult<()> {
let (struct_idx, instance) = match ty {
Type::Struct { idx, .. } => (*idx, [].as_slice()),
Type::StructInstantiation { idx, ty_args, .. } => (*idx, ty_args.as_slice()),
_ => {
return Err(
PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR)
.with_message("inconsistent type".to_owned()),
)

Check warning on line 650 in third_party/move/move-vm/runtime/src/interpreter.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/runtime/src/interpreter.rs#L647-L650

Added lines #L647 - L650 were not covered by tests
},
};
let struct_name = &*loader.name_cache.idx_to_identifier(struct_idx);
if let Some(access) = AccessInstance::new(kind, struct_name, instance, addr) {
self.access_control.check_access(access)?
}

Check warning on line 656 in third_party/move/move-vm/runtime/src/interpreter.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/runtime/src/interpreter.rs#L656

Added line #L656 was not covered by tests
Ok(())
}

/// Exists opcode.
fn exists(
&mut self,
Expand All @@ -644,9 +671,7 @@ impl Interpreter {
let gv = Self::load_resource(loader, data_store, module_store, gas_meter, addr, ty)?;
let exists = gv.exists()?;
gas_meter.charge_exists(is_generic, TypeWithLoader { ty, loader }, exists)?;
if let Some(access) = AccessInstance::read(ty, addr) {
self.access_control.check_access(access)?;
}
self.check_access(loader, AccessKind::Reads, ty, addr)?;
self.operand_stack.push(Value::bool(exists))?;
Ok(())
}
Expand All @@ -672,9 +697,7 @@ impl Interpreter {
TypeWithLoader { ty, loader },
Some(&resource),
)?;
if let Some(access) = AccessInstance::write(ty, addr) {
self.access_control.check_access(access)?;
}
self.check_access(loader, AccessKind::Writes, ty, addr)?;
resource
},
Err(err) => {
Expand Down Expand Up @@ -712,9 +735,7 @@ impl Interpreter {
gv.view().unwrap(),
true,
)?;
if let Some(access) = AccessInstance::write(ty, addr) {
self.access_control.check_access(access)?;
}
self.check_access(loader, AccessKind::Writes, ty, addr)?;
Ok(())
},
Err((err, resource)) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,20 @@ use move_vm_types::loaded_data::{
},
runtime_types::{StructIdentifier, Type},
};
use std::sync::Arc;

/// Loads an access specifier from the file format into the runtime representation.
pub fn load_access_specifier(
module: BinaryIndexedView,
signature_table: &[Vec<Type>],
struct_name_table: &[Arc<StructIdentifier>],
struct_names: &[StructIdentifier],
specifier: &Option<Vec<FF::AccessSpecifier>>,
) -> PartialVMResult<AccessSpecifier> {
if let Some(specs) = specifier {
let mut incls = vec![];
let mut excls = vec![];
for spec in specs {
let resource = load_resource_specifier(
module,
signature_table,
struct_name_table,
&spec.resource,
)?;
let resource =
load_resource_specifier(module, signature_table, struct_names, &spec.resource)?;
let address = load_address_specifier(module, &spec.address)?;
let clause = AccessSpecifierClause {
kind: spec.kind,
Expand All @@ -55,7 +50,7 @@ pub fn load_access_specifier(
fn load_resource_specifier(
module: BinaryIndexedView,
signature_table: &[Vec<Type>],
struct_name_table: &[Arc<StructIdentifier>],
struct_names: &[StructIdentifier],
spec: &FF::ResourceSpecifier,
) -> PartialVMResult<ResourceSpecifier> {
use FF::ResourceSpecifier::*;
Expand All @@ -71,10 +66,10 @@ fn load_resource_specifier(
.ok_or_else(index_out_of_range)?,
)),
Resource(str_idx) => Ok(ResourceSpecifier::Resource(
access_table(struct_name_table, str_idx.0)?.as_ref().clone(),
access_table(struct_names, str_idx.0)?.clone(),
)),
ResourceInstantiation(str_idx, ty_idx) => Ok(ResourceSpecifier::ResourceInstantiation(
access_table(struct_name_table, str_idx.0)?.as_ref().clone(),
access_table(struct_names, str_idx.0)?.clone(),
access_table(signature_table, ty_idx.0)?.clone(),
)),
}
Expand Down
4 changes: 2 additions & 2 deletions third_party/move/move-vm/runtime/src/loader/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl Function {
index: FunctionDefinitionIndex,
module: &CompiledModule,
signature_table: &[Vec<Type>],
struct_name_table: &[Arc<StructIdentifier>],
struct_names: &[StructIdentifier],
) -> PartialVMResult<Self> {
let def = module.function_def_at(index);
let handle = module.function_handle_at(def.function);
Expand Down Expand Up @@ -113,7 +113,7 @@ impl Function {
let access_specifier = load_access_specifier(
BinaryIndexedView::Module(module),
signature_table,
struct_name_table,
struct_names,
&handle.access_specifiers,
)?;

Expand Down
1 change: 0 additions & 1 deletion third_party/move/move-vm/runtime/src/loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ mod type_loader;

pub(crate) use function::{Function, FunctionHandle, FunctionInstantiation, LoadedFunction, Scope};
pub(crate) use modules::{Module, ModuleCache, ModuleStorage, ModuleStorageAdapter};
use move_vm_types::loaded_data::runtime_access_specifier::AccessSpecifier;
pub(crate) use script::{Script, ScriptCache};
use type_loader::intern_type;

Expand Down
16 changes: 12 additions & 4 deletions third_party/move/move-vm/runtime/src/loader/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ impl Module {

let mut create = || {
let mut struct_idxs = vec![];
let mut struct_names = vec![];
// validate the correctness of struct handle references.
for struct_handle in module.struct_handles() {
let struct_name = module.identifier_at(struct_handle.name);
Expand All @@ -283,10 +284,12 @@ impl Module {
.get_struct_type_by_identifier(struct_name, &module_id)?
.check_compatibility(struct_handle)?;
}
struct_idxs.push(name_cache.insert_or_get(StructIdentifier {
let name = StructIdentifier {
module: module_id,
name: struct_name.to_owned(),
}));
};
struct_idxs.push(name_cache.insert_or_get(name.clone()));
struct_names.push(name)
}

// Build signature table
Expand Down Expand Up @@ -327,8 +330,13 @@ impl Module {

for (idx, func) in module.function_defs().iter().enumerate() {
let findex = FunctionDefinitionIndex(idx as TableIndex);
let function =
Function::new(natives, findex, &module, &signature_table, &struct_names)?;
let function = Function::new(
natives,
findex,
&module,
signature_table.as_slice(),
&struct_names,
)?;

function_map.insert(function.name.to_owned(), idx);
function_defs.push(Arc::new(function));
Expand Down
6 changes: 5 additions & 1 deletion third_party/move/move-vm/runtime/src/loader/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ use move_binary_format::{
file_format::{Bytecode, CompiledScript, FunctionDefinitionIndex, Signature, SignatureIndex},
};
use move_core_types::{identifier::Identifier, language_storage::ModuleId, vm_status::StatusCode};
use move_vm_types::loaded_data::runtime_types::{StructIdentifier, Type};
use move_vm_types::loaded_data::{
runtime_access_specifier::AccessSpecifier,
runtime_types::{StructIdentifier, Type},
};
use std::{collections::BTreeMap, sync::Arc};

// A Script is very similar to a `CompiledScript` but data is "transformed" to a representation
Expand Down Expand Up @@ -145,6 +148,7 @@ impl Script {
return_types: return_tys.clone(),
local_types: local_tys,
parameter_types: parameter_tys.clone(),
access_specifier: AccessSpecifier::Any,
});

let mut single_signature_token_map = BTreeMap::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub trait AccessSpecifierEnv {
) -> PartialVMResult<AccountAddress>;
}

/// A struct to represent an access.
/// A struct to represent an access instance (request).
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Debug)]
pub struct AccessInstance {
pub kind: AccessKind,
Expand All @@ -116,8 +116,8 @@ impl AccessSpecifier {
/// may be under-approximated in the presence of exclusions. That is, if
/// `!s.is_empty()`, it is still possible that all concrete accesses fail.
pub fn is_empty(&self) -> bool {
if let AccessSpecifier::Constraint(incl, _) = self {
incl.is_empty()
if let AccessSpecifier::Constraint(incls, _) = self {
incls.is_empty()

Check warning on line 120 in third_party/move/move-vm/types/src/loaded_data/runtime_access_specifier.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/loaded_data/runtime_access_specifier.rs#L118-L120

Added lines #L118 - L120 were not covered by tests
} else {
false

Check warning on line 122 in third_party/move/move-vm/types/src/loaded_data/runtime_access_specifier.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/loaded_data/runtime_access_specifier.rs#L122

Added line #L122 was not covered by tests
}
Expand Down Expand Up @@ -492,12 +492,12 @@ impl AddressSpecifierFunction {
}

impl AccessInstance {
pub fn new(kind: AccessKind, ty: &Type, address: AccountAddress) -> Option<Self> {
let (resource, instance) = match ty {
Type::Struct { name, .. } => (name.as_ref(), &[] as &[Type]),
Type::StructInstantiation { name, ty_args, .. } => (name.as_ref(), ty_args.as_slice()),
_ => return None,
};
pub fn new(
kind: AccessKind,
resource: &StructIdentifier,
instance: &[Type],
address: AccountAddress,
) -> Option<Self> {
Some(AccessInstance {
kind,
resource: resource.clone(),
Expand All @@ -506,12 +506,20 @@ impl AccessInstance {
})
}

pub fn read(ty: &Type, address: AccountAddress) -> Option<Self> {
Self::new(AccessKind::Reads, ty, address)
pub fn read(
resource: &StructIdentifier,
instance: &[Type],
address: AccountAddress,
) -> Option<Self> {
Self::new(AccessKind::Reads, resource, instance, address)
}

Check warning on line 515 in third_party/move/move-vm/types/src/loaded_data/runtime_access_specifier.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/loaded_data/runtime_access_specifier.rs#L509-L515

Added lines #L509 - L515 were not covered by tests

pub fn write(ty: &Type, address: AccountAddress) -> Option<Self> {
Self::new(AccessKind::Writes, ty, address)
pub fn write(
resource: &StructIdentifier,
instance: &[Type],
address: AccountAddress,
) -> Option<Self> {
Self::new(AccessKind::Writes, resource, instance, address)
}

Check warning on line 523 in third_party/move/move-vm/types/src/loaded_data/runtime_access_specifier.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/loaded_data/runtime_access_specifier.rs#L517-L523

Added lines #L517 - L523 were not covered by tests
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -486,15 +486,15 @@ impl fmt::Display for Type {
Address => f.write_str("address"),
Signer => f.write_str("signer"),
Vector(et) => write!(f, "vector<{}>", et),
Struct { name, ability: _ } => f.write_str(&name.to_string()),
Struct { idx, ability: _ } => write!(f, "s#{}", idx.0),

Check warning on line 489 in third_party/move/move-vm/types/src/loaded_data/runtime_types.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/loaded_data/runtime_types.rs#L480-L489

Added lines #L480 - L489 were not covered by tests
StructInstantiation {
name,
idx,
ty_args,
ability: _,
} => write!(
f,
"{}<{}>",
name,
"s#{}<{}>",
idx.0,
ty_args.iter().map(|t| t.to_string()).join(",")
),
Reference(t) => write!(f, "&{}", t),
Expand Down

0 comments on commit 26bd182

Please sign in to comment.