From d87fac76e0538dd7aebbd608eee2c5cfc297c85b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Thu, 22 Jul 2021 21:50:49 +0200 Subject: [PATCH] Fix JIT instruction meter in syscall & unresolved symbol exceptions (#203) * Adds a test which invokes another VM in a syscall. * Fixes two instruction meter bugs in JIT. One in syscalls which throw an exception and one in the unresolved symbol exception. --- src/jit.rs | 26 +++++---- tests/ubpf_execution.rs | 118 ++++++++++++++++++++++++++++++++++------ 2 files changed, 116 insertions(+), 28 deletions(-) diff --git a/src/jit.rs b/src/jit.rs index d3e47ff3..26d284a4 100644 --- a/src/jit.rs +++ b/src/jit.rs @@ -170,7 +170,7 @@ const TARGET_PC_CALLX_UNSUPPORTED_INSTRUCTION: usize = std::usize::MAX - 7; const TARGET_PC_CALL_UNSUPPORTED_INSTRUCTION: usize = std::usize::MAX - 6; const TARGET_PC_DIV_BY_ZERO: usize = std::usize::MAX - 5; const TARGET_PC_EXCEPTION_AT: usize = std::usize::MAX - 4; -const TARGET_PC_SYSCALL_EXCEPTION: usize = std::usize::MAX - 3; +const TARGET_PC_RUST_EXCEPTION: usize = std::usize::MAX - 3; const TARGET_PC_EXIT: usize = std::usize::MAX - 2; const TARGET_PC_EPILOGUE: usize = std::usize::MAX - 1; @@ -1276,15 +1276,7 @@ impl JitCompiler { Argument { index: 2, value: Value::Register(ARGUMENT_REGISTERS[2]) }, Argument { index: 1, value: Value::Register(ARGUMENT_REGISTERS[1]) }, Argument { index: 0, value: Value::Register(RAX) }, // "&mut self" in the "call" method of the SyscallObject - ], None, true)?; - - // Throw error if the result indicates one - X86Instruction::load_immediate(OperandSize::S64, R11, self.pc as i64).emit(self)?; - emit_jcc(self, 0x85, TARGET_PC_SYSCALL_EXCEPTION)?; - - // Store Ok value in result register - X86Instruction::load(OperandSize::S64, RBP, R11, X86IndirectAccess::Offset(slot_on_environment_stack(self, EnvironmentStackSlot::OptRetValPtr))).emit(self)?; - X86Instruction::load(OperandSize::S64, R11, REGISTER_MAP[0], X86IndirectAccess::Offset(8)).emit(self)?; + ], None, false)?; if self.config.enable_instruction_meter { X86Instruction::load(OperandSize::S64, RBP, R11, X86IndirectAccess::Offset(slot_on_environment_stack(self, EnvironmentStackSlot::InsnMeterPtr))).emit(self)?; @@ -1294,6 +1286,15 @@ impl JitCompiler { X86Instruction::store(OperandSize::S64, ARGUMENT_REGISTERS[0], RBP, X86IndirectAccess::Offset(slot_on_environment_stack(self, EnvironmentStackSlot::PrevInsnMeter))).emit(self)?; emit_undo_profile_instruction_count(self, 0)?; } + + // Store Ok value in result register + X86Instruction::load(OperandSize::S64, RBP, R11, X86IndirectAccess::Offset(slot_on_environment_stack(self, EnvironmentStackSlot::OptRetValPtr))).emit(self)?; + X86Instruction::load(OperandSize::S64, R11, REGISTER_MAP[0], X86IndirectAccess::Offset(8)).emit(self)?; + + // Throw error if the result indicates one + X86Instruction::cmp_immediate(OperandSize::S64, R11, 0, Some(X86IndirectAccess::Offset(0))).emit(self)?; + X86Instruction::load_immediate(OperandSize::S64, R11, self.pc as i64).emit(self)?; + emit_jcc(self, 0x85, TARGET_PC_RUST_EXCEPTION)?; } else if let Some(target_pc) = executable.lookup_bpf_function(insn.imm as u32) { emit_bpf_call(self, Value::Constant64(target_pc as i64, false), self.result.pc_section.len() - 1)?; } else { @@ -1306,7 +1307,8 @@ impl JitCompiler { Argument { index: 0, value: Value::RegisterIndirect(RBP, slot_on_environment_stack(self, EnvironmentStackSlot::OptRetValPtr), false) }, ], None, true)?; X86Instruction::load_immediate(OperandSize::S64, R11, self.pc as i64).emit(self)?; - emit_jmp(self, TARGET_PC_SYSCALL_EXCEPTION)?; + emit_validate_instruction_count(self, false, None)?; + emit_jmp(self, TARGET_PC_RUST_EXCEPTION)?; } }, ebpf::CALL_REG => { @@ -1494,7 +1496,7 @@ impl JitCompiler { emit_jmp(self, TARGET_PC_EPILOGUE)?; // Handler for syscall exceptions - set_anchor(self, TARGET_PC_SYSCALL_EXCEPTION); + set_anchor(self, TARGET_PC_RUST_EXCEPTION); emit_profile_instruction_count_of_exception(self, false)?; emit_jmp(self, TARGET_PC_EPILOGUE) } diff --git a/tests/ubpf_execution.rs b/tests/ubpf_execution.rs index 072805dc..d958e398 100644 --- a/tests/ubpf_execution.rs +++ b/tests/ubpf_execution.rs @@ -17,6 +17,7 @@ use solana_rbpf::{ elf::ElfError, error::EbpfError, memory_region::AccessType, + memory_region::MemoryMapping, syscalls::{self, Result}, user_error::UserError, vm::{Config, EbpfVm, Executable, SyscallObject, SyscallRegistry, TestInstructionMeter}, @@ -2701,6 +2702,84 @@ fn test_syscall_with_context() { ); } +pub struct NestedVmSyscall {} +impl SyscallObject for NestedVmSyscall { + fn call( + &mut self, + depth: u64, + throw: u64, + _arg3: u64, + _arg4: u64, + _arg5: u64, + _memory_mapping: &MemoryMapping, + result: &mut Result, + ) { + #[allow(unused_mut)] + if depth > 0 { + let mut syscall_registry = SyscallRegistry::default(); + syscall_registry + .register_syscall_by_name::(b"NestedVmSyscall", NestedVmSyscall::call) + .unwrap(); + let mem = &mut [depth as u8 - 1, throw as u8]; + let mut executable = assemble::( + " + ldabsb 0 + mov64 r1, r0 + ldabsb 1 + mov64 r2, r0 + syscall NestedVmSyscall + exit", + None, + Config::default(), + syscall_registry, + ) + .unwrap(); + #[cfg(not(windows))] + { + executable.jit_compile().unwrap(); + } + let mut vm = EbpfVm::new(executable.as_ref(), mem, &[]).unwrap(); + vm.bind_syscall_context_object(Box::new(NestedVmSyscall {}), None) + .unwrap(); + let mut instruction_meter = TestInstructionMeter { remaining: 6 }; + #[cfg(windows)] + { + *result = vm.execute_program_interpreted(&mut instruction_meter); + } + #[cfg(not(windows))] + { + *result = vm.execute_program_jit(&mut instruction_meter); + } + assert_eq!( + vm.get_total_instruction_count(), + if throw == 0 { 6 } else { 5 } + ); + } else { + *result = if throw == 0 { + Ok(42) + } else { + Err(EbpfError::CallDepthExceeded(33, 0)) + }; + } + } +} + +#[test] +fn test_nested_vm_syscall() { + let config = Config::default(); + let mut nested_vm_syscall = NestedVmSyscall {}; + let memory_mapping = MemoryMapping::new::(vec![], &config).unwrap(); + let mut result = Ok(0); + nested_vm_syscall.call(1, 0, 0, 0, 0, &memory_mapping, &mut result); + assert!(result.unwrap() == 42); + let mut result = Ok(0); + nested_vm_syscall.call(1, 1, 0, 0, 0, &memory_mapping, &mut result); + assert!(matches!(result.unwrap_err(), + EbpfError::CallDepthExceeded(pc, depth) + if pc == 33 && depth == 0 + )); +} + // Elf #[test] @@ -3016,6 +3095,28 @@ fn test_err_capped_before_exception() { }, 2 ); + + test_interpreter_and_jit_asm!( + " + mov64 r1, 0x0 + mov64 r2, 0x0 + add64 r0, 0x0 + add64 r0, 0x0 + syscall Unresolved + add64 r0, 0x0 + exit", + [], + (), + { + |_vm, res: Result| { + matches!(res.unwrap_err(), + EbpfError::ExceededMaxInstructions(pc, initial_insn_count) + if pc == 33 && initial_insn_count == 4 + ) + } + }, + 4 + ); } // Symbols and Relocation @@ -3039,22 +3140,6 @@ fn test_symbol_relocation() { ); } -#[test] -fn test_err_symbol_unresolved() { - test_interpreter_and_jit_asm!( - " - syscall Unresolved - mov64 r0, 0x0 - exit", - [], - (), - { - |_vm, res: Result| matches!(res.unwrap_err(), EbpfError::ElfError(ElfError::UnresolvedSymbol(symbol, pc, offset)) if symbol == "Unknown" && pc == 29 && offset == 0) - }, - 1 - ); -} - #[test] fn test_err_call_unresolved() { test_interpreter_and_jit_asm!( @@ -3065,6 +3150,7 @@ fn test_err_call_unresolved() { mov r4, 4 mov r5, 5 syscall Unresolved + mov64 r0, 0x0 exit", [], (),