Refactor: Turns verifier into a trait (#342)

* Turns verifier into a trait. * Reorders verify() to happen last in from_text_bytes().
solana-labs · Jun 3, 2022 · 0ee6faf · 0ee6faf
1 parent 5969481
commit 0ee6faf
Show file tree

Hide file tree

Showing 21 changed files with 301 additions and 358 deletions.
diff --git a/benches/elf_loader.rs b/benches/elf_loader.rs
@@ -14,6 +14,7 @@ use solana_rbpf::{
     elf::Executable,
     syscalls::{BpfSyscallContext, BpfSyscallU64},
     user_error::UserError,
+    verifier::TautologyVerifier,
     vm::{Config, SyscallObject, SyscallRegistry, TestInstructionMeter},
 };
 use std::{fs::File, io::Read};
@@ -37,9 +38,8 @@ fn bench_load_elf(bencher: &mut Bencher) {
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
     bencher.iter(|| {
-        Executable::<UserError, TestInstructionMeter>::from_elf(
+        Executable::<UserError, TestInstructionMeter>::from_elf::<TautologyVerifier>(
             &elf,
-            None,
             Config::default(),
             syscall_registry(),
         )
@@ -53,13 +53,13 @@ fn bench_load_elf_without_syscall(bencher: &mut Bencher) {
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
     bencher.iter(|| {
-        let executable = Executable::<UserError, TestInstructionMeter>::from_elf(
-            &elf,
-            None,
-            Config::default(),
-            syscall_registry(),
-        )
-        .unwrap();
+        let executable =
+            Executable::<UserError, TestInstructionMeter>::from_elf::<TautologyVerifier>(
+                &elf,
+                Config::default(),
+                syscall_registry(),
+            )
+            .unwrap();
         executable
     });
 }
@@ -70,13 +70,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 executable = Executable::<UserError, TestInstructionMeter>::from_elf(
-            &elf,
-            None,
-            Config::default(),
-            syscall_registry(),
-        )
-        .unwrap();
+        let executable =
+            Executable::<UserError, TestInstructionMeter>::from_elf::<TautologyVerifier>(
+                &elf,
+                Config::default(),
+                syscall_registry(),
+            )
+            .unwrap();
         executable
     });
 }
diff --git a/benches/jit_compile.rs b/benches/jit_compile.rs
@@ -12,6 +12,7 @@ extern crate test;
 use solana_rbpf::{
     elf::Executable,
     user_error::UserError,
+    verifier::TautologyVerifier,
     vm::{Config, EbpfVm, SyscallRegistry, TestInstructionMeter},
 };
 use std::{fs::File, io::Read};
@@ -22,9 +23,8 @@ fn bench_init_vm(bencher: &mut Bencher) {
     let mut file = File::open("tests/elfs/pass_stack_reference.so").unwrap();
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
-    let executable = Executable::<UserError, TestInstructionMeter>::from_elf(
+    let executable = Executable::<UserError, TestInstructionMeter>::from_elf::<TautologyVerifier>(
         &elf,
-        None,
         Config::default(),
         SyscallRegistry::default(),
     )
@@ -40,13 +40,13 @@ fn bench_jit_compile(bencher: &mut Bencher) {
     let mut file = File::open("tests/elfs/pass_stack_reference.so").unwrap();
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
-    let mut executable = Executable::<UserError, TestInstructionMeter>::from_elf(
-        &elf,
-        None,
-        Config::default(),
-        SyscallRegistry::default(),
-    )
-    .unwrap();
+    let mut executable =
+        Executable::<UserError, TestInstructionMeter>::from_elf::<TautologyVerifier>(
+            &elf,
+            Config::default(),
+            SyscallRegistry::default(),
+        )
+        .unwrap();
     bencher.iter(|| {
         Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable).unwrap()
     });

diff --git a/benches/vm_execution.rs b/benches/vm_execution.rs
@@ -14,6 +14,7 @@ use solana_rbpf::{
     elf::Executable,
     memory_region::MemoryRegion,
     user_error::UserError,
+    verifier::TautologyVerifier,
     vm::{Config, EbpfVm, SyscallRegistry, TestInstructionMeter},
 };
 use std::{fs::File, io::Read};
@@ -24,9 +25,8 @@ fn bench_init_interpreter_execution(bencher: &mut Bencher) {
     let mut file = File::open("tests/elfs/pass_stack_reference.so").unwrap();
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
-    let executable = Executable::<UserError, TestInstructionMeter>::from_elf(
+    let executable = Executable::<UserError, TestInstructionMeter>::from_elf::<TautologyVerifier>(
         &elf,
-        None,
         Config::default(),
         SyscallRegistry::default(),
     )
@@ -45,13 +45,13 @@ fn bench_init_jit_execution(bencher: &mut Bencher) {
     let mut file = File::open("tests/elfs/pass_stack_reference.so").unwrap();
     let mut elf = Vec::new();
     file.read_to_end(&mut elf).unwrap();
-    let mut executable = Executable::<UserError, TestInstructionMeter>::from_elf(
-        &elf,
-        None,
-        Config::default(),
-        SyscallRegistry::default(),
-    )
-    .unwrap();
+    let mut executable =
+        Executable::<UserError, TestInstructionMeter>::from_elf::<TautologyVerifier>(
+            &elf,
+            Config::default(),
+            SyscallRegistry::default(),
+        )
+        .unwrap();
     Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable).unwrap();
     let mut vm =
         EbpfVm::<UserError, TestInstructionMeter>::new(&executable, &mut [], Vec::new()).unwrap();
@@ -69,12 +69,11 @@ fn bench_jit_vs_interpreter(
     instruction_meter: u64,
     mem: &mut [u8],
 ) {
-    let mut executable = solana_rbpf::assembler::assemble::<UserError, TestInstructionMeter>(
-        assembly,
-        None,
-        config,
-        SyscallRegistry::default(),
-    )
+    let mut executable = solana_rbpf::assembler::assemble::<
+        TautologyVerifier,
+        UserError,
+        TestInstructionMeter,
+    >(assembly, config, SyscallRegistry::default())
     .unwrap();
     Executable::<UserError, TestInstructionMeter>::jit_compile(&mut executable).unwrap();
     let mem_region = MemoryRegion::new_writable(mem, ebpf::MM_INPUT_START);

diff --git a/cli/src/main.rs b/cli/src/main.rs
@@ -7,7 +7,7 @@ use solana_rbpf::{
     static_analysis::Analysis,
     syscalls::Result,
     user_error::UserError,
-    verifier::check,
+    verifier::SbfVerifier,
     vm::{Config, DynamicAnalysis, EbpfVm, SyscallObject, SyscallRegistry, TestInstructionMeter},
 };
 use std::{fs::File, io::Read, path::Path};
@@ -106,34 +106,21 @@ fn main() {
                 .short('p')
                 .long("prof"),
         )
-        .arg(
-            Arg::new("verify")
-                .about("Run the verifier before execution or disassembly")
-                .short('v')
-                .long("veri"),
-        )
         .get_matches();
 
     let config = Config {
         enable_instruction_tracing: matches.is_present("trace") || matches.is_present("profile"),
         enable_symbol_and_section_labels: true,
         ..Config::default()
     };
-    let verifier: Option<for<'r> fn(&'r [u8], &Config) -> std::result::Result<_, _>> =
-        if matches.is_present("verify") {
-            Some(check)
-        } else {
-            None
-        };
     let syscall_registry = SyscallRegistry::default();
     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();
             file.read_to_end(&mut source).unwrap();
-            assemble::<UserError, TestInstructionMeter>(
+            assemble::<SbfVerifier, UserError, TestInstructionMeter>(
                 std::str::from_utf8(source.as_slice()).unwrap(),
-                verifier,
                 config,
                 syscall_registry,
             )
@@ -142,9 +129,8 @@ 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::<UserError, TestInstructionMeter>::from_elf(
+            Executable::<UserError, TestInstructionMeter>::from_elf::<SbfVerifier>(
                 &elf,
-                verifier,
                 config,
                 syscall_registry,
             )

diff --git a/examples/disassemble.rs b/examples/disassemble.rs
@@ -9,7 +9,7 @@ use solana_rbpf::{
     elf::Executable,
     static_analysis::Analysis,
     user_error::UserError,
-    verifier::check,
+    verifier::SbfVerifier,
     vm::{Config, SyscallRegistry, TestInstructionMeter},
 };
 use std::collections::BTreeMap;
@@ -32,9 +32,8 @@ fn main() {
         0x00, 0x00, 0x00, 0x00, 0xb7, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x95, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00, 0x00,
     ];
-    let executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes(
+    let executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes::<SbfVerifier>(
         &program,
-        Some(check),
         Config::default(),
         SyscallRegistry::default(),
         BTreeMap::default(),

diff --git a/examples/to_json.rs b/examples/to_json.rs
@@ -16,7 +16,7 @@ use solana_rbpf::{
     elf::Executable,
     static_analysis::Analysis,
     user_error::UserError,
-    verifier::check,
+    verifier::SbfVerifier,
     vm::{Config, SyscallRegistry, TestInstructionMeter},
 };
 use std::collections::BTreeMap;
@@ -30,9 +30,8 @@ use std::collections::BTreeMap;
 // * 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::<UserError, TestInstructionMeter>::from_text_bytes(
+    let executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes::<SbfVerifier>(
         &program,
-        Some(check),
         Config::default(),
         SyscallRegistry::default(),
         BTreeMap::default(),

diff --git a/fuzz/fuzz_targets/dumb.rs b/fuzz/fuzz_targets/dumb.rs
@@ -11,7 +11,7 @@ use solana_rbpf::{
     elf::{register_bpf_function, Executable},
     memory_region::MemoryRegion,
     user_error::UserError,
-    verifier::check,
+    verifier::{SbfVerifier, TautologyVerifier, Verifier},
     vm::{EbpfVm, SyscallRegistry, TestInstructionMeter},
 };
 
@@ -29,17 +29,16 @@ struct DumbFuzzData {
 fuzz_target!(|data: DumbFuzzData| {
     let prog = data.prog;
     let config = data.template.into();
-    if check(&prog, &config).is_err() {
+    if SbfVerifier::verify(&prog, &config).is_err() {
         // verify please
         return;
     }
     let mut mem = data.mem;
     let registry = SyscallRegistry::default();
     let mut bpf_functions = BTreeMap::new();
     register_bpf_function(&config, &mut bpf_functions, &registry, 0, "entrypoint").unwrap();
-    let executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes(
+    let executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes::<TautologyVerifier>(
         &prog,
-        None,
         config,
         SyscallRegistry::default(),
         bpf_functions,

diff --git a/fuzz/fuzz_targets/smart.rs b/fuzz/fuzz_targets/smart.rs
@@ -13,7 +13,7 @@ use solana_rbpf::{
     insn_builder::{Arch, IntoBytes},
     memory_region::MemoryRegion,
     user_error::UserError,
-    verifier::check,
+    verifier::{SbfVerifier, TautologyVerifier, Verifier},
     vm::{EbpfVm, SyscallRegistry, TestInstructionMeter},
 };
 
@@ -33,17 +33,16 @@ struct FuzzData {
 fuzz_target!(|data: FuzzData| {
     let prog = make_program(&data.prog, data.arch);
     let config = data.template.into();
-    if check(prog.into_bytes(), &config).is_err() {
+    if SbfVerifier::verify(prog.into_bytes(), &config).is_err() {
         // verify please
         return;
     }
     let mut mem = data.mem;
     let registry = SyscallRegistry::default();
     let mut bpf_functions = BTreeMap::new();
     register_bpf_function(&config, &mut bpf_functions, &registry, 0, "entrypoint").unwrap();
-    let executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes(
+    let executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes::<TautologyVerifier>(
         prog.into_bytes(),
-        None,
         config,
         SyscallRegistry::default(),
         bpf_functions,

diff --git a/fuzz/fuzz_targets/smart_jit_diff.rs b/fuzz/fuzz_targets/smart_jit_diff.rs
@@ -11,7 +11,7 @@ use solana_rbpf::{
     insn_builder::{Arch, Instruction, IntoBytes},
     memory_region::MemoryRegion,
     user_error::UserError,
-    verifier::check,
+    verifier::{SbfVerifier, TautologyVerifier, Verifier},
     vm::{EbpfVm, SyscallRegistry, TestInstructionMeter},
 };
 
@@ -40,7 +40,7 @@ fuzz_target!(|data: FuzzData| {
         .set_imm(data.exit_imm)
         .push();
     let config = data.template.into();
-    if check(prog.into_bytes(), &config).is_err() {
+    if SbfVerifier::verify(prog.into_bytes(), &config).is_err() {
         // verify please
         return;
     }
@@ -49,9 +49,8 @@ fuzz_target!(|data: FuzzData| {
     let registry = SyscallRegistry::default();
     let mut bpf_functions = BTreeMap::new();
     register_bpf_function(&config, &mut bpf_functions, &registry, 0, "entrypoint").unwrap();
-    let mut executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes(
+    let mut executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes::<TautologyVerifier>(
         prog.into_bytes(),
-        None,
         config,
         SyscallRegistry::default(),
         bpf_functions,

diff --git a/fuzz/fuzz_targets/smarter_jit_diff.rs b/fuzz/fuzz_targets/smarter_jit_diff.rs
@@ -13,7 +13,7 @@ use solana_rbpf::{
     memory_region::MemoryRegion,
     static_analysis::Analysis,
     user_error::UserError,
-    verifier::check,
+    verifier::{SbfVerifier, TautologyVerifier, Verifier},
     vm::{EbpfVm, InstructionMeter, SyscallRegistry, TestInstructionMeter},
 };
 
@@ -38,7 +38,7 @@ fn dump_insns<E: UserDefinedError, I: InstructionMeter>(executable: &Executable<
 fuzz_target!(|data: FuzzData| {
     let prog = make_program(&data.prog);
     let config = data.template.into();
-    if check(prog.into_bytes(), &config).is_err() {
+    if SbfVerifier::verify(prog.into_bytes(), &config).is_err() {
         // verify please
         return;
     }
@@ -47,9 +47,8 @@ fuzz_target!(|data: FuzzData| {
     let registry = SyscallRegistry::default();
     let mut bpf_functions = BTreeMap::new();
     register_bpf_function(&config, &mut bpf_functions, &registry, 0, "entrypoint").unwrap();
-    let mut executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes(
+    let mut executable = Executable::<UserError, TestInstructionMeter>::from_text_bytes::<TautologyVerifier>(
         prog.into_bytes(),
-        None,
         config,
         SyscallRegistry::default(),
         bpf_functions,

diff --git a/fuzz/fuzz_targets/verify_semantic_aware.rs b/fuzz/fuzz_targets/verify_semantic_aware.rs
@@ -4,7 +4,7 @@ use libfuzzer_sys::fuzz_target;
 
 use semantic_aware::*;
 use solana_rbpf::insn_builder::IntoBytes;
-use solana_rbpf::verifier::check;
+use solana_rbpf::verifier::{SbfVerifier, Verifier};
 
 use crate::common::ConfigTemplate;
 
@@ -20,5 +20,5 @@ struct FuzzData {
 fuzz_target!(|data: FuzzData| {
     let prog = make_program(&data.prog);
     let config = data.template.into();
-    check(prog.into_bytes(), &config).unwrap();
+    SbfVerifier::verify(prog.into_bytes(), &config).unwrap();
 });