Multiple Breakpoints in Rust: Ownership-Driven Debugger Design

Welcome back to my series on writing a debugger in rust. In our previous post we updated our custom breakpoint handling to support multiple breakpoints in C. And in this post we will do the same thing in Rust.

For reference I’m starting at commit a9447ec.

But this is Rust and not C and we will follow Rust’s best practices which will guide us toward an implementation that eliminates global state and models ownership properly and makes the relationship between inferiors and their breakpoints explicit in the type system. What started off as a C-style port became an ownership-driven rearchitecture.

I started this journey with a new test:

#[test]
fn it_can_handle_more_than_one_breakpoint() {
    let mut bp_main_count: i32 = 0;
    let mut bp_foo_count: i32 = 0;

    let inferior = rusty_trap::trap_inferior_exec(Path::new("./target/debug/loop"), &[]).unwrap();
    let bp_main = rusty_trap::trap_inferior_set_breakpoint(inferior, ADDRESS_OF_MAIN);
    let bp_foo = rusty_trap::trap_inferior_set_breakpoint(inferior, ADDRESS_OF_FOO);
    rusty_trap::trap_inferior_continue(inferior, &mut |passed_inferior, passed_bp| {
        assert_eq!(passed_inferior, inferior);
        if passed_bp == bp_main {
            bp_main_count += 1;
        } else if passed_bp == bp_foo {
            bp_foo_count += 1;
        }
    });

    assert_eq!(bp_main_count, 1);
    assert_eq!(bp_foo_count, 5);
}

This test is similar to our other tests except that those tests set either a breakpoint in main or in foo and this test does both. Then in the lambda / callback it checks which breakpoint was reached and increments the corresponding counter. When it’s all done it checks for expected counts. main is called once and triggers it’s breakpoint once. foo is called in a loop so it triggers 5 times.

So, what do I need to do to make it pass?

We need to allocate breakpoints instead of using one global.
- Put off dynamic allocation until I work through deleting as well.
We need that fancy version of breakpoint_handle that we restructured in the C version.

When writing breakpoint::handle I wanted to mimic what is in the C version and assert(state == INFERIOR_RUNNING);

error[E0369]: binary operation `==` cannot be applied to type `inferior::InferiorState`
  --> src/breakpoint/mod.rs:52:23
   |
52 |     assert!(inf.state == InferiorState::Running);
   |             --------- ^^ ---------------------- inferior::InferiorState
   |             |
   |             inferior::InferiorState
   |
note: an implementation of `PartialEq` might be missing for `inferior::InferiorState`
  --> src/inferior/mod.rs:6:1
   |
