Revert "Moves Config::optimize_rodata into SBPFVersion. (solana-labs#477

)" (solana-labs#480)

0b8680b

fuzz/fuzz_targets/common.rs

@@ -14,6 +14,7 @@ pub struct ConfigTemplate {
     sanitize_user_provided_values: bool,
     encrypt_environment_registers: bool,
     reject_callx_r10: bool,
+    optimize_rodata: bool,
 }
 
 impl<'a> Arbitrary<'a> for ConfigTemplate {
@@ -28,6 +29,7 @@ impl<'a> Arbitrary<'a> for ConfigTemplate {
             sanitize_user_provided_values: bools & (1 << 3) != 0,
             encrypt_environment_registers: bools & (1 << 4) != 0,
             reject_callx_r10: bools & (1 << 6) != 0,
+            optimize_rodata: bools & (1 << 9) != 0,
         })
     }
 
@@ -51,6 +53,7 @@ impl From<ConfigTemplate> for Config {
                 sanitize_user_provided_values,
                 encrypt_environment_registers,
                 reject_callx_r10,
+                optimize_rodata,
             } => Config {
                 max_call_depth,
                 enable_stack_frame_gaps,
@@ -65,6 +68,7 @@ impl From<ConfigTemplate> for Config {
                     0
                 },
                 reject_callx_r10,
+                optimize_rodata,
                 ..Default::default()
             },
         }

src/elf.rs

@@ -290,11 +290,6 @@ impl SBPFVersion {
         self != &SBPFVersion::V1
     }
 
-    /// Avoid copying read only sections when possible
-    pub fn optimize_rodata(&self) -> bool {
-        self != &SBPFVersion::V1
-    }
-
     /// Use dynamic stack frame sizes
     pub fn dynamic_stack_frames(&self) -> bool {
         self != &SBPFVersion::V1
@@ -723,7 +718,7 @@ impl<V: Verifier, C: ContextObject> Executable<V, C> {
         };
 
         if sbpf_version.enable_elf_vaddr() {
-            if !sbpf_version.optimize_rodata() {
+            if !config.optimize_rodata {
                 // When optimize_rodata=false, we allocate a vector and copy all
                 // rodata sections into it. In that case we can't allow virtual
                 // addresses or we'd potentially have to do huge allocations.
@@ -858,7 +853,7 @@ impl<V: Verifier, C: ContextObject> Executable<V, C> {
             if !invalid_offsets {
                 if sbpf_version.enable_elf_vaddr() {
                     // This is enforced in validate()
-                    debug_assert!(sbpf_version.optimize_rodata());
+                    debug_assert!(config.optimize_rodata);
                     if section_addr < section_header.sh_offset() {
                         invalid_offsets = true;
                     } else {
@@ -910,7 +905,7 @@ impl<V: Verifier, C: ContextObject> Executable<V, C> {
                 .saturating_add(1)
                 .saturating_sub(first_ro_section)
                 == n_ro_sections;
-        let ro_section = if sbpf_version.optimize_rodata() && can_borrow {
+        let ro_section = if config.optimize_rodata && can_borrow {
             // Read only sections are grouped together with no intermixed non-ro
             // sections. We can borrow.
 
@@ -937,7 +932,7 @@ impl<V: Verifier, C: ContextObject> Executable<V, C> {
             // Read only and other non-ro sections are mixed. Zero the non-ro
             // sections and and copy the ro ones at their intended offsets.
 
-            if sbpf_version.optimize_rodata() {
+            if config.optimize_rodata {
                 // The rodata region starts at MM_PROGRAM_START + offset,
                 // [MM_PROGRAM_START, MM_PROGRAM_START + offset) is not
                 // mappable. We only need to allocate highest_addr - lowest_addr
@@ -1967,7 +1962,10 @@ mod test {
 
     #[test]
     fn test_owned_ro_region_initial_gap_mappable() {
-        let config = Config::default();
+        let config = Config {
+            optimize_rodata: false,
+            ..Config::default()
+        };
         let elf_bytes = [0u8; 512];
 
         // the first section starts at a non-zero offset
@@ -1991,7 +1989,7 @@ mod test {
         };
 
         // [s1.sh_addr..s3.sh_addr + s3.sh_size] is where the readonly data is.
-        // But for backwards compatibility (sbpf_version.optimize_rodata()=false)
+        // But for backwards compatibility (config.optimize_rodata=false)
         // [0..s1.sh_addr] is mappable too (and zeroed).
         assert!(matches!(
             ro_region.vm_to_host(ebpf::MM_PROGRAM_START, s3.sh_addr + s3.sh_size),
@@ -2065,7 +2063,10 @@ mod test {
 
     #[test]
     fn test_borrowed_ro_sections_disabled() {
-        let config = Config::default();
+        let config = Config {
+            optimize_rodata: false,
+            ..Config::default()
+        };
         let elf_bytes = [0u8; 512];
 
         // s1 and s2 are contiguous, the rodata section can be borrowed from the
@@ -2078,7 +2079,7 @@ mod test {
         assert!(matches!(
             ElfExecutable::parse_ro_sections(
                 &config,
-                &SBPFVersion::V1,
+                &SBPFVersion::V1, // v2 requires optimize_rodata=true
                 sections,
                 &elf_bytes,
             ),

src/memory_region.rs

@@ -122,7 +122,7 @@ impl MemoryRegion {
     /// Convert a virtual machine address into a host address
     pub fn vm_to_host(&self, vm_addr: u64, len: u64) -> ProgramResult {
         // This can happen if a region starts at an offset from the base region
-        // address, eg with rodata regions if sbpf_version.optimize_rodata()=true, see
+        // address, eg with rodata regions if config.optimize_rodata = true, see
         // Elf::get_ro_region.
         if vm_addr < self.vm_addr {
             return ProgramResult::Err(Box::new(EbpfError::InvalidVirtualAddress(vm_addr)));

src/vm.rs

@@ -228,6 +228,8 @@ pub struct Config {
     pub external_internal_function_hash_collision: bool,
     /// Have the verifier reject "callx r10"
     pub reject_callx_r10: bool,
+    /// Avoid copying read only sections when possible
+    pub optimize_rodata: bool,
     /// Use the new ELF parser
     pub new_elf_parser: bool,
     /// Use aligned memory mapping
@@ -263,6 +265,7 @@ impl Default for Config {
                 >> PROGRAM_ENVIRONMENT_KEY_SHIFT,
             external_internal_function_hash_collision: true,
             reject_callx_r10: true,
+            optimize_rodata: true,
             new_elf_parser: true,
             aligned_memory_mapping: true,
             enable_sbpf_v1: true,

tests/execution.rs

@@ -3098,7 +3098,10 @@ fn test_load_elf_empty_rodata() {
 
 #[test]
 fn test_load_elf_rodata() {
-    let config = Config::default();
+    let config = Config {
+        optimize_rodata: true,
+        ..Config::default()
+    };
     test_interpreter_and_jit_elf!(
         "tests/elfs/rodata.so",
         config,
@@ -3111,7 +3114,10 @@ fn test_load_elf_rodata() {
 
 #[test]
 fn test_load_elf_rodata_sbpfv1() {
-    let config = Config::default();
+    let config = Config {
+        optimize_rodata: false,
+        ..Config::default()
+    };
     test_interpreter_and_jit_elf!(
         "tests/elfs/rodata_sbpfv1.so",
         config,