Removes the dense function registry from loaders.

benches/elf_loader.rs

@@ -10,24 +10,17 @@ extern crate solana_sbpf;
 extern crate test;
 extern crate test_utils;
 
-use solana_sbpf::{
-    elf::Executable,
-    program::{BuiltinFunction, BuiltinProgram, FunctionRegistry},
-    vm::Config,
-};
+use solana_sbpf::{elf::Executable, program::BuiltinProgram, vm::Config};
 use std::{fs::File, io::Read, sync::Arc};
 use test::Bencher;
 use test_utils::{syscalls, TestContextObject};
 
 fn loader() -> Arc<BuiltinProgram<TestContextObject>> {
-    let mut function_registry = FunctionRegistry::<BuiltinFunction<TestContextObject>>::default();
-    function_registry
-        .register_function_hashed(*b"log", syscalls::SyscallString::vm)
+    let mut loader = BuiltinProgram::new_loader(Config::default());
+    loader
+        .register_function("log", syscalls::SyscallString::vm)
         .unwrap();
-    Arc::new(BuiltinProgram::new_loader(
-        Config::default(),
-        function_registry,
-    ))
+    Arc::new(loader)
 }
 
 #[bench]

benches/vm_execution.rs

@@ -10,12 +10,7 @@ extern crate solana_sbpf;
 extern crate test;
 
 #[cfg(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64"))]
-use solana_sbpf::{
-    ebpf,
-    memory_region::MemoryRegion,
-    program::{FunctionRegistry, SBPFVersion},
-    vm::Config,
-};
+use solana_sbpf::{ebpf, memory_region::MemoryRegion, program::SBPFVersion, vm::Config};
 use solana_sbpf::{elf::Executable, program::BuiltinProgram, verifier::RequisiteVerifier};
 use std::{fs::File, io::Read, sync::Arc};
 use test::Bencher;
