From bbb73c355d54d550a592027cc8f66f744fac8a9f Mon Sep 17 00:00:00 2001 From: Joonatan Saarhelo Date: Thu, 19 Sep 2024 00:52:30 +0200 Subject: [PATCH] use generic statics for panic as well --- crates/vm2/src/instruction_handlers/heap_access.rs | 13 +++++++------ crates/vm2/src/instruction_handlers/mod.rs | 5 ++++- crates/vm2/src/instruction_handlers/pointer.rs | 5 +++-- crates/vm2/src/instruction_handlers/precompiles.rs | 4 ++-- crates/vm2/src/instruction_handlers/ret.rs | 13 +++++++++++++ .../vm2/src/single_instruction_test/into_zk_evm.rs | 2 +- .../src/single_instruction_test/state_to_zk_evm.rs | 8 ++++---- crates/vm2/src/single_instruction_test/vm.rs | 8 ++------ crates/vm2/src/vm.rs | 11 +---------- 9 files changed, 37 insertions(+), 32 deletions(-) diff --git a/crates/vm2/src/instruction_handlers/heap_access.rs b/crates/vm2/src/instruction_handlers/heap_access.rs index 0fbd265..7218fc9 100644 --- a/crates/vm2/src/instruction_handlers/heap_access.rs +++ b/crates/vm2/src/instruction_handlers/heap_access.rs @@ -4,6 +4,7 @@ use zksync_vm2_interface::{opcodes, HeapId, OpcodeType, Tracer}; use super::{ common::{boilerplate, full_boilerplate}, monomorphization::{match_boolean, match_reg_imm, monomorphize, parameterize}, + ret::spontaneous_panic, }; use crate::{ addressing_modes::{ @@ -77,7 +78,7 @@ fn load( let new_bound = address.wrapping_add(32); if grow_heap::<_, _, H>(&mut vm.state, new_bound).is_err() { - vm.state.current_frame.pc = &*vm.panic; + vm.state.current_frame.pc = spontaneous_panic(); return; }; @@ -85,7 +86,7 @@ fn load( // TODO PLA-974 revert to not growing the heap on failure as soon as zk_evm is fixed if bigger_than_last_address(pointer) { let _ = vm.state.use_gas(u32::MAX); - vm.state.current_frame.pc = &*vm.panic; + vm.state.current_frame.pc = spontaneous_panic(); return; } @@ -119,7 +120,7 @@ where let new_bound = address.wrapping_add(32); if grow_heap::<_, _, H>(&mut vm.state, new_bound).is_err() { - vm.state.current_frame.pc = &*vm.panic; + vm.state.current_frame.pc = spontaneous_panic(); return ExecutionStatus::Running; } @@ -127,7 +128,7 @@ where // TODO PLA-974 revert to not growing the heap on failure as soon as zk_evm is fixed if bigger_than_last_address(pointer) { let _ = vm.state.use_gas(u32::MAX); - vm.state.current_frame.pc = &*vm.panic; + vm.state.current_frame.pc = spontaneous_panic(); return ExecutionStatus::Running; } @@ -170,7 +171,7 @@ fn load_pointer( boilerplate::(vm, world, tracer, |vm, args| { let (input, input_is_pointer) = Register1::get_with_pointer_flag(args, &mut vm.state); if !input_is_pointer { - vm.state.current_frame.pc = &*vm.panic; + vm.state.current_frame.pc = spontaneous_panic(); return; } let pointer = FatPointer::from(input); @@ -179,7 +180,7 @@ fn load_pointer( // but if offset + 32 is not representable, we panic, even if we could've read some bytes. // This is not a bug, this is how it must work to be backwards compatible. if pointer.offset > LAST_ADDRESS { - vm.state.current_frame.pc = &*vm.panic; + vm.state.current_frame.pc = spontaneous_panic(); return; }; diff --git a/crates/vm2/src/instruction_handlers/mod.rs b/crates/vm2/src/instruction_handlers/mod.rs index 9a27ea2..ac88afb 100644 --- a/crates/vm2/src/instruction_handlers/mod.rs +++ b/crates/vm2/src/instruction_handlers/mod.rs @@ -1,7 +1,10 @@ +#[cfg(feature = "single_instruction_test")] +pub(crate) use ret::spontaneous_panic; + pub(crate) use self::{ context::address_into_u256, heap_access::{AuxHeap, Heap}, - ret::{invalid_instruction, RETURN_COST}, + ret::invalid_instruction, }; mod binop; diff --git a/crates/vm2/src/instruction_handlers/pointer.rs b/crates/vm2/src/instruction_handlers/pointer.rs index e224ec1..3e0ade8 100644 --- a/crates/vm2/src/instruction_handlers/pointer.rs +++ b/crates/vm2/src/instruction_handlers/pointer.rs @@ -9,6 +9,7 @@ use super::{ monomorphization::{ match_boolean, match_destination, match_source, monomorphize, parameterize, }, + ret::spontaneous_panic, }; use crate::{ addressing_modes::{ @@ -39,12 +40,12 @@ fn ptr }; if !a_is_pointer || b_is_pointer { - vm.state.current_frame.pc = &*vm.panic; + vm.state.current_frame.pc = spontaneous_panic(); return; } let Some(result) = Op::perform(a, b) else { - vm.state.current_frame.pc = &*vm.panic; + vm.state.current_frame.pc = spontaneous_panic(); return; }; diff --git a/crates/vm2/src/instruction_handlers/precompiles.rs b/crates/vm2/src/instruction_handlers/precompiles.rs index 4ad5e7f..cb36487 100644 --- a/crates/vm2/src/instruction_handlers/precompiles.rs +++ b/crates/vm2/src/instruction_handlers/precompiles.rs @@ -17,7 +17,7 @@ use zkevm_opcode_defs::{ }; use zksync_vm2_interface::{opcodes, CycleStats, HeapId, Tracer}; -use super::common::boilerplate_ext; +use super::{common::boilerplate_ext, ret::spontaneous_panic}; use crate::{ addressing_modes::{Arguments, Destination, Register1, Register2, Source}, heap::Heaps, @@ -35,7 +35,7 @@ fn precompile_call( // This is safe because system contracts are trusted let aux_data = PrecompileAuxData::from_u256(Register2::get(args, &mut vm.state)); let Ok(()) = vm.state.use_gas(aux_data.extra_ergs_cost) else { - vm.state.current_frame.pc = &*vm.panic; + vm.state.current_frame.pc = spontaneous_panic(); return; }; diff --git a/crates/vm2/src/instruction_handlers/ret.rs b/crates/vm2/src/instruction_handlers/ret.rs index 0a810cc..71d51b5 100644 --- a/crates/vm2/src/instruction_handlers/ret.rs +++ b/crates/vm2/src/instruction_handlers/ret.rs @@ -182,13 +182,26 @@ fn invalid( } trait GenericStatics { + const PANIC: Instruction; const INVALID: Instruction; } impl GenericStatics for () { + const PANIC: Instruction = Instruction { + handler: ret::, + arguments: Arguments::new(Predicate::Always, RETURN_COST, ModeRequirements::none()), + }; const INVALID: Instruction = Instruction::from_invalid(); } +// The following functions return references that live for 'static. +// They aren't marked as such because returning any lifetime is more ergonomic. + +/// Point the program counter at this instruction when a panic occurs during the logic of and instruction. +pub(crate) fn spontaneous_panic<'a, T: Tracer, W>() -> &'a Instruction { + &<()>::PANIC +} + /// Panics, burning all available gas. pub(crate) fn invalid_instruction<'a, T: Tracer, W>() -> &'a Instruction { &<()>::INVALID diff --git a/crates/vm2/src/single_instruction_test/into_zk_evm.rs b/crates/vm2/src/single_instruction_test/into_zk_evm.rs index 9829007..b5863ca 100644 --- a/crates/vm2/src/single_instruction_test/into_zk_evm.rs +++ b/crates/vm2/src/single_instruction_test/into_zk_evm.rs @@ -36,7 +36,7 @@ pub fn vm2_to_zk_evm>( event_sink.start_frame(zk_evm::aux_structures::Timestamp(0)); VmState { - local_state: vm2_state_to_zk_evm_state(&vm.state, &*vm.panic), + local_state: vm2_state_to_zk_evm_state(&vm.state), block_properties: BlockProperties { default_aa_code_hash: U256::from_big_endian(&vm.settings.default_aa_code_hash), evm_simulator_code_hash: U256::from_big_endian(&vm.settings.evm_interpreter_code_hash), diff --git a/crates/vm2/src/single_instruction_test/state_to_zk_evm.rs b/crates/vm2/src/single_instruction_test/state_to_zk_evm.rs index 8db4d6c..32e08fe 100644 --- a/crates/vm2/src/single_instruction_test/state_to_zk_evm.rs +++ b/crates/vm2/src/single_instruction_test/state_to_zk_evm.rs @@ -6,16 +6,16 @@ use zk_evm::{ vm_state::{execution_stack::CallStackEntry, Callstack, PrimitiveValue, VmLocalState}, }; use zkevm_opcode_defs::decoding::EncodingModeProduction; +use zksync_vm2_interface::Tracer; use crate::{ callframe::{Callframe, NearCallFrame}, + instruction_handlers::spontaneous_panic, state::State, - Instruction, }; -pub(crate) fn vm2_state_to_zk_evm_state( +pub(crate) fn vm2_state_to_zk_evm_state( state: &State, - panic: &Instruction, ) -> VmLocalState<8, EncodingModeProduction> { // zk_evm requires an unused bottom frame let mut callframes: Vec<_> = iter::once(CallStackEntry::empty_context()) @@ -51,7 +51,7 @@ pub(crate) fn vm2_state_to_zk_evm_state( memory_page_counter: 3000, absolute_execution_step: 0, tx_number_in_block: state.transaction_number, - pending_exception: state.current_frame.pc == panic, + pending_exception: state.current_frame.pc == spontaneous_panic(), previous_super_pc: 0, // Same as current pc so the instruction is read from previous_code_word context_u128_register: state.context_u128, callstack: Callstack { diff --git a/crates/vm2/src/single_instruction_test/vm.rs b/crates/vm2/src/single_instruction_test/vm.rs index 9a67c74..6f297bf 100644 --- a/crates/vm2/src/single_instruction_test/vm.rs +++ b/crates/vm2/src/single_instruction_test/vm.rs @@ -4,8 +4,8 @@ use zksync_vm2_interface::{HeapId, Tracer}; use super::{heap::Heaps, stack::StackPool}; use crate::{ - addressing_modes::Arguments, callframe::Callframe, fat_pointer::FatPointer, state::State, - Instruction, ModeRequirements, Predicate, Settings, VirtualMachine, World, WorldDiff, + callframe::Callframe, fat_pointer::FatPointer, state::State, Settings, VirtualMachine, World, + WorldDiff, }; impl VirtualMachine { @@ -79,10 +79,6 @@ impl<'a, T: Tracer, W: World> Arbitrary<'a> for VirtualMachine { settings: u.arbitrary()?, world_diff: WorldDiff::default(), stack_pool: StackPool {}, - panic: Box::new(Instruction::from_panic( - None, - Arguments::new(Predicate::Always, 5, ModeRequirements::none()), - )), snapshot: None, }) } diff --git a/crates/vm2/src/vm.rs b/crates/vm2/src/vm.rs index bb8344e..6c8d436 100644 --- a/crates/vm2/src/vm.rs +++ b/crates/vm2/src/vm.rs @@ -4,15 +4,13 @@ use primitive_types::H160; use zksync_vm2_interface::{opcodes::TypeLevelCallingMode, CallingMode, HeapId, Tracer}; use crate::{ - addressing_modes::Arguments, callframe::{Callframe, FrameRemnant}, decommit::u256_into_address, instruction::ExecutionStatus, - instruction_handlers::RETURN_COST, stack::StackPool, state::{State, StateSnapshot}, world_diff::{ExternalSnapshot, Snapshot, WorldDiff}, - ExecutionEnd, Instruction, ModeRequirements, Predicate, Program, World, + ExecutionEnd, Program, World, }; /// [`VirtualMachine`] settings. @@ -33,9 +31,6 @@ pub struct VirtualMachine { pub(crate) state: State, pub(crate) settings: Settings, pub(crate) stack_pool: StackPool, - /// Instruction that is jumped to when things go wrong while executing another. - /// Boxed, so the pointer isn't invalidated by moves. - pub(crate) panic: Box>, pub(crate) snapshot: Option, } @@ -66,10 +61,6 @@ impl> VirtualMachine { ), settings, stack_pool, - panic: Box::new(Instruction::from_panic( - None, - Arguments::new(Predicate::Always, RETURN_COST, ModeRequirements::none()), - )), snapshot: None, } }