diff --git a/cli/src/program.rs b/cli/src/program.rs index d4be4c8aa8..baf189372f 100644 --- a/cli/src/program.rs +++ b/cli/src/program.rs @@ -2083,7 +2083,7 @@ fn read_and_verify_elf(program_location: &str) -> Result, Box::from_elf( @@ -2092,7 +2092,7 @@ fn read_and_verify_elf(program_location: &str) -> Result, Box InstructionError { - ic_msg!(invoke_context, "{}", e); - InstructionError::InvalidAccountData -} - mod executor_metrics { + use super::*; + #[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) { + 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), @@ -144,27 +157,26 @@ fn try_borrow_account<'a>( } } -pub fn create_executor( - programdata_account_index: IndexOfAccount, - programdata_offset: usize, - invoke_context: &mut InvokeContext, +fn create_executor_from_bytes( + feature_set: &FeatureSet, + compute_budget: &ComputeBudget, + log_collector: Option>>, + create_executor_metrics: &mut executor_metrics::CreateMetrics, + programdata: &[u8], use_jit: bool, reject_deployment_of_broken_elfs: bool, - disable_deploy_of_alloc_free_syscall: bool, ) -> Result, InstructionError> { 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()); let register_syscall_result = - syscalls::register_syscalls(invoke_context, disable_deploy_of_alloc_free_syscall); + syscalls::register_syscalls(feature_set, disable_deploy_of_alloc_free_syscall); register_syscalls_time.stop(); - invoke_context.timings.create_executor_register_syscalls_us = invoke_context - .timings - .create_executor_register_syscalls_us - .saturating_add(register_syscalls_time.as_us()); + create_executor_metrics.register_syscalls_us = register_syscalls_time.as_us(); let syscall_registry = register_syscall_result.map_err(|e| { - ic_msg!(invoke_context, "Failed to register syscalls: {}", e); + ic_logger_msg!(log_collector, "Failed to register syscalls: {}", e); InstructionError::ProgramEnvironmentSetupFailure })?; - let compute_budget = invoke_context.get_compute_budget(); let config = Config { max_call_depth: compute_budget.max_call_depth, stack_frame_size: compute_budget.stack_frame_size, @@ -177,12 +189,9 @@ pub fn create_executor( noop_instruction_rate: 256, sanitize_user_provided_values: true, encrypt_environment_registers: true, - syscall_bpf_function_hash_collision: invoke_context - .feature_set + syscall_bpf_function_hash_collision: feature_set .is_active(&error_on_syscall_bpf_function_hash_collisions::id()), - reject_callx_r10: invoke_context - .feature_set - .is_active(&reject_callx_r10::id()), + reject_callx_r10: feature_set.is_active(&reject_callx_r10::id()), dynamic_stack_frames: false, enable_sdiv: false, optimize_rodata: false, @@ -193,65 +202,117 @@ pub fn create_executor( aligned_memory_mapping: true, // Warning, do not use `Config::default()` so that configuration here is explicit. }; - let mut create_executor_metrics = executor_metrics::CreateMetrics::default(); - let executable = { - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let programdata = try_borrow_account( - transaction_context, - instruction_context, - programdata_account_index, - )?; - create_executor_metrics.program_id = programdata.get_key().to_string(); - let mut load_elf_time = Measure::start("load_elf_time"); - let executable = Executable::::from_elf( - programdata - .get_data() - .get(programdata_offset..) - .ok_or(InstructionError::AccountDataTooSmall)?, - config, - syscall_registry, - ); - load_elf_time.stop(); - create_executor_metrics.load_elf_us = load_elf_time.as_us(); - invoke_context.timings.create_executor_load_elf_us = invoke_context - .timings - .create_executor_load_elf_us - .saturating_add(create_executor_metrics.load_elf_us); - executable - } - .map_err(|e| map_ebpf_error(invoke_context, e))?; + let mut load_elf_time = Measure::start("load_elf_time"); + let executable = + Executable::::from_elf(programdata, config, syscall_registry) + .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"); let mut verified_executable = VerifiedExecutable::::from_executable(executable) - .map_err(|e| map_ebpf_error(invoke_context, e))?; + .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(); - invoke_context.timings.create_executor_verify_code_us = invoke_context - .timings - .create_executor_verify_code_us - .saturating_add(create_executor_metrics.verify_code_us); if use_jit { 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(); - invoke_context.timings.create_executor_jit_compile_us = invoke_context - .timings - .create_executor_jit_compile_us - .saturating_add(create_executor_metrics.jit_compile_us); if let Err(err) = jit_compile_result { - ic_msg!(invoke_context, "Failed to compile program {:?}", err); + ic_logger_msg!(log_collector, "Failed to compile program {:?}", err); return Err(InstructionError::ProgramFailedToCompile); } } - create_executor_metrics.submit_datapoint(); Ok(Arc::new(BpfExecutor { verified_executable, use_jit, })) } +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 = if bpf_loader_upgradeable::check_id(program.get_owner()) { + if let UpgradeableLoaderState::Program { + programdata_address, + } = program.get_state()? + { + if &programdata_address != programdata.get_key() { + ic_logger_msg!( + log_collector, + "Wrong ProgramData account for this Program account" + ); + return Err(InstructionError::InvalidArgument); + } + if !matches!( + programdata.get_state()?, + UpgradeableLoaderState::ProgramData { + slot: _, + upgrade_authority_address: _, + } + ) { + ic_logger_msg!(log_collector, "Program has been closed"); + return Err(InstructionError::InvalidAccountData); + } + UpgradeableLoaderState::size_of_programdata_metadata() + } else { + ic_logger_msg!(log_collector, "Invalid Program account"); + return Err(InstructionError::InvalidAccountData); + } + } else { + 0 + }; + + if let Some(ref tx_executor_cache) = tx_executor_cache { + if let Some(executor) = tx_executor_cache.get(program.get_key()) { + return Ok((executor, None)); + } + } + + let mut create_executor_metrics = executor_metrics::CreateMetrics { + program_id: program.get_key().to_string(), + ..executor_metrics::CreateMetrics::default() + }; + let executor = create_executor_from_bytes( + feature_set, + compute_budget, + log_collector, + &mut create_executor_metrics, + programdata + .get_data() + .get(programdata_offset..) + .ok_or(InstructionError::AccountDataTooSmall)?, + 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(), false); + } + Ok((executor, Some(create_executor_metrics))) +} + fn write_program_data( program_account_index: IndexOfAccount, program_data_offset: usize, @@ -397,76 +458,35 @@ fn process_instruction_common( invoke_context.get_stack_height() > 1 ); - 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 program_data_offset = if bpf_loader_upgradeable::check_id(program.get_owner()) { - if let UpgradeableLoaderState::Program { - programdata_address, - } = program.get_state()? - { - if programdata_address != *first_account_key { - ic_logger_msg!( - log_collector, - "Wrong ProgramData account for this Program account" - ); - return Err(InstructionError::InvalidArgument); - } - if !matches!( - instruction_context - .try_borrow_program_account(transaction_context, first_instruction_account)? - .get_state()?, - UpgradeableLoaderState::ProgramData { - slot: _, - upgrade_authority_address: _, - } - ) { - ic_logger_msg!(log_collector, "Program has been closed"); - return Err(InstructionError::InvalidAccountData); - } - UpgradeableLoaderState::size_of_programdata_metadata() - } else { - ic_logger_msg!(log_collector, "Invalid Program account"); - return Err(InstructionError::InvalidAccountData); - } + let programdata = if program_account_index == first_instruction_account { + None } else { - 0 - }; - drop(program); - - let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time"); - let cached_executor = invoke_context.tx_executor_cache.borrow().get(program_id); - let executor = if let Some(executor) = cached_executor { - executor - } else { - let executor = create_executor( + Some(try_borrow_account( + transaction_context, + instruction_context, first_instruction_account, - program_data_offset, - invoke_context, - use_jit, - false, /* reject_deployment_of_broken_elfs */ - // allow _sol_alloc_free syscall for execution - false, /* disable_sol_alloc_free_syscall */ - )?; - 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)?; - invoke_context - .tx_executor_cache - .borrow_mut() - .set(*program_id, executor.clone(), false); - executor + )?) }; + let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time"); + let (executor, create_executor_metrics) = create_executor_from_account( + &invoke_context.feature_set, + invoke_context.get_compute_budget(), + log_collector, + Some(invoke_context.tx_executor_cache.borrow_mut()), + &program, + programdata.as_ref().unwrap_or(&program), + use_jit, + )?; + drop(program); + drop(programdata); get_or_create_executor_time.stop(); saturating_add_assign!( 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); + } executor.execute(program_account_index, invoke_context) } else { @@ -677,16 +697,26 @@ fn process_loader_upgradeable_instruction( invoke_context.native_invoke(instruction, signers.as_slice())?; // Load and verify the program bits - let executor = create_executor( - first_instruction_account.saturating_add(3), - buffer_data_offset, - invoke_context, + let transaction_context = &invoke_context.transaction_context; + let instruction_context = transaction_context.get_current_instruction_context()?; + let buffer = + instruction_context.try_borrow_instruction_account(transaction_context, 3)?; + let mut create_executor_metrics = executor_metrics::CreateMetrics::default(); + let executor = create_executor_from_bytes( + &invoke_context.feature_set, + invoke_context.get_compute_budget(), + invoke_context.get_log_collector(), + &mut create_executor_metrics, + buffer + .get_data() + .get(buffer_data_offset..) + .ok_or(InstructionError::AccountDataTooSmall)?, use_jit, true, - invoke_context - .feature_set - .is_active(&disable_deploy_of_alloc_free_syscall::id()), )?; + drop(buffer); + create_executor_metrics.program_id = new_program_id.to_string(); + create_executor_metrics.submit_datapoint(&mut invoke_context.timings); invoke_context .tx_executor_cache .borrow_mut() @@ -848,16 +878,24 @@ fn process_loader_upgradeable_instruction( drop(programdata); // Load and verify the program bits - let executor = create_executor( - first_instruction_account.saturating_add(2), - buffer_data_offset, - invoke_context, + let buffer = + instruction_context.try_borrow_instruction_account(transaction_context, 2)?; + let mut create_executor_metrics = executor_metrics::CreateMetrics::default(); + let executor = create_executor_from_bytes( + &invoke_context.feature_set, + invoke_context.get_compute_budget(), + invoke_context.get_log_collector(), + &mut create_executor_metrics, + buffer + .get_data() + .get(buffer_data_offset..) + .ok_or(InstructionError::AccountDataTooSmall)?, use_jit, true, - invoke_context - .feature_set - .is_active(&disable_deploy_of_alloc_free_syscall::id()), )?; + drop(buffer); + create_executor_metrics.program_id = new_program_id.to_string(); + create_executor_metrics.submit_datapoint(&mut invoke_context.timings); invoke_context .tx_executor_cache .borrow_mut() @@ -1238,7 +1276,7 @@ fn process_loader_instruction( let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); let program_id = instruction_context.get_last_program_key(transaction_context)?; - let program = instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let mut program = instruction_context.try_borrow_instruction_account(transaction_context, 0)?; if program.get_owner() != program_id { ic_msg!( invoke_context, @@ -1247,13 +1285,13 @@ fn process_loader_instruction( return Err(InstructionError::IncorrectProgramId); } let is_program_signer = program.is_signer(); - drop(program); match limited_deserialize(instruction_data)? { LoaderInstruction::Write { offset, bytes } => { if !is_program_signer { ic_msg!(invoke_context, "Program account did not sign"); return Err(InstructionError::MissingRequiredSignature); } + drop(program); write_program_data( first_instruction_account, offset as usize, @@ -1266,20 +1304,18 @@ fn process_loader_instruction( ic_msg!(invoke_context, "key[0] did not sign the transaction"); return Err(InstructionError::MissingRequiredSignature); } - let executor = create_executor( - first_instruction_account, - 0, - invoke_context, + let mut create_executor_metrics = executor_metrics::CreateMetrics::default(); + let executor = create_executor_from_bytes( + &invoke_context.feature_set, + invoke_context.get_compute_budget(), + invoke_context.get_log_collector(), + &mut create_executor_metrics, + program.get_data(), use_jit, true, - invoke_context - .feature_set - .is_active(&disable_deploy_of_alloc_free_syscall::id()), )?; - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let mut program = - instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + create_executor_metrics.program_id = program.get_key().to_string(); + create_executor_metrics.submit_datapoint(&mut invoke_context.timings); invoke_context .tx_executor_cache .borrow_mut() diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index a21526c064..e5d9be8040 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -28,6 +28,7 @@ use { account_info::AccountInfo, blake3, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable, entrypoint::{BPF_ALIGN_OF_U128, MAX_PERMITTED_DATA_INCREASE, SUCCESS}, + feature_set::FeatureSet, feature_set::{ self, blake3_syscall_enabled, check_physical_overlapping, curve25519_syscall_enabled, disable_cpi_setting_executable_and_rent_epoch, disable_fees_sysvar, @@ -151,18 +152,12 @@ macro_rules! register_feature_gated_syscall { } pub fn register_syscalls( - invoke_context: &mut InvokeContext, + feature_set: &FeatureSet, disable_deploy_of_alloc_free_syscall: bool, ) -> Result { - let blake3_syscall_enabled = invoke_context - .feature_set - .is_active(&blake3_syscall_enabled::id()); - let curve25519_syscall_enabled = invoke_context - .feature_set - .is_active(&curve25519_syscall_enabled::id()); - let disable_fees_sysvar = invoke_context - .feature_set - .is_active(&disable_fees_sysvar::id()); + let blake3_syscall_enabled = feature_set.is_active(&blake3_syscall_enabled::id()); + let curve25519_syscall_enabled = feature_set.is_active(&curve25519_syscall_enabled::id()); + let disable_fees_sysvar = feature_set.is_active(&disable_fees_sysvar::id()); let is_abi_v2 = false; let mut syscall_registry = SyscallRegistry::default(); diff --git a/programs/sbf/benches/bpf_loader.rs b/programs/sbf/benches/bpf_loader.rs index c5ef8c57fe..3db833ec13 100644 --- a/programs/sbf/benches/bpf_loader.rs +++ b/programs/sbf/benches/bpf_loader.rs @@ -110,7 +110,7 @@ fn bench_program_alu(bencher: &mut Bencher) { let executable = Executable::::from_elf( &elf, Config::default(), - register_syscalls(invoke_context, true).unwrap(), + register_syscalls(&invoke_context.feature_set, true).unwrap(), ) .unwrap(); @@ -238,7 +238,7 @@ fn bench_create_vm(bencher: &mut Bencher) { let executable = Executable::::from_elf( &elf, Config::default(), - register_syscalls(invoke_context, true).unwrap(), + register_syscalls(&invoke_context.feature_set, true).unwrap(), ) .unwrap(); @@ -285,7 +285,7 @@ fn bench_instruction_count_tuner(_bencher: &mut Bencher) { let executable = Executable::::from_elf( &elf, Config::default(), - register_syscalls(invoke_context, true).unwrap(), + register_syscalls(&invoke_context.feature_set, true).unwrap(), ) .unwrap(); diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index b5119f9ae6..aa049455f7 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -234,7 +234,11 @@ fn run_program(name: &str) -> u64 { let executable = Executable::::from_elf( &data, config, - register_syscalls(invoke_context, true /* no sol_alloc_free */).unwrap(), + register_syscalls( + &invoke_context.feature_set, + true, /* no sol_alloc_free */ + ) + .unwrap(), ) .unwrap(); diff --git a/rbpf-cli/src/main.rs b/rbpf-cli/src/main.rs index fcc97ae257..c8c94c6245 100644 --- a/rbpf-cli/src/main.rs +++ b/rbpf-cli/src/main.rs @@ -252,7 +252,7 @@ native machine code before execting it in the virtual machine.", file.seek(SeekFrom::Start(0)).unwrap(); let mut contents = Vec::new(); file.read_to_end(&mut contents).unwrap(); - let syscall_registry = register_syscalls(&mut invoke_context, true).unwrap(); + let syscall_registry = register_syscalls(&invoke_context.feature_set, true).unwrap(); let executable = if magic == [0x7f, 0x45, 0x4c, 0x46] { Executable::::from_elf(&contents, config, syscall_registry) .map_err(|err| format!("Executable constructor failed: {:?}", err))