6  | pub enum InferiorState {
   | ^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq`
help: consider annotating `inferior::InferiorState` with `#[derive(PartialEq)]`
  --> src/inferior/mod.rs:6:1
   |
6  + #[derive(PartialEq)]
7  | pub enum InferiorState {
   |

So instead I wrote:

    match inf.state {
	InferiorState::Running => (),
	_ => panic!("Unhandled error in breakpoint::handle"),
    }

Which works, and I guess I realized this would work because the old version matched on inf.state to check for Running vs SingleStepping. But, is this really what I have to do? And what does match do that’s different than ==?

Anyway, here’s the completed breakpoint::handle

pub fn handle<F>(inf: Inferior, callback: &mut F) -> InferiorState
where
    F: FnMut(TrapInferior, TrapBreakpoint),
{
    let inferior = inf.pid;
    let (user_breakpoint, bp) =
        find_breakpoint_matching_inferior_instruction_pointer(inf)
        .expect("Could not find breakpoint");

    match inf.state {
        InferiorState::Running => (),
        _ => panic!("Unhandled error in breakpoint::handle"),
    }
    callback(inferior, user_breakpoint);
    step_over(inferior, bp);
    return match waitpid(Pid::from_raw(inf.pid), None) {
        Ok(WaitStatus::Stopped(_pid, signal::SIGTRAP)) => {
            set(inferior, bp);
            cont(inferior);
            InferiorState::Running
        }
        Ok(WaitStatus::Exited(_pid, _code)) => InferiorState::Running,
        Ok(WaitStatus::Stopped(_pid, signal)) => {
            panic!(
                "Unexpected stop on signal {} in breakpoint::handle.  State: {}",
                signal, inf.state as i32
            )
        }
        Ok(_) => panic!("Unexpected stop in breakpoint::handle"),
        Err(_) => panic!("Unhandled error in breakpoint::handle"),
    };
}

Recall, the goal here is to both handle the initial breakpoint and single step in one call to breakpoint::handle. This seems to work for the new test. And it seems to work for each test individually but it doesn’t work when I run all the tests:

running 4 tests
test it_can_exec ... ok
test it_can_handle_a_breakpoint_more_than_once ... ok
test it_can_handle_more_than_one_breakpoint ... FAILED
test it_can_set_breakpoints ... FAILED

failures:

---- it_can_handle_more_than_one_breakpoint stdout ----

thread 'it_can_handle_more_than_one_breakpoint' panicked at tests/lib.rs:64:5:
assertion `left == right` failed
  left: 0
 right: 5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- it_can_set_breakpoints stdout ----

thread 'it_can_set_breakpoints' panicked at tests/lib.rs:28:5:
assertion `left == right` failed
  left: 0
 right: 1


failures:
	it_can_handle_more_than_one_breakpoint
	it_can_set_breakpoints

test result: FAILED. 2 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

This feels again like the tests are being run concurrently but I’m still running cargo test -- --test-threads 1. Can I confirm test are running sequentially by putting a mutex in the tests?

I mutexed the tests based on this post.

modified   tests/lib.rs
@@ -1,65 +1,89 @@
 extern crate rusty_trap;
 use std::path::Path;
+use std::sync::Mutex;
 
 const ADDRESS_OF_MAIN: u64 = 0x55555555b9f4;
 const ADDRESS_OF_FOO: u64 = 0x55555555b9e0;
 
+static LIBRARY_USER: Mutex<Executor> = Mutex::new(Executor);
+
+#[derive(Clone, Copy)]
+struct Executor;
+
+impl Executor {
+    fn run_test(self, f: impl FnOnce()) {
+        f();
+    }
+}
+
 #[test]
 fn it_can_exec() {
-    let inferior = rusty_trap::trap_inferior_exec(Path::new("./target/debug/twelve"), &[]).unwrap();
-    assert_eq!(
-        12,
-        rusty_trap::trap_inferior_continue(inferior, &mut |_, _| {})
-    );
+    LIBRARY_USER.lock().unwrap().run_test(|| {
+        let inferior =
+            rusty_trap::trap_inferior_exec(Path::new("./target/debug/twelve"), &[]).unwrap();
+        assert_eq!(
+            12,
+            rusty_trap::trap_inferior_continue(inferior, &mut |_, _| {})
+        );
+    });
 }

I’ve only shown the first test, the rest are wrapped in this executor the same way. The tests still fail which I guess is expected. But the mutexing also fails? Here are the test failures:

---- it_can_handle_more_than_one_breakpoint stdout ----

thread 'it_can_handle_more_than_one_breakpoint' panicked at tests/lib.rs:87:9:
assertion `left == right` failed
  left: 0
 right: 5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- it_can_set_breakpoints stdout ----

thread 'it_can_set_breakpoints' panicked at tests/lib.rs:33:25:
called `Result::unwrap()` on an `Err` value: PoisonError { .. }

Oh, ok this makes sense the rust docs for Mutex

The mutexes in this module implement a strategy called “poisoning” where a mutex is considered poisoned whenever a thread panics while holding the mutex. Once a mutex is poisoned, all other threads are unable to access the data by default as it is likely tainted (some invariant is not being upheld).

So when the first test failed it panicked and poisoned the mutex. Then the next test that tries to acquire the mutex finds it poisoned and panics as well. I think this does confirm the tests are running synchronously.

So then what’s wrong?

Is there some state being carried, with the rusty_trap library, across tests?
- Of course there is some state, we have a global vector of breakpoints after all. But it should work well enough to run 3 tests back to back, right?
- But I don’t want to rule it out. I mean I think it should work but I also the whole system would work and it doesn’t so something’s wrong in my understanding.

When things don't work as we expect it means we misunderstand something about the system. When this happens question everything, trust nothing you think you know.

That said, I hope the tests are correct. I removed the mutexing since it doesn’t make a difference - the tests are simpler without it. And, I’d rather see the test fail than the mutex fail. Next, I want to use these tests to learn more about the problem. The errors look like this:

running 4 tests
test it_can_exec ... ok
test it_can_handle_a_breakpoint_more_than_once ... ok
test it_can_handle_more_than_one_breakpoint ... FAILED
test it_can_set_breakpoints ... ok

failures:

---- it_can_handle_more_than_one_breakpoint stdout ----

thread 'it_can_handle_more_than_one_breakpoint' panicked at tests/lib.rs:64:5:
assertion `left == right` failed
  left: 0
 right: 5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
	it_can_handle_more_than_one_breakpoint

test result: FAILED. 3 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

This happens the same way every time which is expected. And again, if I were to comment out the other tests then it_can_handle_more_than_one_breakpoint would pass.

What the failure says is that the breakpoint in main was hit once as expected but the breakpoint in foo was never hit. One thing that the test doesn’t capture is if we stopped at the wrong address. The test should check for this, if this did happen that should also count as a test failure and it should be called out differently.

Here’s a test that does that:

#[test]
fn it_can_handle_more_than_one_breakpoint() {
    let mut bp_main_count: i32 = 0;
    let mut bp_foo_count: i32 = 0;

    let inferior = rusty_trap::trap_inferior_exec(Path::new("./target/debug/loop"), &[]).unwrap();
    let bp_main = rusty_trap::trap_inferior_set_breakpoint(inferior, ADDRESS_OF_MAIN);
    let bp_foo = rusty_trap::trap_inferior_set_breakpoint(inferior, ADDRESS_OF_FOO);
    rusty_trap::trap_inferior_continue(inferior, &mut |passed_inferior, passed_bp| {
        assert_eq!(passed_inferior, inferior);
        if passed_bp == bp_main {
            bp_main_count += 1;
        } else if passed_bp == bp_foo {
            bp_foo_count += 1;
        } else {
            panic!(    // How do I write this as an assertion?  Or should I?
                "Unexpected breakpoint {} encountered.  Expected {} or {}",
                passed_bp, bp_main, bp_foo
            );
        }
    });

    assert_eq!(bp_main_count, 1);
    assert_eq!(bp_foo_count, 5);
}

And when I run this test I get this (along with the failure):

Unexpected breakpoint 0 encountered.  Expected 1 or 2

Recall that the breakpoints passed out to the user are just indices into GLOBAL_BREAKPOINTS. So, something is getting crossed where one of the breakpoints from the previous test is being hit.

Another issue is that we have global_inferior but the 4 different tests call exec_inferior 4 different times. So this is likely one problem though I would think that global_inferior would be overwritten and it would be fine (I’m mean, it’s an awful code smell but I don’t think it would lead to this bug)

Another thought is that somewhere I forgot to update breakpoint handling to pick the right breakpoint and so we find the wrong one.

Oh, as I write that down I realize that find_breakpoint_matching_inferior_instruction_pointer is wrong. Remember, it looks like this:

fn find_breakpoint_matching_inferior_instruction_pointer(inf: Inferior) -> Option<(TrapBreakpoint, Breakpoint)> {
    let ip = ptrace_util::get_instruction_pointer(inf.pid).as_i64();
    unsafe { for (idx, bp) in GLOBAL_BREAKPOINTS.iter().enumerate() {
	if bp.target_address.as_i64() == ip - 1 {
	    return Some((idx, *bp))
	}
    } };
    None
}

So, it does this linear search through GLOBAL_BREAKPOINTS until it finds a matching target_address. Well, most of the breakpoints in here have the same target address because we are running unit tests that all break in loop:foo which has the same address every time.

This is a mess and just shows one of the main problems with global variables that Rust is trying to solve. And I worked around all of Rust’s efforts by putting unsafe everywhere and paid the price.

So let’s do this right and get rid of GLOBAL_BREAKPOINTS. Earlier I said that I would

Put off dynamic allocation until I work through deleting as well.

So let’s think about deleting. I had been thinking to mimic the C API in which there is a trap_inferior_remove_breakpoint function that the user can call to delete their breakpoints. But this is a very C style API. Between trap_inferior_set_breakpoint and trap_inferior_remove_breakpoint it basically says: “Hi user, here’s your breakpoint it’s your problem now. Free it, lose it, whatever. I don’t care”. That’s not a Rusty attitude.

In Rust I guess the right idea here is to give the user a breakpoint that they own and that can clean itself up when it goes out of scope. Also it should not be a reference to anything global or unsafe. There is a second piece to this which is that we can’t delete the breakpoint while the inferior is running. So the C version sets the breakpoint in this deferred removal state and cleans it up later. I don’t know if I want to do this in Rust the same way. One thing I don’t like about setting this deferred stat is that it is the only mutation the breakpoint goes through. The breakpoints could be immutable if we do this a different way. This also feels like reference counting: the user has one reference and a running inferior with the breakpoint installed has another reference. Rust has reference counting support so I’ll keep that in mind.

Anyway, I don’t really want to get into the feature of deleting breakpoints right now because we currently have a failing test and should fix that first. Deleting breakpoints will require new tests. So let’s at least start with a basic dynamically allocated breakpoint and worry about proper deletion afterwards. I think none of the tests we have at the moment will trigger a problem with deleting a breakpoint while the inferior is running.

So I think I want to Box up a new breakpoint?

First we need to get rid of Breakpoint and TrapBreakpoint or TrapBreakpoint is Box?

I think something like this would work:

struct Breakpoint {
    shift: u64,
    target_address: InferiorPointer,
    aligned_address: InferiorPointer,
    original_breakpoint_word: i64,
}

pub type TrapBreakpoint = Box<Breakpoint>;

// ...

pub fn trap_inferior_set_breakpoint(inferior: TrapInferior, location: u64) -> TrapBreakpoint {
    let aligned_address = location & !0x7u64;
    let user_bp = Box::new(Breakpoint {
        shift: (location - aligned_address) * 8,
        aligned_address: InferiorPointer(aligned_address),
        target_address: InferiorPointer(location),
        original_breakpoint_word: peek_text(inferior, InferiorPointer(aligned_address)),
    });

    set(inferior, *user_bp);

    user_bp
}

Let me fix all the other uses. Hmm, how do I implement find_breakpoint_matching_inferior_instruction_pointer? It is called when all we know is that the inferior stopped somewhere and want to find the corresponding breakpoint.

We do know the inferior so maybe the inferior should hold on to breakpoints.
- But how do we remove the reference from the inferior?

Ok, just trying to push through incremental changes is not working well. Let’s take a step back and do some systems thinking about ownership and users.

Breakpoints:

Who uses the them / who needs to access the it?
- User needs it to determine when its hit.
  - On the other hand this comparing breakpoints in the callback is kind of gross. The user is passed the breakpoint in the callback.
    - Is comparing against the object something a client needs to do or just an artifact of the testing? This is also inherited from the C version that uses handles.
    - Worst case the callback could compare against the addresses.
- The breakpoint module needs access to the breakpoint to handle it properly.
Who should own it? Or who should control it’s lifetime?
- The user should decide if it is set or deactivated for flow control.
  - In order to deactivate (and also to deallocate) the user would need a way to refer to a breakpoint.
    - When creating the breakpoint with trap_inferior_set_breakpoint the user pases the address to refer to the breakpoint.
- Should the user decide if it is deallocated? Should it ever be deallocated?
  - If we have to search for a breakpoint then never deallocating them means a larger search space. Then again if we don’t need an index as a handle we can use a different datastructure, like a map.

Ok, that was a good exploration of the requirements. And note I used a question and answer investigation to work through it. I think the takeaway is:

The user doesn’t really need to own the breakpoint. The user certainly doesn’t need to look inside the Breakpoint struct.
The user does need a way to refer to breakpoints and the target_address might be a good one.
The user should control if a breakpoint is active or not but doesn’t necessarily need to control the lifetime.
It isn’t clear that deleting breakpoints is really useful for lifetime of the inferior.
The breakpoint module does need access to the breakpoint and the internals of the Breakpoint structure.

Rust’s ownership model offers surprising clarity when leaned into fully.

One thing I could do now is to create tests for all these behaviors and then enforce them. However, we still have a failing test and need to get past that first.

A design that these requirements seems to suggest is:

Store the breakpoints in a data structure attached to the inferior.
The data structure should be a map that maps target_address to Breakpoint.
The user can just refer to breakpoints by target address.
breakpoint::handle can pass the target_address to the callback and the tests can compare against the addresses rather than comparing breakpoints or breakpoint handles.

Ok, so how to do we get from where we are with the failing test to this new design? And using something like behavior driven development would be nice.

Approaching the Problem With a Clear Mind

I took my dogs for a walk and thought about this problem. This is a good way to clear my head and remove distractions, at least some kinds of distractions.

The key is getting past this failing test. I’ve only called that out like 3 times now.
I should do something to get the test to pass by brute force just so we can make progress and then clean up later.
This will give us passing tests we can use to validate our work step by step.
This will also let us write new tests for additional features.

To get the test passing by brute force I first thought of trying run each test as a separate run of the test program. This would acheive the goals but then I realized there is another thing we can try. I can try to reset GLOBAL_BREAKPOINTS between tests. Whenever a new inferior is created I could reset the breakpoints. From a code standpoint this will be messy, inferior has to reach into breakpoint to reset, but for the moment that’s fine. And we should have this fixed as we implement our new design. So let’s get started breaking through this failing test.

As I started to work on this I realized there is a simpler solution. If we return target_address as the breakpoint handle then it should also make the tests pass. We will still end up creating multiple breakpoints with the same target address but from User’s perspective it won’t matter. It only sees the target_address. This is a mess internally but it’s enough to break through this log jam and get us working. It’s also a step toward our new design in that we do want to use the target_address as the breakpoint identifier.

Ok, that does it. Here are the changes to the breakpoint module which just return the new TrapBreakpoint type:

modified   src/breakpoint/mod.rs
@@ -14,7 +14,7 @@ struct Breakpoint {
     original_breakpoint_word: i64,
 }
 
-pub type TrapBreakpoint = usize;
+pub type TrapBreakpoint = InferiorPointer;
 
 static mut GLOBAL_BREAKPOINTS: Vec<Breakpoint> = Vec::<Breakpoint>::new();
 
@@ -33,9 +33,9 @@ fn set(inferior: TrapInferior, bp: Breakpoint) {
 
 fn find_breakpoint_matching_inferior_instruction_pointer(inf: Inferior) -> Option<(TrapBreakpoint, Breakpoint)> {
     let ip = ptrace_util::get_instruction_pointer(inf.pid).as_i64();
-    unsafe { for (idx, bp) in GLOBAL_BREAKPOINTS.iter().enumerate() {
+    unsafe { for bp in &GLOBAL_BREAKPOINTS {
 	if bp.target_address.as_i64() == ip - 1 {
-	    return Some((idx, *bp))
+	    return Some((bp.target_address, *bp))
 	}
     } };
     None
@@ -76,7 +76,7 @@ where
 
 pub fn trap_inferior_set_breakpoint(inferior: TrapInferior, location: u64) -> TrapBreakpoint {
     let aligned_address = location & !0x7u64;
-    let user_bp = unsafe {
+    let index = unsafe {
 	GLOBAL_BREAKPOINTS.push(Breakpoint {
             shift: (location - aligned_address) * 8,
             aligned_address: InferiorPointer(aligned_address),
@@ -86,7 +86,7 @@ pub fn trap_inferior_set_breakpoint(inferior: TrapInferior, location: u64) -> Tr
 	GLOBAL_BREAKPOINTS.len() - 1
     };
 
-    set(inferior, unsafe {GLOBAL_BREAKPOINTS[user_bp]});
+    set(inferior, unsafe {GLOBAL_BREAKPOINTS[index]});
 
-    user_bp
+    InferiorPointer(location)
 }

And I needed a few changes to make InferiorPointer a little more usable. I would actually like it if the tests could pass these in but I’ll need a little more work to support that.

modified   src/inferior/mod.rs
@@ -1,6 +1,7 @@
 use libc::c_void;
 use libc::pid_t;
 use std::ops::{Add, Sub};
+use std::fmt;
 
 #[derive(Copy, Clone)]
 pub enum InferiorState {
@@ -17,7 +18,7 @@ pub struct Inferior {
 
 pub type TrapInferior = pid_t;
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, PartialOrd, Debug)]
 pub struct InferiorPointer(pub u64);
 impl InferiorPointer {
     pub fn as_voidptr(&self) -> *mut c_void {
@@ -52,3 +53,9 @@ impl Sub<i64> for InferiorPointer {
         }
     }
 }
+impl fmt::Display for InferiorPointer {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        let &InferiorPointer(u) = self;
+	write!(f, "{}", u)
+    }
+}

I’ve implemented traits for PartialEq and PartialOrd so I can compare InferiorPointer structs and Display to print them out.

Enjoying the post? Sign up for the newsletter or support the work on Buy Me a Coffee .

With this the tests pass. There are warnings that we will want to clean up, but they pass. Let’s keep score on the warnings so we can see if we make progress on them along the way (we should as many are related to GLOBAL_BREAKPOINTS):

warning: `rusty_trap` (lib) generated 7 warnings
warning: `rusty_trap` (lib test) generated 7 warnings (7 duplicates)

...

running 4 tests
test it_can_exec ... ok
test it_can_handle_a_breakpoint_more_than_once ... ok
test it_can_handle_more_than_one_breakpoint ... ok
test it_can_set_breakpoints ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

From here we can start implementing the new design, recall that we want to:

Store the breakpoints in a data structure attached to the inferior.
The data structure should be a map that maps target_address to Breakpoint.
The user can just refer to breakpoints by target address.
breakpoint::handle can pass the target_address to the callback and the tests can compare against the addresses rather than comparing breakpoints or breakpoint handles.

And we’ve already completed “The user can just refer to breakpoints by target address” and “breakpoint::handle can pass the target_address to the callback”. Though there are some simplifications we can make. find_breakpoint_matching_inferior_instruction_pointer doesn’t need to return Option<(TrapBreakpoint, Breakpoint)> since TrapBreakpoint is now Breakpoint::target_address. I’ll make that change now.

modified   src/breakpoint/mod.rs
@@ -31,11 +31,11 @@ fn set(inferior: TrapInferior, bp: Breakpoint) {
     poke_text(inferior, bp.aligned_address, modified);
 }
 
-fn find_breakpoint_matching_inferior_instruction_pointer(inf: Inferior) -> Option<(TrapBreakpoint, Breakpoint)> {
+fn find_breakpoint_matching_inferior_instruction_pointer(inf: Inferior) -> Option<Breakpoint> {
     let ip = ptrace_util::get_instruction_pointer(inf.pid).as_i64();
     unsafe { for bp in &GLOBAL_BREAKPOINTS {
 	if bp.target_address.as_i64() == ip - 1 {
-	    return Some((bp.target_address, *bp))
+	    return Some(*bp)
 	}
     } };
     None
@@ -46,7 +46,7 @@ where
     F: FnMut(TrapInferior, TrapBreakpoint),
 {
     let inferior = inf.pid;
-    let (user_breakpoint, bp) =
+    let bp =
         find_breakpoint_matching_inferior_instruction_pointer(inf)
         .expect("Could not find breakpoint");
 
@@ -54,7 +54,7 @@ where
         InferiorState::Running => (),
         _ => panic!("Unhandled error in breakpoint::handle"),
     }
-    callback(inferior, user_breakpoint);
+    callback(inferior, bp.target_address);
     step_over(inferior, bp);
     return match waitpid(Pid::from_raw(inf.pid), None) {
         Ok(WaitStatus::Stopped(_pid, signal::SIGTRAP)) => {

Ok, and that still passes and still has 7 warnings but these two design items are completely done.

Next, let’s add a map to the inferior. I started simply with this:

#[derive(Copy, Clone)]
pub struct Inferior {
    pub pid: pid_t,
    pub state: InferiorState,
    pub breakpoints: HashMap<InferiorPointer, Breakpoint>  // <== Added this
}

but this doesn’t build because I’ve dervied the Copy trait:

error[E0204]: the trait `Copy` cannot be implemented for this type
  --> src/inferior/mod.rs:16:10
   |
16 | #[derive(Copy, Clone)]
   |          ^^^^
...
20 |     pub breakpoints: HashMap<InferiorPointer, Breakpoint>
   |     ----------------------------------------------------- this field does not implement `Copy`

This makes sense, we don’t want to copy the map. So I should remove Copy, what about Clone?

Differs from Copy in that Copy is implicit and an inexpensive bit-wise copy, while Clone is always explicit and may or may not be expensive. [1]

Right, this is the deep copy. I guess there is no harm in deriving this. I’ll remove Copy and see what the fallout is. And it begins here:

error[E0382]: use of moved value: `inf`
  --> src/breakpoint/mod.rs:69:25
   |
44 | pub fn handle<F>(inf: Inferior, callback: &mut F) -> InferiorState
   |                  --- move occurs because `inf` has type `inferior::Inferior`, which does not implement the `Copy` trait
...
50 |         find_breakpoint_matching_inferior_instruction_pointer(inf)
   |                                                               --- value moved here
...
69 |                 signal, inf.state as i32
   |                         ^^^^^^^^^ value used here after move
   |
note: consider changing this parameter type in function `find_breakpoint_matching_inferior_instruction_pointer` to borrow instead if owning the value isn't necessary
   |
34 | fn find_breakpoint_matching_inferior_instruction_pointer(inf: Inferior) -> Option<B...
   |    -----------------------------------------------------      ^^^^^^^^ this parameter takes ownership of the value
   |    |
   |    in this function
help: consider cloning the value if the performance cost is acceptable
   |
50 |         find_breakpoint_matching_inferior_instruction_pointer(inf.clone())
   |                                                                  ++++++++

Ok, so find_breakpoint_matching_inferior_instruction_pointer can either borrow or I can pass a clone. find_breakpoint_matching_inferior_instruction_pointer doesn’t need to own the inferior it just needs to use it to look up the instruction pointer. Oh, it will use the new breakpoints hashmap as well. So, it should borrow, immutably. Ok, that’s easy enough:

modified   src/breakpoint/mod.rs
@@ -31,7 +31,7 @@ fn set(inferior: TrapInferior, bp: Breakpoint) {
     poke_text(inferior, bp.aligned_address, modified);
 }
 
-fn find_breakpoint_matching_inferior_instruction_pointer(inf: Inferior) -> Option<Breakpoint> {
+fn find_breakpoint_matching_inferior_instruction_pointer(inf: &Inferior) -> Option<Breakpoint> {
     let ip = ptrace_util::get_instruction_pointer(inf.pid).as_i64();
     unsafe { for bp in &GLOBAL_BREAKPOINTS {
 	if bp.target_address.as_i64() == ip - 1 {

And then fix the call site as well.

Then we have another similar error:

error[E0382]: assign to part of moved value: `inf`
  --> src/lib.rs:88:9
   |
85 |       let mut inf = unsafe { global_inferior };
   |           ------- move occurs because `inf` has type `inferior::Inferior`, which does not implement the `Copy` trait
86 |       ptrace_util::cont(inf.pid);
87 |       loop {
   |       ---- inside of this loop
88 | /         inf.state = match waitpid(Pid::from_raw(inf.pid), None) {
89 | |             Ok(WaitStatus::Exited(_pid, code)) => return code,
90 | |             Ok(WaitStatus::Stopped(_pid, signal::SIGTRAP)) => breakpoint::handle(inf, c...
   | |                                                                                  --- value moved here
91 | |             Ok(WaitStatus::Stopped(_pid, signal)) => {
...  |
98 | |             Err(_) => panic!("Unhandled error in trap_inferior_continue"),
99 | |         };
   | |_________^ value partially assigned here after move
   |
note: consider changing this parameter type in function `handle` to borrow instead if owning the value isn't necessary
  --> src/breakpoint/mod.rs:44:23
   |
44 | pub fn handle<F>(inf: Inferior, callback: &mut F) -> InferiorState
   |        ------         ^^^^^^^^ this parameter takes ownership of the value
   |        |
   |        in this function
help: consider cloning the value if the performance cost is acceptable
   |
90 |             Ok(WaitStatus::Stopped(_pid, signal::SIGTRAP)) => breakpoint::handle(inf.clone(), callback),
   |                                                                                     ++++++++

Ah, ok this is dealing with the global_inferior. One option is to just compute the state and then unsafe assign into the global_inferior. But I think we will be getting rid of global_inferior soon because there are a number of other errors and warnings related to it. The main set of issues being related to initializing global_inferior in the first place. I’ve added this line to try to initialize breakpoints but it fails:

@@ -26,6 +27,7 @@ use breakpoint::TrapBreakpoint;
 static mut global_inferior: Inferior = Inferior {
     pid: 0,
     state: InferiorState::Stopped,
+    breakpoints: HashMap::new(),
 };

error[E0015]: cannot call non-const associated function `HashMap::<inferior::InferiorPointer, Breakpoint>::new` in statics
  --> src/lib.rs:30:18
   |
30 |     breakpoints: HashMap::new(),
   |                  ^^^^^^^^^^^^^^
   |
   = note: calls in statics are limited to constant functions, tuple structs and tuple variants
   = note: consider wrapping this expression in `std::sync::LazyLock::new(|| ...)`

And also here, I haven’t initialized breakpoints but don’t expect it to go well:

error[E0063]: missing field `breakpoints` in initializer of `inferior::Inferior`
  --> src/lib.rs:58:69
   |
58 |         Ok(WaitStatus::Stopped(pid, signal::Signal::SIGTRAP)) => Ok(Inferior {
   |                                                                     ^^^^^^^^ missing `breakpoints`

Well, maybe this one would be easier to initialize since it isn’t static global. But, I think the right option here is to dynamically allocate the inferior and return it (and its ownership) to the user. Then borrow it from them during these operations. Hmm, is that right?

If the user passes it into trap_inferior_continue then rusty_trap is borrowing it from them.
When rusty_trap calls the callback it lends it back to the user.
So the user won’t be able to reference their original copy.
The tests won’t be able to write: assert_eq!(passed_inferior, inferior);
- Instead it could compare the process ids.
Would a real app want to compare?
- I guess it would as that’s the only way for it to know which breakpoint is which.

Note, all of this depends on the app needing more than one inferior which might be rare but is an intended feature of the debugger. This gets into what the purpose of a debugger like this is for. I’ll come back to that at the end of the post.

But let’s move forward with dynamically allocating the inferior. First, a function, new, to intialize the Inferior.

#[derive(Copy, Clone, PartialEq, PartialOrd, Debug)]
pub struct InferiorPointer(pub u64);
impl InferiorPointer {
    // ...
    pub fn new(pid: pid_t) -> Inferior {
	    Inferior {
			pid,
			state: InferiorState::Stopped,
			breakpoints: HashMap::new(),
		}
    }
}

Then the rest:

modified   src/breakpoint/mod.rs
@@ -18,17 +18,17 @@ pub type TrapBreakpoint = InferiorPointer;
 
 static mut GLOBAL_BREAKPOINTS: Vec<Breakpoint> = Vec::<Breakpoint>::new();
 
-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) {
+    poke_text(inferior.pid, bp.aligned_address, bp.original_breakpoint_word);
+    set_instruction_pointer(inferior.pid, bp.target_address);
+    single_step(inferior.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);
 }
 
 fn find_breakpoint_matching_inferior_instruction_pointer(inf: &Inferior) -> Option<Breakpoint> {
@@ -41,32 +41,31 @@ fn find_breakpoint_matching_inferior_instruction_pointer(inf: &Inferior) -> Opti
     None
 }
 
-pub fn handle<F>(inf: Inferior, callback: &mut F) -> InferiorState
+pub fn handle<F>(inferior: &mut Inferior, callback: &mut F) -> InferiorState
 where
-    F: FnMut(TrapInferior, TrapBreakpoint),
+    F: FnMut(&TrapInferior, TrapBreakpoint),
 {
-    let inferior = inf.pid;
     let bp =
-        find_breakpoint_matching_inferior_instruction_pointer(&inf)
+        find_breakpoint_matching_inferior_instruction_pointer(inferior)
         .expect("Could not find breakpoint");
 
-    match inf.state {
+    match inferior.state {
         InferiorState::Running => (),
         _ => panic!("Unhandled error in breakpoint::handle"),
     }
     callback(inferior, bp.target_address);
     step_over(inferior, bp);
-    return match waitpid(Pid::from_raw(inf.pid), None) {
+    return match waitpid(Pid::from_raw(inferior.pid), None) {
         Ok(WaitStatus::Stopped(_pid, signal::SIGTRAP)) => {
             set(inferior, bp);
-            cont(inferior);
+            cont(inferior.pid);
             InferiorState::Running
         }
         Ok(WaitStatus::Exited(_pid, _code)) => InferiorState::Running,
         Ok(WaitStatus::Stopped(_pid, signal)) => {
             panic!(
                 "Unexpected stop on signal {} in breakpoint::handle.  State: {}",
-                signal, inf.state as i32
+                signal, inferior.state as i32
             )
         }
         Ok(_) => panic!("Unexpected stop in breakpoint::handle"),
@@ -74,14 +73,14 @@ where
     };
 }
 
-pub fn trap_inferior_set_breakpoint(inferior: TrapInferior, location: u64) -> TrapBreakpoint {
+pub fn trap_inferior_set_breakpoint(inferior: &TrapInferior, location: u64) -> TrapBreakpoint {
     let aligned_address = location & !0x7u64;
     let index = unsafe {
 	GLOBAL_BREAKPOINTS.push(Breakpoint {
             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)),
 	});
 	GLOBAL_BREAKPOINTS.len() - 1
     };
modified   src/inferior/mod.rs
@@ -17,10 +17,20 @@ pub enum InferiorState {
 pub struct Inferior {
     pub pid: pid_t,
     pub state: InferiorState,
-    pub breakpoints: HashMap<InferiorPointer, Breakpoint>  // Added this
+    pub breakpoints: HashMap<InferiorPointer, Breakpoint>
 }
 
-pub type TrapInferior = pid_t;
+impl Inferior {
+    pub fn new(pid: pid_t) -> Inferior {
+	Inferior {
+	    pid,
+	    state: InferiorState::Stopped,
+	    breakpoints: HashMap::new(),
+	}
+    }
+}
+
+pub type TrapInferior = Inferior;
 
 #[derive(Copy, Clone, PartialEq, PartialOrd, Debug)]
 pub struct InferiorPointer(pub u64);
modified   src/lib.rs
@@ -10,7 +10,6 @@ use nix::{
     unistd::{execve, fork, ForkResult},
     Error,
 };
-use std::collections::HashMap;
 use std::ffi::{CStr, CString};
 use std::path::Path;
 
@@ -24,12 +23,6 @@ mod breakpoint;
 pub use self::breakpoint::trap_inferior_set_breakpoint;
 use breakpoint::TrapBreakpoint;
 
-static mut global_inferior: Inferior = Inferior {
-    pid: 0,
-    state: InferiorState::Stopped,
-    breakpoints: HashMap::new(),
-};
-
 fn disable_address_space_layout_randomization() {
     unsafe {
         let old = libc::personality(0xffffffff);
@@ -55,49 +48,46 @@ fn exec_inferior(filename: &Path, args: &[&str]) {
 fn attach_inferior(raw_pid: pid_t) -> Result<Inferior, Error> {
     let nix_pid = Pid::from_raw(raw_pid);
     match waitpid(nix_pid, None) {
-        Ok(WaitStatus::Stopped(pid, signal::Signal::SIGTRAP)) => Ok(Inferior {
-            pid: pid.into(),
-            state: InferiorState::Running,
-        }),
+        Ok(WaitStatus::Stopped(pid, signal::Signal::SIGTRAP)) => Ok(Inferior::new(pid.into())),
         Ok(_) => panic!("Unexpected stop in attach_inferior"),
         Err(e) => Err(e),
     }
 }
 
-pub fn trap_inferior_exec(filename: &Path, args: &[&str]) -> Result<TrapInferior, Error> {
+pub fn trap_inferior_exec(filename: &Path, args: &[&str]) -> Result<Inferior, Error> {
     loop {
         match unsafe { fork() } {
-            Ok(ForkResult::Child) => exec_inferior(filename, args),
+            Ok(ForkResult::Child) => {
+		exec_inferior(filename, args);
+		unreachable!();
+	    }
             Ok(ForkResult::Parent { child: pid }) => {
-                unsafe { global_inferior = attach_inferior(pid.into()).ok().unwrap() };
-                return Ok(pid.into());
-            }
+		return attach_inferior(pid.into())
+	    }
             Err(Error::EAGAIN) => continue,
             Err(e) => return Err(e),
         }
     }
 }
 
-pub fn trap_inferior_continue<F>(inferior: TrapInferior, callback: &mut F) -> i32
+pub fn trap_inferior_continue<F>(inferior: &mut TrapInferior, callback: &mut F) -> i32
 where
-    F: FnMut(TrapInferior, TrapBreakpoint),
+    F: FnMut(&TrapInferior, TrapBreakpoint),
 {
-    let mut inf = unsafe { global_inferior };
-    ptrace_util::cont(inf.pid);
+    ptrace_util::cont(inferior.pid);
     loop {
-        inf.state = match waitpid(Pid::from_raw(inf.pid), None) {
+        inferior.state = match waitpid(Pid::from_raw(inferior.pid), None) {
             Ok(WaitStatus::Exited(_pid, code)) => return code,
-            Ok(WaitStatus::Stopped(_pid, signal::SIGTRAP)) => breakpoint::handle(inf, callback),
+            Ok(WaitStatus::Stopped(_pid, signal::SIGTRAP)) =>
+		breakpoint::handle(inferior, callback),
             Ok(WaitStatus::Stopped(_pid, signal)) => {
                 panic!(
                     "Unexpected stop on signal {} in trap_inferior_continue.  State: {}",
-                    signal, inf.state as i32
+                    signal, inferior.state as i32
                 )
             }
             Ok(_) => panic!("Unexpected stop in trap_inferior_continue"),
             Err(_) => panic!("Unhandled error in trap_inferior_continue"),
         };
-
-        unsafe { global_inferior = inf };
     }
 }

And then we have the problem I expected that the tests can’t reference the inferior because they lent it to trap_inferior_continue:

error[E0502]: cannot borrow `inferior` as immutable because it is also borrowed as mutable
  --> tests/lib.rs:24:60
   |
24 |     rusty_trap::trap_inferior_continue(&mut inferior, &mut |passed_inferior, passed_bp| {
   |     ---------------------------------- -------------       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here
   |     |                                  |
   |     |                                  mutable borrow occurs here
   |     mutable borrow later used by call
25 |         assert_eq!(passed_inferior.pid, inferior.pid);
   |                                         ------------ second borrow occurs due to use of `inferior` in closure

Ok, good well at least I’m understanding Rust well enough to have expected this. For now, the test will need to copy out the pid before lending the inferior back to rusty_trap

And with that, the tests pass. And, we are down to only 4 warnings!

warning: unused variable: `args`
  --> src/lib.rs:33:35
   |
33 | fn exec_inferior(filename: &Path, args: &[&str]) {
   |                                   ^^^^ help: if this is intentional, prefix it with an underscore: `_args`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: creating a shared reference to mutable static
  --> src/breakpoint/mod.rs:36:24
   |
36 |     unsafe { for bp in &GLOBAL_BREAKPOINTS {
   |                        ^^^^^^^^^^^^^^^^^^^ shared reference to mutable static
   |
   = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
   = note: shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives
   = note: `#[warn(static_mut_refs)]` on by default
help: use `&raw const` instead to create a raw pointer
   |
36 |     unsafe { for bp in &raw const GLOBAL_BREAKPOINTS {
   |                         +++++++++

warning: creating a mutable reference to mutable static
  --> src/breakpoint/mod.rs:79:2
   |
79 | /     GLOBAL_BREAKPOINTS.push(Breakpoint {
80 | |             shift: (location - aligned_address) * 8,
81 | |             aligned_address: InferiorPointer(aligned_address),
82 | |             target_address: InferiorPointer(location),
83 | |             original_breakpoint_word: peek_text(inferior.pid, InferiorPointer(aligned_address)),
84 | |     });
   | |______^ mutable reference to mutable static
   |
   = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
   = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives

warning: creating a shared reference to mutable static
  --> src/breakpoint/mod.rs:85:2
   |
85 |     GLOBAL_BREAKPOINTS.len() - 1
   |     ^^^^^^^^^^^^^^^^^^^^^^^^ shared reference to mutable static
   |
   = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
   = note: shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives

warning: `rusty_trap` (lib) generated 4 warnings
warning: `rusty_trap` (lib test) generated 4 warnings (4 duplicates)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.36s
     Running unittests src/lib.rs (/home/jkain/projects/rusty_trap/target/debug/deps/rusty_trap-dd302d51f38b1a8c)

running 4 tests
test it_can_exec ... ok
test it_can_handle_a_breakpoint_more_than_once ... ok
test it_can_handle_more_than_one_breakpoint ... ok
test it_can_set_breakpoints ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Ok, so these warnings all about GLOBAL_BREAKPOINTS. We have added the breakpoints HashMap to the inferior but aren’t using it yet. Let’s do that now.

The main change is to update find_breakpoint_matching_inferior_instruction_pointer like this:

-fn find_breakpoint_matching_inferior_instruction_pointer(inf: &Inferior) -> Option<Breakpoint> {
-    let ip = ptrace_util::get_instruction_pointer(inf.pid).as_i64();
-    unsafe { for bp in &GLOBAL_BREAKPOINTS {
-	if bp.target_address.as_i64() == ip - 1 {
-	    return Some(*bp)
-	}
-    } };
-    None
+fn find_breakpoint_matching_inferior_instruction_pointer(inf: &Inferior) -> Option<&Breakpoint> {
+    let InferiorPointer(ip) = get_instruction_pointer(inf.pid);
+    let ip = InferiorPointer(ip - 1);
+    return inf.breakpoints.get(&ip);
 }

Then just some minor changes to change Breakpoint to &Breakpoint, etc. And finally, we remove GLOBAL_BREAKPOINTS from everything else:

modified   src/breakpoint/mod.rs
@@ -67,19 +65,17 @@ where
     };
 }
 
-pub fn trap_inferior_set_breakpoint(inferior: &TrapInferior, location: u64) -> TrapBreakpoint {
+pub fn trap_inferior_set_breakpoint(inferior: &mut TrapInferior, location: u64) -> TrapBreakpoint {
     let aligned_address = location & !0x7u64;
-    let index = unsafe {
-	GLOBAL_BREAKPOINTS.push(Breakpoint {
-            shift: (location - aligned_address) * 8,
-            aligned_address: InferiorPointer(aligned_address),
-            target_address: InferiorPointer(location),
-            original_breakpoint_word: peek_text(inferior.pid, InferiorPointer(aligned_address)),
-	});
-	GLOBAL_BREAKPOINTS.len() - 1
-    };
+    let target_address = InferiorPointer(location);
+    inferior.breakpoints.insert(target_address, Breakpoint {
+        shift: (location - aligned_address) * 8,
+        aligned_address: InferiorPointer(aligned_address),
+        target_address,
+        original_breakpoint_word: peek_text(inferior.pid, InferiorPointer(aligned_address)),
+    });
 
-    set(inferior, unsafe {GLOBAL_BREAKPOINTS[index]});
+    set(inferior, inferior.breakpoints.get(&target_address).unwrap());
 
     InferiorPointer(location)
 }
modified   tests/lib.rs
@@ -21,7 +21,7 @@ fn it_can_set_breakpoints() {
     let mut inferior =
         rusty_trap::trap_inferior_exec(Path::new("./target/debug/twelve"), &[]).unwrap();
     let expected_pid = inferior.pid;
-    let bp = rusty_trap::trap_inferior_set_breakpoint(&inferior, 0x000055555555b821);
+    let bp = rusty_trap::trap_inferior_set_breakpoint(&mut inferior, 0x000055555555b821);
     rusty_trap::trap_inferior_continue(&mut inferior, &mut |passed_inferior, passed_bp| {
         assert_eq!(passed_inferior.pid, expected_pid);
         assert_eq!(passed_bp, bp);
@@ -38,7 +38,7 @@ fn it_can_handle_a_breakpoint_more_than_once() {
     let mut inferior =
         rusty_trap::trap_inferior_exec(Path::new("./target/debug/loop"), &[]).unwrap();
     let expected_pid = inferior.pid;
-    let bp = rusty_trap::trap_inferior_set_breakpoint(&inferior, ADDRESS_OF_FOO);
+    let bp = rusty_trap::trap_inferior_set_breakpoint(&mut inferior, ADDRESS_OF_FOO);
     rusty_trap::trap_inferior_continue(&mut inferior, &mut |passed_inferior, passed_bp| {
         assert_eq!(passed_inferior.pid, expected_pid);
         assert_eq!(passed_bp, bp);
@@ -56,8 +56,8 @@ fn it_can_handle_more_than_one_breakpoint() {
     let mut inferior =
         rusty_trap::trap_inferior_exec(Path::new("./target/debug/loop"), &[]).unwrap();
     let expected_pid = inferior.pid;
-    let bp_main = rusty_trap::trap_inferior_set_breakpoint(&inferior, ADDRESS_OF_MAIN);
-    let bp_foo = rusty_trap::trap_inferior_set_breakpoint(&inferior, ADDRESS_OF_FOO);
+    let bp_main = rusty_trap::trap_inferior_set_breakpoint(&mut inferior, ADDRESS_OF_MAIN);
+    let bp_foo = rusty_trap::trap_inferior_set_breakpoint(&mut inferior, ADDRESS_OF_FOO);
     rusty_trap::trap_inferior_continue(&mut inferior, &mut |passed_inferior, passed_bp| {
         assert_eq!(passed_inferior.pid, expected_pid);
         if passed_bp == bp_main {

And the tests pass and there is only one warning left:

warning: unused variable: `args`
  --> src/lib.rs:33:35
   |
33 | fn exec_inferior(filename: &Path, args: &[&str]) {
   |                                   ^^^^ help: if this is intentional, prefix it with an underscore: `_args`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: `rusty_trap` (lib) generated 1 warning

Ok, I guess this was a placeholder I left in case the user wants to pass arguments to the inferior. In general this should be supported but I’m not going to build it and it’s not really that interesting for this blog. So, I’m just going to take the compiler’s advice and rename the argument _args_. If you have an interest in this please go ahead and implement it.

And now, it’s clean. And not only that, the tests pass without the --test-threads 1 option. Nice.

Aside on API design?

Now, I say it’s clean meaning it compiled without any warnings. But I don’t really like the API, it doesn’t feel clean to me. For you rustaceans that have a better knowledge of idiomatic Rust than I do, what’s the right way to create APIs like trap_inferior_exec? I’m dissatisfied with the way it is now because it forces the user to have a mutable Inferior just so that it can pass it back into other rusty_trap functions. The user doesn’t really want to know what’s in there or modify anything. They would depend on rusty_trap to do that for them. So I’d like to return at the very least an immutable object. I guess it’s mutable because they expect rusty_trap to mutate it. Let me know in the comments what you think about this or if you have an suggestions for a cleaner API.

But for now I’ve pushed and merged a PR:

Conclusion: From Global State to Ownership

In this post, we’ve seen how implementing multiple breakpoints in Rust pushed us to reconsider our design approach fundamentally. What started as a seemingly straightforward port from C evolved into an exploration of Rust’s ownership principles and safer design patterns.

Throughout this journey, I applied the question and answer investigation approach to systematically break down problems:

Breakpoints:

Who uses them / who needs to access them?

Who should own them? Or who should control their lifetime?

Should the user decide if it is deallocated? Should it ever be deallocated?

This structured questioning helped clarify requirements and guide us toward better solutions. When I hit roadblocks, I asked specific questions about ownership relationships and data flow, letting the answers drive implementation decisions.

Our journey took us from:

A design with problematic global state and unsafe code
Through a failing test that exposed the limitations of that approach
To a cleaner design with explicit ownership of breakpoints tied to their inferior

The key lessons we learned include:

Global state is particularly problematic in Rust, which actively encourages you to make relationships explicit through the type system
Rust’s ownership model guides you toward designs where data relationships are clear and statically verified.
Even temporary “brute force” solutions can help you make incremental progress toward a better design
Asking the right questions about ownership and responsibility drives better architecture decisions

While our current API still has room for improvement, we’ve eliminated most of the unsafe code and made the relationship between inferiors and their breakpoints much more explicit. The tests now pass reliably without special handling for concurrency issues - a testament to the benefits of thoughful ownership.

What’s Next?

Looking ahead, there are several opportunities to improve our debugger:

Symbol tables: We’ll need to parse the inferiors symbol table to make function names to addresses rather than the fragile hard coding of addresses that we have now.
Dynamic library support: Using the debugger hook for loading and debugging dynamic libraries.
Multiple threads: Adding support for debugging multi-threaded applications.

Symbol tables will likely be next major topic, as they’re essential for providing a usable debugging experience. The Rust ecosystem’s strong support for parsing binary formats should make this an interesting and productive exploration.

Through this series, we’re not just learning how to build a debugger – we’re discovering how Rust’s design principles can guide us toward more robust software architecture decisions. The ownership-driven design we’ve developed here will continue to evolve as we add more complex debugging capabilities.

This post is part of my Writing a Debugger series.
View all posts and upcoming topics →