Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce return instruction in the (dis)assembler #609

Merged
merged 2 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,15 @@ fn make_instruction_map(sbpf_version: SBPFVersion) -> HashMap<String, (Instructi
result.insert(name.to_string(), (inst_type, opc))
};

if sbpf_version == SBPFVersion::V1 {
entry("exit", NoOperand, ebpf::EXIT);
entry("return", NoOperand, ebpf::EXIT);
} else {
entry("exit", NoOperand, ebpf::RETURN);
entry("return", NoOperand, ebpf::RETURN);
}

// Miscellaneous.
entry("exit", NoOperand, ebpf::EXIT);
entry("ja", JumpUnconditional, ebpf::JA);
entry("syscall", Syscall, ebpf::CALL_IMM);
entry("call", CallImm, ebpf::CALL_IMM);
Expand Down Expand Up @@ -296,7 +303,7 @@ fn resolve_label(
/// # 0xbf, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
/// # 0xdc, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00,
/// # 0x87, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
/// # 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]);
/// # 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]);
/// ```
///
/// This will produce the following output:
Expand Down
5 changes: 4 additions & 1 deletion src/disassembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,10 @@ pub fn disassemble_instruction<C: ContextObject>(
desc = format!("{name} {function_name}");
},
ebpf::CALL_REG => { name = "callx"; desc = format!("{} r{}", name, if sbpf_version.callx_uses_src_reg() { insn.src } else { insn.imm as u8 }); },
ebpf::EXIT => { name = "exit"; desc = name.to_string(); },
ebpf::EXIT if !sbpf_version.static_syscalls() => { name = "exit"; desc = name.to_string(); },
LucasSte marked this conversation as resolved.
Show resolved Hide resolved
ebpf::EXIT if sbpf_version.static_syscalls() => { name = "return"; desc = name.to_string(); },
ebpf::RETURN if !sbpf_version.static_syscalls() => { name = "exit"; desc = name.to_string(); },
ebpf::RETURN if sbpf_version.static_syscalls() => { name = "return"; desc = name.to_string(); },

_ => { name = "unknown"; desc = format!("{} opcode={:#x}", name, insn.opc); },
};
Expand Down
4 changes: 3 additions & 1 deletion src/ebpf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,10 @@ pub const JSLE_REG: u8 = BPF_JMP | BPF_X | BPF_JSLE;
pub const CALL_IMM: u8 = BPF_JMP | BPF_CALL;
/// BPF opcode: tail call.
pub const CALL_REG: u8 = BPF_JMP | BPF_X | BPF_CALL;
/// BPF opcode: `exit` /// `return r0`.
/// BPF opcode: `exit` /// `return r0`. /// Valid only for SBPFv1
pub const EXIT: u8 = BPF_JMP | BPF_EXIT;
/// BPF opcode: `return` /// `return r0`. /// Valid only for SBPFv2
pub const RETURN: u8 = BPF_JMP | BPF_X | BPF_EXIT;

// Used in JIT
/// Mask to extract the operation class from an operation code.
Expand Down
4 changes: 2 additions & 2 deletions src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,8 @@ impl<'a, 'b, C: ContextObject> Interpreter<'a, 'b, C> {
throw_error!(self, EbpfError::UnsupportedInstruction);
}
}

