From d5525c28f62550df263910145994bd1a047d7d9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Thu, 13 Jul 2023 14:35:09 +0200 Subject: [PATCH] Refactor - FunctionRegistry (#484) * Turns FunctionRegistry into a struct. * Use FunctionRegistry in BuiltinProgram too. --- benches/elf_loader.rs | 15 +- benches/jit_compile.rs | 6 +- benches/vm_execution.rs | 11 +- cli/src/main.rs | 16 +- examples/disassemble.rs | 6 +- examples/to_json.rs | 6 +- fuzz/fuzz_targets/dumb.rs | 6 +- fuzz/fuzz_targets/smart.rs | 6 +- fuzz/fuzz_targets/smart_jit_diff.rs | 6 +- fuzz/fuzz_targets/smarter_jit_diff.rs | 6 +- fuzz/fuzz_targets/verify_semantic_aware.rs | 3 +- src/assembler.rs | 32 +-- src/disassembler.rs | 12 +- src/elf.rs | 280 +++++++++++++-------- src/interpreter.rs | 6 +- src/jit.rs | 35 +-- src/static_analysis.rs | 20 +- src/verifier.rs | 12 +- src/vm.rs | 94 +++---- tests/assembler.rs | 14 +- tests/disassembler.rs | 12 +- tests/execution.rs | 54 ++-- tests/verifier.rs | 81 +++--- 23 files changed, 402 insertions(+), 337 deletions(-) diff --git a/benches/elf_loader.rs b/benches/elf_loader.rs index 57451249..deecd307 100644 --- a/benches/elf_loader.rs +++ b/benches/elf_loader.rs @@ -11,20 +11,23 @@ extern crate test; extern crate test_utils; use solana_rbpf::{ - elf::Executable, + elf::{Executable, FunctionRegistry}, syscalls, verifier::TautologyVerifier, - vm::{BuiltinProgram, Config, TestContextObject}, + vm::{BuiltinFunction, BuiltinProgram, Config, TestContextObject}, }; use std::{fs::File, io::Read, sync::Arc}; use test::Bencher; fn loader() -> Arc> { - let mut loader = BuiltinProgram::new_loader(Config::default()); - loader - .register_function(b"log", syscalls::bpf_syscall_string) + let mut function_registry = FunctionRegistry::>::default(); + function_registry + .register_function_hashed(*b"log", syscalls::bpf_syscall_string) .unwrap(); - Arc::new(loader) + Arc::new(BuiltinProgram::new_loader( + Config::default(), + function_registry, + )) } #[bench] diff --git a/benches/jit_compile.rs b/benches/jit_compile.rs index a6cbb42f..ccdb4be9 100644 --- a/benches/jit_compile.rs +++ b/benches/jit_compile.rs @@ -12,7 +12,7 @@ extern crate test; use solana_rbpf::{ elf::Executable, verifier::{RequisiteVerifier, TautologyVerifier}, - vm::{BuiltinProgram, Config, TestContextObject}, + vm::{BuiltinProgram, TestContextObject}, }; use std::{fs::File, io::Read, sync::Arc}; use test::Bencher; @@ -25,7 +25,7 @@ fn bench_init_vm(bencher: &mut Bencher) { file.read_to_end(&mut elf).unwrap(); let executable = Executable::::from_elf( &elf, - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let verified_executable = @@ -52,7 +52,7 @@ fn bench_jit_compile(bencher: &mut Bencher) { file.read_to_end(&mut elf).unwrap(); let executable = Executable::::from_elf( &elf, - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let mut verified_executable = diff --git a/benches/vm_execution.rs b/benches/vm_execution.rs index 36cb4216..3bf2e829 100644 --- a/benches/vm_execution.rs +++ b/benches/vm_execution.rs @@ -11,7 +11,7 @@ extern crate test; use solana_rbpf::{ ebpf, - elf::Executable, + elf::{Executable, FunctionRegistry}, memory_region::MemoryRegion, verifier::{RequisiteVerifier, TautologyVerifier}, vm::{BuiltinProgram, Config, TestContextObject}, @@ -27,7 +27,7 @@ fn bench_init_interpreter_start(bencher: &mut Bencher) { file.read_to_end(&mut elf).unwrap(); let executable = Executable::::from_elf( &elf, - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let verified_executable = @@ -56,7 +56,7 @@ fn bench_init_jit_start(bencher: &mut Bencher) { file.read_to_end(&mut elf).unwrap(); let executable = Executable::::from_elf( &elf, - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let mut verified_executable = @@ -88,7 +88,10 @@ fn bench_jit_vs_interpreter( ) { let executable = solana_rbpf::assembler::assemble::( assembly, - Arc::new(BuiltinProgram::new_loader(config)), + Arc::new(BuiltinProgram::new_loader( + config, + FunctionRegistry::default(), + )), ) .unwrap(); let mut verified_executable = diff --git a/cli/src/main.rs b/cli/src/main.rs index 291b7e2c..c11521c7 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -3,7 +3,7 @@ use solana_rbpf::{ aligned_memory::AlignedMemory, assembler::assemble, ebpf, - elf::Executable, + elf::{Executable, FunctionRegistry}, memory_region::{MemoryMapping, MemoryRegion}, static_analysis::Analysis, verifier::{RequisiteVerifier, TautologyVerifier}, @@ -92,11 +92,15 @@ fn main() { ) .get_matches(); - let loader = Arc::new(BuiltinProgram::new_loader(Config { - enable_instruction_tracing: matches.is_present("trace") || matches.is_present("profile"), - enable_symbol_and_section_labels: true, - ..Config::default() - })); + let loader = Arc::new(BuiltinProgram::new_loader( + Config { + enable_instruction_tracing: matches.is_present("trace") + || matches.is_present("profile"), + enable_symbol_and_section_labels: true, + ..Config::default() + }, + FunctionRegistry::default(), + )); let executable = match matches.value_of("assembler") { Some(asm_file_name) => { let mut file = File::open(Path::new(asm_file_name)).unwrap(); diff --git a/examples/disassemble.rs b/examples/disassemble.rs index 02134b09..eab69b94 100644 --- a/examples/disassemble.rs +++ b/examples/disassemble.rs @@ -6,10 +6,10 @@ extern crate solana_rbpf; use solana_rbpf::{ - elf::{Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, static_analysis::Analysis, verifier::TautologyVerifier, - vm::{BuiltinProgram, FunctionRegistry, TestContextObject}, + vm::{BuiltinProgram, TestContextObject}, }; use std::sync::Arc; @@ -31,7 +31,7 @@ fn main() { 0x00, 0x00, 0x00, 0x00, 0xb7, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, ]; - let loader = Arc::new(BuiltinProgram::default()); + let loader = Arc::new(BuiltinProgram::new_mock()); let executable = Executable::::from_text_bytes( program, loader, diff --git a/examples/to_json.rs b/examples/to_json.rs index 3938470c..dc3974bf 100644 --- a/examples/to_json.rs +++ b/examples/to_json.rs @@ -12,10 +12,10 @@ use std::path::PathBuf; extern crate solana_rbpf; use solana_rbpf::{ - elf::{Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, static_analysis::Analysis, verifier::TautologyVerifier, - vm::{BuiltinProgram, FunctionRegistry, TestContextObject}, + vm::{BuiltinProgram, TestContextObject}, }; use std::sync::Arc; // Turn a program into a JSON string. @@ -30,7 +30,7 @@ use std::sync::Arc; fn to_json(program: &[u8]) -> String { let executable = Executable::::from_text_bytes( program, - Arc::new(BuiltinProgram::default()), + Arc::new(BuiltinProgram::new_mock()), SBPFVersion::V2, FunctionRegistry::default(), ) diff --git a/fuzz/fuzz_targets/dumb.rs b/fuzz/fuzz_targets/dumb.rs index d452d881..ab8ace34 100644 --- a/fuzz/fuzz_targets/dumb.rs +++ b/fuzz/fuzz_targets/dumb.rs @@ -6,10 +6,10 @@ use libfuzzer_sys::fuzz_target; use solana_rbpf::{ ebpf, - elf::{Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, memory_region::MemoryRegion, verifier::{RequisiteVerifier, TautologyVerifier, Verifier}, - vm::{BuiltinProgram, FunctionRegistry, TestContextObject}, + vm::{BuiltinProgram, TestContextObject}, }; use test_utils::create_vm; @@ -35,7 +35,7 @@ fuzz_target!(|data: DumbFuzzData| { let mut mem = data.mem; let executable = Executable::::from_text_bytes( &prog, - std::sync::Arc::new(BuiltinProgram::new_loader(config)), + std::sync::Arc::new(BuiltinProgram::new_loader(config, FunctionRegistry::default())), SBPFVersion::V2, function_registry, ) diff --git a/fuzz/fuzz_targets/smart.rs b/fuzz/fuzz_targets/smart.rs index 5f04fd57..92d82889 100644 --- a/fuzz/fuzz_targets/smart.rs +++ b/fuzz/fuzz_targets/smart.rs @@ -7,11 +7,11 @@ use libfuzzer_sys::fuzz_target; use grammar_aware::*; use solana_rbpf::{ ebpf, - elf::{Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, insn_builder::{Arch, IntoBytes}, memory_region::MemoryRegion, verifier::{RequisiteVerifier, TautologyVerifier, Verifier}, - vm::{BuiltinProgram, FunctionRegistry, TestContextObject}, + vm::{BuiltinProgram, TestContextObject}, }; use test_utils::create_vm; @@ -39,7 +39,7 @@ fuzz_target!(|data: FuzzData| { let mut mem = data.mem; let executable = Executable::::from_text_bytes( prog.into_bytes(), - std::sync::Arc::new(BuiltinProgram::new_loader(config)), + std::sync::Arc::new(BuiltinProgram::new_loader(config, FunctionRegistry::default())), SBPFVersion::V2, function_registry, ) diff --git a/fuzz/fuzz_targets/smart_jit_diff.rs b/fuzz/fuzz_targets/smart_jit_diff.rs index 3605ba41..a6c2e99e 100644 --- a/fuzz/fuzz_targets/smart_jit_diff.rs +++ b/fuzz/fuzz_targets/smart_jit_diff.rs @@ -5,11 +5,11 @@ use libfuzzer_sys::fuzz_target; use grammar_aware::*; use solana_rbpf::{ ebpf, - elf::{Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, insn_builder::{Arch, Instruction, IntoBytes}, memory_region::MemoryRegion, verifier::{RequisiteVerifier, TautologyVerifier, Verifier}, - vm::{BuiltinProgram, FunctionRegistry, TestContextObject}, + vm::{BuiltinProgram, TestContextObject}, }; use test_utils::create_vm; @@ -47,7 +47,7 @@ fuzz_target!(|data: FuzzData| { let mut jit_mem = data.mem; let mut executable = Executable::::from_text_bytes( prog.into_bytes(), - std::sync::Arc::new(BuiltinProgram::new_loader(config)), + std::sync::Arc::new(BuiltinProgram::new_loader(config, FunctionRegistry::default())), SBPFVersion::V2, function_registry, ) diff --git a/fuzz/fuzz_targets/smarter_jit_diff.rs b/fuzz/fuzz_targets/smarter_jit_diff.rs index 73a722f8..d2639f9e 100644 --- a/fuzz/fuzz_targets/smarter_jit_diff.rs +++ b/fuzz/fuzz_targets/smarter_jit_diff.rs @@ -5,13 +5,13 @@ use libfuzzer_sys::fuzz_target; use semantic_aware::*; use solana_rbpf::{ ebpf, - elf::{Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, insn_builder::IntoBytes, memory_region::MemoryRegion, static_analysis::Analysis, verifier::{RequisiteVerifier, TautologyVerifier, Verifier}, vm::{ - BuiltinProgram, ContextObject, FunctionRegistry, TestContextObject, + BuiltinProgram, ContextObject, TestContextObject, }, }; use test_utils::create_vm; @@ -46,7 +46,7 @@ fuzz_target!(|data: FuzzData| { let mut jit_mem = data.mem; let mut executable = Executable::::from_text_bytes( prog.into_bytes(), - std::sync::Arc::new(BuiltinProgram::new_loader(config)), + std::sync::Arc::new(BuiltinProgram::new_loader(config, FunctionRegistry::default())), SBPFVersion::V2, function_registry, ) diff --git a/fuzz/fuzz_targets/verify_semantic_aware.rs b/fuzz/fuzz_targets/verify_semantic_aware.rs index 3438017b..01308cea 100644 --- a/fuzz/fuzz_targets/verify_semantic_aware.rs +++ b/fuzz/fuzz_targets/verify_semantic_aware.rs @@ -4,10 +4,9 @@ use libfuzzer_sys::fuzz_target; use semantic_aware::*; use solana_rbpf::{ - elf::SBPFVersion, + elf::{FunctionRegistry, SBPFVersion}, insn_builder::IntoBytes, verifier::{RequisiteVerifier, Verifier}, - vm::FunctionRegistry, }; use crate::common::ConfigTemplate; diff --git a/src/assembler.rs b/src/assembler.rs index 07985d1f..89ef50e8 100644 --- a/src/assembler.rs +++ b/src/assembler.rs @@ -18,9 +18,9 @@ use crate::{ Statement, }, ebpf::{self, Insn}, - elf::{register_internal_function, Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, verifier::TautologyVerifier, - vm::{BuiltinProgram, ContextObject, FunctionRegistry}, + vm::{BuiltinProgram, ContextObject}, }; use std::{collections::HashMap, sync::Arc}; @@ -191,7 +191,7 @@ fn insn(opc: u8, dst: i64, src: i64, off: i64, imm: i64) -> Result /// be16 r0 /// neg64 r2 /// exit", -/// std::sync::Arc::new(BuiltinProgram::new_loader(Config::default())), +/// std::sync::Arc::new(BuiltinProgram::new_mock()), /// ).unwrap(); /// let program = executable.get_text_bytes().1; /// println!("{:?}", program); @@ -244,14 +244,9 @@ pub fn assemble( match statement { Statement::Label { name } => { if name.starts_with("function_") || name == "entrypoint" { - register_internal_function( - &mut function_registry, - &loader, - &SBPFVersion::V2, - insn_ptr, - name.as_bytes(), - ) - .map_err(|_| format!("Label hash collision {name}"))?; + function_registry + .register_function(insn_ptr as u32, name.as_bytes().to_vec(), insn_ptr) + .map_err(|_| format!("Label hash collision {name}"))?; } labels.insert(name.as_str(), insn_ptr); } @@ -298,14 +293,13 @@ pub fn assemble( (CallImm, [Integer(imm)]) => { let target_pc = *imm + insn_ptr as i64 + 1; let label = format!("function_{}", target_pc as usize); - register_internal_function( - &mut function_registry, - &loader, - &SBPFVersion::V2, - target_pc as usize, - label.as_bytes(), - ) - .map_err(|_| format!("Label hash collision {name}"))?; + function_registry + .register_function( + target_pc as u32, + label.as_bytes().to_vec(), + target_pc as usize, + ) + .map_err(|_| format!("Label hash collision {name}"))?; insn(opc, 0, 1, 0, target_pc) } (CallReg, [Register(dst)]) => insn(opc, 0, 0, 0, *dst), diff --git a/src/disassembler.rs b/src/disassembler.rs index 86f4804c..eb4f4ce9 100644 --- a/src/disassembler.rs +++ b/src/disassembler.rs @@ -10,9 +10,9 @@ use crate::{ ebpf, - elf::SBPFVersion, + elf::{FunctionRegistry, SBPFVersion}, static_analysis::CfgNode, - vm::{BuiltinProgram, ContextObject, FunctionRegistry}, + vm::{BuiltinProgram, ContextObject}, }; use std::collections::BTreeMap; @@ -123,7 +123,7 @@ fn jmp_reg_str(name: &str, insn: &ebpf::Insn, cfg_nodes: &BTreeMap( insn: &ebpf::Insn, cfg_nodes: &BTreeMap, - function_registry: &FunctionRegistry, + function_registry: &FunctionRegistry, loader: &BuiltinProgram, sbpf_version: &SBPFVersion, ) -> String { @@ -254,17 +254,17 @@ pub fn disassemble_instruction( let mut function_name = None; if sbpf_version.static_syscalls() { if insn.src != 0 { - function_name = Some(resolve_label(cfg_nodes, insn.imm as usize)); + function_name = Some(resolve_label(cfg_nodes, insn.imm as usize).to_string()); } } else { - function_name = function_registry.get(&(insn.imm as u32)).map(|(_, function_name)| function_name.as_str()); + function_name = function_registry.lookup_by_key(insn.imm as u32).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string()); } let function_name = if let Some(function_name) = function_name { name = "call"; function_name } else { name = "syscall"; - loader.lookup_function(insn.imm as u32).map(|(function_name, _)| std::str::from_utf8(function_name).unwrap()).unwrap_or("[invalid]") + loader.get_function_registry().lookup_by_key(insn.imm as u32).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string()).unwrap_or_else(|| "[invalid]".to_string()) }; desc = format!("{name} {function_name}"); }, diff --git a/src/elf.rs b/src/elf.rs index 996b31de..9546ac83 100644 --- a/src/elf.rs +++ b/src/elf.rs @@ -23,7 +23,7 @@ use crate::{ error::EbpfError, memory_region::MemoryRegion, verifier::{TautologyVerifier, Verifier}, - vm::{BuiltinProgram, Config, ContextObject, FunctionRegistry}, + vm::{BuiltinProgram, Config, ContextObject}, }; #[cfg(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64"))] @@ -113,59 +113,6 @@ pub enum ElfError { InvalidProgramHeader, } -/// Generates the hash by which a symbol can be called -pub fn hash_internal_function(pc: usize, name: &[u8]) -> u32 { - if name == b"entrypoint" { - ebpf::hash_symbol_name(b"entrypoint") - } else { - let mut key = [0u8; mem::size_of::()]; - LittleEndian::write_u64(&mut key, pc as u64); - ebpf::hash_symbol_name(&key) - } -} - -/// Register a symbol or throw ElfError::SymbolHashCollision -pub fn register_internal_function( - function_registry: &mut FunctionRegistry, - loader: &BuiltinProgram, - sbpf_version: &SBPFVersion, - pc: usize, - name: &[u8], -) -> Result { - let config = loader.get_config(); - let key = if sbpf_version.static_syscalls() { - // With static_syscalls normal function calls and syscalls are differentiated in the ISA. - // Thus, we don't need to hash them here anymore and collisions are gone as well. - pc as u32 - } else { - let hash = hash_internal_function(pc, name); - if config.external_internal_function_hash_collision - && loader.lookup_function(hash).is_some() - { - return Err(ElfError::SymbolHashCollision(hash)); - } - hash - }; - match function_registry.entry(key) { - Entry::Vacant(entry) => { - entry.insert(( - pc, - if config.enable_symbol_and_section_labels || name == b"entrypoint" { - String::from_utf8_lossy(name).to_string() - } else { - String::default() - }, - )); - } - Entry::Occupied(entry) => { - if entry.get().0 != pc { - return Err(ElfError::SymbolHashCollision(key)); - } - } - } - Ok(key) -} - // For more information on the BPF instruction set: // https://github.com/iovisor/bpf-docs/blob/master/eBPF.md @@ -301,6 +248,140 @@ impl SBPFVersion { } } +/// Holds the function symbols of an Executable +#[derive(Debug, PartialEq)] +pub struct FunctionRegistry { + pub(crate) map: BTreeMap, T)>, +} + +impl Default for FunctionRegistry { + fn default() -> Self { + Self { + map: BTreeMap::new(), + } + } +} + +impl FunctionRegistry { + /// Register a symbol with an explicit key + pub fn register_function( + &mut self, + key: u32, + name: impl Into>, + value: T, + ) -> Result<(), ElfError> { + match self.map.entry(key) { + Entry::Vacant(entry) => { + entry.insert((name.into(), value)); + } + Entry::Occupied(entry) => { + if entry.get().1 != value { + return Err(ElfError::SymbolHashCollision(key)); + } + } + } + Ok(()) + } + + /// Register a symbol with an implicit key + pub fn register_function_hashed( + &mut self, + name: impl Into>, + value: T, + ) -> Result { + let name = name.into(); + let key = ebpf::hash_symbol_name(name.as_slice()); + self.register_function(key, name, value)?; + Ok(key) + } + + /// Used for transitioning from SBPFv1 to SBPFv2 + fn register_function_hashed_legacy( + &mut self, + loader: &BuiltinProgram, + hash_symbol_name: bool, + name: impl Into>, + value: T, + ) -> Result + where + usize: From, + { + let name = name.into(); + let config = loader.get_config(); + let key = if hash_symbol_name { + let hash = if name == b"entrypoint" { + ebpf::hash_symbol_name(b"entrypoint") + } else { + ebpf::hash_symbol_name(&usize::from(value).to_le_bytes()) + }; + if config.external_internal_function_hash_collision + && loader.get_function_registry().lookup_by_key(hash).is_some() + { + return Err(ElfError::SymbolHashCollision(hash)); + } + hash + } else { + usize::from(value) as u32 + }; + self.register_function( + key, + if config.enable_symbol_and_section_labels || name == b"entrypoint" { + name + } else { + Vec::default() + }, + value, + )?; + Ok(key) + } + + /// Unregister a symbol again + pub fn unregister_function(&mut self, key: u32) { + self.map.remove(&key); + } + + /// Iterate over all keys + pub fn keys(&self) -> impl Iterator + '_ { + self.map.keys().cloned() + } + + /// Iterate over all entries + pub fn iter(&self) -> impl Iterator + '_ { + self.map + .iter() + .map(|(key, (name, value))| (*key, (name.as_slice(), *value))) + } + + /// Get a function by its key + pub fn lookup_by_key(&self, key: u32) -> Option<(&[u8], T)> { + // String::from_utf8_lossy(function_name).as_str() + self.map + .get(&key) + .map(|(function_name, value)| (function_name.as_slice(), *value)) + } + + /// Get a function by its name + pub fn lookup_by_name(&self, name: &[u8]) -> Option<(&[u8], T)> { + self.map + .values() + .find(|(function_name, _value)| function_name == name) + .map(|(function_name, value)| (function_name.as_slice(), *value)) + } + + /// Calculate memory size + pub fn mem_size(&self) -> usize { + mem::size_of::().saturating_add(self.map.iter().fold( + 0, + |state: usize, (_, (name, value))| { + state.saturating_add( + mem::size_of_val(value) + .saturating_add(mem::size_of_val(name).saturating_add(name.capacity())), + ) + }, + )) + } +} + /// Elf loader/relocator #[derive(Debug, PartialEq)] pub struct Executable { @@ -317,7 +398,7 @@ pub struct Executable { /// Address of the entry point entry_pc: usize, /// Call resolution map (hash, pc, name) - function_registry: FunctionRegistry, + function_registry: FunctionRegistry, /// Loader built-in program loader: Arc>, /// Compiled program and argument @@ -382,11 +463,6 @@ impl Executable { self.text_section_info.offset_range.start as u64 } - /// Get a symbol's instruction offset - pub fn lookup_internal_function(&self, hash: u32) -> Option { - self.function_registry.get(&hash).map(|(pc, _name)| *pc) - } - /// Get the loader built-in program pub fn get_loader(&self) -> &Arc> { &self.loader @@ -419,8 +495,8 @@ impl Executable { Ok(()) } - /// Get normal functions (if debug symbols are not stripped) - pub fn get_function_registry(&self) -> &FunctionRegistry { + /// Get the function registry + pub fn get_function_registry(&self) -> &FunctionRegistry { &self.function_registry } @@ -429,23 +505,19 @@ impl Executable { text_bytes: &[u8], loader: Arc>, sbpf_version: SBPFVersion, - mut function_registry: FunctionRegistry, + mut function_registry: FunctionRegistry, ) -> Result { let elf_bytes = AlignedMemory::from_slice(text_bytes); let config = loader.get_config(); let enable_symbol_and_section_labels = config.enable_symbol_and_section_labels; - let entry_pc = if let Some((pc, _name)) = function_registry - .values() - .find(|(_pc, name)| name.as_bytes() == b"entrypoint") - { - *pc + let entry_pc = if let Some((_name, pc)) = function_registry.lookup_by_name(b"entrypoint") { + pc } else { - register_internal_function( - &mut function_registry, + function_registry.register_function_hashed_legacy( &loader, - &sbpf_version, + !sbpf_version.static_syscalls(), + *b"entrypoint", 0, - b"entrypoint", )?; 0 }; @@ -559,14 +631,13 @@ impl Executable { } let entry_pc = if let Some(entry_pc) = (offset as usize).checked_div(ebpf::INSN_SIZE) { if !sbpf_version.static_syscalls() { - function_registry.remove(&ebpf::hash_symbol_name(b"entrypoint")); + function_registry.unregister_function(ebpf::hash_symbol_name(b"entrypoint")); } - register_internal_function( - &mut function_registry, + function_registry.register_function_hashed_legacy( &loader, - &sbpf_version, + !sbpf_version.static_syscalls(), + *b"entrypoint", entry_pc, - b"entrypoint", )?; entry_pc } else { @@ -611,15 +682,7 @@ impl Executable { // text section info .saturating_add(self.text_section_info.mem_size()) // bpf functions - .saturating_add(mem::size_of_val(&self.function_registry)) - .saturating_add(self.function_registry - .iter() - .fold(0, |state: usize, (_, (val, name))| state - .saturating_add(mem::size_of_val(val) - .saturating_add(mem::size_of_val(&name) - .saturating_add(name.capacity()))))) - // loader built-in program - .saturating_add(self.loader.mem_size()); + .saturating_add(self.function_registry.mem_size()); #[cfg(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64"))] { @@ -926,7 +989,7 @@ impl Executable { /// Relocates the ELF in-place fn relocate<'a, P: ElfParser<'a>>( - function_registry: &mut FunctionRegistry, + function_registry: &mut FunctionRegistry, loader: &BuiltinProgram, elf: &'a P, elf_bytes: &mut [u8], @@ -967,12 +1030,11 @@ impl Executable { } else { String::default() }; - let key = register_internal_function( - function_registry, + let key = function_registry.register_function_hashed_legacy( loader, - &sbpf_version, - target_pc as usize, + !sbpf_version.static_syscalls(), name.as_bytes(), + target_pc as usize, )?; let offset = i.saturating_mul(ebpf::INSN_SIZE).saturating_add(4); let checked_slice = text_bytes @@ -1210,19 +1272,20 @@ impl Executable { as usize) .checked_div(ebpf::INSN_SIZE) .unwrap_or_default(); - register_internal_function( - function_registry, + function_registry.register_function_hashed_legacy( loader, - &sbpf_version, - target_pc, + !sbpf_version.static_syscalls(), name, + target_pc, )? } else { // Else it's a syscall let hash = *syscall_cache .entry(symbol.st_name()) .or_insert_with(|| ebpf::hash_symbol_name(name)); - if config.reject_broken_elfs && loader.lookup_function(hash).is_none() { + if config.reject_broken_elfs + && loader.get_function_registry().lookup_by_key(hash).is_none() + { return Err(ElfError::UnresolvedSymbol( String::from_utf8_lossy(name).to_string(), r_offset @@ -1261,12 +1324,11 @@ impl Executable { let name = elf .symbol_name(symbol.st_name() as Elf64Word) .ok_or_else(|| ElfError::UnknownSymbol(symbol.st_name() as usize))?; - register_internal_function( - function_registry, + function_registry.register_function_hashed_legacy( loader, - &sbpf_version, - target_pc, + !sbpf_version.static_syscalls(), name, + target_pc, )?; } } @@ -1316,7 +1378,7 @@ mod test { }, fuzz::fuzz, syscalls, - vm::{ProgramResult, TestContextObject}, + vm::{BuiltinFunction, ProgramResult, TestContextObject}, }; use rand::{distributions::Uniform, Rng}; use std::{fs::File, io::Read}; @@ -1324,14 +1386,18 @@ mod test { type ElfExecutable = Executable; fn loader() -> Arc> { - let mut loader = BuiltinProgram::new_loader(Config::default()); - loader - .register_function(b"log", syscalls::bpf_syscall_string) + let mut function_registry = + FunctionRegistry::>::default(); + function_registry + .register_function_hashed(*b"log", syscalls::bpf_syscall_string) .unwrap(); - loader - .register_function(b"log_64", syscalls::bpf_syscall_u64) + function_registry + .register_function_hashed(*b"log_64", syscalls::bpf_syscall_u64) .unwrap(); - Arc::new(loader) + Arc::new(BuiltinProgram::new_loader( + Config::default(), + function_registry, + )) } #[test] diff --git a/src/interpreter.rs b/src/interpreter.rs index b6c229c8..c7876de2 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -426,7 +426,7 @@ impl<'a, 'b, V: Verifier, C: ContextObject> Interpreter<'a, 'b, V, C> { if !self.check_pc(pc) { return false; } - if self.executable.get_sbpf_version().static_syscalls() && self.executable.lookup_internal_function(self.pc as u32).is_none() { + if self.executable.get_sbpf_version().static_syscalls() && self.executable.get_function_registry().lookup_by_key(self.pc as u32).is_none() { self.due_insn_count += 1; throw_error!(self, EbpfError::UnsupportedInstruction(self.pc + ebpf::ELF_INSN_DUMP_OFFSET)); } @@ -443,7 +443,7 @@ impl<'a, 'b, V: Verifier, C: ContextObject> Interpreter<'a, 'b, V, C> { }; if external { - if let Some((_function_name, function)) = self.executable.get_loader().lookup_function(insn.imm as u32) { + if let Some((_function_name, function)) = self.executable.get_loader().get_function_registry().lookup_by_key(insn.imm as u32) { resolved = true; if config.enable_instruction_meter { @@ -471,7 +471,7 @@ impl<'a, 'b, V: Verifier, C: ContextObject> Interpreter<'a, 'b, V, C> { } if internal && !resolved { - if let Some(target_pc) = self.executable.lookup_internal_function(insn.imm as u32) { + if let Some((_function_name, target_pc)) = self.executable.get_function_registry().lookup_by_key(insn.imm as u32) { resolved = true; // make BPF to BPF call diff --git a/src/jit.rs b/src/jit.rs index 5eb6df0f..4dd9e5a7 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -599,7 +599,7 @@ impl<'a, V: Verifier, C: ContextObject> JitCompiler<'a, V, C> { }; if external { - if let Some((_function_name, function)) = self.executable.get_loader().lookup_function(insn.imm as u32) { + if let Some((_function_name, function)) = self.executable.get_loader().get_function_registry().lookup_by_key(insn.imm as u32) { self.emit_validate_and_profile_instruction_count(true, Some(0)); self.emit_ins(X86Instruction::load_immediate(OperandSize::S64, R11, function as usize as i64)); self.emit_ins(X86Instruction::call_immediate(self.relative_to_anchor(ANCHOR_EXTERNAL_FUNCTION_CALL, 5))); @@ -609,7 +609,7 @@ impl<'a, V: Verifier, C: ContextObject> JitCompiler<'a, V, C> { } if internal { - if let Some(target_pc) = self.executable.lookup_internal_function(insn.imm as u32) { + if let Some((_function_name, target_pc)) = self.executable.get_function_registry().lookup_by_key(insn.imm as u32) { self.emit_internal_call(Value::Constant64(target_pc as i64, false)); resolved = true; } @@ -1533,13 +1533,13 @@ impl<'a, V: Verifier, C: ContextObject> JitCompiler<'a, V, C> { if self.executable.get_sbpf_version().static_syscalls() { let mut prev_pc = 0; for current_pc in self.executable.get_function_registry().keys() { - if *current_pc as usize >= self.result.pc_section.len() { + if current_pc as usize >= self.result.pc_section.len() { break; } - for pc in prev_pc..*current_pc as usize { + for pc in prev_pc..current_pc as usize { self.result.pc_section[pc] = call_unsupported_instruction; } - prev_pc = *current_pc as usize + 1; + prev_pc = current_pc as usize + 1; } for pc in prev_pc..self.result.pc_section.len() { self.result.pc_section[pc] = call_unsupported_instruction; @@ -1552,10 +1552,10 @@ impl<'a, V: Verifier, C: ContextObject> JitCompiler<'a, V, C> { mod tests { use super::*; use crate::{ - elf::SBPFVersion, + elf::{FunctionRegistry, SBPFVersion}, syscalls, verifier::TautologyVerifier, - vm::{BuiltinProgram, FunctionRegistry, TestContextObject}, + vm::{BuiltinFunction, BuiltinProgram, TestContextObject}, }; use byteorder::{ByteOrder, LittleEndian}; use std::sync::Arc; @@ -1598,15 +1598,22 @@ mod tests { fn create_mockup_executable( program: &[u8], ) -> Executable { - let mut loader = BuiltinProgram::new_loader(Config { - noop_instruction_rate: 0, - ..Config::default() - }); - loader - .register_function(b"gather_bytes", syscalls::bpf_gather_bytes) + let mut function_registry = + FunctionRegistry::>::default(); + function_registry + .register_function_hashed(*b"gather_bytes", syscalls::bpf_gather_bytes) .unwrap(); + let loader = BuiltinProgram::new_loader( + Config { + noop_instruction_rate: 0, + ..Config::default() + }, + function_registry, + ); let mut function_registry = FunctionRegistry::default(); - function_registry.insert(8, (8, "function_foo".to_string())); + function_registry + .register_function(8, *b"function_foo", 8) + .unwrap(); Executable::::from_text_bytes( program, Arc::new(loader), diff --git a/src/static_analysis.rs b/src/static_analysis.rs index fb0a44cd..3c9670b3 100644 --- a/src/static_analysis.rs +++ b/src/static_analysis.rs @@ -4,7 +4,7 @@ use crate::disassembler::disassemble_instruction; use crate::{ ebpf, - elf::{self, Executable}, + elf::Executable, error::EbpfError, verifier::{TautologyVerifier, Verifier}, vm::{ContextObject, DynamicAnalysis, TestContextObject}, @@ -153,8 +153,11 @@ impl<'a> Analysis<'a> { ) -> Result { let (_program_vm_addr, program) = executable.get_text_bytes(); let mut functions = BTreeMap::new(); - for (key, (pc, name)) in executable.get_function_registry().iter() { - functions.insert(*pc, (*key, name.clone())); + for (key, (function_name, pc)) in executable.get_function_registry().iter() { + functions.insert( + pc, + (key, String::from_utf8_lossy(function_name).to_string()), + ); } debug_assert!( program.len() % ebpf::INSN_SIZE == 0, @@ -228,7 +231,8 @@ impl<'a> Analysis<'a> { if let Some((function_name, _function)) = self .executable .get_loader() - .lookup_function(insn.imm as u32) + .get_function_registry() + .lookup_by_key(insn.imm as u32) { if function_name == b"abort" { self.cfg_nodes @@ -236,8 +240,10 @@ impl<'a> Analysis<'a> { .or_insert_with(CfgNode::default); cfg_edges.insert(insn.ptr, (insn.opc, Vec::new())); } - } else if let Some(target_pc) = - self.executable.lookup_internal_function(insn.imm as u32) + } else if let Some((_function_name, target_pc)) = self + .executable + .get_function_registry() + .lookup_by_key(insn.imm as u32) { self.cfg_nodes .entry(insn.ptr + 1) @@ -844,7 +850,7 @@ impl<'a> Analysis<'a> { cfg_node.sources.push(self.super_root); self.functions.entry(*v).or_insert_with(|| { let name = format!("function_{}", *v); - let hash = elf::hash_internal_function(*v, name.as_bytes()); + let hash = ebpf::hash_symbol_name(name.as_bytes()); (hash, name) }); } diff --git a/src/verifier.rs b/src/verifier.rs index b0485b03..4b83e086 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -25,8 +25,8 @@ use crate::{ ebpf, - elf::SBPFVersion, - vm::{Config, FunctionRegistry}, + elf::{FunctionRegistry, SBPFVersion}, + vm::Config, }; use thiserror::Error; @@ -100,7 +100,7 @@ pub trait Verifier { prog: &[u8], config: &Config, sbpf_version: &SBPFVersion, - function_registry: &FunctionRegistry, + function_registry: &FunctionRegistry, ) -> Result<(), VerifierError>; } @@ -226,11 +226,11 @@ pub struct RequisiteVerifier {} impl Verifier for RequisiteVerifier { /// Check the program against the verifier's rules #[rustfmt::skip] - fn verify(prog: &[u8], config: &Config, sbpf_version: &SBPFVersion, function_registry: &FunctionRegistry) -> Result<(), VerifierError> { + fn verify(prog: &[u8], config: &Config, sbpf_version: &SBPFVersion, function_registry: &FunctionRegistry) -> Result<(), VerifierError> { check_prog_len(prog)?; let program_range = 0..prog.len() / ebpf::INSN_SIZE; - let mut function_iter = function_registry.keys().map(|insn_ptr| *insn_ptr as usize).peekable(); + let mut function_iter = function_registry.keys().map(|insn_ptr| insn_ptr as usize).peekable(); let mut function_range = program_range.start..program_range.end; let mut insn_ptr: usize = 0; while (insn_ptr + 1) * ebpf::INSN_SIZE <= prog.len() { @@ -394,7 +394,7 @@ impl Verifier for TautologyVerifier { _prog: &[u8], _config: &Config, _sbpf_version: &SBPFVersion, - _function_registry: &FunctionRegistry, + _function_registry: &FunctionRegistry, ) -> std::result::Result<(), VerifierError> { Ok(()) } diff --git a/src/vm.rs b/src/vm.rs index d02bd3aa..721d1ebf 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -14,7 +14,7 @@ use crate::{ ebpf, - elf::{Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, error::EbpfError, interpreter::Interpreter, memory_region::MemoryMapping, @@ -22,12 +22,7 @@ use crate::{ verifier::{TautologyVerifier, Verifier}, }; use rand::Rng; -use std::{ - collections::{BTreeMap, HashMap}, - fmt::Debug, - mem, - sync::Arc, -}; +use std::{collections::BTreeMap, fmt::Debug, mem, sync::Arc}; /// Same as `Result` but provides a stable memory layout #[derive(Debug)] @@ -94,52 +89,52 @@ impl From> for StableResult { /// Return value of programs and syscalls pub type ProgramResult = StableResult>; -/// Holds the function symbols of an Executable -pub type FunctionRegistry = BTreeMap; - /// Syscall function without context pub type BuiltinFunction = fn(&mut C, u64, u64, u64, u64, u64, &mut MemoryMapping, &mut ProgramResult); /// Represents the interface to a fixed functionality program +#[derive(PartialEq)] pub struct BuiltinProgram { /// Holds the Config if this is a loader program config: Option>, /// Function pointers by symbol - functions: HashMap)>, + functions: FunctionRegistry>, } impl BuiltinProgram { /// Constructs a loader built-in program - pub fn new_loader(config: Config) -> Self { + pub fn new_loader(config: Config, functions: FunctionRegistry>) -> Self { Self { config: Some(Box::new(config)), - functions: HashMap::new(), + functions, } } - /// Get the configuration settings assuming this is a loader program - pub fn get_config(&self) -> &Config { - self.config.as_ref().unwrap() + /// Constructs a built-in program + pub fn new_builtin(functions: FunctionRegistry>) -> Self { + Self { + config: None, + functions, + } } - /// Register a built-in function - pub fn register_function( - &mut self, - name: &'static [u8], - function: BuiltinFunction, - ) -> Result<(), EbpfError> { - let key = ebpf::hash_symbol_name(name); - if self.functions.insert(key, (name, function)).is_some() { - Err(EbpfError::FunctionAlreadyRegistered(key as usize)) - } else { - Ok(()) + /// Constructs a mock loader built-in program + pub fn new_mock() -> Self { + Self { + config: Some(Box::default()), + functions: FunctionRegistry::default(), } } - /// Get a symbol's function pointer - pub fn lookup_function(&self, key: u32) -> Option<(&'static [u8], BuiltinFunction)> { - self.functions.get(&key).cloned() + /// Get the configuration settings assuming this is a loader program + pub fn get_config(&self) -> &Config { + self.config.as_ref().unwrap() + } + + /// Get the function registry + pub fn get_function_registry(&self) -> &FunctionRegistry> { + &self.functions } /// Calculate memory size @@ -150,46 +145,19 @@ impl BuiltinProgram { } else { 0 } - + self.functions.capacity() - * mem::size_of::<(u32, (&'static [u8], BuiltinFunction))>() - } -} - -impl Default for BuiltinProgram { - fn default() -> Self { - Self { - config: None, - functions: HashMap::new(), - } + + self.functions.mem_size() } } impl Debug for BuiltinProgram { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { writeln!(f, "{:?}", unsafe { - std::mem::transmute::<_, &HashMap>(&self.functions) + std::mem::transmute::<_, &BTreeMap>(&self.functions.map) })?; Ok(()) } } -impl PartialEq for BuiltinProgram { - fn eq(&self, other: &Self) -> bool { - if self.config != other.config { - return false; - } - for ((a_key, a_function), (b_key, b_function)) in - self.functions.iter().zip(other.functions.iter()) - { - if a_key != b_key || a_function as *const _ as usize != b_function as *const _ as usize - { - return false; - } - } - true - } -} - /// Shift the Config::runtime_environment_key by this many bits to the LSB /// /// 3 bits for 8 Byte alignment, and 1 bit to have encoding space for the RuntimeEnvironment. @@ -286,7 +254,7 @@ impl Executable { text_bytes: &[u8], loader: Arc>, sbpf_version: SBPFVersion, - function_registry: FunctionRegistry, + function_registry: FunctionRegistry, ) -> Result { Executable::new_from_text_bytes(text_bytes, loader, sbpf_version, function_registry) .map_err(EbpfError::ElfError) @@ -401,10 +369,10 @@ pub struct CallFrame { /// use solana_rbpf::{ /// aligned_memory::AlignedMemory, /// ebpf, -/// elf::{Executable, SBPFVersion}, +/// elf::{Executable, FunctionRegistry, SBPFVersion}, /// memory_region::{MemoryMapping, MemoryRegion}, /// verifier::{TautologyVerifier, RequisiteVerifier}, -/// vm::{BuiltinProgram, Config, EbpfVm, FunctionRegistry, TestContextObject}, +/// vm::{BuiltinProgram, Config, EbpfVm, TestContextObject}, /// }; /// /// let prog = &[ @@ -414,7 +382,7 @@ pub struct CallFrame { /// 0xaa, 0xbb, 0x11, 0x22, 0xcc, 0xdd /// ]; /// -/// let loader = std::sync::Arc::new(BuiltinProgram::new_loader(Config::default())); +/// let loader = std::sync::Arc::new(BuiltinProgram::new_mock()); /// let function_registry = FunctionRegistry::default(); /// let mut executable = Executable::::from_text_bytes(prog, loader, SBPFVersion::V2, function_registry).unwrap(); /// let verified_executable = Executable::::verified(executable).unwrap(); diff --git a/tests/assembler.rs b/tests/assembler.rs index 7b3070b9..bbf402ee 100644 --- a/tests/assembler.rs +++ b/tests/assembler.rs @@ -11,16 +11,13 @@ extern crate test_utils; use solana_rbpf::{ assembler::assemble, ebpf, - vm::{BuiltinProgram, Config, TestContextObject}, + vm::{BuiltinProgram, TestContextObject}, }; use std::sync::Arc; use test_utils::{TCP_SACK_ASM, TCP_SACK_BIN}; fn asm(src: &str) -> Result, String> { - let executable = assemble::( - src, - Arc::new(BuiltinProgram::new_loader(Config::default())), - )?; + let executable = assemble::(src, Arc::new(BuiltinProgram::new_mock()))?; let (_program_vm_addr, program) = executable.get_text_bytes(); Ok((0..program.len() / ebpf::INSN_SIZE) .map(|insn_ptr| ebpf::get_insn(program, insn_ptr)) @@ -539,11 +536,8 @@ fn test_large_immediate() { #[test] fn test_tcp_sack() { - let executable = assemble::( - TCP_SACK_ASM, - Arc::new(BuiltinProgram::new_loader(Config::default())), - ) - .unwrap(); + let executable = + assemble::(TCP_SACK_ASM, Arc::new(BuiltinProgram::new_mock())).unwrap(); let (_program_vm_addr, program) = executable.get_text_bytes(); assert_eq!(program, TCP_SACK_BIN.to_vec()); } diff --git a/tests/disassembler.rs b/tests/disassembler.rs index edaab2c5..cda792d7 100644 --- a/tests/disassembler.rs +++ b/tests/disassembler.rs @@ -9,6 +9,7 @@ extern crate solana_rbpf; use solana_rbpf::{ assembler::assemble, + elf::FunctionRegistry, static_analysis::Analysis, vm::{BuiltinProgram, Config, TestContextObject}, }; @@ -18,10 +19,13 @@ use std::sync::Arc; macro_rules! disasm { ($src:expr) => {{ let src = $src; - let loader = BuiltinProgram::new_loader(Config { - enable_symbol_and_section_labels: true, - ..Config::default() - }); + let loader = BuiltinProgram::new_loader( + Config { + enable_symbol_and_section_labels: true, + ..Config::default() + }, + FunctionRegistry::default(), + ); let executable = assemble::(src, Arc::new(loader)).unwrap(); let analysis = Analysis::from_executable(&executable).unwrap(); let mut reasm = Vec::new(); diff --git a/tests/execution.rs b/tests/execution.rs index 39beb207..a2b4ae19 100644 --- a/tests/execution.rs +++ b/tests/execution.rs @@ -18,14 +18,14 @@ use rand::{rngs::SmallRng, RngCore, SeedableRng}; use solana_rbpf::{ assembler::assemble, ebpf, - elf::{Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, error::EbpfError, memory_region::{AccessType, MemoryMapping, MemoryRegion}, static_analysis::Analysis, syscalls, verifier::{RequisiteVerifier, TautologyVerifier}, vm::{ - BuiltinProgram, Config, ContextObject, FunctionRegistry, ProgramResult, TestContextObject, + BuiltinFunction, BuiltinProgram, Config, ContextObject, ProgramResult, TestContextObject, }, }; use std::{fs::File, io::Read, sync::Arc}; @@ -36,9 +36,9 @@ use test_utils::{ const INSTRUCTION_METER_BUDGET: u64 = 1024; macro_rules! test_interpreter_and_jit { - (register, $loader:expr, $location:expr => $syscall_function:expr) => { - $loader - .register_function($location.as_bytes(), $syscall_function) + (register, $function_registry:expr, $location:expr => $syscall_function:expr) => { + $function_registry + .register_function_hashed($location.as_bytes(), $syscall_function) .unwrap(); }; ($executable:expr, $mem:tt, $context_object:expr, $expected_result:expr $(,)?) => { @@ -132,9 +132,9 @@ macro_rules! test_interpreter_and_jit_asm { ($source:tt, $config:tt, $mem:tt, ($($location:expr => $syscall_function:expr),* $(,)?), $context_object:expr, $expected_result:expr $(,)?) => { #[allow(unused_mut)] { - let mut loader = BuiltinProgram::new_loader($config); - $(test_interpreter_and_jit!(register, loader, $location => $syscall_function);)* - let loader = Arc::new(loader); + let mut function_registry = FunctionRegistry::>::default(); + $(test_interpreter_and_jit!(register, function_registry, $location => $syscall_function);)* + let loader = Arc::new(BuiltinProgram::new_loader($config, function_registry)); let mut executable = assemble($source, loader).unwrap(); test_interpreter_and_jit!(executable, $mem, $context_object, $expected_result); } @@ -158,9 +158,9 @@ macro_rules! test_interpreter_and_jit_elf { file.read_to_end(&mut elf).unwrap(); #[allow(unused_mut)] { - let mut loader = BuiltinProgram::new_loader($config); - $(test_interpreter_and_jit!(register, loader, $location => $syscall_function);)* - let loader = Arc::new(loader); + let mut function_registry = FunctionRegistry::>::default(); + $(test_interpreter_and_jit!(register, function_registry, $location => $syscall_function);)* + let loader = Arc::new(BuiltinProgram::new_loader($config, function_registry)); let mut executable = Executable::::from_elf(&elf, loader).unwrap(); test_interpreter_and_jit!(executable, $mem, $context_object, $expected_result); } @@ -2560,7 +2560,7 @@ fn test_err_mem_access_out_of_bound() { prog[0] = ebpf::LD_DW_IMM; prog[16] = ebpf::ST_B_IMM; prog[24] = ebpf::EXIT; - let loader = Arc::new(BuiltinProgram::new_loader(Config::default())); + let loader = Arc::new(BuiltinProgram::new_mock()); for address in [0x2u64, 0x8002u64, 0x80000002u64, 0x8000000000000002u64] { LittleEndian::write_u32(&mut prog[4..], address as u32); LittleEndian::write_u32(&mut prog[12..], (address >> 32) as u32); @@ -3009,10 +3009,12 @@ fn nested_vm_syscall( }; #[allow(unused_mut)] if depth > 0 { - let mut loader = BuiltinProgram::new_loader(Config::default()); - loader - .register_function(b"nested_vm_syscall", nested_vm_syscall) + let mut function_registry = + FunctionRegistry::>::default(); + function_registry + .register_function_hashed(*b"nested_vm_syscall", nested_vm_syscall) .unwrap(); + let loader = BuiltinProgram::new_loader(Config::default(), function_registry); let mem = [depth as u8 - 1, throw as u8]; let mut executable = assemble::( " @@ -3382,10 +3384,13 @@ fn test_syscall_static() { #[test] fn test_err_unresolved_syscall_reloc_64_32() { - let loader = BuiltinProgram::new_loader(Config { - reject_broken_elfs: true, - ..Config::default() - }); + let loader = BuiltinProgram::new_loader( + Config { + reject_broken_elfs: true, + ..Config::default() + }, + FunctionRegistry::default(), + ); let mut file = File::open("tests/elfs/syscall_reloc_64_32.so").unwrap(); let mut elf = Vec::new(); file.read_to_end(&mut elf).unwrap(); @@ -3786,10 +3791,13 @@ fn execute_generated_program(prog: &[u8]) -> bool { let mem_size = 1024 * 1024; let executable = Executable::::from_text_bytes( prog, - Arc::new(BuiltinProgram::new_loader(Config { - enable_instruction_tracing: true, - ..Config::default() - })), + Arc::new(BuiltinProgram::new_loader( + Config { + enable_instruction_tracing: true, + ..Config::default() + }, + FunctionRegistry::default(), + )), SBPFVersion::V2, FunctionRegistry::default(), ); diff --git a/tests/verifier.rs b/tests/verifier.rs index 7629eeda..1c847536 100644 --- a/tests/verifier.rs +++ b/tests/verifier.rs @@ -25,9 +25,9 @@ extern crate thiserror; use solana_rbpf::{ assembler::assemble, ebpf, - elf::{Executable, SBPFVersion}, + elf::{Executable, FunctionRegistry, SBPFVersion}, verifier::{RequisiteVerifier, TautologyVerifier, Verifier, VerifierError}, - vm::{BuiltinProgram, Config, FunctionRegistry, TestContextObject}, + vm::{BuiltinProgram, Config, TestContextObject}, }; use std::sync::Arc; use test_utils::create_vm; @@ -46,7 +46,7 @@ impl Verifier for ContradictionVerifier { _prog: &[u8], _config: &Config, _sbpf_version: &SBPFVersion, - _function_registry: &FunctionRegistry, + _function_registry: &FunctionRegistry, ) -> std::result::Result<(), VerifierError> { Err(VerifierError::NoProgram) } @@ -58,7 +58,7 @@ fn test_verifier_success() { " mov32 r0, 0xBEE exit", - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let verified_executable = @@ -81,7 +81,7 @@ fn test_verifier_fail() { " mov32 r0, 0xBEE exit", - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let _verified_executable = @@ -96,7 +96,7 @@ fn test_verifier_err_div_by_zero_imm() { mov32 r0, 1 div32 r0, 0 exit", - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let _verified_executable = @@ -113,7 +113,7 @@ fn test_verifier_err_endian_size() { ]; let executable = Executable::::from_text_bytes( prog, - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), SBPFVersion::V2, FunctionRegistry::default(), ) @@ -132,7 +132,7 @@ fn test_verifier_err_incomplete_lddw() { ]; let executable = Executable::::from_text_bytes( prog, - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), SBPFVersion::V2, FunctionRegistry::default(), ) @@ -150,10 +150,13 @@ fn test_verifier_err_invalid_reg_dst() { " mov r11, 1 exit", - Arc::new(BuiltinProgram::new_loader(Config { - enable_sbpf_v2, - ..Config::default() - })), + Arc::new(BuiltinProgram::new_loader( + Config { + enable_sbpf_v2, + ..Config::default() + }, + FunctionRegistry::default(), + )), ) .unwrap(); let result = Executable::::verified(executable) @@ -175,10 +178,13 @@ fn test_verifier_err_invalid_reg_src() { " mov r0, r11 exit", - Arc::new(BuiltinProgram::new_loader(Config { - enable_sbpf_v2, - ..Config::default() - })), + Arc::new(BuiltinProgram::new_loader( + Config { + enable_sbpf_v2, + ..Config::default() + }, + FunctionRegistry::default(), + )), ) .unwrap(); let result = Executable::::verified(executable) @@ -198,10 +204,13 @@ fn test_verifier_resize_stack_ptr_success() { sub r11, 1 add r11, 1 exit", - Arc::new(BuiltinProgram::new_loader(Config { - enable_stack_frame_gaps: false, - ..Config::default() - })), + Arc::new(BuiltinProgram::new_loader( + Config { + enable_stack_frame_gaps: false, + ..Config::default() + }, + FunctionRegistry::default(), + )), ) .unwrap(); let _verified_executable = @@ -216,7 +225,7 @@ fn test_verifier_err_jmp_lddw() { ja +1 lddw r0, 0x1122334455667788 exit", - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let _verified_executable = @@ -231,7 +240,7 @@ fn test_verifier_err_call_lddw() { call 1 lddw r0, 0x1122334455667788 exit", - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let _verified_executable = @@ -246,7 +255,7 @@ fn test_verifier_err_function_fallthrough() { mov r0, r1 function_foo: exit", - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let _verified_executable = @@ -260,7 +269,7 @@ fn test_verifier_err_jmp_out() { " ja +2 exit", - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let _verified_executable = @@ -274,7 +283,7 @@ fn test_verifier_err_jmp_out_start() { " ja -2 exit", - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let _verified_executable = @@ -290,7 +299,7 @@ fn test_verifier_err_unknown_opcode() { ]; let executable = Executable::::from_text_bytes( prog, - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), SBPFVersion::V2, FunctionRegistry::default(), ) @@ -306,7 +315,7 @@ fn test_verifier_err_write_r10() { " mov r10, 1 exit", - Arc::new(BuiltinProgram::new_loader(Config::default())), + Arc::new(BuiltinProgram::new_mock()), ) .unwrap(); let _verified_executable = @@ -341,11 +350,8 @@ fn test_verifier_err_all_shift_overflows() { for (overflowing_instruction, expected) in testcases { let assembly = format!("\n{overflowing_instruction}\nexit"); - let executable = assemble::( - &assembly, - Arc::new(BuiltinProgram::new_loader(Config::default())), - ) - .unwrap(); + let executable = + assemble::(&assembly, Arc::new(BuiltinProgram::new_mock())).unwrap(); let result = Executable::::verified(executable) .map_err(|err| format!("Executable constructor {err:?}")); match expected { @@ -375,10 +381,13 @@ fn test_sdiv_disabled() { let assembly = format!("\n{instruction}\nexit"); let executable = assemble::( &assembly, - Arc::new(BuiltinProgram::new_loader(Config { - enable_sbpf_v2, - ..Config::default() - })), + Arc::new(BuiltinProgram::new_loader( + Config { + enable_sbpf_v2, + ..Config::default() + }, + FunctionRegistry::default(), + )), ) .unwrap(); let result = Executable::::verified(executable)