In the last post we implemented a state machine to manage breakpoints in rusty_trap. We also added support for hitting a single breakpoint multiple times. Now, I want to start adding support for multiple distinct breakpoints.

One way to approach adding a new feature is to refactor to make the code easily accept the new feature. That’s what I set out to do today, but it didn’t go the way I planned.

Multiple Breakpoints

I still start with writing a test for my goal - to support multiple breakpoints. This will help me identify the refactorings I need:

#[test] fn it_can_handle_multiple_breakpoints () { let mut foo_count : i32 = 0 ; let mut main_count : i32 = 0 ; let inferior = rusty_trap :: trap_inferior_exec ( Path :: new ( "./target/debug/loop" ), & []) . unwrap (); let bp_foo = rusty_trap :: trap_inferior_set_breakpoint ( inferior , 0x5555555587b0 ); let bp_main = rusty_trap :: trap_inferior_set_breakpoint ( inferior , 0x555555558bd0 ); rusty_trap :: trap_inferior_continue ( inferior , & mut | passed_inferior , bp_passed | { assert_eq ! ( passed_inferior , inferior ); if bp_passed == bp_foo { foo_count += 1 } else if bp_passed == bp_main { main_count += 1 } else { panic ! ( "Unknown breakpoint {}" , bp_passed ) } }); assert_eq ! ( foo_count , 5 ); assert_eq ! ( main_count , 5 ) }

and of course this test fails which is expected. Though the error isn’t exactly what I expected:

---- it_can_handle_multiple_breakpoints stdout ---- thread 'it_can_handle_multiple_breakpoints' panicked at 'assertion failed: `(left == right)` (left: `2`, right: `5`)', tests/lib.rs:63

So it looks like we only detected 2 two hits to bp_foo . I would have expected to get all 5 an maybe more (due to misattribution of bp_main).

I also see this

test it_can_handle_multiple_breakpoints ... thread '<main>' panicked at 'assertion failed: c.borrow().is_none()', ../src/libstd/sys/common/thread_info.rs:54 FAILED

I’m not sure exactly what this is about or when it happened. The test must have gotten at least two assert_eq!(foo_count, 5);

Implementation

As I mentioned, the test should point me in the direction I need to refactor. So, I need to consider: hat do we need to do in order to implement this feature?

Looking over breakpoint/mod.rs I think we need to

Remove global_breakpoint in favor of a vector of breakpoints

Update breakpoint::handle to lookup the correct breakpoint

to lookup the correct breakpoint Fix trap_inferior_set_breakpoint to add to the vector of breakpoints

Also, instead of having a global vector of breakpoints. I’ll keep the vector in the Inferior . This will help when we need to support multiple inferiors. One problem is that this will create a circular dependency between inferior and breakpoint modules.

I need some private structure for breakpoint module that fits into Inferior . How do I do this in Rust? Maybe I can use a generic that lets me store/fetch it in a Vector . A little googling turns up that I can use the Any trait.

If I want to put a Vector inside Inferior then I will need to remove the Copy trait from Inferior . I don’t want to to be copying the Vector s. Given this I should remove global_inferior .

Remove global_inferior

I’ll do this by dynamically allocating an inferior and returning it to the application. This eliminates the need for Copy trait on Inferior.

I start by commenting out global_inferior and letting the compiler tell me what to fix:

