Skip to content

Commit

Permalink
Continued re-design of paranoid mode (now called Runtime Type Check) (#…
Browse files Browse the repository at this point in the history 
…15437)

* further redesign

* fixed a typo
  • Loading branch information
@ziaptos
ziaptos authored Dec 2, 2024
1 parent 5d87d94 commit 94e548c
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 43 deletions.
96 changes: 53 additions & 43 deletions third_party/move/move-vm/runtime/src/interpreter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl InterpreterImpl {
active_modules: HashSet::new(),
};

if loader.vm_config().paranoid_type_checks {
if interpreter.paranoid_type_checks {
interpreter.execute_main::<FullRuntimeTypeCheck>(
loader,
data_store,
Expand Down Expand Up @@ -257,7 +257,7 @@ impl InterpreterImpl {
.build_loaded_function_from_handle_and_ty_args(fh_idx, vec![])
.map_err(|e| self.set_location(e))?;

if self.paranoid_type_checks {
if RTTCheck::should_perform_checks() {
self.check_friend_or_private_call(&current_frame.function, &function)?;
}

Expand All @@ -282,7 +282,7 @@ impl InterpreterImpl {
.map_err(|e| set_err_info!(current_frame, e))?;

if function.is_native() {
self.call_native(
self.call_native::<RTTCheck>(
&mut current_frame,
&resolver,
data_store,
Expand All @@ -293,7 +293,12 @@ impl InterpreterImpl {
)?;
continue;
}
self.set_new_call_frame(&mut current_frame, gas_meter, loader, function)?;
self.set_new_call_frame::<RTTCheck>(
&mut current_frame,
gas_meter,
loader,
function,
)?;
},
ExitCode::CallGeneric(idx) => {
let ty_args = resolver
Expand All @@ -307,7 +312,7 @@ impl InterpreterImpl {
.build_loaded_function_from_instantiation_and_ty_args(idx, ty_args)
.map_err(|e| self.set_location(e))?;

if self.paranoid_type_checks {
if RTTCheck::should_perform_checks() {
self.check_friend_or_private_call(&current_frame.function, &function)?;
}

Expand Down Expand Up @@ -335,7 +340,7 @@ impl InterpreterImpl {
.map_err(|e| set_err_info!(current_frame, e))?;

if function.is_native() {
self.call_native(
self.call_native::<RTTCheck>(
&mut current_frame,
&resolver,
data_store,
Expand All @@ -346,13 +351,18 @@ impl InterpreterImpl {
)?;
continue;
}
self.set_new_call_frame(&mut current_frame, gas_meter, loader, function)?;
self.set_new_call_frame::<RTTCheck>(
&mut current_frame,
gas_meter,
loader,
function,
)?;
},
}
}
}

fn set_new_call_frame(
fn set_new_call_frame<RTTCheck: RuntimeTypeCheck>(
&mut self,
current_frame: &mut Frame,
gas_meter: &mut impl GasMeter,
Expand Down Expand Up @@ -380,7 +390,7 @@ impl InterpreterImpl {
}

let mut frame = self
.make_call_frame(gas_meter, loader, function)
.make_call_frame::<RTTCheck>(gas_meter, loader, function)
.map_err(|err| {
self.attach_state_if_invariant_violation(self.set_location(err), current_frame)
})?;
Expand All @@ -404,7 +414,7 @@ impl InterpreterImpl {
///
/// Native functions do not push a frame at the moment and as such errors from a native
/// function are incorrectly attributed to the caller.
fn make_call_frame(
fn make_call_frame<RTTCheck: RuntimeTypeCheck>(
&mut self,
gas_meter: &mut impl GasMeter,
loader: &Loader,
Expand All @@ -421,7 +431,7 @@ impl InterpreterImpl {
)?;

let ty_args = function.ty_args();
if self.paranoid_type_checks {
if RTTCheck::should_perform_checks() {
let ty = self.operand_stack.pop_ty()?;
let expected_ty = &function.local_tys()[num_param_tys - i - 1];
if !ty_args.is_empty() {
Expand Down Expand Up @@ -479,7 +489,7 @@ impl InterpreterImpl {
}

/// Call a native functions.
fn call_native(
fn call_native<RTTCheck: RuntimeTypeCheck>(
&mut self,
current_frame: &mut Frame,
resolver: &Resolver,
Expand All @@ -490,7 +500,7 @@ impl InterpreterImpl {
function: &LoadedFunction,
) -> VMResult<()> {
// Note: refactor if native functions push a frame on the stack
self.call_native_impl(
self.call_native_impl::<RTTCheck>(
current_frame,
resolver,
data_store,
Expand All @@ -517,7 +527,7 @@ impl InterpreterImpl {
})
}

fn call_native_impl(
fn call_native_impl<RTTCheck: RuntimeTypeCheck>(
&mut self,
current_frame: &mut Frame,
resolver: &Resolver,
Expand All @@ -537,7 +547,7 @@ impl InterpreterImpl {
let mut arg_tys = VecDeque::new();

let ty_args = function.ty_args();
if self.paranoid_type_checks {
if RTTCheck::should_perform_checks() {
for i in 0..num_param_tys {
let ty = self.operand_stack.pop_ty()?;
let expected_ty = &function.param_tys()[num_param_tys - i - 1];
Expand Down Expand Up @@ -594,7 +604,7 @@ impl InterpreterImpl {
self.operand_stack.push(value)?;
}

if self.paranoid_type_checks {
if RTTCheck::should_perform_checks() {
for ty in function.return_tys() {
let ty = ty_builder.create_ty_with_subst(ty, ty_args)?;
self.operand_stack.push_ty(ty)?;
Expand Down Expand Up @@ -689,15 +699,20 @@ impl InterpreterImpl {
}

// Maintaining the type stack for the paranoid mode using calling convention mentioned above.
if self.paranoid_type_checks {
if RTTCheck::should_perform_checks() {
arg_tys.pop_back();
for ty in arg_tys {
self.operand_stack.push_ty(ty)?;
}
}

self.set_new_call_frame(current_frame, gas_meter, resolver.loader(), target_func)
.map_err(|err| err.to_partial())
self.set_new_call_frame::<RTTCheck>(
current_frame,
gas_meter,
resolver.loader(),
target_func,
)
.map_err(|err| err.to_partial())
},
NativeResult::LoadModule { module_name } => {
let arena_id = traversal_context
Expand Down Expand Up @@ -1293,7 +1308,7 @@ impl Stack {
Ok(args)
}

fn check_balance(&self) -> PartialVMResult<()> {
pub(crate) fn check_balance(&self) -> PartialVMResult<()> {
if self.types.len() != self.value.len() {
return Err(
PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR).with_message(
Expand Down Expand Up @@ -1511,17 +1526,15 @@ impl Frame {
// The reason for this design is we charge gas during instruction execution and we want to perform checks only after
// proper gas has been charged for each instruction.

if interpreter.paranoid_type_checks {
interpreter.operand_stack.check_balance()?;
RTTCheck::pre_execution_type_stack_transition(
&self.local_tys,
&self.locals,
self.function.ty_args(),
resolver,
&mut interpreter.operand_stack,
instruction,
)?;
}
RTTCheck::check_operand_stack_balance(&interpreter.operand_stack)?;
RTTCheck::pre_execution_type_stack_transition(
&self.local_tys,
&self.locals,
self.function.ty_args(),
resolver,
&mut interpreter.operand_stack,
instruction,
)?;

match instruction {
Bytecode::Pop => {
Expand Down Expand Up @@ -2300,18 +2313,15 @@ impl Frame {
vec_ref.swap(idx1, idx2, ty)?;
},
}
if interpreter.paranoid_type_checks {
RTTCheck::post_execution_type_stack_transition(
&self.local_tys,
self.function.ty_args(),
resolver,
&mut interpreter.operand_stack,
&mut self.ty_cache,
instruction,
)?;

interpreter.operand_stack.check_balance()?;
}
RTTCheck::post_execution_type_stack_transition(
&self.local_tys,
self.function.ty_args(),
resolver,
&mut interpreter.operand_stack,
&mut self.ty_cache,
instruction,
)?;
RTTCheck::check_operand_stack_balance(&interpreter.operand_stack)?;

// invariant: advance to pc +1 is iff instruction at pc executed without aborting
self.pc += 1;
Expand Down
24 changes: 24 additions & 0 deletions third_party/move/move-vm/runtime/src/runtime_type_checks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ pub(crate) trait RuntimeTypeCheck {
ty_cache: &mut FrameTypeCache,
instruction: &Bytecode,
) -> PartialVMResult<()>;

/// Paranoid check that operand and type stacks have the same size
fn check_operand_stack_balance(operand_stack: &Stack) -> PartialVMResult<()>;

/// For any other checks are performed externally
fn should_perform_checks() -> bool;
}

fn verify_pack<'a>(
Expand Down Expand Up @@ -91,6 +97,15 @@ impl RuntimeTypeCheck for NoRuntimeTypeCheck {
) -> PartialVMResult<()> {
Ok(())
}

fn check_operand_stack_balance(_operand_stack: &Stack) -> PartialVMResult<()> {
Ok(())
}

#[inline(always)]
fn should_perform_checks() -> bool {
false
}
}

impl RuntimeTypeCheck for FullRuntimeTypeCheck {
Expand Down Expand Up @@ -688,4 +703,13 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck {
}
Ok(())
}

fn check_operand_stack_balance(operand_stack: &Stack) -> PartialVMResult<()> {
operand_stack.check_balance()
}

#[inline(always)]
fn should_perform_checks() -> bool {
true
}
}

0 comments on commit 94e548c

Please sign in to comment.