Replaces "sub r11, imm" with "add r11, -imm". (#488)

solana-labs · Jul 25, 2023 · 48f271f · 48f271f
1 parent 4c68373
commit 48f271f
Show file tree

Hide file tree

Showing 7 changed files with 14 additions and 33 deletions.
diff --git a/benches/vm_execution.rs b/benches/vm_execution.rs
@@ -259,7 +259,7 @@ fn bench_jit_vs_interpreter_call_depth_dynamic(bencher: &mut Bencher) {
     jlt r6, 1024, -4
     exit
     function_foo:
-    sub r11, 4
+    add r11, -4
     stw [r10-4], 0x11223344
     mov r6, r1
     jeq r6, 0, +3

diff --git a/src/interpreter.rs b/src/interpreter.rs
@@ -187,21 +187,14 @@ impl<'a, 'b, V: Verifier, C: ContextObject> Interpreter<'a, 'b, V, C> {
         }
 
         match insn.opc {
-            _ if dst == STACK_PTR_REG && self.executable.get_sbpf_version().dynamic_stack_frames() => {
+            ebpf::ADD64_IMM if dst == STACK_PTR_REG && self.executable.get_sbpf_version().dynamic_stack_frames() => {
                 // Let the stack overflow. For legitimate programs, this is a nearly
                 // impossible condition to hit since programs are metered and we already
                 // enforce a maximum call depth. For programs that intentionally mess
                 // around with the stack pointer, MemoryRegion::map will return
                 // InvalidVirtualAddress(stack_ptr) once an invalid stack address is
                 // accessed.
-                match insn.opc {
-                    ebpf::SUB64_IMM => { self.vm.stack_pointer = self.vm.stack_pointer.overflowing_add(-insn.imm as u64).0; }
-                    ebpf::ADD64_IMM => { self.vm.stack_pointer = self.vm.stack_pointer.overflowing_add(insn.imm as u64).0; }
-                    _ => {
-                        #[cfg(debug_assertions)]
-                        unreachable!("unexpected insn on r11")
-                    }
-                }
+                self.vm.stack_pointer = self.vm.stack_pointer.overflowing_add(insn.imm as u64).0;
             }
 
             ebpf::LD_DW_IMM  => {

diff --git a/src/jit.rs b/src/jit.rs
@@ -392,16 +392,9 @@ impl<'a, V: Verifier, C: ContextObject> JitCompiler<'a, V, C> {
             let target_pc = (self.pc as isize + insn.off as isize + 1) as usize;
 
             match insn.opc {
-                _ if insn.dst == STACK_PTR_REG as u8 && self.executable.get_sbpf_version().dynamic_stack_frames() => {
+                ebpf::ADD64_IMM if insn.dst == STACK_PTR_REG as u8 && self.executable.get_sbpf_version().dynamic_stack_frames() => {
                     let stack_ptr_access = X86IndirectAccess::Offset(self.slot_on_environment_stack(RuntimeEnvironmentSlot::StackPointer));
-                    match insn.opc {
-                        ebpf::SUB64_IMM => self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x81, 5, RBP, insn.imm, Some(stack_ptr_access))),
-                        ebpf::ADD64_IMM => self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x81, 0, RBP, insn.imm, Some(stack_ptr_access))),
-                        _ => {
-                            #[cfg(debug_assertions)]
-                            unreachable!("unexpected insn on r11")
-                        }
-                    }
+                    self.emit_ins(X86Instruction::alu(OperandSize::S64, 0x81, 0, RBP, insn.imm, Some(stack_ptr_access)));
                 }
 
                 ebpf::LD_DW_IMM  => {

diff --git a/src/verifier.rs b/src/verifier.rs
@@ -182,12 +182,7 @@ fn check_registers(
 
     match (insn.dst, store) {
         (0..=9, _) | (10, true) => Ok(()),
-        (11, _)
-            if sbpf_version.dynamic_stack_frames()
-                && (insn.opc == ebpf::SUB64_IMM || insn.opc == ebpf::ADD64_IMM) =>
-        {
-            Ok(())
-        }
+        (11, _) if sbpf_version.dynamic_stack_frames() && insn.opc == ebpf::ADD64_IMM => Ok(()),
         (10, false) => Err(VerifierError::CannotWriteR10(adj_insn_ptr(insn_ptr))),
         (_, _) => Err(VerifierError::InvalidDestinationRegister(adj_insn_ptr(
             insn_ptr,

diff --git a/tests/elfs/relative_call.so b/tests/elfs/relative_call.so
diff --git a/tests/execution.rs b/tests/execution.rs
@@ -2383,11 +2383,11 @@ fn test_err_dynamic_stack_ptr_overflow() {
     // stack_ptr -= stack_ptr + 1
     test_interpreter_and_jit_asm!(
         "
-        sub r11, 0x7FFFFFFF
-        sub r11, 0x7FFFFFFF
-        sub r11, 0x7FFFFFFF
-        sub r11, 0x7FFFFFFF
-        sub r11, 0x14005
+        add r11, -0x7FFFFFFF
+        add r11, -0x7FFFFFFF
+        add r11, -0x7FFFFFFF
+        add r11, -0x7FFFFFFF
+        add r11, -0x14005
         call function_foo
         exit
         function_foo:
@@ -2435,7 +2435,7 @@ fn test_dynamic_frame_ptr() {
     // to the top of the stack
     test_interpreter_and_jit_asm!(
         "
-        sub r11, 8
+        add r11, -8
         call function_foo
         exit
         function_foo:
@@ -2452,7 +2452,7 @@ fn test_dynamic_frame_ptr() {
     // is restored
     test_interpreter_and_jit_asm!(
         "
-        sub r11, 8
+        add r11, -8
         call function_foo
         mov r0, r10
         exit

diff --git a/tests/verifier.rs b/tests/verifier.rs
@@ -201,7 +201,7 @@ fn test_verifier_err_invalid_reg_src() {
 fn test_verifier_resize_stack_ptr_success() {
     let executable = assemble::<TestContextObject>(
         "
-        sub r11, 1
+        add r11, -1
         add r11, 1
         exit",
         Arc::new(BuiltinProgram::new_loader(