From 3a4ba72daf51d7bef42303ef17b680f9cc30974e Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Fri, 17 Feb 2023 03:47:45 -0800 Subject: [PATCH] Remove executors trait and replace with LoadedProgram (#30348) * Remove executors trait and replace with LoadedProgram * fill in deployment slot * address review comments * fix clippy warnings * address review comments * fix failures caught by sbf tests --- program-runtime/src/executor.rs | 39 -- program-runtime/src/executor_cache.rs | 72 ++-- program-runtime/src/lib.rs | 1 - program-runtime/src/loaded_programs.rs | 94 ++++- programs/bpf_loader/src/lib.rs | 492 +++++++++++-------------- runtime/src/bank.rs | 21 +- runtime/src/bank/tests.rs | 71 +++- 7 files changed, 398 insertions(+), 392 deletions(-) delete mode 100644 program-runtime/src/executor.rs diff --git a/program-runtime/src/executor.rs b/program-runtime/src/executor.rs deleted file mode 100644 index 8d0894b180..0000000000 --- a/program-runtime/src/executor.rs +++ /dev/null @@ -1,39 +0,0 @@ -use { - crate::{invoke_context::InvokeContext, timings::ExecuteDetailsTimings}, - solana_sdk::{instruction::InstructionError, saturating_add_assign}, -}; - -/// Program executor -pub trait Executor: std::fmt::Debug + Send + Sync { - /// Execute the program - fn execute(&self, invoke_context: &mut InvokeContext) -> Result<(), InstructionError>; -} - -#[derive(Debug, Default)] -pub struct CreateMetrics { - pub program_id: String, - pub register_syscalls_us: u64, - pub load_elf_us: u64, - pub verify_code_us: u64, - pub jit_compile_us: u64, -} - -impl CreateMetrics { - pub fn submit_datapoint(&self, timings: &mut ExecuteDetailsTimings) { - saturating_add_assign!( - timings.create_executor_register_syscalls_us, - self.register_syscalls_us - ); - saturating_add_assign!(timings.create_executor_load_elf_us, self.load_elf_us); - saturating_add_assign!(timings.create_executor_verify_code_us, self.verify_code_us); - saturating_add_assign!(timings.create_executor_jit_compile_us, self.jit_compile_us); - datapoint_trace!( - "create_executor_trace", - ("program_id", self.program_id, String), - ("register_syscalls_us", self.register_syscalls_us, i64), - ("load_elf_us", self.load_elf_us, i64), - ("verify_code_us", self.verify_code_us, i64), - ("jit_compile_us", self.jit_compile_us, i64), - ); - } -} diff --git a/program-runtime/src/executor_cache.rs b/program-runtime/src/executor_cache.rs index 22b68cb90a..24a5d3d899 100644 --- a/program-runtime/src/executor_cache.rs +++ b/program-runtime/src/executor_cache.rs @@ -1,5 +1,5 @@ use { - crate::executor::Executor, + crate::loaded_programs::LoadedProgram, log::*, rand::Rng, solana_sdk::{pubkey::Pubkey, saturating_add_assign, slot_history::Slot, stake_history::Epoch}, @@ -21,36 +21,35 @@ use { #[derive(Default, Debug)] pub struct TransactionExecutorCache { /// Executors or tombstones which are visible during the transaction - pub visible: HashMap>>, + pub visible: HashMap>, /// Executors of programs which were re-/deploymed during the transaction - pub deployments: HashMap>, + pub deployments: HashMap>, /// Executors which were missing in the cache and not re-/deploymed during the transaction - pub add_to_cache: HashMap>, + pub add_to_cache: HashMap>, } impl TransactionExecutorCache { - pub fn new(executable_keys: impl Iterator)>) -> Self { + pub fn new(executable_keys: impl Iterator)>) -> Self { Self { - visible: executable_keys - .map(|(id, executor)| (id, Some(executor))) - .collect(), + visible: executable_keys.collect(), deployments: HashMap::new(), add_to_cache: HashMap::new(), } } - pub fn get(&self, key: &Pubkey) -> Option>> { + pub fn get(&self, key: &Pubkey) -> Option> { self.visible.get(key).cloned() } pub fn set_tombstone(&mut self, key: Pubkey) { - self.visible.insert(key, None); + self.visible + .insert(key, Arc::new(LoadedProgram::new_tombstone())); } pub fn set( &mut self, key: Pubkey, - executor: Arc, + executor: Arc, upgrade: bool, delay_visibility_of_program_deployment: bool, ) { @@ -60,22 +59,22 @@ impl TransactionExecutorCache { // we don't load the new version from the database as it should remain invisible self.set_tombstone(key); } else { - self.visible.insert(key, Some(executor.clone())); + self.visible.insert(key, executor.clone()); } self.deployments.insert(key, executor); } else { - self.visible.insert(key, Some(executor.clone())); + self.visible.insert(key, executor.clone()); self.add_to_cache.insert(key, executor); } } - pub fn get_executors_added_to_the_cache(&mut self) -> HashMap> { + pub fn get_executors_added_to_the_cache(&mut self) -> HashMap> { let mut executors = HashMap::new(); std::mem::swap(&mut executors, &mut self.add_to_cache); executors } - pub fn get_executors_which_were_deployed(&mut self) -> HashMap> { + pub fn get_executors_which_were_deployed(&mut self) -> HashMap> { let mut executors = HashMap::new(); std::mem::swap(&mut executors, &mut self.deployments); executors @@ -90,7 +89,7 @@ pub const MAX_CACHED_EXECUTORS: usize = 256; pub struct BankExecutorCacheEntry { prev_epoch_count: u64, epoch_count: AtomicU64, - executor: Arc, + executor: Arc, pub hit_count: AtomicU64, } @@ -175,7 +174,7 @@ impl BankExecutorCache { } } - pub fn get(&self, pubkey: &Pubkey) -> Option> { + pub fn get(&self, pubkey: &Pubkey) -> Option> { if let Some(entry) = self.executors.get(pubkey) { self.stats.hits.fetch_add(1, Relaxed); entry.epoch_count.fetch_add(1, Relaxed); @@ -187,12 +186,12 @@ impl BankExecutorCache { } } - pub fn put(&mut self, executors: impl Iterator)>) { + pub fn put(&mut self, executors: impl Iterator)>) { let mut new_executors: Vec<_> = executors .filter_map(|(key, executor)| { if let Some(mut entry) = self.remove(&key) { self.stats.replacements.fetch_add(1, Relaxed); - entry.executor = executor.clone(); + entry.executor = executor; let _ = self.executors.insert(key, entry); None } else { @@ -353,20 +352,7 @@ impl Stats { #[allow(clippy::indexing_slicing)] #[cfg(test)] mod tests { - use { - super::*, crate::invoke_context::InvokeContext, solana_sdk::instruction::InstructionError, - }; - - #[derive(Debug)] - struct TestExecutor {} - impl Executor for TestExecutor { - fn execute( - &self, - _invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { - Ok(()) - } - } + use super::*; #[test] fn test_executor_cache() { @@ -374,7 +360,7 @@ mod tests { let key2 = solana_sdk::pubkey::new_rand(); let key3 = solana_sdk::pubkey::new_rand(); let key4 = solana_sdk::pubkey::new_rand(); - let executor: Arc = Arc::new(TestExecutor {}); + let executor = Arc::new(LoadedProgram::default()); let mut cache = BankExecutorCache::new(3, 0); cache.put([(key1, executor.clone())].into_iter()); @@ -398,7 +384,7 @@ mod tests { assert!(cache.get(&key4).is_some()); assert!(cache.get(&key4).is_some()); assert!(cache.get(&key4).is_some()); - cache.put([(key3, executor.clone())].into_iter()); + cache.put([(key3, executor)].into_iter()); assert!(cache.get(&key3).is_some()); let num_retained = [&key1, &key2, &key4] .iter() @@ -413,7 +399,7 @@ mod tests { let key2 = solana_sdk::pubkey::new_rand(); let key3 = solana_sdk::pubkey::new_rand(); let key4 = solana_sdk::pubkey::new_rand(); - let executor: Arc = Arc::new(TestExecutor {}); + let executor = Arc::new(LoadedProgram::default()); let mut cache = BankExecutorCache::new(3, 0); assert!(cache.current_epoch == 0); @@ -452,7 +438,7 @@ mod tests { cache = BankExecutorCache::new_from_parent_bank_executors(&cache, 2); assert!(cache.current_epoch == 2); - cache.put([(key3, executor.clone())].into_iter()); + cache.put([(key3, executor)].into_iter()); assert!(cache.get(&key3).is_some()); } @@ -461,7 +447,7 @@ mod tests { let key1 = solana_sdk::pubkey::new_rand(); let key2 = solana_sdk::pubkey::new_rand(); let key3 = solana_sdk::pubkey::new_rand(); - let executor: Arc = Arc::new(TestExecutor {}); + let executor = Arc::new(LoadedProgram::default()); let mut cache = BankExecutorCache::new(2, 0); cache.put([(key1, executor.clone())].into_iter()); @@ -480,7 +466,7 @@ mod tests { entries.sort_by_key(|(_, v)| *v); assert!(entries[0].1 < entries[1].1); - cache.put([(key3, executor.clone())].into_iter()); + cache.put([(key3, executor)].into_iter()); assert!(cache.get(&entries[0].0).is_none()); assert!(cache.get(&entries[1].0).is_some()); } @@ -491,7 +477,7 @@ mod tests { let one_hit_wonder = Pubkey::new_unique(); let popular = Pubkey::new_unique(); - let executor: Arc = Arc::new(TestExecutor {}); + let executor = Arc::new(LoadedProgram::default()); // make sure we're starting from where we think we are assert_eq!(cache.stats.one_hit_wonders.load(Relaxed), 0); @@ -511,7 +497,7 @@ mod tests { assert_eq!(cache.executors[&popular].hit_count.load(Relaxed), 2); // evict "popular program" - cache.put([(one_hit_wonder, executor.clone())].into_iter()); + cache.put([(one_hit_wonder, executor)].into_iter()); assert_eq!(cache.executors[&one_hit_wonder].hit_count.load(Relaxed), 1); // one-hit-wonder counter not incremented @@ -576,7 +562,7 @@ mod tests { let program_id1 = Pubkey::new_unique(); let program_id2 = Pubkey::new_unique(); - let executor: Arc = Arc::new(TestExecutor {}); + let executor = Arc::new(LoadedProgram::default()); // make sure we're starting from where we think we are assert_eq!(ComparableStats::from(&cache.stats), expected_stats,); @@ -606,7 +592,7 @@ mod tests { assert_eq!(ComparableStats::from(&cache.stats), expected_stats); // evict an executor - cache.put([(Pubkey::new_unique(), executor.clone())].into_iter()); + cache.put([(Pubkey::new_unique(), executor)].into_iter()); expected_stats.insertions += 1; expected_stats.evictions.insert(program_id2, 1); assert_eq!(ComparableStats::from(&cache.stats), expected_stats); diff --git a/program-runtime/src/lib.rs b/program-runtime/src/lib.rs index f7a47e114f..bf3a5d7062 100644 --- a/program-runtime/src/lib.rs +++ b/program-runtime/src/lib.rs @@ -11,7 +11,6 @@ extern crate solana_metrics; pub mod accounts_data_meter; pub mod compute_budget; -pub mod executor; pub mod executor_cache; pub mod invoke_context; pub mod loaded_programs; diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index b8e2f6c491..b1619766ec 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -1,5 +1,6 @@ use { - crate::invoke_context::InvokeContext, + crate::{invoke_context::InvokeContext, timings::ExecuteDetailsTimings}, + solana_measure::measure::Measure, solana_rbpf::{ elf::Executable, error::EbpfError, @@ -8,6 +9,7 @@ use { }, solana_sdk::{ bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable, clock::Slot, pubkey::Pubkey, + saturating_add_assign, }, std::{ collections::HashMap, @@ -66,7 +68,13 @@ impl Debug for LoadedProgramType { } } -#[derive(Debug)] +impl Default for LoadedProgramType { + fn default() -> Self { + LoadedProgramType::Invalid + } +} + +#[derive(Debug, Default)] pub struct LoadedProgram { /// The program of this entry pub program: LoadedProgramType, @@ -80,6 +88,35 @@ pub struct LoadedProgram { pub usage_counter: AtomicU64, } +#[derive(Debug, Default)] +pub struct LoadProgramMetrics { + pub program_id: String, + pub register_syscalls_us: u64, + pub load_elf_us: u64, + pub verify_code_us: u64, + pub jit_compile_us: u64, +} + +impl LoadProgramMetrics { + pub fn submit_datapoint(&self, timings: &mut ExecuteDetailsTimings) { + saturating_add_assign!( + timings.create_executor_register_syscalls_us, + self.register_syscalls_us + ); + saturating_add_assign!(timings.create_executor_load_elf_us, self.load_elf_us); + saturating_add_assign!(timings.create_executor_verify_code_us, self.verify_code_us); + saturating_add_assign!(timings.create_executor_jit_compile_us, self.jit_compile_us); + datapoint_trace!( + "create_executor_trace", + ("program_id", self.program_id, String), + ("register_syscalls_us", self.register_syscalls_us, i64), + ("load_elf_us", self.load_elf_us, i64), + ("verify_code_us", self.verify_code_us, i64), + ("jit_compile_us", self.jit_compile_us, i64), + ); + } +} + impl LoadedProgram { /// Creates a new user program pub fn new( @@ -88,16 +125,42 @@ impl LoadedProgram { deployment_slot: Slot, elf_bytes: &[u8], account_size: usize, + use_jit: bool, + metrics: &mut LoadProgramMetrics, ) -> Result { - let program = if bpf_loader_deprecated::check_id(loader_key) { - let executable = Executable::load(elf_bytes, loader.clone())?; + let mut load_elf_time = Measure::start("load_elf_time"); + let executable = Executable::load(elf_bytes, loader.clone())?; + load_elf_time.stop(); + metrics.load_elf_us = load_elf_time.as_us(); + + let mut verify_code_time = Measure::start("verify_code_time"); + + // Allowing mut here, since it may be needed for jit compile, which is under a config flag + #[allow(unused_mut)] + let mut program = if bpf_loader_deprecated::check_id(loader_key) { LoadedProgramType::LegacyV0(VerifiedExecutable::from_executable(executable)?) } else if bpf_loader::check_id(loader_key) || bpf_loader_upgradeable::check_id(loader_key) { - let executable = Executable::load(elf_bytes, loader.clone())?; LoadedProgramType::LegacyV1(VerifiedExecutable::from_executable(executable)?) } else { panic!(); }; + verify_code_time.stop(); + metrics.verify_code_us = verify_code_time.as_us(); + + if use_jit { + #[cfg(all(not(target_os = "windows"), target_arch = "x86_64"))] + { + let mut jit_compile_time = Measure::start("jit_compile_time"); + match &mut program { + LoadedProgramType::LegacyV0(executable) => executable.jit_compile(), + LoadedProgramType::LegacyV1(executable) => executable.jit_compile(), + _ => Err(EbpfError::JitNotCompiled), + }?; + jit_compile_time.stop(); + metrics.jit_compile_us = jit_compile_time.as_us(); + } + } + Ok(Self { deployment_slot, account_size, @@ -120,6 +183,20 @@ impl LoadedProgram { program: LoadedProgramType::BuiltIn(program), } } + + pub fn new_tombstone() -> Self { + Self { + program: LoadedProgramType::Invalid, + account_size: 0, + deployment_slot: 0, + effective_slot: 0, + usage_counter: AtomicU64::default(), + } + } + + pub fn is_tombstone(&self) -> bool { + matches!(self.program, LoadedProgramType::Invalid) + } } #[derive(Debug, Default)] @@ -245,6 +322,13 @@ mod tests { }, }; + #[test] + fn test_tombstone() { + let tombstone = LoadedProgram::new_tombstone(); + assert!(matches!(tombstone.program, LoadedProgramType::Invalid)); + assert!(tombstone.is_tombstone()); + } + struct TestForkGraph { relation: BlockRelation, } diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 19a4a9f0f2..0a6762b4d8 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -18,11 +18,10 @@ use { solana_measure::measure::Measure, solana_program_runtime::{ compute_budget::ComputeBudget, - executor::{CreateMetrics, Executor}, executor_cache::TransactionExecutorCache, ic_logger_msg, ic_msg, invoke_context::InvokeContext, - loaded_programs::LoadedProgram, + loaded_programs::{LoadProgramMetrics, LoadedProgram, LoadedProgramType}, log_collector::LogCollector, stable_log, sysvar_cache::get_sysvar_with_account_check, @@ -30,7 +29,6 @@ use { solana_rbpf::{ aligned_memory::AlignedMemory, ebpf::{HOST_ALIGN, MM_HEAP_START}, - elf::Executable, error::{EbpfError, UserDefinedError}, memory_region::MemoryRegion, verifier::{RequisiteVerifier, VerifierError}, @@ -121,15 +119,19 @@ fn try_borrow_account<'a>( } } -fn create_executor_from_bytes( +#[allow(clippy::too_many_arguments)] +pub fn load_program_from_bytes( feature_set: &FeatureSet, compute_budget: &ComputeBudget, log_collector: Option>>, - create_executor_metrics: &mut CreateMetrics, + load_program_metrics: &mut LoadProgramMetrics, programdata: &[u8], + loader_key: &Pubkey, + account_size: usize, + deployment_slot: Slot, use_jit: bool, reject_deployment_of_broken_elfs: bool, -) -> Result, InstructionError> { +) -> Result { let mut register_syscalls_time = Measure::start("register_syscalls_time"); let disable_deploy_of_alloc_free_syscall = reject_deployment_of_broken_elfs && feature_set.is_active(&disable_deploy_of_alloc_free_syscall::id()); @@ -145,41 +147,22 @@ fn create_executor_from_bytes( InstructionError::ProgramEnvironmentSetupFailure })?; register_syscalls_time.stop(); - create_executor_metrics.register_syscalls_us = register_syscalls_time.as_us(); - let mut load_elf_time = Measure::start("load_elf_time"); - let executable = Executable::::from_elf(programdata, loader).map_err(|err| { + load_program_metrics.register_syscalls_us = register_syscalls_time.as_us(); + + let loaded_program = LoadedProgram::new( + loader_key, + loader, + deployment_slot, + programdata, + account_size, + use_jit, + load_program_metrics, + ) + .map_err(|err| { ic_logger_msg!(log_collector, "{}", err); InstructionError::InvalidAccountData - }); - load_elf_time.stop(); - create_executor_metrics.load_elf_us = load_elf_time.as_us(); - let executable = executable?; - let mut verify_code_time = Measure::start("verify_code_time"); - #[allow(unused_mut)] - let mut verified_executable = - VerifiedExecutable::::from_executable(executable) - .map_err(|err| { - ic_logger_msg!(log_collector, "{}", err); - InstructionError::InvalidAccountData - })?; - verify_code_time.stop(); - create_executor_metrics.verify_code_us = verify_code_time.as_us(); - if use_jit { - #[cfg(all(not(target_os = "windows"), target_arch = "x86_64"))] - { - let mut jit_compile_time = Measure::start("jit_compile_time"); - let jit_compile_result = verified_executable.jit_compile(); - jit_compile_time.stop(); - create_executor_metrics.jit_compile_us = jit_compile_time.as_us(); - if let Err(err) = jit_compile_result { - ic_logger_msg!(log_collector, "Failed to compile program {:?}", err); - return Err(InstructionError::ProgramFailedToCompile); - } - } - } - Ok(Arc::new(BpfExecutor { - verified_executable, - })) + })?; + Ok(loaded_program) } fn get_programdata_offset_and_depoyment_offset( @@ -215,9 +198,11 @@ pub fn load_program_from_account( feature_set: &FeatureSet, compute_budget: &ComputeBudget, log_collector: Option>>, + tx_executor_cache: Option>, program: &BorrowedAccount, programdata: &BorrowedAccount, -) -> Result<(LoadedProgram, Option), InstructionError> { + use_jit: bool, +) -> Result<(Arc, Option), InstructionError> { if !check_loader_id(program.get_owner()) { ic_logger_msg!( log_collector, @@ -228,128 +213,82 @@ pub fn load_program_from_account( let (programdata_offset, deployment_slot) = get_programdata_offset_and_depoyment_offset(&log_collector, program, programdata)?; + + if let Some(ref tx_executor_cache) = tx_executor_cache { + if let Some(loaded_program) = tx_executor_cache.get(program.get_key()) { + if loaded_program.is_tombstone() { + // We cached that the Executor does not exist, abort + // This case can only happen once delay_visibility_of_program_deployment is active. + return Err(InstructionError::InvalidAccountData); + } + // Executor exists and is cached, use it + return Ok((loaded_program, None)); + } + } + let programdata_size = if programdata_offset != 0 { programdata.get_data().len() } else { 0 }; - let mut create_executor_metrics = CreateMetrics { + let mut load_program_metrics = LoadProgramMetrics { program_id: program.get_key().to_string(), - ..CreateMetrics::default() + ..LoadProgramMetrics::default() }; - let mut register_syscalls_time = Measure::start("register_syscalls_time"); - let loader = syscalls::create_loader(feature_set, compute_budget, false, false, false) - .map_err(|e| { - ic_logger_msg!(log_collector, "Failed to register syscalls: {}", e); - InstructionError::ProgramEnvironmentSetupFailure - })?; - register_syscalls_time.stop(); - create_executor_metrics.register_syscalls_us = register_syscalls_time.as_us(); - - let mut load_elf_time = Measure::start("load_elf_time"); - let loaded_program = LoadedProgram::new( - program.get_owner(), - loader, - deployment_slot, - programdata - .get_data() - .get(programdata_offset..) - .ok_or(InstructionError::AccountDataTooSmall)?, - program.get_data().len().saturating_add(programdata_size), - ) - .map_err(|err| { - ic_logger_msg!(log_collector, "{}", err); - InstructionError::InvalidAccountData - })?; - load_elf_time.stop(); - create_executor_metrics.load_elf_us = load_elf_time.as_us(); - - Ok((loaded_program, Some(create_executor_metrics))) -} - -pub fn create_executor_from_account( - feature_set: &FeatureSet, - compute_budget: &ComputeBudget, - log_collector: Option>>, - tx_executor_cache: Option>, - program: &BorrowedAccount, - programdata: &BorrowedAccount, - use_jit: bool, -) -> Result<(Arc, Option), InstructionError> { - if !check_loader_id(program.get_owner()) { - ic_logger_msg!( - log_collector, - "Executable account not owned by the BPF loader" - ); - return Err(InstructionError::IncorrectProgramId); - } - - let (programdata_offset, _) = - get_programdata_offset_and_depoyment_offset(&log_collector, program, programdata)?; - - if let Some(ref tx_executor_cache) = tx_executor_cache { - match tx_executor_cache.get(program.get_key()) { - // Executor exists and is cached, use it - Some(Some(executor)) => return Ok((executor, None)), - // We cached that the Executor does not exist, abort - // This case can only happen once delay_visibility_of_program_deployment is active. - Some(None) => return Err(InstructionError::InvalidAccountData), - // Nothing cached, try to load from account instead - None => {} - } - } - - let mut create_executor_metrics = CreateMetrics { - program_id: program.get_key().to_string(), - ..CreateMetrics::default() - }; - let executor = create_executor_from_bytes( + let loaded_program = Arc::new(load_program_from_bytes( feature_set, compute_budget, log_collector, - &mut create_executor_metrics, + &mut load_program_metrics, programdata .get_data() .get(programdata_offset..) .ok_or(InstructionError::AccountDataTooSmall)?, + program.get_owner(), + program.get_data().len().saturating_add(programdata_size), + deployment_slot, use_jit, false, /* reject_deployment_of_broken_elfs */ - )?; + )?); if let Some(mut tx_executor_cache) = tx_executor_cache { tx_executor_cache.set( *program.get_key(), - executor.clone(), + loaded_program.clone(), false, feature_set.is_active(&delay_visibility_of_program_deployment::id()), ); } - Ok((executor, Some(create_executor_metrics))) + + Ok((loaded_program, Some(load_program_metrics))) } macro_rules! deploy_program { - ($invoke_context:expr, $use_jit:expr, $program_id:expr, - $drop:expr, $new_programdata:expr $(,)?) => {{ + ($invoke_context:expr, $use_jit:expr, $program_id:expr, $loader_key:expr, + $account_size:expr, $slot:expr, $drop:expr, $new_programdata:expr $(,)?) => {{ let delay_visibility_of_program_deployment = $invoke_context .feature_set .is_active(&delay_visibility_of_program_deployment::id()); - let mut create_executor_metrics = CreateMetrics::default(); - let executor = create_executor_from_bytes( + let mut load_program_metrics = LoadProgramMetrics::default(); + let executor = load_program_from_bytes( &$invoke_context.feature_set, $invoke_context.get_compute_budget(), $invoke_context.get_log_collector(), - &mut create_executor_metrics, + &mut load_program_metrics, $new_programdata, + $loader_key, + $account_size, + $slot, $use_jit, true, )?; $drop - create_executor_metrics.program_id = $program_id.to_string(); - create_executor_metrics.submit_datapoint(&mut $invoke_context.timings); + load_program_metrics.program_id = $program_id.to_string(); + load_program_metrics.submit_datapoint(&mut $invoke_context.timings); $invoke_context.tx_executor_cache.borrow_mut().set( $program_id, - executor, + Arc::new(executor), true, delay_visibility_of_program_deployment, ); @@ -516,7 +455,7 @@ fn process_instruction_common( )?) }; let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time"); - let (executor, create_executor_metrics) = create_executor_from_account( + let (executor, load_program_metrics) = load_program_from_account( &invoke_context.feature_set, invoke_context.get_compute_budget(), log_collector, @@ -532,11 +471,15 @@ fn process_instruction_common( invoke_context.timings.get_or_create_executor_us, get_or_create_executor_time.as_us() ); - if let Some(create_executor_metrics) = create_executor_metrics { - create_executor_metrics.submit_datapoint(&mut invoke_context.timings); + if let Some(load_program_metrics) = load_program_metrics { + load_program_metrics.submit_datapoint(&mut invoke_context.timings); + } + match &executor.program { + LoadedProgramType::Invalid => Err(InstructionError::InvalidAccountData), + LoadedProgramType::LegacyV0(executable) => execute(executable, invoke_context), + LoadedProgramType::LegacyV1(executable) => execute(executable, invoke_context), + LoadedProgramType::BuiltIn(_) => Err(InstructionError::IncorrectProgramId), } - - executor.execute(invoke_context) } else { drop(program); debug_assert_eq!(first_instruction_account, 1); @@ -715,6 +658,7 @@ fn process_loader_upgradeable_instruction( buffer.set_lamports(0)?; } + let owner_id = *program_id; let mut instruction = system_instruction::create_account( &payer_key, &programdata_key, @@ -747,6 +691,9 @@ fn process_loader_upgradeable_instruction( invoke_context, use_jit, new_program_id, + &owner_id, + UpgradeableLoaderState::size_of_program().saturating_add(programdata_len), + clock.slot, { drop(buffer); }, @@ -894,7 +841,7 @@ fn process_loader_upgradeable_instruction( ); return Err(InstructionError::InsufficientFunds); } - if let UpgradeableLoaderState::ProgramData { + let deployment_slot = if let UpgradeableLoaderState::ProgramData { slot, upgrade_authority_address, } = programdata.get_state()? @@ -919,10 +866,12 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "Upgrade authority did not sign"); return Err(InstructionError::MissingRequiredSignature); } + slot } else { ic_logger_msg!(log_collector, "Invalid ProgramData account"); return Err(InstructionError::InvalidAccountData); - } + }; + let programdata_len = programdata.get_data().len(); drop(programdata); // Load and verify the program bits @@ -932,6 +881,9 @@ fn process_loader_upgradeable_instruction( invoke_context, use_jit, new_program_id, + program_id, + UpgradeableLoaderState::size_of_program().saturating_add(programdata_len), + deployment_slot, { drop(buffer); }, @@ -1457,6 +1409,9 @@ fn process_loader_instruction( invoke_context, use_jit, *program.get_key(), + program.get_owner(), + program.get_data().len(), + 0, {}, program.get_data(), ); @@ -1468,161 +1423,145 @@ fn process_loader_instruction( Ok(()) } -/// BPF Loader's Executor implementation -pub struct BpfExecutor { - verified_executable: VerifiedExecutable>, -} +fn execute( + executable: &VerifiedExecutable>, + invoke_context: &mut InvokeContext, +) -> Result<(), InstructionError> { + let log_collector = invoke_context.get_log_collector(); + let stack_height = invoke_context.get_stack_height(); + let transaction_context = &invoke_context.transaction_context; + let instruction_context = transaction_context.get_current_instruction_context()?; + let program_id = *instruction_context.get_last_program_key(transaction_context)?; + #[cfg(any(target_os = "windows", not(target_arch = "x86_64")))] + let use_jit = false; + #[cfg(all(not(target_os = "windows"), target_arch = "x86_64"))] + let use_jit = executable.get_executable().get_compiled_program().is_some(); -// Well, implement Debug for solana_rbpf::vm::Executable in solana-rbpf... -impl Debug for BpfExecutor { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!(f, "BpfExecutor({self:p})") - } -} + let mut serialize_time = Measure::start("serialize"); + let (parameter_bytes, regions, account_lengths) = serialize_parameters( + invoke_context.transaction_context, + instruction_context, + invoke_context + .feature_set + .is_active(&cap_bpf_program_instruction_accounts::ID), + )?; + serialize_time.stop(); -impl Executor for BpfExecutor { - fn execute(&self, invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { - let log_collector = invoke_context.get_log_collector(); - let stack_height = invoke_context.get_stack_height(); - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let program_id = *instruction_context.get_last_program_key(transaction_context)?; - #[cfg(any(target_os = "windows", not(target_arch = "x86_64")))] - let use_jit = false; - #[cfg(all(not(target_os = "windows"), target_arch = "x86_64"))] - let use_jit = self - .verified_executable - .get_executable() - .get_compiled_program() - .is_some(); - - let mut serialize_time = Measure::start("serialize"); - let (parameter_bytes, regions, account_lengths) = serialize_parameters( - invoke_context.transaction_context, - instruction_context, - invoke_context - .feature_set - .is_active(&cap_bpf_program_instruction_accounts::ID), - )?; - serialize_time.stop(); - - let mut create_vm_time = Measure::start("create_vm"); - let mut execute_time; - let execution_result = { - let compute_meter_prev = invoke_context.get_remaining(); - let mut vm = match create_vm( - // We dropped the lifetime tracking in the Executor by setting it to 'static, - // thus we need to reintroduce the correct lifetime of InvokeContext here again. - unsafe { std::mem::transmute(&self.verified_executable) }, - regions, - account_lengths, - invoke_context, - ) { - Ok(info) => info, - Err(e) => { - ic_logger_msg!(log_collector, "Failed to create SBF VM: {}", e); - return Err(InstructionError::ProgramEnvironmentSetupFailure); - } - }; - create_vm_time.stop(); - - execute_time = Measure::start("execute"); - stable_log::program_invoke(&log_collector, &program_id, stack_height); - let (compute_units_consumed, result) = vm.execute_program(!use_jit); - drop(vm); - ic_logger_msg!( - log_collector, - "Program {} consumed {} of {} compute units", - &program_id, - compute_units_consumed, - compute_meter_prev - ); - let (_returned_from_program_id, return_data) = - invoke_context.transaction_context.get_return_data(); - if !return_data.is_empty() { - stable_log::program_return(&log_collector, &program_id, return_data); - } - match result { - ProgramResult::Ok(status) if status != SUCCESS => { - let error: InstructionError = if (status - == MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED - && !invoke_context - .feature_set - .is_active(&cap_accounts_data_allocations_per_transaction::id())) - || (status == MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED - && !invoke_context - .feature_set - .is_active(&limit_max_instruction_trace_length::id())) - { - // Until the cap_accounts_data_allocations_per_transaction feature is - // enabled, map the `MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED` error to `InvalidError`. - // Until the limit_max_instruction_trace_length feature is - // enabled, map the `MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED` error to `InvalidError`. - InstructionError::InvalidError - } else { - status.into() - }; - stable_log::program_failure(&log_collector, &program_id, &error); - Err(error) - } - ProgramResult::Err(error) => { - let error = match error { - /*EbpfError::UserError(user_error) if let BpfError::SyscallError( - SyscallError::InstructionError(instruction_error), - ) = user_error.downcast_ref::().unwrap() => instruction_error.clone(),*/ - EbpfError::UserError(user_error) - if matches!( - user_error.downcast_ref::().unwrap(), - BpfError::SyscallError(SyscallError::InstructionError(_)), - ) => - { - match user_error.downcast_ref::().unwrap() { - BpfError::SyscallError(SyscallError::InstructionError( - instruction_error, - )) => instruction_error.clone(), - _ => unreachable!(), - } - } - err => { - ic_logger_msg!(log_collector, "Program failed to complete: {}", err); - InstructionError::ProgramFailedToComplete - } - }; - stable_log::program_failure(&log_collector, &program_id, &error); - Err(error) - } - _ => Ok(()), + let mut create_vm_time = Measure::start("create_vm"); + let mut execute_time; + let execution_result = { + let compute_meter_prev = invoke_context.get_remaining(); + let mut vm = match create_vm( + // We dropped the lifetime tracking in the Executor by setting it to 'static, + // thus we need to reintroduce the correct lifetime of InvokeContext here again. + unsafe { std::mem::transmute(executable) }, + regions, + account_lengths, + invoke_context, + ) { + Ok(info) => info, + Err(e) => { + ic_logger_msg!(log_collector, "Failed to create SBF VM: {}", e); + return Err(InstructionError::ProgramEnvironmentSetupFailure); } }; - execute_time.stop(); + create_vm_time.stop(); - let mut deserialize_time = Measure::start("deserialize"); - let execute_or_deserialize_result = execution_result.and_then(|_| { - deserialize_parameters( - invoke_context.transaction_context, - invoke_context - .transaction_context - .get_current_instruction_context()?, - parameter_bytes.as_slice(), - invoke_context.get_orig_account_lengths()?, - ) - }); - deserialize_time.stop(); - - // Update the timings - let timings = &mut invoke_context.timings; - timings.serialize_us = timings.serialize_us.saturating_add(serialize_time.as_us()); - timings.create_vm_us = timings.create_vm_us.saturating_add(create_vm_time.as_us()); - timings.execute_us = timings.execute_us.saturating_add(execute_time.as_us()); - timings.deserialize_us = timings - .deserialize_us - .saturating_add(deserialize_time.as_us()); - - if execute_or_deserialize_result.is_ok() { - stable_log::program_success(&log_collector, &program_id); + execute_time = Measure::start("execute"); + stable_log::program_invoke(&log_collector, &program_id, stack_height); + let (compute_units_consumed, result) = vm.execute_program(!use_jit); + drop(vm); + ic_logger_msg!( + log_collector, + "Program {} consumed {} of {} compute units", + &program_id, + compute_units_consumed, + compute_meter_prev + ); + let (_returned_from_program_id, return_data) = + invoke_context.transaction_context.get_return_data(); + if !return_data.is_empty() { + stable_log::program_return(&log_collector, &program_id, return_data); } - execute_or_deserialize_result + match result { + ProgramResult::Ok(status) if status != SUCCESS => { + let error: InstructionError = if (status == MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED + && !invoke_context + .feature_set + .is_active(&cap_accounts_data_allocations_per_transaction::id())) + || (status == MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED + && !invoke_context + .feature_set + .is_active(&limit_max_instruction_trace_length::id())) + { + // Until the cap_accounts_data_allocations_per_transaction feature is + // enabled, map the `MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED` error to `InvalidError`. + // Until the limit_max_instruction_trace_length feature is + // enabled, map the `MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED` error to `InvalidError`. + InstructionError::InvalidError + } else { + status.into() + }; + stable_log::program_failure(&log_collector, &program_id, &error); + Err(error) + } + ProgramResult::Err(error) => { + let error = match error { + /*EbpfError::UserError(user_error) if let BpfError::SyscallError( + SyscallError::InstructionError(instruction_error), + ) = user_error.downcast_ref::().unwrap() => instruction_error.clone(),*/ + EbpfError::UserError(user_error) + if matches!( + user_error.downcast_ref::().unwrap(), + BpfError::SyscallError(SyscallError::InstructionError(_)), + ) => + { + match user_error.downcast_ref::().unwrap() { + BpfError::SyscallError(SyscallError::InstructionError( + instruction_error, + )) => instruction_error.clone(), + _ => unreachable!(), + } + } + err => { + ic_logger_msg!(log_collector, "Program failed to complete: {}", err); + InstructionError::ProgramFailedToComplete + } + }; + stable_log::program_failure(&log_collector, &program_id, &error); + Err(error) + } + _ => Ok(()), + } + }; + execute_time.stop(); + + let mut deserialize_time = Measure::start("deserialize"); + let execute_or_deserialize_result = execution_result.and_then(|_| { + deserialize_parameters( + invoke_context.transaction_context, + invoke_context + .transaction_context + .get_current_instruction_context()?, + parameter_bytes.as_slice(), + invoke_context.get_orig_account_lengths()?, + ) + }); + deserialize_time.stop(); + + // Update the timings + let timings = &mut invoke_context.timings; + timings.serialize_us = timings.serialize_us.saturating_add(serialize_time.as_us()); + timings.create_vm_us = timings.create_vm_us.saturating_add(create_vm_time.as_us()); + timings.execute_us = timings.execute_us.saturating_add(execute_time.as_us()); + timings.deserialize_us = timings + .deserialize_us + .saturating_add(deserialize_time.as_us()); + + if execute_or_deserialize_result.is_ok() { + stable_log::program_success(&log_collector, &program_id); } + execute_or_deserialize_result } #[cfg(test)] @@ -1633,6 +1572,7 @@ mod tests { solana_program_runtime::invoke_context::mock_process_instruction, solana_rbpf::{ ebpf::MM_INPUT_START, + elf::Executable, verifier::Verifier, vm::{BuiltInProgram, Config, ContextObject, FunctionRegistry}, }, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index aa046e523f..e8bdda6ffd 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -92,10 +92,9 @@ use { solana_program_runtime::{ accounts_data_meter::MAX_ACCOUNTS_DATA_LEN, compute_budget::{self, ComputeBudget}, - executor::Executor, executor_cache::{BankExecutorCache, TransactionExecutorCache, MAX_CACHED_EXECUTORS}, invoke_context::{BuiltinProgram, ProcessInstructionWithContext}, - loaded_programs::{LoadedPrograms, WorkingSlot}, + loaded_programs::{LoadedProgram, LoadedPrograms, WorkingSlot}, log_collector::LogCollector, sysvar_cache::SysvarCache, timings::{ExecuteTimingType, ExecuteTimings}, @@ -4056,8 +4055,13 @@ impl Bank { } } + /// Remove an executor from the bank's cache + fn remove_executor(&self, pubkey: &Pubkey) { + let _ = self.executor_cache.write().unwrap().remove(pubkey); + } + #[allow(dead_code)] // Preparation for BankExecutorCache rework - fn create_executor(&self, pubkey: &Pubkey) -> Result> { + fn load_program(&self, pubkey: &Pubkey) -> Result> { let program = if let Some(program) = self.get_account_with_fixed_root(pubkey) { program } else { @@ -4110,24 +4114,19 @@ impl Bank { } else { None }; - solana_bpf_loader_program::create_executor_from_account( + solana_bpf_loader_program::load_program_from_account( &self.feature_set, &self.runtime_config.compute_budget.unwrap_or_default(), None, // log_collector - None, // tx_executor_cache + None, &program, programdata.as_ref().unwrap_or(&program), self.runtime_config.bpf_jit, ) - .map(|(executor, _create_executor_metrics)| executor) + .map(|(loaded_program, _create_executor_metrics)| loaded_program) .map_err(|err| TransactionError::InstructionError(0, err)) } - /// Remove an executor from the bank's cache - fn remove_executor(&self, pubkey: &Pubkey) { - let _ = self.executor_cache.write().unwrap().remove(pubkey); - } - pub fn clear_executors(&self) { self.executor_cache.write().unwrap().clear(); } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 46758a39b5..96356c95b6 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -33,9 +33,9 @@ use { solana_logger, solana_program_runtime::{ compute_budget::{self, ComputeBudget, MAX_COMPUTE_UNIT_LIMIT}, - executor::Executor, executor_cache::TransactionExecutorCache, invoke_context::{mock_process_instruction, InvokeContext}, + loaded_programs::{LoadedProgram, LoadedProgramType}, prioritization_fee::{PrioritizationFeeDetails, PrioritizationFeeType}, timings::ExecuteTimings, }, @@ -7663,17 +7663,6 @@ fn test_reconfigure_token2_native_mint() { assert_eq!(native_mint_account.owner(), &inline_spl_token::id()); } -#[derive(Debug)] -struct TestExecutor {} -impl Executor for TestExecutor { - fn execute( - &self, - _invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { - Ok(()) - } -} - #[test] fn test_bank_executor_cache() { solana_logger::setup(); @@ -7686,7 +7675,7 @@ fn test_bank_executor_cache() { let key3 = solana_sdk::pubkey::new_rand(); let key4 = solana_sdk::pubkey::new_rand(); let key5 = solana_sdk::pubkey::new_rand(); - let executor: Arc = Arc::new(TestExecutor {}); + let executor = Arc::new(LoadedProgram::default()); fn new_executable_account(owner: Pubkey) -> AccountSharedData { AccountSharedData::from(Account { @@ -7719,7 +7708,7 @@ fn test_bank_executor_cache() { executors.set(key1, executor.clone(), false, true); executors.set(key2, executor.clone(), false, true); executors.set(key3, executor.clone(), true, true); - executors.set(key4, executor.clone(), false, true); + executors.set(key4, executor, false, true); let executors = Rc::new(RefCell::new(executors)); // store Missing @@ -7776,7 +7765,6 @@ fn test_bank_executor_cache() { programdata_account.set_rent_epoch(1); bank.store_account_and_update_capitalization(&key1, &program_account); bank.store_account_and_update_capitalization(&programdata_key, &programdata_account); - bank.create_executor(&key1).unwrap(); // Remove all bank.remove_executor(&key1); @@ -7787,6 +7775,55 @@ fn test_bank_executor_cache() { assert_eq!(stored_executors.borrow().visible.len(), 0); } +#[test] +fn test_bank_load_program() { + solana_logger::setup(); + + let (genesis_config, _) = create_genesis_config(1); + let bank = Bank::new_for_tests(&genesis_config); + + let key1 = solana_sdk::pubkey::new_rand(); + + let mut file = File::open("../programs/bpf_loader/test_elfs/out/noop_aligned.so").unwrap(); + let mut elf = Vec::new(); + file.read_to_end(&mut elf).unwrap(); + let programdata_key = solana_sdk::pubkey::new_rand(); + let mut program_account = AccountSharedData::new_data( + 40, + &UpgradeableLoaderState::Program { + programdata_address: programdata_key, + }, + &bpf_loader_upgradeable::id(), + ) + .unwrap(); + program_account.set_executable(true); + program_account.set_rent_epoch(1); + let programdata_data_offset = UpgradeableLoaderState::size_of_programdata_metadata(); + let mut programdata_account = AccountSharedData::new( + 40, + programdata_data_offset + elf.len(), + &bpf_loader_upgradeable::id(), + ); + programdata_account + .set_state(&UpgradeableLoaderState::ProgramData { + slot: 42, + upgrade_authority_address: None, + }) + .unwrap(); + programdata_account.data_mut()[programdata_data_offset..].copy_from_slice(&elf); + programdata_account.set_rent_epoch(1); + bank.store_account_and_update_capitalization(&key1, &program_account); + bank.store_account_and_update_capitalization(&programdata_key, &programdata_account); + let program = bank.load_program(&key1); + assert!(program.is_ok()); + let program = program.unwrap(); + assert!(matches!(program.program, LoadedProgramType::LegacyV1(_))); + assert_eq!( + program.account_size, + program_account.data().len() + programdata_account.data().len() + ); +} + #[test] fn test_bank_executor_cow() { solana_logger::setup(); @@ -7796,7 +7833,7 @@ fn test_bank_executor_cow() { let key1 = solana_sdk::pubkey::new_rand(); let key2 = solana_sdk::pubkey::new_rand(); - let executor: Arc = Arc::new(TestExecutor {}); + let executor = Arc::new(LoadedProgram::default()); let executable_account = AccountSharedData::from(Account { owner: bpf_loader_upgradeable::id(), executable: true, @@ -7825,7 +7862,7 @@ fn test_bank_executor_cow() { assert_eq!(executors.borrow().visible.len(), 1); let mut executors = TransactionExecutorCache::default(); - executors.set(key2, executor.clone(), false, true); + executors.set(key2, executor, false, true); let executors = Rc::new(RefCell::new(executors)); fork1.store_executors_which_added_to_the_cache(&executors);