Refactor - Remove generic parameter Verifier from Executable (#500)

* Removes the generic parameter "V: Verifier" from Executable. * Removes TautologyVerifier from public interface. * Turns Executable::verified() into executable.verify()
solana-labs · Aug 22, 2023 · 6ae9e4b · 6ae9e4b
1 parent 91f8af8
commit 6ae9e4b
Show file tree

Hide file tree

Showing 22 changed files with 184 additions and 286 deletions.
diff --git a/benches/elf_loader.rs b/benches/elf_loader.rs
@@ -13,7 +13,6 @@ extern crate test_utils;
 use solana_rbpf::{
     elf::{Executable, FunctionRegistry},
     syscalls,
-    verifier::TautologyVerifier,
     vm::{BuiltinFunction, BuiltinProgram, Config, TestContextObject},
 };
 use std::{fs::File, io::Read, sync::Arc};
@@ -36,9 +35,7 @@ fn bench_load_sbpfv1(bencher: &mut Bencher) {
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
     let loader = loader();
-    bencher.iter(|| {
-        Executable::<TautologyVerifier, TestContextObject>::from_elf(&elf, loader.clone()).unwrap()
-    });
+    bencher.iter(|| Executable::<TestContextObject>::from_elf(&elf, loader.clone()).unwrap());
 }
 
 #[bench]
@@ -47,7 +44,5 @@ fn bench_load_sbpfv2(bencher: &mut Bencher) {
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
     let loader = loader();
-    bencher.iter(|| {
-        Executable::<TautologyVerifier, TestContextObject>::from_elf(&elf, loader.clone()).unwrap()
-    });
+    bencher.iter(|| Executable::<TestContextObject>::from_elf(&elf, loader.clone()).unwrap());
 }
diff --git a/benches/jit_compile.rs b/benches/jit_compile.rs
@@ -11,7 +11,7 @@ extern crate test;
 
 use solana_rbpf::{
     elf::Executable,
-    verifier::{RequisiteVerifier, TautologyVerifier},
+    verifier::RequisiteVerifier,
     vm::{BuiltinProgram, TestContextObject},
 };
 use std::{fs::File, io::Read, sync::Arc};
@@ -23,18 +23,15 @@ fn bench_init_vm(bencher: &mut Bencher) {
     let mut file = File::open("tests/elfs/relative_call.so").unwrap();
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
-    let executable = Executable::<TautologyVerifier, TestContextObject>::from_elf(
-        &elf,
-        Arc::new(BuiltinProgram::new_mock()),
-    )
-    .unwrap();
-    let verified_executable =
-        Executable::<RequisiteVerifier, TestContextObject>::verified(executable).unwrap();
+    let executable =
+        Executable::<TestContextObject>::from_elf(&elf, Arc::new(BuiltinProgram::new_mock()))
+            .unwrap();
+    executable.verify::<RequisiteVerifier>().unwrap();
     bencher.iter(|| {
         let mut context_object = TestContextObject::default();
         create_vm!(
             _vm,
-            &verified_executable,
+            &executable,
             &mut context_object,
             stack,
             heap,
@@ -50,12 +47,9 @@ fn bench_jit_compile(bencher: &mut Bencher) {
     let mut file = File::open("tests/elfs/relative_call.so").unwrap();
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
-    let executable = Executable::<TautologyVerifier, TestContextObject>::from_elf(
-        &elf,
-        Arc::new(BuiltinProgram::new_mock()),
-    )
-    .unwrap();
-    let mut verified_executable =
-        Executable::<RequisiteVerifier, TestContextObject>::verified(executable).unwrap();
-    bencher.iter(|| verified_executable.jit_compile().unwrap());
+    let mut executable =
+        Executable::<TestContextObject>::from_elf(&elf, Arc::new(BuiltinProgram::new_mock()))
+            .unwrap();
+    executable.verify::<RequisiteVerifier>().unwrap();
+    bencher.iter(|| executable.jit_compile().unwrap());
 }
diff --git a/benches/vm_execution.rs b/benches/vm_execution.rs
@@ -13,7 +13,7 @@ use solana_rbpf::{
     ebpf,
     elf::{Executable, FunctionRegistry},
     memory_region::MemoryRegion,
-    verifier::{RequisiteVerifier, TautologyVerifier},
+    verifier::RequisiteVerifier,
     vm::{BuiltinProgram, Config, TestContextObject},
 };
 use std::{fs::File, io::Read, sync::Arc};
@@ -25,17 +25,14 @@ fn bench_init_interpreter_start(bencher: &mut Bencher) {
     let mut file = File::open("tests/elfs/rodata_section.so").unwrap();
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
-    let executable = Executable::<TautologyVerifier, TestContextObject>::from_elf(
-        &elf,
-        Arc::new(BuiltinProgram::new_mock()),
-    )
-    .unwrap();
-    let verified_executable =
-        Executable::<RequisiteVerifier, TestContextObject>::verified(executable).unwrap();
+    let executable =
+        Executable::<TestContextObject>::from_elf(&elf, Arc::new(BuiltinProgram::new_mock()))
+            .unwrap();
+    executable.verify::<RequisiteVerifier>().unwrap();
     let mut context_object = TestContextObject::default();
     create_vm!(
         vm,
-        &verified_executable,
+        &executable,
         &mut context_object,
         stack,
         heap,
@@ -44,7 +41,7 @@ fn bench_init_interpreter_start(bencher: &mut Bencher) {
     );
     bencher.iter(|| {
         vm.context_object_pointer.remaining = 37;
-        vm.execute_program(&verified_executable, true).1.unwrap()
+        vm.execute_program(&executable, true).1.unwrap()
     });
 }
 
@@ -54,18 +51,15 @@ fn bench_init_jit_start(bencher: &mut Bencher) {
     let mut file = File::open("tests/elfs/rodata_section.so").unwrap();
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
-    let executable = Executable::<TautologyVerifier, TestContextObject>::from_elf(
-        &elf,
-        Arc::new(BuiltinProgram::new_mock()),
-    )
-    .unwrap();
-    let mut verified_executable =
-        Executable::<RequisiteVerifier, TestContextObject>::verified(executable).unwrap();
-    verified_executable.jit_compile().unwrap();
+    let mut executable =
+        Executable::<TestContextObject>::from_elf(&elf, Arc::new(BuiltinProgram::new_mock()))
+            .unwrap();
+    executable.verify::<RequisiteVerifier>().unwrap();
+    executable.jit_compile().unwrap();
     let mut context_object = TestContextObject::default();
     create_vm!(
         vm,
-        &verified_executable,
+        &executable,
         &mut context_object,
         stack,
         heap,
@@ -74,7 +68,7 @@ fn bench_init_jit_start(bencher: &mut Bencher) {
     );
     bencher.iter(|| {
         vm.context_object_pointer.remaining = 37;
-        vm.execute_program(&verified_executable, false).1.unwrap()
+        vm.execute_program(&executable, false).1.unwrap()
     });
 }
 
@@ -86,22 +80,21 @@ fn bench_jit_vs_interpreter(
     instruction_meter: u64,
     mem: &mut [u8],
 ) {
-    let executable = solana_rbpf::assembler::assemble::<TestContextObject>(
+    let mut executable = solana_rbpf::assembler::assemble::<TestContextObject>(
         assembly,
         Arc::new(BuiltinProgram::new_loader(
             config,
             FunctionRegistry::default(),
         )),
     )
     .unwrap();
-    let mut verified_executable =
-        Executable::<RequisiteVerifier, TestContextObject>::verified(executable).unwrap();
-    verified_executable.jit_compile().unwrap();
+    executable.verify::<RequisiteVerifier>().unwrap();
+    executable.jit_compile().unwrap();
     let mut context_object = TestContextObject::default();
     let mem_region = MemoryRegion::new_writable(mem, ebpf::MM_INPUT_START);
     create_vm!(
         vm,
-        &verified_executable,
+        &executable,
         &mut context_object,
         stack,
         heap,
@@ -112,8 +105,7 @@ fn bench_jit_vs_interpreter(
         .bench(|bencher| {
             bencher.iter(|| {
                 vm.context_object_pointer.remaining = instruction_meter;
-                let (instruction_count_interpreter, result) =
-                    vm.execute_program(&verified_executable, true);
+                let (instruction_count_interpreter, result) = vm.execute_program(&executable, true);
                 assert!(result.is_ok(), "{:?}", result);
                 assert_eq!(instruction_count_interpreter, instruction_meter);
             });
@@ -125,8 +117,7 @@ fn bench_jit_vs_interpreter(
         .bench(|bencher| {
             bencher.iter(|| {
                 vm.context_object_pointer.remaining = instruction_meter;
-                let (instruction_count_jit, result) =
-                    vm.execute_program(&verified_executable, false);
+                let (instruction_count_jit, result) = vm.execute_program(&executable, false);
                 assert!(result.is_ok(), "{:?}", result);
                 assert_eq!(instruction_count_jit, instruction_meter);
             });

diff --git a/cli/src/main.rs b/cli/src/main.rs
@@ -6,7 +6,7 @@ use solana_rbpf::{
     elf::{Executable, FunctionRegistry},
     memory_region::{MemoryMapping, MemoryRegion},
     static_analysis::Analysis,
-    verifier::{RequisiteVerifier, TautologyVerifier},
+    verifier::RequisiteVerifier,
     vm::{BuiltinProgram, Config, DynamicAnalysis, EbpfVm, TestContextObject},
 };
 use std::{fs::File, io::Read, path::Path, sync::Arc};
@@ -101,7 +101,8 @@ fn main() {
         },
         FunctionRegistry::default(),
     ));
-    let executable = match matches.value_of("assembler") {
+    #[allow(unused_mut)]
+    let mut executable = match matches.value_of("assembler") {
         Some(asm_file_name) => {
             let mut file = File::open(Path::new(asm_file_name)).unwrap();
             let mut source = Vec::new();
@@ -112,15 +113,13 @@ fn main() {
             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::<TautologyVerifier, TestContextObject>::from_elf(&elf, loader)
+            Executable::<TestContextObject>::from_elf(&elf, loader)
                 .map_err(|err| format!("Executable constructor failed: {err:?}"))
         }
     }
     .unwrap();
 
-    #[allow(unused_mut)]
-    let verified_executable =
-        Executable::<RequisiteVerifier, TestContextObject>::verified(executable).unwrap();
+    executable.verify::<RequisiteVerifier>().unwrap();
 
     let mut mem = match matches.value_of("input").unwrap().parse::<usize>() {
         Ok(allocate) => vec![0u8; allocate],
@@ -133,7 +132,7 @@ fn main() {
     };
     #[cfg(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64"))]
     if matches.value_of("use") == Some("jit") {
-        verified_executable.jit_compile().unwrap();
+        executable.jit_compile().unwrap();
     }
     let mut context_object = TestContextObject::new(
         matches
@@ -142,8 +141,8 @@ fn main() {
             .parse::<u64>()
             .unwrap(),
     );
-    let config = verified_executable.get_config();
-    let sbpf_version = verified_executable.get_sbpf_version();
+    let config = executable.get_config();
+    let sbpf_version = executable.get_sbpf_version();
     let mut stack = AlignedMemory::<{ ebpf::HOST_ALIGN }>::zero_filled(config.stack_size());
     let stack_len = stack.len();
     let mut heap = AlignedMemory::<{ ebpf::HOST_ALIGN }>::zero_filled(
@@ -154,7 +153,7 @@ fn main() {
             .unwrap(),
     );
     let regions: Vec<MemoryRegion> = vec![
-        verified_executable.get_ro_region(),
+        executable.get_ro_region(),
         MemoryRegion::new_writable_gapped(
             stack.as_slice_mut(),
             ebpf::MM_STACK_START,
@@ -171,8 +170,8 @@ fn main() {
     let memory_mapping = MemoryMapping::new(regions, config, sbpf_version).unwrap();
 
     let mut vm = EbpfVm::new(
-        verified_executable.get_config(),
-        verified_executable.get_sbpf_version(),
+        executable.get_config(),
+        executable.get_sbpf_version(),
         &mut context_object,
         memory_mapping,
         stack_len,
@@ -183,7 +182,7 @@ fn main() {
         || matches.is_present("trace")
         || matches.is_present("profile")
     {
-        Some(Analysis::from_executable(&verified_executable).unwrap())
+        Some(Analysis::from_executable(&executable).unwrap())
     } else {
         None
     };
@@ -212,10 +211,8 @@ fn main() {
     if matches.value_of("use").unwrap() == "debugger" {
         vm.debug_port = Some(matches.value_of("port").unwrap().parse::<u16>().unwrap());
     }
-    let (instruction_count, result) = vm.execute_program(
-        &verified_executable,
-        matches.value_of("use").unwrap() != "jit",
-    );
+    let (instruction_count, result) =
+        vm.execute_program(&executable, matches.value_of("use").unwrap() != "jit");
     println!("Result: {result:?}");
     println!("Instruction Count: {instruction_count}");
     if matches.is_present("trace") {

diff --git a/examples/disassemble.rs b/examples/disassemble.rs
@@ -8,7 +8,6 @@ extern crate solana_rbpf;
 use solana_rbpf::{
     elf::{Executable, FunctionRegistry, SBPFVersion},
     static_analysis::Analysis,
-    verifier::TautologyVerifier,
     vm::{BuiltinProgram, TestContextObject},
 };
 use std::sync::Arc;
@@ -32,7 +31,7 @@ fn main() {
         0x00, 0x00, 0x00, 0x00, 0x00,
     ];
     let loader = Arc::new(BuiltinProgram::new_mock());
-    let executable = Executable::<TautologyVerifier, TestContextObject>::from_text_bytes(
+    let executable = Executable::<TestContextObject>::from_text_bytes(
         program,
         loader,
         SBPFVersion::V2,

diff --git a/examples/to_json.rs b/examples/to_json.rs
@@ -14,7 +14,6 @@ extern crate solana_rbpf;
 use solana_rbpf::{
     elf::{Executable, FunctionRegistry, SBPFVersion},
     static_analysis::Analysis,
-    verifier::TautologyVerifier,
     vm::{BuiltinProgram, TestContextObject},
 };
 use std::sync::Arc;
@@ -28,7 +27,7 @@ use std::sync::Arc;
 // * Print integers as integers, and not as strings containing their hexadecimal representation
 //   (just replace the relevant `format!()` calls by the commented values.
 fn to_json(program: &[u8]) -> String {
-    let executable = Executable::<TautologyVerifier, TestContextObject>::from_text_bytes(
+    let executable = Executable::<TestContextObject>::from_text_bytes(
         program,
         Arc::new(BuiltinProgram::new_mock()),
         SBPFVersion::V2,

diff --git a/fuzz/fuzz_targets/dumb.rs b/fuzz/fuzz_targets/dumb.rs
@@ -8,7 +8,7 @@ use solana_rbpf::{
     ebpf,
     elf::{Executable, FunctionRegistry, SBPFVersion},
     memory_region::MemoryRegion,
-    verifier::{RequisiteVerifier, TautologyVerifier, Verifier},
+    verifier::{RequisiteVerifier, Verifier},
     vm::{BuiltinProgram, TestContextObject},
 };
 use test_utils::create_vm;
@@ -33,7 +33,7 @@ fuzz_target!(|data: DumbFuzzData| {
         return;
     }
     let mut mem = data.mem;
-    let executable = Executable::<TautologyVerifier, TestContextObject>::from_text_bytes(
+    let executable = Executable::<TestContextObject>::from_text_bytes(
         &prog,
         std::sync::Arc::new(BuiltinProgram::new_loader(config, FunctionRegistry::default())),
         SBPFVersion::V2,

diff --git a/fuzz/fuzz_targets/smart.rs b/fuzz/fuzz_targets/smart.rs
@@ -10,7 +10,7 @@ use solana_rbpf::{
     elf::{Executable, FunctionRegistry, SBPFVersion},
     insn_builder::{Arch, IntoBytes},
     memory_region::MemoryRegion,
-    verifier::{RequisiteVerifier, TautologyVerifier, Verifier},
+    verifier::{RequisiteVerifier, Verifier},
     vm::{BuiltinProgram, TestContextObject},
 };
 use test_utils::create_vm;
@@ -37,7 +37,7 @@ fuzz_target!(|data: FuzzData| {
         return;
     }
     let mut mem = data.mem;
-    let executable = Executable::<TautologyVerifier, TestContextObject>::from_text_bytes(
+    let executable = Executable::<TestContextObject>::from_text_bytes(
         prog.into_bytes(),
         std::sync::Arc::new(BuiltinProgram::new_loader(config, FunctionRegistry::default())),
         SBPFVersion::V2,

diff --git a/fuzz/fuzz_targets/smart_jit_diff.rs b/fuzz/fuzz_targets/smart_jit_diff.rs
@@ -8,7 +8,7 @@ use solana_rbpf::{
     elf::{Executable, FunctionRegistry, SBPFVersion},
     insn_builder::{Arch, Instruction, IntoBytes},
     memory_region::MemoryRegion,
-    verifier::{RequisiteVerifier, TautologyVerifier, Verifier},
+    verifier::{RequisiteVerifier, Verifier},
     vm::{BuiltinProgram, TestContextObject},
 };
 use test_utils::create_vm;
@@ -45,7 +45,7 @@ fuzz_target!(|data: FuzzData| {
     }
     let mut interp_mem = data.mem.clone();
     let mut jit_mem = data.mem;
-    let mut executable = Executable::<TautologyVerifier, TestContextObject>::from_text_bytes(
+    let mut executable = Executable::<TestContextObject>::from_text_bytes(
         prog.into_bytes(),
         std::sync::Arc::new(BuiltinProgram::new_loader(config, FunctionRegistry::default())),
         SBPFVersion::V2,