Skip to content

Commit 3cf48f4

Browse files
LichtsoLichtso
committed
Some control flow operations were not tested before.
1 parent 75d5bc8 commit 3cf48f4

File tree

3 files changed

+55
-46
lines changed

3 files changed

+55
-46
lines changed

‎src/disassembler.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -267,19 +267,16 @@ pub fn disassemble_instruction<C: ContextObject>(
267267
ebpf::JSLE_REG => { name = "jsle"; desc = jmp_reg_str(name, insn, cfg_nodes); },
268268
ebpf::CALL_IMM => {
269269
let key = sbpf_version.calculate_call_imm_target_pc(pc, insn.imm);
270-
let function_name = function_registry.lookup_by_key(key).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string());
271-
let function_name = if let Some(function_name) = function_name {
272-
name = "call";
273-
function_name
274-
} else {
270+
let mut name = "call";
271+
let mut function_name = function_registry.lookup_by_key(key).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string());
272+
if !sbpf_version.static_syscalls() && function_name.is_none() {
275273
name = "syscall";
276-
loader.get_function_registry(sbpf_version).lookup_by_key(insn.imm as u32).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string()).unwrap_or_else(|| "[invalid]".to_string())
277-
};
278-
desc = format!("{name} {function_name}");
274+
function_name = loader.get_function_registry(sbpf_version).lookup_by_key(insn.imm as u32).map(|(function_name, _)| String::from_utf8_lossy(function_name).to_string());
275+
}
276+
desc = format!("{} {}", name, function_name.unwrap_or_else(|| "[invalid]".to_string()));
279277
},
280278
ebpf::CALL_REG => { name = "callx"; desc = format!("{} r{}", name, if sbpf_version.callx_uses_src_reg() { insn.src } else { insn.imm as u8 }); },
281-
ebpf::EXIT
282-
| ebpf::RETURN if !sbpf_version.static_syscalls() => { name = "exit"; desc = name.to_string(); },
279+
ebpf::EXIT if !sbpf_version.static_syscalls() => { name = "exit"; desc = name.to_string(); },
283280
ebpf::RETURN if sbpf_version.static_syscalls() => { name = "return"; desc = name.to_string(); },
284281
ebpf::SYSCALL if sbpf_version.static_syscalls() => { desc = format!("syscall {}", insn.imm); },
285282

