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

Implement RegId for Mips and MSP430 #38

Merged
merged 7 commits into from Jan 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 32 additions & 2 deletions src/arch/mips/mod.rs
Expand Up @@ -9,7 +9,7 @@ pub mod reg;
///
/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
/// more info about the `RegIdImpl` type parameter.
pub enum Mips<RegIdImpl: RegId> {
pub enum Mips<RegIdImpl: RegId = reg::id::MipsRegId<u32>> {
#[doc(hidden)]
_Marker(core::marker::PhantomData<RegIdImpl>),
}
Expand All @@ -18,11 +18,17 @@ pub enum Mips<RegIdImpl: RegId> {
///
/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
/// more info about the `RegIdImpl` type parameter.
pub enum Mips64<RegIdImpl: RegId> {
pub enum Mips64<RegIdImpl: RegId = reg::id::MipsRegId<u64>> {
#[doc(hidden)]
_Marker(core::marker::PhantomData<RegIdImpl>),
}

/// Implements `Arch` for 32-bit MIPS with the DSP feature enabled.
pub enum MipsWithDsp {}

/// Implements `Arch` for 64-bit MIPS with the DSP feature enabled.
pub enum Mips64WithDsp {}

impl<RegIdImpl: RegId> Arch for Mips<RegIdImpl> {
type Usize = u32;
type Registers = reg::MipsCoreRegs<u32>;
Expand All @@ -42,3 +48,27 @@ impl<RegIdImpl: RegId> Arch for Mips64<RegIdImpl> {
Some(r#"<target version="1.0"><architecture>mips64</architecture></target>"#)
}
}

impl Arch for MipsWithDsp {
type Usize = u32;
type Registers = reg::MipsCoreRegsWithDsp<u32>;
type RegId = reg::id::MipsRegId<u32>;

fn target_description_xml() -> Option<&'static str> {
Some(
r#"<target version="1.0"><architecture>mips</architecture><feature name="org.gnu.gdb.mips.dsp"></feature></target>"#,
)
}
}

impl Arch for Mips64WithDsp {
type Usize = u64;
type Registers = reg::MipsCoreRegsWithDsp<u64>;
type RegId = reg::id::MipsRegId<u64>;

fn target_description_xml() -> Option<&'static str> {
Some(
r#"<target version="1.0"><architecture>mips64</architecture><feature name="org.gnu.gdb.mips.dsp"></feature></target>"#,
)
}
}
131 changes: 129 additions & 2 deletions src/arch/mips/reg/id.rs
@@ -1,2 +1,129 @@
// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
// pub enum MipsRegId {}
use crate::arch::RegId;

/// MIPS register identifier.
#[derive(Debug, Clone, Copy)]
#[non_exhaustive]
pub enum MipsRegId<U> {
/// General purpose registers (R0-R31)
Gpr(u8),
/// Status register
Status,
/// Low register
Lo,
/// High register
Hi,
/// Bad Virtual Address register
Badvaddr,
/// Exception Cause register
Cause,
/// Program Counter
Pc,
/// Floating point registers (F0-F31)
Fpr(u8),
/// Floating-point Control Status register
Fcsr,
/// Floating-point Implementation Register
Fir,
/// High 1 register
Hi1,
/// Low 1 register
Lo1,
/// High 2 register
Hi2,
/// Low 2 register
Lo2,
/// High 3 register
Hi3,
/// Low 3 register
Lo3,
/// DSP Control register
Dspctl,
/// Restart register
Restart,
#[doc(hidden)]
_Size(U),
}

fn from_raw_id<U>(id: usize) -> Option<(MipsRegId<U>, usize)> {
let reg = match id {
0..=31 => MipsRegId::Gpr(id as u8),
32 => MipsRegId::Status,
33 => MipsRegId::Lo,
34 => MipsRegId::Hi,
35 => MipsRegId::Badvaddr,
36 => MipsRegId::Cause,
37 => MipsRegId::Pc,
38..=69 => MipsRegId::Fpr((id as u8) - 38),
70 => MipsRegId::Fcsr,
71 => MipsRegId::Fir,
72 => MipsRegId::Hi1,
73 => MipsRegId::Lo1,
74 => MipsRegId::Hi2,
75 => MipsRegId::Lo2,
76 => MipsRegId::Hi3,
77 => MipsRegId::Lo3,
// `MipsRegId::Dspctl` is the only register that will always be 4 bytes wide
78 => return Some((MipsRegId::Dspctl, 4)),
79 => MipsRegId::Restart,
_ => return None,
};

let ptrsize = core::mem::size_of::<U>();
Some((reg, ptrsize))
}

impl RegId for MipsRegId<u32> {
fn from_raw_id(id: usize) -> Option<(Self, usize)> {
from_raw_id::<u32>(id)
}
}

impl RegId for MipsRegId<u64> {
fn from_raw_id(id: usize) -> Option<(Self, usize)> {
from_raw_id::<u64>(id)
}
}

#[cfg(test)]
mod tests {
use crate::arch::traits::RegId;
use crate::arch::traits::Registers;

fn test<Rs: Registers, RId: RegId>() {
// Obtain the data length written by `gdb_serialize` by passing a custom
// closure.
let mut serialized_data_len = 0;
let counter = |b: Option<u8>| {
if b.is_some() {
serialized_data_len += 1;
}
};
Rs::default().gdb_serialize(counter);

// Accumulate register sizes returned by `from_raw_id`.
let mut i = 0;
let mut sum_reg_sizes = 0;
while let Some((_, size)) = RId::from_raw_id(i) {
sum_reg_sizes += size;
i += 1;
}

assert_eq!(serialized_data_len, sum_reg_sizes);
}

#[test]
fn test_mips32() {
test::<
crate::arch::mips::reg::MipsCoreRegsWithDsp<u32>,
crate::arch::mips::reg::id::MipsRegId<u32>,
>()
}

#[test]
fn test_mips64() {
test::<
crate::arch::mips::reg::MipsCoreRegsWithDsp<u64>,
crate::arch::mips::reg::id::MipsRegId<u64>,
>()
}
}
128 changes: 121 additions & 7 deletions src/arch/mips/reg/mips.rs
@@ -1,16 +1,18 @@
use crate::arch::Registers;
use crate::internal::LeBytes;
use core::convert::TryInto;

use num_traits::PrimInt;

use crate::arch::Registers;
use crate::internal::LeBytes;

/// MIPS registers.
///
/// The register width is set to `u32` or `u64` based on the `<U>` type.
///
/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-cpu.xml
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct MipsCoreRegs<U> {
/// General purpose registers (R0-R32)
/// General purpose registers (R0-R31)
pub r: [U; 32],
/// Low register (regnum 33)
pub lo: U,
Expand Down Expand Up @@ -42,14 +44,50 @@ pub struct MipsCp0Regs<U> {
/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-fpu.xml
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct MipsFpuRegs<U> {
/// FP registers (F0-F32) starting at regnum 38
/// FP registers (F0-F31) starting at regnum 38
pub r: [U; 32],
/// Floating-point Control Status register
pub fcsr: U,
/// Floating-point Implementation Register
pub fir: U,
}

/// MIPS DSP registers.
///
/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-dsp.xml
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct MipsDspRegs<U> {
/// High 1 register (regnum 72)
pub hi1: U,
/// Low 1 register (regnum 73)
pub lo1: U,
/// High 2 register (regnum 74)
pub hi2: U,
/// Low 2 register (regnum 75)
pub lo2: U,
/// High 3 register (regnum 76)
pub hi3: U,
/// Low 3 register (regnum 77)
pub lo3: U,
/// DSP Control register (regnum 78)
/// Note: This register will always be 32-bit regardless of the target
/// https://sourceware.org/gdb/current/onlinedocs/gdb/MIPS-Features.html#MIPS-Features
pub dspctl: u32,
/// Restart register (regnum 79)
pub restart: U,
}

/// MIPS core and DSP registers.
///
/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-dsp-linux.xml
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct MipsCoreRegsWithDsp<U> {
/// Core registers
pub core: MipsCoreRegs<U>,
/// DSP registers
pub dsp: MipsDspRegs<U>,
}

impl<U> Registers for MipsCoreRegs<U>
where
U: PrimInt + LeBytes + Default + core::fmt::Debug,
Expand Down Expand Up @@ -99,11 +137,12 @@ where
fn gdb_deserialize(&mut self, bytes: &[u8]) -> Result<(), ()> {
let ptrsize = core::mem::size_of::<U>();

// ensure bytes.chunks_exact(ptrsize) won't panic
if bytes.len() % ptrsize != 0 {
// Ensure bytes contains enough data for all 72 registers
if bytes.len() < ptrsize * 72 {
Comment on lines -102 to +141
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two checks aren't equivalent.

For example, if bytes.len() == ptrsize * 72 + 1, then chunks_exact will panic.

You can leave this condition in, but do not remove the other one.

return Err(());
}

// All core registers are the same size
let mut regs = bytes
.chunks_exact(ptrsize)
.map(|c| U::from_le_bytes(c).unwrap());
Expand Down Expand Up @@ -136,10 +175,85 @@ where
self.fpu.fcsr = regs.next().ok_or(())?;
self.fpu.fir = regs.next().ok_or(())?;

if regs.next().is_some() {
Ok(())
}
}

impl<U> Registers for MipsCoreRegsWithDsp<U>
where
U: PrimInt + LeBytes + Default + core::fmt::Debug,
{
fn gdb_serialize(&self, mut write_byte: impl FnMut(Option<u8>)) {
macro_rules! write_le_bytes {
($value:expr) => {
let mut buf = [0; 16];
// infallible (unless digit is a >128 bit number)
let len = $value.to_le_bytes(&mut buf).unwrap();
let buf = &buf[..len];
for b in buf {
write_byte(Some(*b));
}
};
}

// Serialize the core registers first
self.core.gdb_serialize(&mut write_byte);

// Write the DSP registers
write_le_bytes!(&self.dsp.hi1);
write_le_bytes!(&self.dsp.lo1);
write_le_bytes!(&self.dsp.hi2);
write_le_bytes!(&self.dsp.lo2);
write_le_bytes!(&self.dsp.hi3);
write_le_bytes!(&self.dsp.lo3);

for b in &self.dsp.dspctl.to_le_bytes() {
write_byte(Some(*b));
}

write_le_bytes!(&self.dsp.restart);
}

fn gdb_deserialize(&mut self, bytes: &[u8]) -> Result<(), ()> {
// Deserialize the core registers first
self.core.gdb_deserialize(bytes)?;

// Ensure bytes contains enough data for all 79 registers of target-width
// and the dspctl register which is always 4 bytes
let ptrsize = core::mem::size_of::<U>();
if bytes.len() < (ptrsize * 79) + 4 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment - this check is not sufficient to ensure that chunks_exact won't panic.

You can leave it in, but you still need the other check as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue I'm seeing with if bytes.len() % ptrsize != 0 { is when U is a u64 there will be 79 registers of size 64 and one register of size 32 (dspctl), thus this check will fail. It looks like chunks_exact actually doesn't panic when there are extra elements, it should still split the bytes up until the end with a possible remainder of 4 bytes.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, yep, you're right.

Well, in any case, I decided to see if the optimizer was smart enough to get rid of any panics, and yep, sure enough it seems to have done the trick:

https://godbolt.org/z/8zsKf9

So yeah, this check can be left as-is.

return Err(());
}

// Calculate the offsets to the DSP registers based on the ptrsize
let dspregs_start = ptrsize * 72;
let dspctl_start = ptrsize * 78;

// Read up until the dspctl register
let mut regs = bytes[dspregs_start..dspctl_start]
.chunks_exact(ptrsize)
.map(|c| U::from_le_bytes(c).unwrap());

self.dsp.hi1 = regs.next().ok_or(())?;
self.dsp.lo1 = regs.next().ok_or(())?;
self.dsp.hi2 = regs.next().ok_or(())?;
self.dsp.lo2 = regs.next().ok_or(())?;
self.dsp.hi3 = regs.next().ok_or(())?;
self.dsp.lo3 = regs.next().ok_or(())?;

// Dspctl will always be a u32
self.dsp.dspctl =
u32::from_le_bytes(bytes[dspctl_start..dspctl_start + 4].try_into().unwrap());

// Only 4 or 8 bytes should remain to be read
self.dsp.restart = U::from_le_bytes(
bytes[dspctl_start + 4..]
.chunks_exact(ptrsize)
.next()
.ok_or(())?,
)
.unwrap();

Ok(())
}
}
1 change: 1 addition & 0 deletions src/arch/mips/reg/mod.rs
Expand Up @@ -6,5 +6,6 @@ pub mod id;
mod mips;

pub use mips::MipsCoreRegs;
pub use mips::MipsCoreRegsWithDsp;
pub use mips::MipsCp0Regs;
pub use mips::MipsFpuRegs;
2 changes: 1 addition & 1 deletion src/arch/msp430/mod.rs
Expand Up @@ -9,7 +9,7 @@ pub mod reg;
///
/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
/// more info about the `RegIdImpl` type parameter.
pub enum Msp430<RegIdImpl: RegId> {
pub enum Msp430<RegIdImpl: RegId = reg::id::Msp430RegId> {
#[doc(hidden)]
_Marker(core::marker::PhantomData<RegIdImpl>),
}
Expand Down