ebpf::EXIT => {
ebpf::RETURN
| ebpf::EXIT => {
if self.vm.call_depth == 0 {
if config.enable_instruction_meter && self.vm.due_insn_count > self.vm.previous_instruction_meter {
throw_error!(self, EbpfError::ExceededMaxInstructions);
Expand Down
3 changes: 2 additions & 1 deletion src/jit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,8 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
};
self.emit_internal_call(Value::Register(target_pc));
},
ebpf::EXIT => {
ebpf::RETURN
| ebpf::EXIT => {
self.emit_validate_instruction_count(true, Some(self.pc));

let call_depth_access = X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::CallDepth));
Expand Down
3 changes: 2 additions & 1 deletion src/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ impl Verifier for RequisiteVerifier {
function_range.end = *function_iter.peek().unwrap_or(&program_range.end);
let insn = ebpf::get_insn(prog, function_range.end.saturating_sub(1));
match insn.opc {
ebpf::JA | ebpf::EXIT => {},
ebpf::JA | ebpf::RETURN => {},
_ => return Err(VerifierError::InvalidFunction(
function_range.end.saturating_sub(1),
)),
Expand Down Expand Up @@ -391,6 +391,7 @@ impl Verifier for RequisiteVerifier {
ebpf::CALL_IMM => {},
ebpf::CALL_REG => { check_callx_register(&insn, insn_ptr, sbpf_version)?; },
ebpf::EXIT => {},
ebpf::RETURN => {},

_ => {
return Err(VerifierError::UnknownOpCode(insn.opc, insn_ptr));
Expand Down
2 changes: 1 addition & 1 deletion src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ pub struct CallFrame {
/// };
///
/// let prog = &[
/// 0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 // exit
/// 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 // exit
/// ];
/// let mem = &mut [
/// 0xaa, 0xbb, 0x11, 0x22, 0xcc, 0xdd
Expand Down
2 changes: 1 addition & 1 deletion test_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ pub const TCP_SACK_BIN: [u8; 352] = [
0x6d, 0x32, 0xee, 0xff, 0x00, 0x00, 0x00, 0x00, //
0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, //
0xb7, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, //
0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
];

pub const TCP_SACK_MATCH: [u8; 78] = [
Expand Down
52 changes: 48 additions & 4 deletions tests/assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,19 @@
extern crate solana_rbpf;
extern crate test_utils;

use solana_rbpf::program::{FunctionRegistry, SBPFVersion};
use solana_rbpf::vm::Config;
use solana_rbpf::{assembler::assemble, ebpf, program::BuiltinProgram, vm::TestContextObject};
use std::sync::Arc;
use test_utils::{TCP_SACK_ASM, TCP_SACK_BIN};

fn asm(src: &str) -> Result<Vec<ebpf::Insn>, String> {
let executable = assemble::<TestContextObject>(src, Arc::new(BuiltinProgram::new_mock()))?;
asm_with_config(src, Config::default())
}

fn asm_with_config(src: &str, config: Config) -> Result<Vec<ebpf::Insn>, String> {
let loader = BuiltinProgram::new_loader(config, FunctionRegistry::default());
let executable = assemble::<TestContextObject>(src, Arc::new(loader))?;
let (_program_vm_addr, program) = executable.get_text_bytes();
Ok((0..program.len() / ebpf::INSN_SIZE)
.map(|insn_ptr| ebpf::get_insn(program, insn_ptr))
Expand Down Expand Up @@ -50,7 +57,34 @@ fn test_fill() {
// Example for InstructionType::NoOperand.
#[test]
fn test_exit() {
assert_eq!(asm("exit"), Ok(vec![insn(0, ebpf::EXIT, 0, 0, 0, 0)]));
let config = Config {
enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1,
..Config::default()
};
assert_eq!(
asm_with_config("exit", config.clone()),
Ok(vec![insn(0, ebpf::EXIT, 0, 0, 0, 0)])
);
assert_eq!(
asm_with_config("return", config),
Ok(vec![insn(0, ebpf::EXIT, 0, 0, 0, 0)])
);
}

#[test]
fn test_return() {
let config = Config {
enabled_sbpf_versions: SBPFVersion::V2..=SBPFVersion::V2,
..Config::default()
};
assert_eq!(
asm_with_config("exit", config.clone()),
Ok(vec![insn(0, ebpf::RETURN, 0, 0, 0, 0)])
);
assert_eq!(
asm_with_config("return", config),
Ok(vec![insn(0, ebpf::RETURN, 0, 0, 0, 0)])
);
}

// Example for InstructionType::AluBinary.
Expand Down Expand Up @@ -471,8 +505,18 @@ fn test_large_immediate() {

#[test]
fn test_tcp_sack() {
let executable =
assemble::<TestContextObject>(TCP_SACK_ASM, Arc::new(BuiltinProgram::new_mock())).unwrap();
let config = Config {
enabled_sbpf_versions: SBPFVersion::V2..=SBPFVersion::V2,
..Config::default()
};
let executable = assemble::<TestContextObject>(
TCP_SACK_ASM,
Arc::new(BuiltinProgram::new_loader(
config,
FunctionRegistry::default(),
)),
)
.unwrap();
let (_program_vm_addr, program) = executable.get_text_bytes();
assert_eq!(program, TCP_SACK_BIN.to_vec());
}
Expand Down
43 changes: 29 additions & 14 deletions tests/disassembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// copied, modified, or distributed except according to those terms.

extern crate solana_rbpf;
use solana_rbpf::program::SBPFVersion;
use solana_rbpf::{
assembler::assemble,
program::{BuiltinProgram, FunctionRegistry},
Expand All @@ -18,14 +19,15 @@ use std::sync::Arc;
// Using a macro to keep actual line numbers in failure output
macro_rules! disasm {
($src:expr) => {{
let config = Config {
enable_symbol_and_section_labels: true,
..Config::default()
};
disasm!($src, config);
}};
($src:expr, $config:expr) => {{
let src = $src;
let loader = BuiltinProgram::new_loader(
Config {
enable_symbol_and_section_labels: true,
..Config::default()
},
FunctionRegistry::default(),
);
let loader = BuiltinProgram::new_loader($config, FunctionRegistry::default());
let executable = assemble::<TestContextObject>(src, Arc::new(loader)).unwrap();
let analysis = Analysis::from_executable(&executable).unwrap();
let mut reasm = Vec::new();
Expand All @@ -42,7 +44,20 @@ fn test_empty() {
// Example for InstructionType::NoOperand.
#[test]
fn test_exit() {
disasm!("entrypoint:\n exit\n");
let config = Config {
enabled_sbpf_versions: SBPFVersion::V1..=SBPFVersion::V1,
..Config::default()
};
disasm!("entrypoint:\n exit\n", config);
}

#[test]
fn test_return() {
let config = Config {
enabled_sbpf_versions: SBPFVersion::V2..=SBPFVersion::V2,
..Config::default()
};
disasm!("entrypoint:\n return\n", config);
}

// Example for InstructionType::AluBinary.
Expand Down Expand Up @@ -80,7 +95,7 @@ fn test_ja() {
"entrypoint:
ja lbb_1
lbb_1:
exit
return
"
);
}
Expand All @@ -92,14 +107,14 @@ fn test_jeq() {
"entrypoint:
jeq r1, 4, lbb_1
lbb_1:
exit
return
"
);
disasm!(
"entrypoint:
jeq r1, r3, lbb_1
lbb_1:
exit
return
"
);
}
Expand All @@ -112,7 +127,7 @@ fn test_call() {
call function_1

function_1:
exit
return
"
);
}
Expand Down Expand Up @@ -301,7 +316,7 @@ fn test_jump_conditional() {
jslt r1, r2, lbb_11
jsle r1, r2, lbb_11
lbb_11:
exit
return
"
);

Expand All @@ -319,7 +334,7 @@ lbb_11:
jslt r1, 2, lbb_11
jsle r1, 2, lbb_11
lbb_11:
exit
return
"
);
}
Expand Down
8 changes: 4 additions & 4 deletions tests/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ fn test_pqr() {
prog[24] = ebpf::HOR64_IMM;
prog[25] = 1; // dst = R1
prog[33] = 16; // src = R1
prog[40] = ebpf::EXIT;
prog[40] = ebpf::RETURN;
let loader = Arc::new(BuiltinProgram::new_mock());
for (opc, dst, src, expected_result) in [
(ebpf::UHMUL64_IMM, 13u64, 4u64, 0u64),
Expand Down Expand Up @@ -841,7 +841,7 @@ fn test_pqr() {
fn test_err_divide_by_zero() {
let mut prog = [0; 24];
prog[0] = ebpf::MOV32_IMM;
prog[16] = ebpf::EXIT;
prog[16] = ebpf::RETURN;
let loader = Arc::new(BuiltinProgram::new_mock());
for opc in [
ebpf::UDIV32_REG,
Expand Down Expand Up @@ -882,7 +882,7 @@ fn test_err_divide_overflow() {
LittleEndian::write_i32(&mut prog[20..], -1);
prog[25] = 16; // src = R1
LittleEndian::write_i32(&mut prog[28..], -1);
prog[32] = ebpf::EXIT;
prog[32] = ebpf::RETURN;
let loader = Arc::new(BuiltinProgram::new_mock());
for opc in [
ebpf::SDIV32_IMM,
Expand Down Expand Up @@ -2261,7 +2261,7 @@ fn test_err_mem_access_out_of_bound() {
prog[0] = ebpf::MOV32_IMM;
prog[8] = ebpf::HOR64_IMM;
prog[16] = ebpf::ST_1B_IMM;
prog[24] = ebpf::EXIT;
prog[24] = ebpf::RETURN;
let loader = Arc::new(BuiltinProgram::new_mock());
for address in [0x2u64, 0x8002u64, 0x80000002u64, 0x8000000000000002u64] {
LittleEndian::write_u32(&mut prog[4..], address as u32);
Expand Down
8 changes: 4 additions & 4 deletions tests/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn test_verifier_err_endian_size() {
let prog = &[
0xdc, 0x01, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, //
0xb7, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
];
let executable = Executable::<TestContextObject>::from_text_bytes(
prog,
Expand Down Expand Up @@ -290,7 +290,7 @@ fn test_verifier_err_jmp_out_start() {
fn test_verifier_err_unknown_opcode() {
let prog = &[
0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
];
let executable = Executable::<TestContextObject>::from_text_bytes(
prog,
Expand All @@ -307,7 +307,7 @@ fn test_verifier_err_unknown_opcode() {
fn test_verifier_unknown_sycall() {
let prog = &[
0x85, 0x00, 0x00, 0x00, 0xfe, 0xc3, 0xf5, 0x6b, // call 0x6bf5c3fe
0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // exit
0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // return
];
let executable = Executable::<TestContextObject>::from_text_bytes(
prog,
Expand All @@ -323,7 +323,7 @@ fn test_verifier_unknown_sycall() {
fn test_verifier_known_syscall() {
let prog = &[
0x85, 0x00, 0x00, 0x00, 0xfe, 0xc3, 0xf5, 0x6b, // call 0x6bf5c3fe
0x95, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // exit
0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // return
];
let mut function_registry = FunctionRegistry::<BuiltinFunction<TestContextObject>>::default();
function_registry
Expand Down