‎src/jit.rs

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1719,12 +1719,14 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
17191719
mod tests {
17201720
use super::*;
17211721
use crate::{
1722+
disassembler::disassemble_instruction,
17221723
program::{BuiltinProgram, FunctionRegistry, SBPFVersion},
1724+
static_analysis::CfgNode,
17231725
syscalls,
17241726
vm::TestContextObject,
17251727
};
17261728
use byteorder::{ByteOrder, LittleEndian};
1727-
use std::sync::Arc;
1729+
use std::{collections::BTreeMap, sync::Arc};
17281730

17291731
#[test]
17301732
fn test_runtime_environment_slots() {
@@ -1838,35 +1840,45 @@ mod tests {
18381840
<= MACHINE_CODE_PER_INSTRUCTION_METER_CHECKPOINT
18391841
);
18401842

1843+
let mut cfg_nodes = BTreeMap::new();
1844+
cfg_nodes.insert(
1845+
8,
1846+
CfgNode {
1847+
label: std::string::String::from("label"),
1848+
..CfgNode::default()
1849+
},
1850+
);
1851+
18411852
for sbpf_version in [SBPFVersion::V0, SBPFVersion::V3] {
1853+
println!("opcode;machine_code_length_per_instruction;assembly");
18421854
let empty_program_machine_code_length =
18431855
empty_program_machine_code_length_per_version[sbpf_version as usize];
18441856

18451857
for mut opcode in 0x00..=0xFF {
18461858
let (registers, immediate) = match opcode {
1847-
0x85 | 0x8D => (0x88, 8),
1848-
0x86 => {
1859+
0x85 if !sbpf_version.static_syscalls() => (0x00, Some(8)),
1860+
0x85 if sbpf_version.static_syscalls() => (0x00, None),
1861+
0x8D => (0x88, Some(0)),
1862+
0x95 if sbpf_version.static_syscalls() => (0x00, Some(1)),
1863+
0xE5 if !sbpf_version.static_syscalls() => {
18491864
// Put external function calls on a separate loop iteration
18501865
opcode = 0x85;
1851-
(0x00, 0x91020CDD)
1866+
(0x00, Some(0x91020CDD))
18521867
}
1853-
0x87 => {
1868+
0xF5 => {
18541869
// Put invalid function calls on a separate loop iteration
18551870
opcode = 0x85;
1856-
(0x88, 0x91020CDD)
1871+
(0x00, Some(0x91020CD0))
18571872
}
1858-
0x95 => {
1859-
// Put a valid syscall
1860-
(0, 1)
1861-
}
1862-
0xD4 | 0xDC => (0x88, 16),
1863-
_ => (0x88, 0xFFFFFFFF),
1873+
0xD4 | 0xDC => (0x88, Some(16)),
1874+
_ => (0x88, Some(0xFFFFFFFF)),
18641875
};
18651876
for pc in 0..INSTRUCTION_COUNT {
18661877
prog[pc * ebpf::INSN_SIZE] = opcode;
18671878
prog[pc * ebpf::INSN_SIZE + 1] = registers;
1868-
prog[pc * ebpf::INSN_SIZE + 2] = 0xFF;
1869-
prog[pc * ebpf::INSN_SIZE + 3] = 0xFF;
1879+
let offset = 7_u16.wrapping_sub(pc as u16);
1880+
LittleEndian::write_u16(&mut prog[pc * ebpf::INSN_SIZE + 2..], offset);
1881+
let immediate = immediate.unwrap_or_else(|| 7_u32.wrapping_sub(pc as u32));
18701882
LittleEndian::write_u32(&mut prog[pc * ebpf::INSN_SIZE + 4..], immediate);
18711883
}
18721884
let config = Config {
@@ -1900,12 +1912,19 @@ mod tests {
19001912
(machine_code_length_per_instruction + 0.5) as usize
19011913
<= MAX_MACHINE_CODE_LENGTH_PER_INSTRUCTION
19021914
);
1903-
/*println!("opcode={:02X} machine_code_length_per_instruction={}", opcode, machine_code_length_per_instruction);
1904-
let analysis = crate::static_analysis::Analysis::from_executable(&executable).unwrap();
1905-
{
1906-
let stdout = std::io::stdout();
1907-
analysis.disassemble(&mut stdout.lock()).unwrap();
1908-
}*/
1915+
let insn = ebpf::get_insn_unchecked(&prog, 0);
1916+
let assembly = disassemble_instruction(
1917+
&insn,
1918+
0,
1919+
&cfg_nodes,
1920+
executable.get_function_registry(),
1921+
executable.get_loader(),
1922+
executable.get_sbpf_version(),
1923+
);
1924+
println!(
1925+
"{:02X};{:>7.3};{}",
1926+
opcode, machine_code_length_per_instruction, assembly
1927+
);
19091928
}
19101929
}
19111930
}

‎src/static_analysis.rs

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -230,24 +230,13 @@ impl<'a> Analysis<'a> {
230230
self.cfg_nodes.entry(*pc).or_default();
231231
}
232232
let mut cfg_edges = BTreeMap::new();
233-
for insn in self.instructions.iter() {
233+
for (pc, insn) in self.instructions.iter().enumerate() {
234234
let target_pc = (insn.ptr as isize + insn.off as isize + 1) as usize;
235235
match insn.opc {
236236
ebpf::CALL_IMM => {
237-
if let Some((function_name, _function)) = self
238-
.executable
239-
.get_loader()
240-
.get_function_registry(sbpf_version)
241-
.lookup_by_key(insn.imm as u32)
242-
{
243-
if function_name == b"abort" {
244-
self.cfg_nodes.entry(insn.ptr + 1).or_default();
245-
cfg_edges.insert(insn.ptr, (insn.opc, Vec::new()));
246-
}
247-
} else if let Some((_function_name, target_pc)) = self
248-
.executable
249-
.get_function_registry()
250-
.lookup_by_key(insn.imm as u32)
237+
let key = sbpf_version.calculate_call_imm_target_pc(pc, insn.imm);
238+
if let Some((_function_name, target_pc)) =
239+
self.executable.get_function_registry().lookup_by_key(key)
251240
{
252241
self.cfg_nodes.entry(insn.ptr + 1).or_default();
253242
self.cfg_nodes.entry(target_pc).or_default();
@@ -269,7 +258,11 @@ impl<'a> Analysis<'a> {
269258
};
270259
cfg_edges.insert(insn.ptr, (insn.opc, destinations));
271260
}
272-
ebpf::EXIT => {
261+
ebpf::EXIT if !sbpf_version.static_syscalls() => {
262+
self.cfg_nodes.entry(insn.ptr + 1).or_default();
263+
cfg_edges.insert(insn.ptr, (insn.opc, Vec::new()));
264+
}
265+
ebpf::RETURN if sbpf_version.static_syscalls() => {
273266
self.cfg_nodes.entry(insn.ptr + 1).or_default();
274267
cfg_edges.insert(insn.ptr, (insn.opc, Vec::new()));
275268
}

0 commit comments

Comments
 (0)