Skip to content
This repository has been archived by the owner on Jan 10, 2025. It is now read-only.

Commit

Permalink
Fixes inconsistencies of the pc field in the error reporting of "call…
Browse files Browse the repository at this point in the history
…" and "callx" instructions. (#133)

Also adds two tests for it.
  • Loading branch information
Lichtso authored Jan 7, 2021
1 parent 1c2a34d commit 180afeb
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 18 deletions.
4 changes: 2 additions & 2 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl AnalysisResult {
for insn in result.instructions.iter() {
match insn.opc {
ebpf::CALL_IMM => {
if let Some(target_pc) = executable.lookup_bpf_call(insn.imm as u32) {
if let Some(target_pc) = executable.lookup_bpf_function(insn.imm as u32) {
// result.sources.insert(insn.ptr, vec![*target_pc]);
if !result.destinations.contains_key(target_pc) {
result.destinations.insert(*target_pc, Label {
Expand Down Expand Up @@ -199,7 +199,7 @@ impl AnalysisResult {
ebpf::CALL_IMM => {
insn.desc = if let Some(syscall_name) = syscalls.get(&(insn.imm as u32)) {
format!("syscall {}", syscall_name)
} else if let Some(target_pc) = executable.lookup_bpf_call(insn.imm as u32)
} else if let Some(target_pc) = executable.lookup_bpf_function(insn.imm as u32)
{
format!("call {}", resolve_label!(result.destinations, target_pc))
} else {
Expand Down
7 changes: 6 additions & 1 deletion src/elf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,13 @@ impl<E: UserDefinedError, I: InstructionMeter> Executable<E, I> for EBpfElf<E, I
Ok(self.entrypoint)
}

/// Set a symbol's instruction offset
fn define_bpf_function(&mut self, hash: u32, pc: usize) {
self.calls.insert(hash, pc);
}

/// Get a symbol's instruction offset
fn lookup_bpf_call(&self, hash: u32) -> Option<&usize> {
fn lookup_bpf_function(&self, hash: u32) -> Option<&usize> {
self.calls.get(&hash)
}

Expand Down
8 changes: 4 additions & 4 deletions src/jit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ fn emit_bpf_call(jit: &mut JitCompiler, dst: Value, number_of_instructions: usiz

match dst {
Value::Register(_reg) => {
emit_validate_and_profile_instruction_count(jit, true, None);
emit_validate_and_profile_instruction_count(jit, false, None);

emit_mov(jit, OperationWidth::Bit64, REGISTER_MAP[0], R11);
emit_pop(jit, REGISTER_MAP[0]);
Expand All @@ -613,7 +613,7 @@ fn emit_bpf_call(jit: &mut JitCompiler, dst: Value, number_of_instructions: usiz
emit1(jit, 0xd3);
},
Value::Constant64(target_pc) => {
emit_validate_and_profile_instruction_count(jit, true, Some(target_pc as usize));
emit_validate_and_profile_instruction_count(jit, false, Some(target_pc as usize));
emit_call(jit, target_pc as usize);
},
_ => panic!()
Expand Down Expand Up @@ -1204,15 +1204,15 @@ impl<'a> JitCompiler<'a> {
emit_undo_profile_instruction_count(self, 0);
}
} else {
match executable.lookup_bpf_call(insn.imm as u32) {
match executable.lookup_bpf_function(insn.imm as u32) {
Some(target_pc) => {
emit_bpf_call(self, Value::Constant64(*target_pc as i64), self.pc_section.len() - 1);
},
None => {
// executable.report_unresolved_symbol(self.pc)?;
// Workaround for unresolved symbols in ELF: Report error at runtime instead of compiletime
let fat_ptr: DynTraitFatPointer = unsafe { std::mem::transmute(executable) };
emit_rust_call(self, fat_ptr.vtable.methods[9], &[
emit_rust_call(self, fat_ptr.vtable.methods[10], &[
Argument { index: 0, value: Value::RegisterIndirect(RBP, -8 * (CALLEE_SAVED_REGISTERS.len() + 1) as i32) }, // Pointer to optional typed return value
Argument { index: 1, value: Value::Constant64(fat_ptr.data as i64) },
Argument { index: 2, value: Value::Constant64(self.pc as i64) },
Expand Down
6 changes: 4 additions & 2 deletions src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,10 @@ pub trait Executable<E: UserDefinedError, I: InstructionMeter>: Send + Sync {
fn get_ro_sections(&self) -> Result<Vec<(u64, &[u8])>, EbpfError<E>>;
/// Get the entry point offset into the text section
fn get_entrypoint_instruction_offset(&self) -> Result<usize, EbpfError<E>>;
/// Set a symbol's instruction offset
fn define_bpf_function(&mut self, hash: u32, pc: usize);
/// Get a symbol's instruction offset
fn lookup_bpf_call(&self, hash: u32) -> Option<&usize>;
fn lookup_bpf_function(&self, hash: u32) -> Option<&usize>;
/// Get the syscall registry
fn get_syscall_registry(&self) -> &SyscallRegistry;
/// Set (overwrite) the syscall registry
Expand Down Expand Up @@ -886,7 +888,7 @@ impl<'a, E: UserDefinedError, I: InstructionMeter> EbpfVm<'a, E, I> {
if instruction_meter_enabled {
remaining_insn_count = instruction_meter.get_remaining();
}
} else if let Some(target_pc) = self.executable.lookup_bpf_call(insn.imm as u32) {
} else if let Some(target_pc) = self.executable.lookup_bpf_function(insn.imm as u32) {
// make BPF to BPF call
reg[ebpf::STACK_REG] = self.frames.push(
&reg[ebpf::FIRST_SCRATCH_REG
Expand Down
80 changes: 71 additions & 9 deletions tests/ubpf_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,8 @@ macro_rules! test_interpreter_and_jit {
let mut vm = EbpfVm::new($executable.as_ref(), &mut mem, &[]).unwrap();
test_interpreter_and_jit!(2, vm, $($location => $syscall_function; $syscall_context_object),*);
let result = vm.execute_program_interpreted(&mut TestInstructionMeter { remaining: $expected_instruction_count });
let tracer_interpreter = vm.get_tracer().clone();
let mut tracer_display = String::new();
tracer_interpreter.write(&mut tracer_display, vm.get_program()).unwrap();
println!("{}", tracer_display);
assert!(check_closure(&vm, result));
(vm.get_total_instruction_count(), tracer_interpreter)
(vm.get_total_instruction_count(), vm.get_tracer().clone())
};
#[cfg(not(windows))]
{
Expand All @@ -65,7 +61,15 @@ macro_rules! test_interpreter_and_jit {
test_interpreter_and_jit!(2, vm, $($location => $syscall_function; $syscall_context_object),*);
let result = vm.execute_program_jit(&mut TestInstructionMeter { remaining: $expected_instruction_count });
let tracer_jit = vm.get_tracer();
assert!(solana_rbpf::vm::Tracer::compare(&_tracer_interpreter, tracer_jit));
if !solana_rbpf::vm::Tracer::compare(&_tracer_interpreter, tracer_jit) {
let mut tracer_display = String::new();
_tracer_interpreter.write(&mut tracer_display, vm.get_program()).unwrap();
println!("{}", tracer_display);
let mut tracer_display = String::new();
tracer_jit.write(&mut tracer_display, vm.get_program()).unwrap();
println!("{}", tracer_display);
panic!();
}
if $executable.get_config().enable_instruction_meter {
let instruction_count_jit = vm.get_total_instruction_count();
assert_eq!(instruction_count_interpreter, instruction_count_jit);
Expand Down Expand Up @@ -2463,9 +2467,7 @@ fn test_err_reg_stack_depth() {
callx 0x0
exit",
[],
(
hash_symbol_name(b"log") => BpfSyscallString::call; BpfSyscallString {},
),
(),
{
|_vm, res: Result| {
matches!(res.unwrap_err(),
Expand Down Expand Up @@ -2739,6 +2741,66 @@ fn test_tight_infinite_loop_unconditional() {
);
}

#[test]
fn test_tight_infinite_recursion() {
let program = assemble(
"
mov64 r3, 0x41414141
call 0x53075d44
exit",
)
.unwrap();
let config = Config {
enable_instruction_tracing: true,
..Config::default()
};
let mut executable =
Executable::<UserError, TestInstructionMeter>::from_text_bytes(&program, None, config)
.unwrap();
executable.define_bpf_function(0x53075d44, 0x0);
#[allow(unused_mut)]
{
test_interpreter_and_jit!(
executable,
[],
(),
{
|_vm, res: Result| {
matches!(res.unwrap_err(),
EbpfError::ExceededMaxInstructions(pc, initial_insn_count)
if pc == 31 && initial_insn_count == 4
)
}
},
4
);
}
}

#[test]
fn test_tight_infinite_recursion_callx() {
test_interpreter_and_jit_asm!(
"
mov64 r8, 0x1
lsh64 r8, 0x20
or64 r8, 0x18
mov64 r3, 0x41414141
callx 8
exit",
[],
(),
{
|_vm, res: Result| {
matches!(res.unwrap_err(),
EbpfError::ExceededMaxInstructions(pc, initial_insn_count)
if pc == 34 && initial_insn_count == 7
)
}
},
7
);
}

#[test]
fn test_instruction_count_syscall() {
test_interpreter_and_jit_asm!(
Expand Down

0 comments on commit 180afeb

Please sign in to comment.