This post is an exercise in refactoring Rust code. I’ll be clearning up code written a few posts ago for rusty_trap. In the process I’ll explore a few topics that are new to me: operator overloading and other traits.

This post is part my larger series on writing a debugger in C and Rust. If you haven’t read the earlier posts you may want to go back and start from the beginning.

Cleanup

The first thing I want to do is extract ptrace_util functions to provide a better interface around ptrace. The existing ptrace code is terrible to look at:

ptrace(PTRACE_POKETEXT, inferior, target_address as * mut c_void,
           original as * mut c_void)
        .ok()
        .expect("Failed PTRACE_POKETEXT");

and defeats a lot of Rust’s type checking. If I write wrapper functions I can enforce the type safety up to the wrapper. I started by creating a new file, lib/ptrace_util/mod.rs. First, I’ll move the ptrace_util_get_instruction_pointer and ptrace_util_set_instruction_pointer functions and the user::reg offsets to the new file. I also changed the names to just get_instruction_pointer and set_instruction_pointer since they are now enclosed in a module called ptrace_util. Corespondingly, I changed the calls to ptrace_util::get_instruction_pointer and ptrace_util::set_instruction_pointer. Finally, I ran the tests and the tests passed.

Next, I look at PTRACE_CONT

ptrace(PTRACE_CONT, pid, ptr::null_mut(), ptr::null_mut())
    .ok()
    .expect("Failed PTRACE_CONTINUE");

The only real argument to ptrace here is pid so the utility function has this signature:

pub fn continue(pid: pid_t) -> ();

There is PTRACE_TRACEME which is simply:

pub fn trace_me() -> () {
    ptrace(PTRACE_TRACEME, 0, ptr::null_mut(), ptr::null_mut())
        .ok()
        .expect("Failed PTRACE_TRACEME");
}

PTRACE_POKETEXT is a more intersting example

ptrace(PTRACE_POKETEXT, inferior, target_address as * mut c_void,
       original as * mut c_void)
    .ok()
    .expect("Failed PTRACE_POKETEXT");

all the arguments are used. We need a pid, an address at which to poke and a value to poke. PTRACE_PEEKTEXT is similar:

pub fn poke_text(pid: pid_t, address: usize, value: i64) -> () {
    ptrace(PTRACE_POKETEXT, pid, address as * mut c_void, value as * mut c_void)
        .ok()
        .expect("Failed PTRACE_POKETEXT");
}

After moving all these functions and updating the code to use them I ran to tests to insure they still pass.

The next step is to work out the right types for the value and addresses in peek and poke. I need to be more precise about the types of addresses. This is something I glossed over in the C version. In C proper types are easy to overlook, but in Rust it just feels wrong. I’ve saved this work so that we’ve already finished the initial refactoring. Extracting out these functions using the existing types for addresses gives us a better sense of what we have so we can better figure out where we want to go.

The most interesting type to be aware of are the pointers that are used within the the inferior. These are different than pointers within the debugger itself for example we could have a 64 bit version of rusty_trap debugging a 32 bit program. The ptrace C API uses void * to represent this but this isn’t what we want to enforce. In safe Rust code we will define our own type. Then within the ptrace wrapper we can conver to void * as needed.

I’ll use the newtype pattern to create an InferiorPointer type which is backed by a u64.

pub struct InferiorPointer(pub u64);

The next step is to use this in the ptrace utility functions. First, I’ll update ptrace_util::get_instruction_pointer:

pub fn get_instruction_pointer(pid: pid_t) -> InferiorPointer {
    return ptrace(PTRACE_PEEKUSER, pid, user::regs::RIP as * mut c_void, ptr::null_mut())
        .ok()
        .expect("Failed PTRACE_PEEKUSER") as InferiorPointer;
}