Compiling rusty_trap v0.1.0 (file:///home/joseph/src/rust/rusty_trap) src/lib.rs:79:26: 79:41 error: unresolved name `global_inferior` [E0425] src/lib.rs:79 unsafe { global_inferior = attach_inferior(pid).ok().unwrap() }; ^~~~~~~~~~~~~~~ src/lib.rs:79:26: 79:41 help: run `rustc --explain E0425` to see a detailed explanation src/lib.rs:91:28: 91:43 error: unresolved name `global_inferior` [E0425] src/lib.rs:91 let mut inf = unsafe { global_inferior }; ^~~~~~~~~~~~~~~ src/lib.rs:91:28: 91:43 help: run `rustc --explain E0425` to see a detailed explanation src/lib.rs:105:18: 105:33 error: unresolved name `global_inferior` [E0425] src/lib.rs:105 unsafe { global_inferior = inf }; ^~~~~~~~~~~~~~~ src/lib.rs:105:18: 105:33 help: run `rustc --explain E0425` to see a detailed explanation error: aborting due to 3 previous errors Could not compile `rusty_trap`.

Changes to inferior/mod.rs:

diff --git a/src/inferior/mod.rs b/src/inferior/mod.rs index 17eaa77..e5eaa55 100644 --- a/src/inferior/mod.rs +++ b/src/inferior/mod.rs @@ -1,6 +1,9 @@ use libc::pid_t; use libc::c_void; use std::ops::{Add, Sub}; +use std::any::Any; +use std::vec::Vec; #[derive(Copy, Clone)] pub enum InferiorState { @@ -9,13 +12,14 @@ pub enum InferiorState { SingleStepping } -#[derive(Copy, Clone)] +//#[derive(Copy, Clone)] pub struct Inferior { pub pid: pid_t, - pub state: InferiorState + pub state: InferiorState, + pub privates: Vec<Box<Any>> } -pub type TrapInferior = pid_t; +pub type TrapInferior = Box<Inferior>; #[derive(Copy, Clone)] pub struct InferiorPointer(pub u64);

Right now, the only important changes is the change to TrapInferior . The changes to Inferior are still work in progress.

Changes to lib.rs:

diff --git a/src/lib.rs b/src/lib.rs index ac9e444..70ded88 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -29,13 +29,13 @@ struct Breakpoint { original_breakpoint_word : i64 } -static mut global_breakpoint : Breakpoint = Breakpoint { - shift: 0, - target_address: InferiorPointer(0), - aligned_address: InferiorPointer(0), - original_breakpoint_word: 0 -}; -static mut global_inferior : Inferior = Inferior { pid: 0, state: InferiorState::Stopped }; +// static mut global_breakpoint : Breakpoint = Breakpoint { +// shift: 0, +// target_address: InferiorPointer(0), +// aligned_address: InferiorPointer(0), +// original_breakpoint_word: 0 +// }; +//static mut global_inferior : Inferior = Inferior { pid: 0, state: InferiorState::Stopped, privates: vec![] }; mod ffi { use libc::{c_int, c_long}; @@ -62,10 +62,10 @@ fn exec_inferior(filename: &Path, args: &[&str]) -> () { unreachable!(); } -fn attach_inferior(pid: pid_t) -> Result<Inferior, Error> { +fn attach_inferior(pid: pid_t) -> Result<Box<Inferior>, Error> { match waitpid(pid, None) { Ok(WaitStatus::Stopped(pid, signal::SIGTRAP)) => - return Ok(Inferior {pid: pid, state: InferiorState::Running}), + return Ok(Box::new(Inferior {pid: pid, state: InferiorState::Running, privates: vec![]})), Ok(_) => panic!("Unexpected stop in attach_inferior"), Err(e) => return Err(e) } @@ -76,8 +76,8 @@ pub fn trap_inferior_exec(filename: &Path, args: &[&str]) -> Result<TrapInferior match fork() { Ok(Child) => exec_inferior(filename, args), Ok(Parent(pid)) => { - unsafe { global_inferior = attach_inferior(pid).ok().unwrap() }; - return Ok(pid) + let inf = attach_inferior(pid).ok().unwrap(); + return Ok(inf) }, Err(Error::Sys(errno::EAGAIN)) => continue, Err(e) => return Err(e) @@ -86,22 +86,20 @@ pub fn trap_inferior_exec(filename: &Path, args: &[&str]) -> Result<TrapInferior } pub fn trap_inferior_continue<F>(inferior: TrapInferior, callback: &mut F) -> i8 - where F: FnMut(TrapInferior, TrapBreakpoint) -> () { + where F: FnMut(&TrapInferior, TrapBreakpoint) -> () { - let mut inf = unsafe { global_inferior }; + let mut inf = inferior; ptrace_util::cont(inf.pid); loop { inf.state = match waitpid(inf.pid, None) { Ok(WaitStatus::Exited(_pid, code)) => return code, Ok(WaitStatus::Stopped(_pid, signal::SIGTRAP)) => - breakpoint::handle(inf, callback), + breakpoint::handle(&inf, callback), Ok(WaitStatus::Stopped(_pid, signal)) => { panic!("Unexpected stop on signal {} in trap_inferior_continue. State: {}", signal, inf.state as i32) }, Ok(_) => panic!("Unexpected stop in trap_inferior_continue"), Err(_) => panic!("Unhandled error in trap_inferior_continue") }; - - unsafe { global_inferior = inf }; } }

Here I’ve commened out global_inferior and allocated an Inferior dynamically in attach_inferior using Box . I’ve also started passing around references to the Box<Inferior> rather than moving the pointer.

Changes to breakpoint/mod.rs:

diff --git a/src/breakpoint/mod.rs b/src/breakpoint/mod.rs index a069427..1d4aa1d 100644 --- a/src/breakpoint/mod.rs +++ b/src/breakpoint/mod.rs @@ -19,33 +19,33 @@ static mut global_breakpoint : Breakpoint = Breakpoint { }; -fn step_over(inferior: TrapInferior, bp: Breakpoint) -> () { - poke_text(inferior, bp.aligned_address, bp.original_breakpoint_word); - set_instruction_pointer(inferior, bp.target_address); - single_step(inferior); +fn step_over(inferior: &TrapInferior, bp: Breakpoint) -> () { + let pid = inferior.pid; + poke_text(pid, bp.aligned_address, bp.original_breakpoint_word); + set_instruction_pointer(pid, bp.target_address); + single_step(pid); } -fn set(inferior: TrapInferior, bp: Breakpoint) -> () { +fn set(inferior: &TrapInferior, bp: Breakpoint) -> () { let mut modified = bp.original_breakpoint_word; modified &= !0xFFi64 << bp.shift; modified |= 0xCCi64 << bp.shift; - poke_text(inferior, bp.aligned_address, modified); + poke_text(inferior.pid, bp.aligned_address, modified); } -pub fn handle<F>(inf: Inferior, mut callback: &mut F) -> InferiorState - where F: FnMut(TrapInferior, TrapBreakpoint) -> () { - let inferior = inf.pid; +pub fn handle<F>(inf: &TrapInferior, mut callback: &mut F) -> InferiorState + where F: FnMut(&TrapInferior, TrapBreakpoint) -> () { let bp = unsafe { global_breakpoint }; match inf.state { InferiorState::Running => { - callback(inferior, 0); - step_over(inferior, bp); + callback(&inf, 0); + step_over(&inf, bp); InferiorState::SingleStepping }, InferiorState::SingleStepping => { - set(inferior, bp); - cont(inferior); + set(&inf, bp); + cont(inf.pid); InferiorState::Running }, _ => panic!("Unsupported breakpoint encountered during supported inferior state") @@ -58,10 +58,10 @@ pub fn trap_inferior_set_breakpoint(inferior: TrapInferior, location: u64) -> Tr shift : (location - aligned_address) * 8, aligned_address: InferiorPointer(aligned_address), target_address: InferiorPointer(location), - original_breakpoint_word: peek_text(inferior, InferiorPointer(aligned_address)) + original_breakpoint_word: peek_text(inferior.pid, InferiorPointer(aligned_address)) }; - set(inferior, bp); + set(&inferior, bp); unsafe { global_breakpoint = bp;

These changes are mostly of one of two forms:

Taking the pid field from the Inferior rather than assuming InferiorTrap is a pid. Passing references to the Box<Inferior> around instead of moving. Previously these were copies. Passing references fixes up ownership now that we don’t copy.

These changes all seem well and good until I get to the tests. I expected to have to fix up a few things (in fact the API changed in thatthe callback now takes a reference). But I didn’t understand the real problem until I saw the error:

$ cargo test Compiling rusty_trap v0.1.0 (file:///home/joseph/src/rust/rusty_trap) <std macros>:5:8: 5:18 error: binary operation `==` cannot be applied to type `&Box<rusty_trap::inferior::Inferior>` [E0369] <std macros>:5 if ! ( * left_val == * right_val ) { ^~~~~~~~~~ <std macros>:1:1: 9:39 note: in expansion of assert_eq! tests/lib.rs:19:9: 19:47 note: expansion site note: in expansion of closure expansion tests/lib.rs:18:55: 22:6 note: expansion site

So, it seems that these references are not comparable. Which I guess makes some sense. I actually, shouldn’t have more than one copy of the pointer or reference to the pointer in the first place. That must be violating the borrow rules.

Next steps

I can’t really proceed with this refactoring. I’m going to need to go back to where I started and try something different. I’m thinking I will go back to returning the pid, or some opaque handle, to the application and then using that to lookup the Inferior data structure.

I’ll try again next week.