From c72113b6abc6bff3d4d6538ea2d844a309cee213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 26 Jul 2024 10:26:57 +0200 Subject: [PATCH] Refactor - `Config::enabled_sbpf_versions` (#582) * Removes derive(Copy) from Config. * Replaces enable_sbpf_v1 and enable_sbpf_v2 by enabled_sbpf_versions. --- benches/vm_execution.rs | 15 ++++-- src/assembler.rs | 8 +-- src/elf.rs | 13 ++--- src/program.rs | 2 +- src/vm.rs | 11 ++-- tests/execution.rs | 98 +++++++++++++++++----------------- tests/exercise_instructions.rs | 4 +- tests/verifier.rs | 14 ++--- 8 files changed, 80 insertions(+), 85 deletions(-) diff --git a/benches/vm_execution.rs b/benches/vm_execution.rs index 3fa14c03..e3c6ebc1 100644 --- a/benches/vm_execution.rs +++ b/benches/vm_execution.rs @@ -10,7 +10,12 @@ extern crate solana_rbpf; extern crate test; #[cfg(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64"))] -use solana_rbpf::{ebpf, memory_region::MemoryRegion, program::FunctionRegistry, vm::Config}; +use solana_rbpf::{ + ebpf, + memory_region::MemoryRegion, + program::{FunctionRegistry, SBPFVersion}, + vm::Config, +}; use solana_rbpf::{ elf::Executable, program::BuiltinProgram, verifier::RequisiteVerifier, vm::TestContextObject, }; @@ -166,7 +171,7 @@ fn bench_jit_vs_interpreter_address_translation_stack_fixed(bencher: &mut Benche bencher, ADDRESS_TRANSLATION_STACK_CODE, Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }, 524289, @@ -181,7 +186,7 @@ fn bench_jit_vs_interpreter_address_translation_stack_dynamic(bencher: &mut Benc bencher, ADDRESS_TRANSLATION_STACK_CODE, Config { - enable_sbpf_v2: true, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V2, ..Config::default() }, 524289, @@ -228,7 +233,7 @@ fn bench_jit_vs_interpreter_call_depth_fixed(bencher: &mut Bencher) { call function_foo exit", Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }, 137218, @@ -259,7 +264,7 @@ fn bench_jit_vs_interpreter_call_depth_dynamic(bencher: &mut Bencher) { add r11, 4 exit", Config { - enable_sbpf_v2: true, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V2, ..Config::default() }, 176130, diff --git a/src/assembler.rs b/src/assembler.rs index 4e9a6b57..08c852f3 100644 --- a/src/assembler.rs +++ b/src/assembler.rs @@ -19,7 +19,7 @@ use crate::{ }, ebpf::{self, Insn}, elf::Executable, - program::{BuiltinProgram, FunctionRegistry, SBPFVersion}, + program::{BuiltinProgram, FunctionRegistry}, vm::ContextObject, }; use std::collections::HashMap; @@ -311,11 +311,7 @@ pub fn assemble( src: &str, loader: Arc>, ) -> Result, String> { - let sbpf_version = if loader.get_config().enable_sbpf_v2 { - SBPFVersion::V2 - } else { - SBPFVersion::V1 - }; + let sbpf_version = loader.get_config().enabled_sbpf_versions.end().clone(); let statements = parse(src)?; let instruction_map = make_instruction_map(); diff --git a/src/elf.rs b/src/elf.rs index e63f736b..c5b531d4 100644 --- a/src/elf.rs +++ b/src/elf.rs @@ -516,16 +516,13 @@ impl Executable { } let sbpf_version = if header.e_flags == EF_SBPF_V2 { - if !config.enable_sbpf_v2 { - return Err(ElfError::UnsupportedSBPFVersion); - } SBPFVersion::V2 } else { - if !config.enable_sbpf_v1 { - return Err(ElfError::UnsupportedSBPFVersion); - } SBPFVersion::V1 }; + if !config.enabled_sbpf_versions.contains(&sbpf_version) { + return Err(ElfError::UnsupportedSBPFVersion); + } if sbpf_version.enable_elf_vaddr() { if !config.optimize_rodata { @@ -1456,7 +1453,7 @@ mod test { fn test_sh_offset_not_same_as_vaddr() { let config = Config { reject_broken_elfs: true, - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }; let elf_bytes = [0u8; 512]; @@ -1834,7 +1831,7 @@ mod test { #[test] fn test_reject_rodata_stack_overlap() { let config = Config { - enable_sbpf_v2: true, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V2, ..Config::default() }; let elf_bytes = [0u8; 512]; diff --git a/src/program.rs b/src/program.rs index 7fce2394..0ef76bcc 100644 --- a/src/program.rs +++ b/src/program.rs @@ -9,7 +9,7 @@ use { }; /// Defines a set of sbpf_version of an executable -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, PartialOrd, Eq, Clone)] pub enum SBPFVersion { /// The legacy format V1, diff --git a/src/vm.rs b/src/vm.rs index 720a1212..969a339e 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -48,7 +48,7 @@ pub fn get_runtime_environment_key() -> i32 { } /// VM configuration settings -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Config { /// Maximum call depth pub max_call_depth: usize, @@ -80,10 +80,8 @@ pub struct Config { pub optimize_rodata: bool, /// Use aligned memory mapping pub aligned_memory_mapping: bool, - /// Allow ExecutableCapability::V1 - pub enable_sbpf_v1: bool, - /// Allow ExecutableCapability::V2 - pub enable_sbpf_v2: bool, + /// Allowed [SBPFVersion]s + pub enabled_sbpf_versions: std::ops::RangeInclusive, } impl Config { @@ -111,8 +109,7 @@ impl Default for Config { reject_callx_r10: true, optimize_rodata: true, aligned_memory_mapping: true, - enable_sbpf_v1: true, - enable_sbpf_v2: true, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V2, } } } diff --git a/tests/execution.rs b/tests/execution.rs index d467e497..6d8f151f 100644 --- a/tests/execution.rs +++ b/tests/execution.rs @@ -1943,7 +1943,7 @@ fn test_string_stack() { #[test] fn test_err_dynamic_stack_out_of_bound() { let config = Config { - enable_sbpf_v2: true, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V2, max_call_depth: 3, ..Config::default() }; @@ -1955,7 +1955,7 @@ fn test_err_dynamic_stack_out_of_bound() { " stb [r10-0x3001], 0 exit", - config, + config.clone(), [], (), TestContextObject::new(1), @@ -1972,7 +1972,7 @@ fn test_err_dynamic_stack_out_of_bound() { " stb [r10], 0 exit", - config, + config.clone(), [], (), TestContextObject::new(1), @@ -2027,7 +2027,7 @@ fn test_dynamic_stack_frames_empty() { function_foo: mov r0, r10 exit", - config, + config.clone(), [], (), TestContextObject::new(4), @@ -2049,7 +2049,7 @@ fn test_dynamic_frame_ptr() { function_foo: mov r0, r10 exit", - config, + config.clone(), [], (), TestContextObject::new(5), @@ -2067,7 +2067,7 @@ fn test_dynamic_frame_ptr() { function_foo: exit ", - config, + config.clone(), [], (), TestContextObject::new(5), @@ -2083,9 +2083,9 @@ fn test_entrypoint_exit() { // can't infer anything from the stack size so we track call depth // explicitly. Make sure exit still works with both fixed and dynamic // frames. - for enable_sbpf_v2 in [false, true] { + for highest_sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { let config = Config { - enable_sbpf_v2, + enabled_sbpf_versions: SBPFVersion::V1..=highest_sbpf_version, ..Config::default() }; @@ -2111,9 +2111,9 @@ fn test_entrypoint_exit() { #[test] fn test_stack_call_depth_tracking() { - for enable_sbpf_v2 in [false, true] { + for highest_sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { let config = Config { - enable_sbpf_v2, + enabled_sbpf_versions: SBPFVersion::V1..=highest_sbpf_version, max_call_depth: 2, ..Config::default() }; @@ -2130,7 +2130,7 @@ fn test_stack_call_depth_tracking() { function_foo: exit ", - config, + config.clone(), [], (), TestContextObject::new(5), @@ -2338,7 +2338,7 @@ fn test_bpf_to_bpf_depth() { add64 r1, -1 call function_foo exit", - config, + config.clone(), [max_call_depth as u8], (), TestContextObject::new(max_call_depth as u64 * 4 - 2), @@ -3421,7 +3421,7 @@ fn test_total_chaos() { #[test] fn test_err_fixed_stack_out_of_bound() { let config = Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, max_call_depth: 3, ..Config::default() }; @@ -3445,14 +3445,14 @@ fn test_err_fixed_stack_out_of_bound() { #[test] fn test_lddw() { let config = Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }; test_interpreter_and_jit_asm!( " lddw r0, 0x1122334455667788 exit", - config, + config.clone(), [], (), TestContextObject::new(2), @@ -3462,7 +3462,7 @@ fn test_lddw() { " lddw r0, 0x0000000080000000 exit", - config, + config.clone(), [], (), TestContextObject::new(2), @@ -3481,7 +3481,7 @@ fn test_lddw() { add r0, r1 exit ", - config, + config.clone(), [], (), TestContextObject::new(9), @@ -3495,7 +3495,7 @@ fn test_lddw() { callx r8 lddw r0, 0x1122334455667788 exit", - config, + config.clone(), [], (), TestContextObject::new(4), @@ -3509,7 +3509,7 @@ fn test_lddw() { callx r8 lddw r0, 0x1122334455667788 exit", - config, + config.clone(), [], (), TestContextObject::new(5), @@ -3526,7 +3526,7 @@ fn test_lddw() { lddw r0, 0x1122334455667788 exit ", - config, + config.clone(), [], (), TestContextObject::new(5), @@ -3542,7 +3542,7 @@ fn test_lddw() { lddw r0, 0x1122334455667788 exit ", - config, + config.clone(), [], (), TestContextObject::new(3), @@ -3566,7 +3566,7 @@ fn test_lddw() { #[test] fn test_le() { let config = Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }; test_interpreter_and_jit_asm!( @@ -3574,7 +3574,7 @@ fn test_le() { ldxh r0, [r1] le16 r0 exit", - config, + config.clone(), [0x22, 0x11], (), TestContextObject::new(3), @@ -3585,7 +3585,7 @@ fn test_le() { ldxdw r0, [r1] le16 r0 exit", - config, + config.clone(), [0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88], (), TestContextObject::new(3), @@ -3596,7 +3596,7 @@ fn test_le() { ldxw r0, [r1] le32 r0 exit", - config, + config.clone(), [0x44, 0x33, 0x22, 0x11], (), TestContextObject::new(3), @@ -3607,7 +3607,7 @@ fn test_le() { ldxdw r0, [r1] le32 r0 exit", - config, + config.clone(), [0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88], (), TestContextObject::new(3), @@ -3629,7 +3629,7 @@ fn test_le() { #[test] fn test_neg() { let config = Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }; test_interpreter_and_jit_asm!( @@ -3637,7 +3637,7 @@ fn test_neg() { mov32 r0, 2 neg32 r0 exit", - config, + config.clone(), [], (), TestContextObject::new(3), @@ -3648,7 +3648,7 @@ fn test_neg() { mov r0, 2 neg r0 exit", - config, + config.clone(), [], (), TestContextObject::new(3), @@ -3659,7 +3659,7 @@ fn test_neg() { mov32 r0, 3 sub32 r0, 1 exit", - config, + config.clone(), [], (), TestContextObject::new(3), @@ -3681,7 +3681,7 @@ fn test_neg() { #[test] fn test_callx_imm() { let config = Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }; test_interpreter_and_jit_asm!( @@ -3706,7 +3706,7 @@ fn test_callx_imm() { #[test] fn test_mul() { let config = Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }; test_interpreter_and_jit_asm!( @@ -3714,7 +3714,7 @@ fn test_mul() { mov r0, 3 mul32 r0, 4 exit", - config, + config.clone(), [], (), TestContextObject::new(3), @@ -3726,7 +3726,7 @@ fn test_mul() { mov r1, 4 mul32 r0, r1 exit", - config, + config.clone(), [], (), TestContextObject::new(4), @@ -3738,7 +3738,7 @@ fn test_mul() { mov r1, 4 mul32 r0, r1 exit", - config, + config.clone(), [], (), TestContextObject::new(4), @@ -3749,7 +3749,7 @@ fn test_mul() { mov r0, 0x40000001 mul r0, 4 exit", - config, + config.clone(), [], (), TestContextObject::new(3), @@ -3761,7 +3761,7 @@ fn test_mul() { mov r1, 4 mul r0, r1 exit", - config, + config.clone(), [], (), TestContextObject::new(4), @@ -3783,7 +3783,7 @@ fn test_mul() { #[test] fn test_div() { let config = Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }; test_interpreter_and_jit_asm!( @@ -3792,7 +3792,7 @@ fn test_div() { lddw r1, 0x100000004 div32 r0, r1 exit", - config, + config.clone(), [], (), TestContextObject::new(4), @@ -3803,7 +3803,7 @@ fn test_div() { lddw r0, 0x10000000c div32 r0, 4 exit", - config, + config.clone(), [], (), TestContextObject::new(3), @@ -3815,7 +3815,7 @@ fn test_div() { mov r1, 4 div32 r0, r1 exit", - config, + config.clone(), [], (), TestContextObject::new(4), @@ -3827,7 +3827,7 @@ fn test_div() { lsh r0, 32 div r0, 4 exit", - config, + config.clone(), [], (), TestContextObject::new(4), @@ -3840,7 +3840,7 @@ fn test_div() { mov r1, 4 div r0, r1 exit", - config, + config.clone(), [], (), TestContextObject::new(5), @@ -3852,7 +3852,7 @@ fn test_div() { mov32 r1, 0 div r0, r1 exit", - config, + config.clone(), [], (), TestContextObject::new(3), @@ -3875,7 +3875,7 @@ fn test_div() { #[test] fn test_mod() { let config = Config { - enable_sbpf_v2: false, + enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1, ..Config::default() }; test_interpreter_and_jit_asm!( @@ -3885,7 +3885,7 @@ fn test_mod() { mov32 r1, 13 mod32 r0, r1 exit", - config, + config.clone(), [], (), TestContextObject::new(5), @@ -3896,7 +3896,7 @@ fn test_mod() { lddw r0, 0x100000003 mod32 r0, 3 exit", - config, + config.clone(), [], (), TestContextObject::new(3), @@ -3913,7 +3913,7 @@ fn test_mod() { mod r0, r1 mod r0, 0x658f1778 exit", - config, + config.clone(), [], (), TestContextObject::new(9), @@ -3925,7 +3925,7 @@ fn test_mod() { mov32 r1, 0 mod r0, r1 exit", - config, + config.clone(), [], (), TestContextObject::new(3), diff --git a/tests/exercise_instructions.rs b/tests/exercise_instructions.rs index 889bf0ba..e2fc04b8 100644 --- a/tests/exercise_instructions.rs +++ b/tests/exercise_instructions.rs @@ -17,7 +17,7 @@ use solana_rbpf::{ assembler::assemble, ebpf, memory_region::MemoryRegion, - program::{BuiltinFunction, BuiltinProgram, FunctionRegistry}, + program::{BuiltinFunction, BuiltinProgram, FunctionRegistry, SBPFVersion}, static_analysis::Analysis, verifier::RequisiteVerifier, vm::{Config, ContextObject, TestContextObject}, @@ -540,7 +540,7 @@ fn test_ins(v1: bool, ins: String, prng: &mut SmallRng, cu: u64) { let mut config = Config::default(); if v1 { - config.enable_sbpf_v2 = false; + config.enabled_sbpf_versions = SBPFVersion::V1..=SBPFVersion::V1; } test_interpreter_and_jit_asm!(asm.as_str(), config, input, (), TestContextObject::new(cu)); } diff --git a/tests/verifier.rs b/tests/verifier.rs index 9146f798..b7fb9c23 100644 --- a/tests/verifier.rs +++ b/tests/verifier.rs @@ -153,14 +153,14 @@ fn test_verifier_err_incomplete_lddw() { fn test_verifier_err_invalid_reg_dst() { // r11 is disabled when sbpf_version.dynamic_stack_frames()=false, and only sub and add are // allowed when sbpf_version.dynamic_stack_frames()=true - for enable_sbpf_v2 in [false, true] { + for highest_sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { let executable = assemble::( " mov r11, 1 exit", Arc::new(BuiltinProgram::new_loader( Config { - enable_sbpf_v2, + enabled_sbpf_versions: SBPFVersion::V1..=highest_sbpf_version, ..Config::default() }, FunctionRegistry::default(), @@ -176,14 +176,14 @@ fn test_verifier_err_invalid_reg_dst() { fn test_verifier_err_invalid_reg_src() { // r11 is disabled when sbpf_version.dynamic_stack_frames()=false, and only sub and add are // allowed when sbpf_version.dynamic_stack_frames()=true - for enable_sbpf_v2 in [false, true] { + for highest_sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { let executable = assemble::( " mov r0, r11 exit", Arc::new(BuiltinProgram::new_loader( Config { - enable_sbpf_v2, + enabled_sbpf_versions: SBPFVersion::V1..=highest_sbpf_version, ..Config::default() }, FunctionRegistry::default(), @@ -360,13 +360,13 @@ fn test_sdiv_disabled() { ]; for (opc, instruction) in instructions { - for enable_sbpf_v2 in [true, false] { + for highest_sbpf_version in [SBPFVersion::V1, SBPFVersion::V2] { let assembly = format!("\n{instruction}\nexit"); let executable = assemble::( &assembly, Arc::new(BuiltinProgram::new_loader( Config { - enable_sbpf_v2, + enabled_sbpf_versions: SBPFVersion::V1..=highest_sbpf_version.clone(), ..Config::default() }, FunctionRegistry::default(), @@ -374,7 +374,7 @@ fn test_sdiv_disabled() { ) .unwrap(); let result = executable.verify::(); - if enable_sbpf_v2 { + if highest_sbpf_version == SBPFVersion::V2 { assert!(result.is_ok()); } else { assert_error!(result, "VerifierError(UnknownOpCode({}, {}))", opc, 0);