Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[move-vm] Relax script function rules #175

Merged
merged 1 commit into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion language/benchmarks/src/move_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,13 @@ fn execute<M: Measurement + 'static>(
c.bench_function(fun, |b| {
b.iter(|| {
session
.execute_function(&module_id, fun_name, vec![], vec![], &mut gas_status)
.execute_function_bypass_visibility(
&module_id,
fun_name,
vec![],
Vec::<Vec<u8>>::new(),
&mut gas_status,
)
.unwrap_or_else(|err| {
panic!(
"{:?}::{} failed with {:?}",
Expand Down
78 changes: 41 additions & 37 deletions language/extensions/async/move-async-vm/src/async_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use move_binary_format::errors::{Location, PartialVMError, PartialVMResult, VMEr
use move_core_types::{
account_address::AccountAddress,
effects::{ChangeSet, Event},
gas_schedule::GasAlgebra,
identifier::Identifier,
language_storage::{ModuleId, StructTag, TypeTag},
resolver::MoveResolver,
Expand All @@ -19,7 +20,7 @@ use move_core_types::{
use move_vm_runtime::{
move_vm::MoveVM,
native_functions::{NativeContextExtensions, NativeFunction},
session::{ExecutionResult, Session},
session::{SerializedReturnValues, Session},
};
use move_vm_types::{
gas_schedule::GasStatus,
Expand Down Expand Up @@ -175,25 +176,29 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
}

// Execute the initializer.
let result = self.vm_session.execute_function_for_effects(
&actor.module_id,
&actor.initializer,
vec![],
Vec::<Vec<u8>>::new(),
gas_status,
);
let gas_before = gas_status.remaining_gas().get();
let result = self
.vm_session
.execute_function_bypass_visibility(
&actor.module_id,
&actor.initializer,
vec![],
Vec::<Vec<u8>>::new(),
gas_status,
)
.and_then(|ret| Ok((ret, self.vm_session.finish_with_extensions()?)));
let gas_used = gas_status.remaining_gas().get() - gas_before;

// Process the result, moving the return value of the initializer function into the
// changeset.
match result {
ExecutionResult::Success {
mut change_set,
events,
mut return_values,
gas_used,
mut native_extensions,
..
} => {
Ok((
SerializedReturnValues {
mutable_reference_outputs: _,
mut return_values,
},
(mut change_set, events, mut native_extensions),
)) => {
if return_values.len() != 1 {
Err(async_extension_error(format!(
"inconsistent initializer `{}`",
Expand All @@ -204,7 +209,7 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
&mut change_set,
actor_addr,
actor.state_tag.clone(),
return_values.remove(0),
return_values.remove(0).0,
)
.map_err(partial_vm_error_to_async)?;
let async_ext = native_extensions.remove::<AsyncExtension>();
Expand All @@ -216,7 +221,7 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
})
}
}
ExecutionResult::Fail { error, gas_used } => Err(AsyncError { error, gas_used }),
Err(error) => Err(AsyncError { error, gas_used }),
}
}

Expand Down Expand Up @@ -266,37 +271,36 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
);

// Execute the handler.
let result = self.vm_session.execute_function_for_effects(
module_id,
handler_id,
vec![],
args,
gas_status,
);
let gas_before = gas_status.remaining_gas().get();
let result = self
.vm_session
.execute_function_bypass_visibility(module_id, handler_id, vec![], args, gas_status)
.and_then(|ret| Ok((ret, self.vm_session.finish_with_extensions()?)));

let gas_used = gas_status.remaining_gas().get() - gas_before;

// Process the result, moving the mutated value of the handlers first parameter
// into the changeset.
match result {
ExecutionResult::Success {
mut change_set,
events,
mut mutable_ref_values,
gas_used,
mut native_extensions,
..
} => {
if mutable_ref_values.len() > 1 {
Ok((
SerializedReturnValues {
mut mutable_reference_outputs,
return_values: _,
},
(mut change_set, events, mut native_extensions),
)) => {
if mutable_reference_outputs.len() > 1 {
Err(async_extension_error(format!(
"inconsistent handler `{}`",
handler_id
)))
} else {
if !mutable_ref_values.is_empty() {
if !mutable_reference_outputs.is_empty() {
publish_actor_state(
&mut change_set,
actor_addr,
actor.state_tag.clone(),
mutable_ref_values.remove(0),
mutable_reference_outputs.remove(0).1,
)
.map_err(partial_vm_error_to_async)?;
}
Expand All @@ -309,7 +313,7 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
})
}
}
ExecutionResult::Fail { error, gas_used } => Err(AsyncError { error, gas_used }),
Err(error) => Err(AsyncError { error, gas_used }),
}
}

Expand Down
35 changes: 22 additions & 13 deletions language/move-binary-format/src/check_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,30 @@ impl<'a> BoundsChecker<'a> {
};
bounds_check.verify_impl()?;

let signatures = &script.signatures;
let parameters = &script.parameters;
match signatures.get(parameters.into_index()) {
// The bounds checker has already checked each function definition's code, but a script's
// code exists outside of any function definition. It gets checked here.
Some(signature) => {
bounds_check.check_code(&script.code, &script.type_parameters, signature)
let type_param_count = script.type_parameters.len();

check_bounds_impl(bounds_check.view.signatures(), script.parameters)?;
if let Some(sig) = bounds_check
.view
.signatures()
.get(script.parameters.into_index())
{
for ty in &sig.0 {
bounds_check.check_type_parameter(ty, type_param_count)?
}
None => Err(bounds_error(
StatusCode::INDEX_OUT_OF_BOUNDS,
IndexKind::Signature,
parameters.into_index() as u16,
signatures.len(),
)),
}

// The bounds checker has already checked each function definition's code, but a
// script's code exists outside of any function definition. It gets checked here.
bounds_check.check_code(
&script.code,
&script.type_parameters,
bounds_check
.view
.signatures()
.get(script.parameters.into_index())
.unwrap(),
)
}

pub fn verify_module(module: &'a CompiledModule) -> PartialVMResult<()> {
Expand Down
6 changes: 5 additions & 1 deletion language/move-binary-format/src/file_format_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,12 @@ pub const VERSION_3: u32 = 3;
/// + bytecode for vector operations
pub const VERSION_4: u32 = 4;

/// Version 5: changes compared with version 4
/// +/- script and public(script) verification is now adapter specific
pub const VERSION_5: u32 = 5;

// Mark which version is the latest version
pub const VERSION_MAX: u32 = VERSION_4;
pub const VERSION_MAX: u32 = VERSION_5;

pub(crate) mod versioned_data {
use crate::{errors::*, file_format_common::*};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,19 @@ fn invalid_type_param_in_fn_parameters() {
);
}

#[test]
fn invalid_type_param_in_script_parameters() {
use SignatureToken::*;

let mut s = basic_test_script();
s.parameters = SignatureIndex(1);
s.signatures.push(Signature(vec![TypeParameter(0)]));
assert_eq!(
BoundsChecker::verify_script(&s).unwrap_err().major_status(),
StatusCode::INDEX_OUT_OF_BOUNDS
);
}

#[test]
fn invalid_struct_in_fn_return_() {
use SignatureToken::*;
Expand Down
3 changes: 3 additions & 0 deletions language/move-bytecode-verifier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ pub mod verifier;
pub use check_duplication::DuplicationChecker;
pub use code_unit_verifier::CodeUnitVerifier;
pub use instruction_consistency::InstructionConsistency;
pub use script_signature::{
legacy_script_signature_checks, no_additional_script_signature_checks, FnCheckScriptSignature,
};
pub use signature::SignatureChecker;
pub use struct_defs::RecursiveStructDefChecker;
pub use verifier::{verify_module, verify_script};
Expand Down