Fix ELF Symbol Parsing Performance (#176)

* Cache parsed and hashed syscall names from ELF dynstrtab. * Keeps hashed syscall names around for the disassembler and the CLI tool. * Moves set_syscall_registry() as argument in the constructors of the Executable. * Moves all syscalls into the file src/syscalls.rs * Configurable enforcement of syscalls to be resolved when an ELF file is loaded.
solana-labs · Jul 6, 2021 · 3639ab8 · 3639ab8
1 parent 608a756
commit 3639ab8
Show file tree

Hide file tree

Showing 23 changed files with 428 additions and 343 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -21,7 +21,7 @@ include = [
 [dependencies]
 byteorder = "1.2"
 combine = "3.8.1"
-goblin = "0.3.0"
+goblin = "0.4.2"
 hash32 = "0.1.0"
 libc = "0.2"
 log = "0.4.2"

diff --git a/benches/elf_loader.rs b/benches/elf_loader.rs
@@ -11,12 +11,20 @@ extern crate test;
 extern crate test_utils;
 
 use solana_rbpf::{
+    syscalls::BpfSyscallU64,
     user_error::UserError,
     vm::{Config, DefaultInstructionMeter, Executable, SyscallObject, SyscallRegistry},
 };
 use std::{fs::File, io::Read};
 use test::Bencher;
-use test_utils::BpfSyscallU64;
+
+fn syscall_registry() -> SyscallRegistry {
+    let mut syscall_registry = SyscallRegistry::default();
+    syscall_registry
+        .register_syscall_by_name::<UserError, _>(b"log_64", BpfSyscallU64::call)
+        .unwrap();
+    syscall_registry
+}
 
 #[bench]
 fn bench_load_elf(bencher: &mut Bencher) {
@@ -28,6 +36,7 @@ fn bench_load_elf(bencher: &mut Bencher) {
             &elf,
             None,
             Config::default(),
+            syscall_registry(),
         )
         .unwrap()
     });
@@ -43,6 +52,7 @@ fn bench_load_elf_without_syscall(bencher: &mut Bencher) {
             &elf,
             None,
             Config::default(),
+            syscall_registry(),
         )
         .unwrap();
         executable
@@ -55,17 +65,13 @@ fn bench_load_elf_with_syscall(bencher: &mut Bencher) {
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
     bencher.iter(|| {
-        let mut executable = <dyn Executable<UserError, DefaultInstructionMeter>>::from_elf(
+        let executable = <dyn Executable<UserError, DefaultInstructionMeter>>::from_elf(
             &elf,
             None,
             Config::default(),
+            syscall_registry(),
         )
         .unwrap();
-        let mut syscall_registry = SyscallRegistry::default();
-        syscall_registry
-            .register_syscall_by_name::<UserError, _>(b"log_64", BpfSyscallU64::call)
-            .unwrap();
-        executable.set_syscall_registry(syscall_registry);
         executable
     });
 }
diff --git a/benches/jit_compile.rs b/benches/jit_compile.rs
@@ -11,7 +11,7 @@ extern crate test;
 
 use solana_rbpf::{
     user_error::UserError,
-    vm::{Config, DefaultInstructionMeter, EbpfVm, Executable},
+    vm::{Config, DefaultInstructionMeter, EbpfVm, Executable, SyscallRegistry},
 };
 use std::{fs::File, io::Read};
 use test::Bencher;
@@ -25,6 +25,7 @@ fn bench_init_vm(bencher: &mut Bencher) {
         &elf,
         None,
         Config::default(),
+        SyscallRegistry::default(),
     )
     .unwrap();
     bencher.iter(|| {
@@ -43,6 +44,7 @@ fn bench_jit_compile(bencher: &mut Bencher) {
         &elf,
         None,
         Config::default(),
+        SyscallRegistry::default(),
     )
     .unwrap();
     bencher.iter(|| executable.jit_compile().unwrap());

diff --git a/benches/vm_execution.rs b/benches/vm_execution.rs
@@ -11,7 +11,7 @@ extern crate test;
 
 use solana_rbpf::{
     user_error::UserError,
-    vm::{Config, DefaultInstructionMeter, EbpfVm, Executable},
+    vm::{Config, DefaultInstructionMeter, EbpfVm, Executable, SyscallRegistry},
 };
 use std::{fs::File, io::Read};
 use test::Bencher;
@@ -25,6 +25,7 @@ fn bench_init_interpreter_execution(bencher: &mut Bencher) {
         &elf,
         None,
         Config::default(),
+        SyscallRegistry::default(),
     )
     .unwrap();
     let mut vm =
@@ -46,6 +47,7 @@ fn bench_init_jit_execution(bencher: &mut Bencher) {
         &elf,
         None,
         Config::default(),
+        SyscallRegistry::default(),
     )
     .unwrap();
     executable.jit_compile().unwrap();
@@ -65,11 +67,14 @@ fn bench_jit_vs_interpreter(
     instruction_meter: u64,
     mem: &mut [u8],
 ) {
-    let mut executable = solana_rbpf::assembler::assemble::<
-        UserError,
-        test_utils::TestInstructionMeter,
-    >(assembly, None, Config::default())
-    .unwrap();
+    let mut executable =
+        solana_rbpf::assembler::assemble::<UserError, test_utils::TestInstructionMeter>(
+            assembly,
+            None,
+            Config::default(),
+            SyscallRegistry::default(),
+        )
+        .unwrap();
     executable.jit_compile().unwrap();
     let mut vm = EbpfVm::new(executable.as_ref(), mem, &[]).unwrap();
     let interpreter_summary = bencher

diff --git a/cli/Cargo.lock b/cli/Cargo.lock
diff --git a/cli/src/main.rs b/cli/src/main.rs
@@ -4,12 +4,13 @@ use solana_rbpf::{
     ebpf,
     memory_region::{MemoryMapping, MemoryRegion},
     static_analysis::Analysis,
+    syscalls::Result,
     user_error::UserError,
     verifier::check,
     vm::{Config, DynamicAnalysis, EbpfVm, Executable, SyscallObject, SyscallRegistry},
 };
 use std::{fs::File, io::Read, path::Path};
-use test_utils::{Result, TestInstructionMeter};
+use test_utils::TestInstructionMeter;
 
 struct MockSyscall {
     name: String,
@@ -122,6 +123,10 @@ fn main() {
         } else {
             None
         };
+    let syscall_registry = SyscallRegistry::default();
+    /*for hash in syscalls.keys() {
+        let _ = syscall_registry.register_syscall_by_hash(*hash, MockSyscall::call);
+    }*/
     let mut executable = match matches.value_of("assembler") {
         Some(asm_file_name) => {
             let mut file = File::open(&Path::new(asm_file_name)).unwrap();
@@ -131,24 +136,23 @@ fn main() {
                 std::str::from_utf8(source.as_slice()).unwrap(),
                 verifier,
                 config,
+                syscall_registry,
             )
         }
         None => {
             let mut file = File::open(&Path::new(matches.value_of("elf").unwrap())).unwrap();
             let mut elf = Vec::new();
             file.read_to_end(&mut elf).unwrap();
-            Executable::<UserError, TestInstructionMeter>::from_elf(&elf, verifier, config)
-                .map_err(|err| format!("Executable constructor failed: {:?}", err))
+            <dyn Executable<UserError, TestInstructionMeter>>::from_elf(
+                &elf,
+                verifier,
+                config,
+                syscall_registry,
+            )
+            .map_err(|err| format!("Executable constructor failed: {:?}", err))
         }
     }
     .unwrap();
-
-    let (syscalls, _functions) = executable.get_symbols();
-    let mut syscall_registry = SyscallRegistry::default();
-    for hash in syscalls.keys() {
-        let _ = syscall_registry.register_syscall_by_hash(*hash, MockSyscall::call);
-    }
-    executable.set_syscall_registry(syscall_registry);
     executable.jit_compile().unwrap();
     let analysis = Analysis::from_executable(executable.as_ref());
 
@@ -192,7 +196,7 @@ fn main() {
     ];
     let heap_region = MemoryRegion::new_from_slice(&heap, ebpf::MM_HEAP_START, 0, true);
     let mut vm = EbpfVm::new(executable.as_ref(), &mut mem, &[heap_region]).unwrap();
-    for (hash, name) in &analysis.syscalls {
+    for (hash, name) in analysis.executable.get_syscall_symbols() {
         vm.bind_syscall_context_object(Box::new(MockSyscall { name: name.clone() }), Some(*hash))
             .unwrap();
     }

diff --git a/examples/disassemble.rs b/examples/disassemble.rs
@@ -8,7 +8,7 @@ extern crate solana_rbpf;
 use solana_rbpf::{
     static_analysis::Analysis,
     user_error::UserError,
-    vm::{Config, DefaultInstructionMeter, Executable},
+    vm::{Config, DefaultInstructionMeter, Executable, SyscallRegistry},
 };
 use std::collections::BTreeMap;
 
@@ -32,9 +32,10 @@ fn main() {
     ];
     let executable = <dyn Executable<UserError, DefaultInstructionMeter>>::from_text_bytes(
         &program,
-        BTreeMap::new(),
         None,
         Config::default(),
+        SyscallRegistry::default(),
+        BTreeMap::default(),
     )
     .unwrap();
     let analysis = Analysis::from_executable(executable.as_ref());

diff --git a/examples/to_json.rs b/examples/to_json.rs
@@ -15,7 +15,7 @@ use solana_rbpf::{
     disassembler::disassemble_instruction,
     static_analysis::Analysis,
     user_error::UserError,
-    vm::{Config, DefaultInstructionMeter, Executable},
+    vm::{Config, DefaultInstructionMeter, Executable, SyscallRegistry},
 };
 use std::collections::BTreeMap;
 // Turn a program into a JSON string.
@@ -30,9 +30,10 @@ use std::collections::BTreeMap;
 fn to_json(program: &[u8]) -> String {
     let executable = <dyn Executable<UserError, DefaultInstructionMeter>>::from_text_bytes(
         &program,
-        BTreeMap::new(),
         None,
         Config::default(),
+        SyscallRegistry::default(),
+        BTreeMap::default(),
     )
     .unwrap();
     let analysis = Analysis::from_executable(executable.as_ref());

diff --git a/examples/uptime.rs b/examples/uptime.rs
@@ -45,9 +45,10 @@ fn main() {
     // Create a VM: this one takes no data. Load prog1 in it.
     let executable = <dyn Executable<UserError, DefaultInstructionMeter>>::from_text_bytes(
         prog1,
-        BTreeMap::new(),
         None,
         Config::default(),
+        SyscallRegistry::default(),
+        BTreeMap::default(),
     )
     .unwrap();
     let mut vm =
@@ -67,21 +68,22 @@ fn main() {
     // In the following example we use a syscall to get the elapsed time since boot time: we
     // reimplement uptime in eBPF, in Rust. Because why not.
 
-    let mut executable = <dyn Executable<UserError, DefaultInstructionMeter>>::from_text_bytes(
-        prog2,
-        BTreeMap::new(),
-        None,
-        Config::default(),
-    )
-    .unwrap();
     let mut syscall_registry = SyscallRegistry::default();
     syscall_registry
         .register_syscall_by_hash::<UserError, _>(
             syscalls::BPF_KTIME_GETNS_IDX,
             syscalls::BpfTimeGetNs::call,
         )
         .unwrap();
-    executable.set_syscall_registry(syscall_registry);
+    #[allow(unused_mut)]
+    let mut executable = <dyn Executable<UserError, DefaultInstructionMeter>>::from_text_bytes(
+        prog2,
+        None,
+        Config::default(),
+        syscall_registry,
+        BTreeMap::default(),
+    )
+    .unwrap();
     #[cfg(not(windows))]
     {
         executable.jit_compile().unwrap();

diff --git a/src/assembler.rs b/src/assembler.rs
@@ -19,7 +19,7 @@ use crate::{
     ebpf::{self, Insn},
     elf::register_bpf_function,
     error::UserDefinedError,
-    vm::{Config, Executable, InstructionMeter, Verifier},
+    vm::{Config, Executable, InstructionMeter, SyscallRegistry, Verifier},
 };
 use std::collections::{BTreeMap, HashMap};
 
@@ -178,7 +178,7 @@ fn insn(opc: u8, dst: i64, src: i64, off: i64, imm: i64) -> Result<Insn, String>
 /// # Examples
 ///
 /// ```
-/// use solana_rbpf::{assembler::assemble, user_error::UserError, vm::{Config, DefaultInstructionMeter}};
+/// use solana_rbpf::{assembler::assemble, user_error::UserError, vm::{Config, DefaultInstructionMeter, SyscallRegistry}};
 /// let executable = assemble::<UserError, DefaultInstructionMeter>(
 ///    "add64 r1, 0x605
 ///     mov64 r2, 0x32
@@ -187,7 +187,8 @@ fn insn(opc: u8, dst: i64, src: i64, off: i64, imm: i64) -> Result<Insn, String>
 ///     neg64 r2
 ///     exit",
 ///     None,
-///     Config::default()
+///     Config::default(),
+///     SyscallRegistry::default(),
 /// ).unwrap();
 /// let program = executable.get_text_bytes().unwrap().1;
 /// println!("{:?}", program);
@@ -214,6 +215,7 @@ pub fn assemble<E: UserDefinedError, I: 'static + InstructionMeter>(
     src: &str,
     verifier: Option<Verifier>,
     config: Config,
+    syscall_registry: SyscallRegistry,
 ) -> Result<Box<dyn Executable<E, I>>, String> {
     fn resolve_label(
         insn_ptr: usize,
@@ -358,6 +360,12 @@ pub fn assemble<E: UserDefinedError, I: 'static + InstructionMeter>(
         .map(|insn| insn.to_vec())
         .flatten()
         .collect::<Vec<_>>();
-    <dyn Executable<E, I>>::from_text_bytes(&program, bpf_functions, verifier, config)
-        .map_err(|err| format!("Executable constructor {:?}", err))
+    <dyn Executable<E, I>>::from_text_bytes(
+        &program,
+        verifier,
+        config,
+        syscall_registry,
+        bpf_functions,
+    )
+    .map_err(|err| format!("Executable constructor {:?}", err))
 }
diff --git a/src/disassembler.rs b/src/disassembler.rs
@@ -225,7 +225,7 @@ pub fn disassemble_instruction<E: UserDefinedError, I: InstructionMeter>(insn: &
         ebpf::JSLE_IMM   => { name = "jsle"; desc = jmp_imm_str(name, insn, analysis); },
         ebpf::JSLE_REG   => { name = "jsle"; desc = jmp_reg_str(name, insn, analysis); },
         ebpf::CALL_IMM   => {
-            desc = if let Some(syscall_name) = analysis.syscalls.get(&(insn.imm as u32)) {
+            desc = if let Some(syscall_name) = analysis.executable.get_syscall_symbols().get(&(insn.imm as u32)) {
                 name = "syscall";
                 format!("{} {}", name, syscall_name)
             } else {