Skip to content

Commit

Permalink
Fix JIT instruction meter in syscall & unresolved symbol exceptions (#…
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@Lichtso
Lichtso authored Jul 22, 2021
1 parent 26d0151 commit d87fac7
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 28 deletions.
26 changes: 14 additions & 12 deletions src/jit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)?;
Expand All @@ -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 {
Expand All @@ -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 => {
Expand Down Expand Up @@ -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)
}
Expand Down
118 changes: 102 additions & 16 deletions tests/ubpf_execution.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -2701,6 +2702,84 @@ fn test_syscall_with_context() {
);
}

pub struct NestedVmSyscall {}
impl SyscallObject<UserError> 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::<UserError, _>(b"NestedVmSyscall", NestedVmSyscall::call)
.unwrap();
let mem = &mut [depth as u8 - 1, throw as u8];
let mut executable = assemble::<UserError, TestInstructionMeter>(
"
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::<UserError>(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]
Expand Down Expand Up @@ -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
Expand All @@ -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!(
Expand All @@ -3065,6 +3150,7 @@ fn test_err_call_unresolved() {
mov r4, 4
mov r5, 5
syscall Unresolved
mov64 r0, 0x0
exit",
[],
(),
Expand Down

0 comments on commit d87fac7

Please sign in to comment.