But this is not enough to keep the project compiling:

   Compiling rusty_trap v0.1.0 (file:///home/joseph/src/rust/rusty_trap)
src/ptrace_util.rs:48:12: 50:61src/ptrace_util.rs:48:12: 50:61  error: error: non-scalar cast: `i64` as `ptrace_util::InferiorPointer`non-scalar cast: `i64` as `ptrace_util::InferiorPointer`

src/ptrace_util.rssrc/ptrace_util.rs:48:48      return ptrace(PTRACE_PEEKUSER, pid, user::regs::RIP as * mut c_void, ptr::null_mut())    return ptrace(PTRACE_PEEKUSER, pid, user::regs::RIP as * mut c_void, ptr::null_mut())

src/ptrace_util.rssrc/ptrace_util.rs::49 49         .ok()        .ok()
src/ptrace_util.rs
src/ptrace_util.rs::50 50         .expect("Failed PTRACE_PEEKUSER") as InferiorPointer;
        .expect("Failed PTRACE_PEEKUSER") as InferiorPointer;
src/lib.rs:87:26: 87:72src/lib.rs:87:26: 87:72  error: error: binary operation `-` cannot be applied to type `ptrace_util::InferiorPointer`binary operation `-` cannot be applied to type `ptrace_util::InferiorPointer` [E0369] [E0369]

src/lib.rssrc/lib.rs::8787      let target_address = ptrace_util::get_instruction_pointer(inferior) - 1;    let target_address = ptrace_util::get_instruction_pointer(inferior) - 1;

                                                                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I want to be able to perform arithmetic on InferiorPointer. I can implement the std::ops::Add and std::ops::Sub traits for InferiorPointer:

pub struct InferiorPointer(pub u64);
impl Add<i64> for InferiorPointer {
    type Output = InferiorPointer;
    fn add(self, rhs: i64) -> InferiorPointer {
        let InferiorPointer(u) = self;
        if rhs >= 0 {
            InferiorPointer(u + rhs as u64)
        } else {
            InferiorPointer(u - rhs as u64)
        }
    }
}
impl Sub<i64> for InferiorPointer {
    type Output = InferiorPointer;
    fn sub(self, rhs: i64) -> InferiorPointer {
        let InferiorPointer(u) = self;
        if rhs >= 0 {
            InferiorPointer(u - rhs as u64)
        } else {
            InferiorPointer(u + rhs as u64)
        }
    }
}

This fixes the compilation error I posted above but things still don’t compile:

   Compiling rusty_trap v0.1.0 (file:///home/joseph/src/rust/rusty_trap)
src/ptrace_util.rs:71:12: 73:61src/ptrace_util.rs:71:12: 73:61  error: error: non-scalar cast: `i64` as `ptrace_util::InferiorPointer`
non-scalar cast: `i64` as `ptrace_util::InferiorPointer`src/ptrace_util.rs:71
    return ptrace(PTRACE_PEEKUSER, pid, user::regs::RIP as * mut c_void, ptr::null_mut())
src/ptrace_util.rs:72 src/ptrace_util.rs:        .ok()
src/ptrace_util.rs71:73         .expect("Failed PTRACE_PEEKUSER") as InferiorPointer;     return ptrace(PTRACE_PEEKUSER, pid, user::regs::RIP as * mut c_void, ptr::null_mut())

src/ptrace_util.rs:72         .ok()
src/ptrace_util.rs:73         .expect("Failed PTRACE_PEEKUSER") as InferiorPointer;
src/lib.rs:88:38: 88:52 error: mismatched types:
 expected `usize`,
    found `ptrace_util::InferiorPointer`
(expected usize,
    found struct `ptrace_util::InferiorPointer`) [E0308]
src/lib.rs:88     ptrace_util::poke_text(inferior, target_address, original);

I can’t just cast to InferiorPointer, I need to construct one like this:

pub fn get_instruction_pointer(pid: pid_t) -> InferiorPointer {
    let raw = ptrace(PTRACE_PEEKUSER, pid, user::regs::RIP as * mut c_void,
                     ptr::null_mut())
        .ok()
        .expect("Failed PTRACE_PEEKUSER");
    InferiorPointer(raw as u64)
}

This helps but of course there are still more problems to fix:

   Compiling rusty_trap v0.1.0 (file:///home/joseph/src/rust/rusty_trap)
src/lib.rs:88:38: 88:52src/lib.rs:88:38: 88:52  error: error: mismatched types:
 expected `usize`,
    found `ptrace_util::InferiorPointer`
(expected usize,
    found struct `ptrace_util::InferiorPointer`) [E0308]mismatched types:
 expected `usize`,
    found `ptrace_util::InferiorPointer`
(expected usize,
    found struct `ptrace_util::InferiorPointer`)
 [E0308]src/lib.rs:
88 src/lib.rs:    ptrace_util::poke_text(inferior, target_address, original);
88     ptrace_util::poke_text(inferior, target_address, original);                                                   
                                                   ^~~~~~~~~~~~~~^~~~~~~~~~~~~~

To fix this I need to update ptrace_util::poke_text to take an InferiorPointer as the address rather than as usize. Here’s the code:

pub fn poke_text(pid: pid_t, address: InferiorPointer, value: i64) -> () {
    let InferiorPointer(raw_address) = address;

    ptrace(PTRACE_POKETEXT, pid, raw_address as * mut c_void, value as * mut c_void)
        .ok()
        .expect("Failed PTRACE_POKETEXT");
}

Note that I had to extract the raw address from the InferiorPointer using destructuring and then cast it to * mut c_void.

Following futrher compiler errors, I made similar changes to set_instruction_pointer and peek_text. I needed to update trap_inferior_set_breakpoint to create an InferiorPointer for the aligned address like this:

let original =
    ptrace_util::peek_text(inferior,
                           ptrace_util::InferiorPointer(aligned_address));

After this I end up with a borrow checker error:

Compiling rusty_trap v0.1.0 (file:///home/joseph/src/rust/rusty_trap)
src/lib.rs:92:52: 92:66 error: use of moved value: `target_address` [E0382]
src/lib.rs:92     ptrace_util::set_instruction_pointer(inferior, target_address);
                                                                 ^~~~~~~~~~~~~~
src/lib.rs:88:38: 88:52 note: `target_address` moved here because it has type `ptrace_util::InferiorPointer`, which is non-copyable
src/lib.rs:88     ptrace_util::poke_text(inferior, target_address, original);
                                                   ^~~~~~~~~~~~~~

Ah, my InferiorPointer type needs copy semantics. To do this I need to implement the copy trait which is as simple as:

#[derive(Copy, Clone)]
pub struct InferiorPointer(pub u64);

And with this the tests pass again.

It’s time now to clean up some duplication in the code we ended up with. I ended up with several functions that use the destructuring and casting pattern:

let InferiorPointer(raw_address) = address;
ptrace(op, pid, raw_address as * mut c_void, data);

This is used in 3 different places. It would be nice to abstract this duplicated functionality into a single implementation with an intention revealing name. The API I would like to have is simply:

ptrace(op, pid, address.as_voidptr(), data);

which can be implemented as:

impl InferiorPointer {
    fn as_voidptr(&self) -> * mut c_void {
        let &InferiorPointer(u) = self;
        u as * mut c_void
    }
}

After updating the code to use as_voidptr I made sure to run the tests again to make sure they still pass.

Finally, the InferiorPointer has become a significant portion of ptrace_util.rs and is not really related to ptrace. I will move InferiorPointer to its own file. Here’s the entire inferior_pointer.rs:

use libc::c_void;
use std::ops::{Add, Sub};

#[derive(Copy, Clone)]
pub struct InferiorPointer(pub u64);
impl InferiorPointer {
    pub fn as_voidptr(&self) -> * mut c_void {
        let &InferiorPointer(u) = self;
        u as * mut c_void
    }
}
impl Add<i64> for InferiorPointer {
    type Output = InferiorPointer;
    fn add(self, rhs: i64) -> InferiorPointer {
        let InferiorPointer(u) = self;
        if rhs >= 0 {
            InferiorPointer(u + rhs as u64)
        } else {
            InferiorPointer(u - rhs as u64)
        }
    }
}
impl Sub<i64> for InferiorPointer {
    type Output = InferiorPointer;
    fn sub(self, rhs: i64) -> InferiorPointer {
        let InferiorPointer(u) = self;
        if rhs >= 0 {
            InferiorPointer(u - rhs as u64)
        } else {
            InferiorPointer(u + rhs as u64)
        }
    }
}

Summary

Most of this post was motivated by refactoring in order to clean up a lot of things I didn’t like in the code I had at the end of the previous rust post. While working in Rust I find that code smells are much more pronounced than they are in C. I don’t know if this is because I’m new to Rust or if I’ve grown accustomed to duplication and type unsafety over the years working in C.

Either way I appreciate being able to look at code afresh with Rust and work to make the code cleaner and easier to follow. Also, the tests help me make sure things continue to work.

Next week I’ll continue working in Rust and will implement a state machine to formalize breakpoint handling.