@@ -83,10 +78,7 @@ fn bench_jit_vs_interpreter(
 ) {
     let mut executable = solana_sbpf::assembler::assemble::<TestContextObject>(
         assembly,
-        Arc::new(BuiltinProgram::new_loader(
-            config,
-            FunctionRegistry::default(),
-        )),
+        Arc::new(BuiltinProgram::new_loader(config)),
     )
     .unwrap();
     executable.verify::<RequisiteVerifier>().unwrap();

cli/src/main.rs

@@ -5,7 +5,7 @@ use solana_sbpf::{
     ebpf,
     elf::Executable,
     memory_region::{MemoryMapping, MemoryRegion},
-    program::{BuiltinProgram, FunctionRegistry},
+    program::BuiltinProgram,
     static_analysis::Analysis,
     verifier::RequisiteVerifier,
     vm::{Config, DynamicAnalysis, EbpfVm},
@@ -94,15 +94,11 @@ 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()
-        },
-        FunctionRegistry::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()
+    }));
     #[allow(unused_mut)]
     let mut executable = match matches.value_of("assembler") {
         Some(asm_file_name) => {

fuzz/fuzz_targets/dumb.rs

@@ -39,7 +39,6 @@ fuzz_target!(|data: DumbFuzzData| {
         &prog,
         std::sync::Arc::new(BuiltinProgram::new_loader(
             config,
-            FunctionRegistry::default(),
         )),
         SBPFVersion::V3,
         function_registry,

fuzz/fuzz_targets/smart.rs

@@ -51,7 +51,6 @@ fuzz_target!(|data: FuzzData| {
         prog.into_bytes(),
         std::sync::Arc::new(BuiltinProgram::new_loader(
             config,
-            FunctionRegistry::default(),
         )),
         SBPFVersion::V3,
         function_registry,

fuzz/fuzz_targets/smart_jit_diff.rs

@@ -59,7 +59,6 @@ fuzz_target!(|data: FuzzData| {
         prog.into_bytes(),
         std::sync::Arc::new(BuiltinProgram::new_loader(
             config,
-            FunctionRegistry::default(),
         )),
         SBPFVersion::V3,
         function_registry,

fuzz/fuzz_targets/smarter_jit_diff.rs

@@ -49,7 +49,6 @@ fuzz_target!(|data: FuzzData| {
         prog.into_bytes(),
         std::sync::Arc::new(BuiltinProgram::new_loader(
             config,
-            FunctionRegistry::default(),
         )),
         SBPFVersion::V3,
         function_registry,

src/program.rs

@@ -134,18 +134,6 @@ impl<T: Copy + PartialEq> FunctionRegistry<T> {
         Ok(())
     }
 
-    /// Register a symbol with an implicit key
-    pub fn register_function_hashed(
-        &mut self,
-        name: impl Into<Vec<u8>>,
-        value: T,
-    ) -> Result<u32, ElfError> {
-        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 SBPFv0 to SBPFv3
     pub(crate) fn register_function_hashed_legacy<C: ContextObject>(
         &mut self,
@@ -246,34 +234,28 @@ pub struct BuiltinProgram<C: ContextObject> {
     config: Option<Box<Config>>,
     /// Function pointers by symbol with sparse indexing
     sparse_registry: FunctionRegistry<BuiltinFunction<C>>,
-    /// Function pointers by symbol with dense indexing
-    dense_registry: FunctionRegistry<BuiltinFunction<C>>,
 }
 
 impl<C: ContextObject> PartialEq for BuiltinProgram<C> {
     fn eq(&self, other: &Self) -> bool {
-        self.config.eq(&other.config)
-            && self.sparse_registry.eq(&other.sparse_registry)
-            && self.dense_registry.eq(&other.dense_registry)
+        self.config.eq(&other.config) && self.sparse_registry.eq(&other.sparse_registry)
     }
 }
 
 impl<C: ContextObject> BuiltinProgram<C> {
     /// Constructs a loader built-in program
-    pub fn new_loader(config: Config, functions: FunctionRegistry<BuiltinFunction<C>>) -> Self {
+    pub fn new_loader(config: Config) -> Self {
         Self {
             config: Some(Box::new(config)),
-            sparse_registry: functions,
-            dense_registry: FunctionRegistry::default(),
+            sparse_registry: FunctionRegistry::default(),
         }
     }
 
     /// Constructs a built-in program
-    pub fn new_builtin(functions: FunctionRegistry<BuiltinFunction<C>>) -> Self {
+    pub fn new_builtin() -> Self {
         Self {
             config: None,
-            sparse_registry: functions,
-            dense_registry: FunctionRegistry::default(),
+            sparse_registry: FunctionRegistry::default(),
         }
     }
 
@@ -282,17 +264,6 @@ impl<C: ContextObject> BuiltinProgram<C> {
         Self {
             config: Some(Box::default()),
             sparse_registry: FunctionRegistry::default(),
-            dense_registry: FunctionRegistry::default(),
-        }
-    }
-
-    /// Create a new loader with both dense and sparse function indexation
-    /// Use `BuiltinProgram::register_function` for registrations.
-    pub fn new_loader_with_dense_registration(config: Config) -> Self {
-        Self {
-            config: Some(Box::new(config)),
-            sparse_registry: FunctionRegistry::default(),
-            dense_registry: FunctionRegistry::default(),
         }
     }
 
@@ -304,13 +275,9 @@ impl<C: ContextObject> BuiltinProgram<C> {
     /// Get the function registry depending on the SBPF version
     pub fn get_function_registry(
         &self,
-        sbpf_version: SBPFVersion,
+        _sbpf_version: SBPFVersion,
     ) -> &FunctionRegistry<BuiltinFunction<C>> {
-        if sbpf_version.static_syscalls() {
-            &self.dense_registry
-        } else {
-            &self.sparse_registry
-        }
+        &self.sparse_registry
     }
 
     /// Calculate memory size
@@ -322,19 +289,18 @@ impl<C: ContextObject> BuiltinProgram<C> {
                 0
             })
             .saturating_add(self.sparse_registry.mem_size())
-            .saturating_add(self.dense_registry.mem_size())
     }
 
     /// Register a function both in the sparse and dense registries
     pub fn register_function(
         &mut self,
         name: &str,
-        dense_key: u32,
         value: BuiltinFunction<C>,
     ) -> Result<(), ElfError> {
-        self.sparse_registry.register_function_hashed(name, value)?;
-        self.dense_registry
-            .register_function(dense_key, name, value)
+        let key = ebpf::hash_symbol_name(name.as_bytes());
+        self.sparse_registry
+            .register_function(key, name, value)
+            .map(|_| ())
     }
 }
 
@@ -343,16 +309,12 @@ impl<C: ContextObject> std::fmt::Debug for BuiltinProgram<C> {
         unsafe {
             writeln!(
                 f,
-                "sparse: {:?}\n dense: {:?}",
+                "registry: {:?}",
                 // `derive(Debug)` does not know that `C: ContextObject` does not need to implement `Debug`
                 std::mem::transmute::<
                     &FunctionRegistry<BuiltinFunction<C>>,
                     &FunctionRegistry<usize>,
-                >(&self.sparse_registry,),
-                std::mem::transmute::<
-                    &FunctionRegistry<BuiltinFunction<C>>,
-                    &FunctionRegistry<usize>,
-                >(&self.dense_registry,)
+                >(&self.sparse_registry),
             )?;
         }
         Ok(())

test_utils/src/lib.rs

@@ -397,19 +397,17 @@ macro_rules! test_interpreter_and_jit {
 
 #[macro_export]
 macro_rules! test_interpreter_and_jit_asm {
-    ($source:expr, $config:expr, $mem:tt, $context_object:expr, $expected_result:expr $(,)?) => {
+    ($source:expr, $config:expr, $mem:expr, $context_object:expr, $expected_result:expr $(,)?) => {
         #[allow(unused_mut)]
         {
             let mut config = $config;
             config.enable_instruction_tracing = true;
-            let mut function_registry =
-                FunctionRegistry::<BuiltinFunction<TestContextObject>>::default();
-            let loader = Arc::new(BuiltinProgram::new_loader(config, function_registry));
+            let loader = Arc::new(BuiltinProgram::new_loader(config));
             let mut executable = assemble($source, loader).unwrap();
             test_interpreter_and_jit!(executable, $mem, $context_object, $expected_result);
         }
     };
-    ($source:expr, $mem:tt, $context_object:expr, $expected_result:expr $(,)?) => {
+    ($source:expr, $mem:expr, $context_object:expr, $expected_result:expr $(,)?) => {
         #[allow(unused_mut)]
         {
             test_interpreter_and_jit_asm!(
@@ -423,56 +421,48 @@ macro_rules! test_interpreter_and_jit_asm {
     };
 }
 
+#[macro_export]
+macro_rules! test_syscall_asm {
+    (register, $loader:expr, $syscall_name:expr => $syscall_function:expr) => {
+        let _ = $loader.register_function($syscall_name, $syscall_function).unwrap();
+    };
+    ($source:expr, $mem:expr, ($($syscall_name:expr => $syscall_function:expr),*$(,)?), $context_object:expr, $expected_result:expr $(,)?) => {
+        let mut config = Config {
+            enable_instruction_tracing: true,
+            ..Config::default()
+        };
+        for sbpf_version in [SBPFVersion::V0, SBPFVersion::V3] {
+            config.enabled_sbpf_versions = sbpf_version..=sbpf_version;
+            let mut loader = BuiltinProgram::new_loader(config.clone());
+            $(test_syscall_asm!(register, loader, $syscall_name => $syscall_function);)*
+            let mut executable = assemble($source, Arc::new(loader)).unwrap();
+            test_interpreter_and_jit!(executable, $mem, $context_object, $expected_result);
+        }
+    };
+}
+
 #[macro_export]
 macro_rules! test_interpreter_and_jit_elf {
-    (register, $function_registry:expr, $location:expr => $syscall_function:expr) => {
-        $function_registry
-            .register_function_hashed($location.as_bytes(), $syscall_function)
-            .unwrap();
+    (register, $loader:expr, $location:expr => $syscall_function:expr) => {
+        $loader.register_function($location, $syscall_function).unwrap();
     };
-    ($source:tt, $config:tt, $mem:tt, ($($location:expr => $syscall_function:expr),* $(,)?), $context_object:expr, $expected_result:expr $(,)?) => {
+    ($source:expr, $config:expr, $mem:expr, ($($location:expr => $syscall_function:expr),* $(,)?), $context_object:expr, $expected_result:expr $(,)?) => {
         let mut file = File::open($source).unwrap();
         let mut elf = Vec::new();
         file.read_to_end(&mut elf).unwrap();
         #[allow(unused_mut)]
         {
-            let mut function_registry = FunctionRegistry::<BuiltinFunction<TestContextObject>>::default();
-            $(test_interpreter_and_jit_elf!(register, function_registry, $location => $syscall_function);)*
-            let loader = Arc::new(BuiltinProgram::new_loader($config, function_registry));
-            let mut executable = Executable::<TestContextObject>::from_elf(&elf, loader).unwrap();
+            let mut loader = BuiltinProgram::new_loader($config);
+            $(test_interpreter_and_jit_elf!(register, loader, $location => $syscall_function);)*
+            let mut executable = Executable::<TestContextObject>::from_elf(&elf, Arc::new(loader)).unwrap();
             test_interpreter_and_jit!(executable, $mem, $context_object, $expected_result);
         }
     };
-    ($source:tt, $mem:tt, ($($location:expr => $syscall_function:expr),* $(,)?), $context_object:expr, $expected_result:expr $(,)?) => {
+    ($source:expr, $mem:expr, ($($location:expr => $syscall_function:expr),* $(,)?), $context_object:expr, $expected_result:expr $(,)?) => {
         let config = Config {
             enable_instruction_tracing: true,
             ..Config::default()
         };
         test_interpreter_and_jit_elf!($source, config, $mem, ($($location => $syscall_function),*), $context_object, $expected_result);
     };
 }
-
-#[macro_export]
-macro_rules! test_syscall_asm {
-    (register, $loader:expr, $syscall_number:literal => $syscall_name:expr => $syscall_function:expr) => {
-        let _ = $loader.register_function($syscall_name, $syscall_number, $syscall_function).unwrap();
-    };
-    ($source:tt, $mem:tt, ($($syscall_number:literal => $syscall_name:expr => $syscall_function:expr),*$(,)?), $context_object:expr, $expected_result:expr $(,)?) => {
-        let mut config = Config {
-            enable_instruction_tracing: true,
-            ..Config::default()
-        };
-        for sbpf_version in [SBPFVersion::V0, SBPFVersion::V3] {
-            config.enabled_sbpf_versions = sbpf_version..=sbpf_version;
-            let src = if sbpf_version == SBPFVersion::V0 {
-                format!($source, $($syscall_name, )*)
-            } else {
-                format!($source, $($syscall_number, )*)
-            };
-            let mut loader = BuiltinProgram::new_loader_with_dense_registration(config.clone());
-            $(test_syscall_asm!(register, loader, $syscall_number => $syscall_name => $syscall_function);)*
-            let mut executable = assemble(src.as_str(), Arc::new(loader)).unwrap();
-            test_interpreter_and_jit!(executable, $mem, $context_object, $expected_result);
-        }
-    };
-}

tests/assembler.rs

@@ -10,7 +10,7 @@
 extern crate solana_sbpf;
 extern crate test_utils;
 
-use solana_sbpf::program::{FunctionRegistry, SBPFVersion};
+use solana_sbpf::program::SBPFVersion;
 use solana_sbpf::vm::Config;
 use solana_sbpf::{assembler::assemble, ebpf, program::BuiltinProgram};
 use std::sync::Arc;
@@ -21,7 +21,7 @@ fn asm(src: &str) -> Result<Vec<ebpf::Insn>, String> {
 }
 
 fn asm_with_config(src: &str, config: Config) -> Result<Vec<ebpf::Insn>, String> {
-    let loader = BuiltinProgram::new_loader(config, FunctionRegistry::default());
+    let loader = BuiltinProgram::new_loader(config);
     let executable = assemble::<TestContextObject>(src, Arc::new(loader))?;
     let (_program_vm_addr, program) = executable.get_text_bytes();
     Ok((0..program.len() / ebpf::INSN_SIZE)
@@ -524,14 +524,9 @@ fn test_tcp_sack() {
         enabled_sbpf_versions: SBPFVersion::V3..=SBPFVersion::V3,
         ..Config::default()
     };
-    let executable = assemble::<TestContextObject>(
-        TCP_SACK_ASM,
-        Arc::new(BuiltinProgram::new_loader(
-            config,
-            FunctionRegistry::default(),
-        )),
-    )
-    .unwrap();
+    let executable =
+        assemble::<TestContextObject>(TCP_SACK_ASM, Arc::new(BuiltinProgram::new_loader(config)))
+            .unwrap();
     let (_program_vm_addr, program) = executable.get_text_bytes();
     assert_eq!(program, TCP_SACK_BIN.to_vec());
 }