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.

Sign up for GitHub

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

Continued re-design of paranoid mode (now called Runtime Type Check) #15437

Merged
merged 3 commits into from
Dec 2, 2024
Merged

Continued re-design of paranoid mode (now called Runtime Type Check) #15437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be72e9f
further redesign
ziaptos Dec 2, 2024
7f84d4a
fixed a typo
ziaptos Dec 2, 2024
b3eb0af
Merge branch 'main' into zi/new-type/small-runtime-check-refactor
ziaptos Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 53 additions & 43 deletions third_party/move/move-vm/runtime/src/interpreter.rs
View file
Open in desktop
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
Open in desktop
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: .s in the end of docstrings

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
}
}
Loading