Wrapes Executable in Pin<Box<Executable>> to make sure the pointer st…

…ays stable (in heap) for JITs unresolved symbol error reporting at runtime. (#244)
solana-labs · Dec 14, 2021 · 608db84 · 608db84
1 parent c56ccaa
commit 608db84
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 31 deletions.
diff --git a/benches/jit_compile.rs b/benches/jit_compile.rs
@@ -47,5 +47,7 @@ fn bench_jit_compile(bencher: &mut Bencher) {
         SyscallRegistry::default(),
     )
     .unwrap();
-    bencher.iter(|| executable.jit_compile().unwrap());
+    bencher.iter(|| {
+        Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable).unwrap()
+    });
 }
diff --git a/benches/vm_execution.rs b/benches/vm_execution.rs
@@ -50,7 +50,7 @@ fn bench_init_jit_execution(bencher: &mut Bencher) {
         SyscallRegistry::default(),
     )
     .unwrap();
-    executable.jit_compile().unwrap();
+    Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable).unwrap();
     let mut vm =
         EbpfVm::<UserError, TestInstructionMeter>::new(&executable, &mut [], &mut []).unwrap();
     bencher.iter(|| {
@@ -73,7 +73,7 @@ fn bench_jit_vs_interpreter(
         SyscallRegistry::default(),
     )
     .unwrap();
-    executable.jit_compile().unwrap();
+    Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable).unwrap();
     let mut vm = EbpfVm::new(&executable, &mut [], mem).unwrap();
     let interpreter_summary = bencher
         .bench(|bencher| {

diff --git a/cli/src/main.rs b/cli/src/main.rs
@@ -153,7 +153,7 @@ fn main() {
         }
     }
     .unwrap();
-    executable.jit_compile().unwrap();
+    Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable).unwrap();
     let analysis = Analysis::from_executable(&executable);
 
     match matches.value_of("use") {

diff --git a/examples/uptime.rs b/examples/uptime.rs
@@ -86,7 +86,7 @@ fn main() {
     .unwrap();
     #[cfg(not(windows))]
     {
-        executable.jit_compile().unwrap();
+        Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable).unwrap();
     }
     let mut vm =
         EbpfVm::<UserError, TestInstructionMeter>::new(&executable, &mut [], &mut []).unwrap();

diff --git a/src/assembler.rs b/src/assembler.rs
@@ -22,7 +22,10 @@ use crate::{
     error::UserDefinedError,
     vm::{Config, InstructionMeter, SyscallRegistry, Verifier},
 };
-use std::collections::{BTreeMap, HashMap};
+use std::{
+    collections::{BTreeMap, HashMap},
+    pin::Pin,
+};
 
 #[derive(Clone, Copy, Debug, PartialEq)]
 enum InstructionType {
@@ -217,7 +220,7 @@ pub fn assemble<E: UserDefinedError, I: 'static + InstructionMeter>(
     verifier: Option<Verifier>,
     config: Config,
     syscall_registry: SyscallRegistry,
-) -> Result<Executable<E, I>, String> {
+) -> Result<Pin<Box<Executable<E, I>>>, String> {
     fn resolve_label(
         insn_ptr: usize,
         labels: &HashMap<&str, usize>,

diff --git a/src/elf.rs b/src/elf.rs
@@ -21,7 +21,7 @@ use goblin::{
     elf::{header::*, reloc::*, section_header::*, Elf},
     error::Error as GoblinError,
 };
-use std::{collections::BTreeMap, fmt::Debug, mem, ops::Range, str};
+use std::{collections::BTreeMap, fmt::Debug, mem, ops::Range, pin::Pin, str};
 
 /// Error definitions
 #[derive(Debug, thiserror::Error, PartialEq, Eq)]
@@ -285,8 +285,9 @@ impl<E: UserDefinedError, I: InstructionMeter> Executable<E, I> {
     }
 
     /// JIT compile the executable
-    pub fn jit_compile(&mut self) -> Result<(), EbpfError<E>> {
-        self.compiled_program = Some(JitProgram::<E, I>::new(self)?);
+    pub fn jit_compile(executable: &mut Pin<Box<Self>>) -> Result<(), EbpfError<E>> {
+        // TODO: Turn back to `executable: &mut self` once Self::report_unresolved_symbol() is gone
+        executable.compiled_program = Some(JitProgram::<E, I>::new(executable)?);
         Ok(())
     }
 

diff --git a/src/jit.rs b/src/jit.rs
@@ -15,12 +15,13 @@
 
 extern crate libc;
 
-use std::fmt::Debug;
-use std::mem;
-use std::collections::HashMap;
-use std::fmt::Formatter;
-use std::fmt::Error as FormatterError;
-use std::ops::{Index, IndexMut};
+use std::{
+    collections::HashMap,
+    fmt::{Debug, Error as FormatterError, Formatter},
+    mem,
+    ops::{Index, IndexMut},
+    pin::Pin,
+};
 use rand::{rngs::SmallRng, Rng, SeedableRng};
 
 use crate::{
@@ -147,7 +148,7 @@ impl<E: UserDefinedError, I: InstructionMeter> PartialEq for JitProgram<E, I> {
 }
 
 impl<E: UserDefinedError, I: InstructionMeter> JitProgram<E, I> {
-    pub fn new(executable: &Executable<E, I>) -> Result<Self, EbpfError<E>> {
+    pub fn new(executable: &Pin<Box<Executable<E, I>>>) -> Result<Self, EbpfError<E>> {
         let program = executable.get_text_bytes().1;
         let mut jit = JitCompiler::new::<E>(program, executable.get_config())?;
         jit.compile::<E, I>(executable)?;
@@ -979,7 +980,7 @@ impl JitCompiler {
     }
 
     fn compile<E: UserDefinedError, I: InstructionMeter>(&mut self,
-            executable: &Executable<E, I>) -> Result<(), EbpfError<E>> {
+            executable: &Pin<Box<Executable<E, I>>>) -> Result<(), EbpfError<E>> {
         let (program_vm_addr, program) = executable.get_text_bytes();
         self.program_vm_addr = program_vm_addr;
 
@@ -1300,7 +1301,7 @@ impl JitCompiler {
                         // Workaround for unresolved symbols in ELF: Report error at runtime instead of compiletime
                         emit_rust_call(self, Executable::<E, I>::report_unresolved_symbol as *const _, &[
                             Argument { index: 2, value: Value::Constant64(self.pc as i64, false) },
-                            Argument { index: 1, value: Value::Constant64(executable as *const _ as i64, false) },
+                            Argument { index: 1, value: Value::Constant64(&*executable.as_ref() as *const _ as i64, false) },
                             Argument { index: 0, value: Value::RegisterIndirect(RBP, slot_on_environment_stack(self, EnvironmentStackSlot::OptRetValPtr), false) },
                         ], None, true)?;
                         X86Instruction::load_immediate(OperandSize::S64, R11, self.pc as i64).emit(self)?;

diff --git a/src/vm.rs b/src/vm.rs
@@ -27,6 +27,7 @@ use log::debug;
 use std::{
     collections::{BTreeMap, HashMap},
     fmt::Debug,
+    pin::Pin,
     u32,
 };
 
@@ -240,13 +241,12 @@ impl<E: UserDefinedError, I: 'static + InstructionMeter> Executable<E, I> {
         verifier: Option<Verifier>,
         config: Config,
         syscall_registry: SyscallRegistry,
-    ) -> Result<Self, EbpfError<E>> {
-        let ebpf_elf = Executable::load(config, elf_bytes, syscall_registry)?;
-        let text_bytes = ebpf_elf.get_text_bytes().1;
+    ) -> Result<Pin<Box<Self>>, EbpfError<E>> {
+        let executable = Executable::load(config, elf_bytes, syscall_registry)?;
         if let Some(verifier) = verifier {
-            verifier(text_bytes, &config)?;
+            verifier(executable.get_text_bytes().1, &config)?;
         }
-        Ok(ebpf_elf)
+        Ok(Pin::new(Box::new(executable)))
     }
     /// Creates a verified executable from machine code
     pub fn from_text_bytes(
@@ -255,16 +255,16 @@ impl<E: UserDefinedError, I: 'static + InstructionMeter> Executable<E, I> {
         config: Config,
         syscall_registry: SyscallRegistry,
         bpf_functions: BTreeMap<u32, (usize, String)>,
-    ) -> Result<Self, EbpfError<E>> {
+    ) -> Result<Pin<Box<Self>>, EbpfError<E>> {
         if let Some(verifier) = verifier {
             verifier(text_bytes, &config).map_err(EbpfError::VerifierError)?;
         }
-        Ok(Executable::new_from_text_bytes(
+        Ok(Pin::new(Box::new(Executable::new_from_text_bytes(
             config,
             text_bytes,
             syscall_registry,
             bpf_functions,
-        ))
+        ))))
     }
 }
 
@@ -480,7 +480,7 @@ impl<'a, E: UserDefinedError, I: InstructionMeter> EbpfVm<'a, E, I> {
     /// let mut vm = EbpfVm::<UserError, TestInstructionMeter>::new(&executable, &mut [], &mut []).unwrap();
     /// ```
     pub fn new(
-        executable: &'a Executable<E, I>,
+        executable: &'a Pin<Box<Executable<E, I>>>,
         heap_region: &mut [u8],
         input_region: &mut [u8],
     ) -> Result<EbpfVm<'a, E, I>, EbpfError<E>> {

diff --git a/tests/ubpf_execution.rs b/tests/ubpf_execution.rs
@@ -46,7 +46,7 @@ macro_rules! test_interpreter_and_jit {
         #[cfg(all(not(windows), target_arch = "x86_64"))]
         {
             let check_closure = $check;
-            let compilation_result = $executable.jit_compile();
+            let compilation_result = Executable::<UserError, TestInstructionMeter>::jit_compile(&mut $executable);
             let mut mem = $mem;
             let mut vm = EbpfVm::new(&$executable, &mut [], &mut mem).unwrap();
             match compilation_result {
@@ -2737,7 +2737,8 @@ impl SyscallObject<UserError> for NestedVmSyscall {
             .unwrap();
             #[cfg(all(not(windows), target_arch = "x86_64"))]
             {
-                executable.jit_compile().unwrap();
+                Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable)
+                    .unwrap();
             }
             let mut vm = EbpfVm::new(&executable, &mut [], mem).unwrap();
             vm.bind_syscall_context_object(Box::new(NestedVmSyscall {}), None)
@@ -3412,7 +3413,7 @@ fn execute_generated_program(prog: &[u8]) -> bool {
     } else {
         return false;
     };
-    if executable.jit_compile().is_err() {
+    if Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable).is_err() {
         return false;
     }
     let (instruction_count_interpreter, tracer_interpreter, result_interpreter) = {