Skip to content

Commit

Permalink
docs + #[derive] sweep
Browse files Browse the repository at this point in the history
After merging all the arch PRs, I realized that everyone did their own
thing when it comes to #[derive]s and wording on the various structs,
which made the docs feel kind-of disjointed. This PR makes everything
much more uniform.

For example, all implementors of the `Register` trait are now guaranteed
to implement `Debug`, `Default`, `Clone`, and `PartialEq`. Note that
this does _not_ include `Eq`, as some architectures have explicit IEEE
floating point registers (i.e: PowerPC), which do not implement `Eq`.

Additionally, I've changed all instances of `struct FooArch;` to
`enum FooArch{}`, which ensures that `Arch` implementations are purely a
type level construct.

I may have to revert this at some point, depending on how #12 gets
tackled, but for now, having Arch implementations as enums makes more
sense.
  • Loading branch information
daniel5151 committed Sep 13, 2020
1 parent fffe938 commit 2dfe132
Show file tree
Hide file tree
Showing 22 changed files with 64 additions and 54 deletions.
3 changes: 1 addition & 2 deletions src/arch/arm/mod.rs
Expand Up @@ -5,8 +5,7 @@ use crate::arch::Arch;
pub mod reg;

/// Implements `Arch` for ARMv4T
#[derive(Eq, PartialEq)]
pub struct Armv4t;
pub enum Armv4t {}

impl Arch for Armv4t {
type Usize = u32;
Expand Down
4 changes: 2 additions & 2 deletions src/arch/arm/reg/arm_core.rs
@@ -1,7 +1,7 @@
use crate::arch::{RegId, Registers};

/// 32-bit ARM core register identifier.
#[derive(Debug)]
#[derive(Debug, Clone, Copy)]
pub enum ArmCoreRegId {
/// General purpose registers (R0-R12)
Gpr(u8),
Expand Down Expand Up @@ -36,7 +36,7 @@ impl RegId for ArmCoreRegId {
/// 32-bit ARM core registers.
///
/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/arm/arm-core.xml
#[derive(Default)]
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct ArmCoreRegs {
/// General purpose registers (R0-R12)
pub r: [u32; 13],
Expand Down
2 changes: 1 addition & 1 deletion src/arch/arm/reg/mod.rs
@@ -1,4 +1,4 @@
//! `GdbRegister` structs for various ARM architectures.
//! `Register` structs for various ARM architectures.

mod arm_core;

Expand Down
6 changes: 2 additions & 4 deletions src/arch/mips/mod.rs
Expand Up @@ -5,12 +5,10 @@ use crate::arch::Arch;
pub mod reg;

/// Implements `Arch` for 32-bit MIPS.
#[derive(Eq, PartialEq)]
pub struct Mips;
pub enum Mips {}

/// Implements `Arch` for 64-bit MIPS.
#[derive(Eq, PartialEq)]
pub struct Mips64;
pub enum Mips64 {}

impl Arch for Mips {
type Usize = u32;
Expand Down
10 changes: 5 additions & 5 deletions src/arch/mips/reg/mips.rs
Expand Up @@ -5,11 +5,11 @@ use crate::internal::LeBytes;
use num_traits::PrimInt;

/// MIPS registers.
/// This structure is identical for both 32 and 64-bit MIPS.
///
/// 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(Default)]
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct MipsCoreRegs<U> {
/// General purpose registers (R0-R32)
pub r: [U; 32],
Expand All @@ -28,7 +28,7 @@ pub struct MipsCoreRegs<U> {
/// MIPS CP0 (coprocessor 0) registers.
///
/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-cp0.xml
#[derive(Default)]
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct MipsCp0Regs<U> {
/// Status register (regnum 32)
pub status: U,
Expand All @@ -41,7 +41,7 @@ pub struct MipsCp0Regs<U> {
/// MIPS FPU registers.
///
/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-fpu.xml
#[derive(Default)]
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct MipsFpuRegs<U> {
/// FP registers (F0-F32) starting at regnum 38
pub r: [U; 32],
Expand All @@ -53,7 +53,7 @@ pub struct MipsFpuRegs<U> {

impl<U> Registers for MipsCoreRegs<U>
where
U: PrimInt + LeBytes + Default,
U: PrimInt + LeBytes + Default + core::fmt::Debug,
{
type RegId = RawRegId;

Expand Down
2 changes: 1 addition & 1 deletion src/arch/mips/reg/mod.rs
@@ -1,4 +1,4 @@
//! `GdbRegister` structs for MIPS architectures.
//! `Register` structs for MIPS architectures.

mod mips;

Expand Down
5 changes: 5 additions & 0 deletions src/arch/mod.rs
Expand Up @@ -8,6 +8,11 @@
//!
//! Please consider upstreaming any missing `Arch` implementations you happen to
//! implement yourself!
//!
//! **Disclaimer:** These implementations are all community contributions, and
//! while they are tested (by the PR's author) and code-reviewed, it's not
//! particularly feasible to write detailed tests for each architecture! If you
//! spot a bug in any of the implementations, please file an issue / open a PR!

pub mod arm;
pub mod mips;
Expand Down
3 changes: 1 addition & 2 deletions src/arch/msp430/mod.rs
Expand Up @@ -5,8 +5,7 @@ use crate::arch::Arch;
pub mod reg;

/// Implements `Arch` for standard 16-bit TI-MSP430 MCUs.
#[derive(Eq, PartialEq)]
pub struct Msp430;
pub enum Msp430 {}

impl Arch for Msp430 {
type Usize = u32;
Expand Down
2 changes: 1 addition & 1 deletion src/arch/msp430/reg/mod.rs
@@ -1,4 +1,4 @@
//! `GdbRegister` structs for various TI-MSP430 CPUs.
//! `Register` structs for various TI-MSP430 CPUs.

mod msp430;

Expand Down
2 changes: 1 addition & 1 deletion src/arch/msp430/reg/msp430.rs
Expand Up @@ -2,7 +2,7 @@ use crate::arch::RawRegId;
use crate::arch::Registers;

/// 16-bit TI-MSP430 registers.
#[derive(Default)]
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct Msp430Regs {
/// Program Counter (R0)
pub pc: u16,
Expand Down
3 changes: 1 addition & 2 deletions src/arch/ppc/mod.rs
Expand Up @@ -5,8 +5,7 @@ use crate::arch::Arch;
pub mod reg;

/// Implements `Arch` for 32-bit PowerPC + AltiVec SIMD
#[derive(Eq, PartialEq)]
pub struct PowerPcAltivec32;
pub enum PowerPcAltivec32 {}

impl Arch for PowerPcAltivec32 {
type Usize = u32;
Expand Down
4 changes: 2 additions & 2 deletions src/arch/ppc/reg/common.rs
Expand Up @@ -11,11 +11,11 @@ use core::convert::TryInto;
/// * https://github.com/bminor/binutils-gdb/blob/master/gdb/features/rs6000/power-core.xml
/// * https://github.com/bminor/binutils-gdb/blob/master/gdb/features/rs6000/power-fpu.xml
/// * https://github.com/bminor/binutils-gdb/blob/master/gdb/features/rs6000/power-altivec.xml
#[derive(Default, Debug, PartialEq)]
#[derive(Debug, Default, Clone, PartialEq)]
pub struct PowerPcCommonRegs {
/// General purpose registers
pub r: [u32; 32],
/// Float registers
/// Floating Point registers
pub f: [f64; 32],
/// Program counter
pub pc: u32,
Expand Down
2 changes: 1 addition & 1 deletion src/arch/ppc/reg/mod.rs
@@ -1,4 +1,4 @@
//! Registers for PowerPC architectures
//! `Register` structs for PowerPC architectures

mod common;

Expand Down
8 changes: 3 additions & 5 deletions src/arch/riscv/mod.rs
@@ -1,4 +1,4 @@
//! Support for the [RISC-V](https://riscv.org/) architecture.
//! Implementations for the [RISC-V](https://riscv.org/) architecture.
//!
//! *Note*: currently only supports integer versions of the ISA.

Expand All @@ -7,12 +7,10 @@ use crate::arch::Arch;
pub mod reg;

/// Implements `Arch` for 32-bit RISC-V.
#[derive(Eq, PartialEq)]
pub struct Riscv32;
pub enum Riscv32 {}

/// Implements `Arch` for 64-bit RISC-V.
#[derive(Eq, PartialEq)]
pub struct Riscv64;
pub enum Riscv64 {}

impl Arch for Riscv32 {
type Usize = u32;
Expand Down
2 changes: 1 addition & 1 deletion src/arch/riscv/reg/mod.rs
@@ -1,4 +1,4 @@
//! `GdbRegister` structs for RISC-V architectures.
//! `Register` structs for RISC-V architectures.

mod riscv;

Expand Down
6 changes: 3 additions & 3 deletions src/arch/riscv/reg/riscv.rs
Expand Up @@ -4,7 +4,7 @@ use crate::arch::{RegId, Registers};
use crate::internal::LeBytes;

/// RISC-V Register identifier.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy)]
pub enum RiscvRegId {
/// General Purpose Register (x0-x31).
Gpr(u8),
Expand Down Expand Up @@ -38,7 +38,7 @@ impl RegId for RiscvRegId {
/// Useful links:
/// * [GNU binutils-gdb XML descriptions](https://github.com/bminor/binutils-gdb/blob/master/gdb/features/riscv)
/// * [riscv-tdep.h](https://github.com/bminor/binutils-gdb/blob/master/gdb/riscv-tdep.h)
#[derive(Default)]
#[derive(Debug, Default, Clone, PartialEq)]
pub struct RiscvCoreRegs<U> {
/// General purpose registers (x0-x31)
pub x: [U; 32],
Expand All @@ -48,7 +48,7 @@ pub struct RiscvCoreRegs<U> {

impl<U> Registers for RiscvCoreRegs<U>
where
U: PrimInt + LeBytes + Default,
U: PrimInt + LeBytes + Default + core::fmt::Debug,
{
type RegId = RiscvRegId;

Expand Down
22 changes: 14 additions & 8 deletions src/arch/traits.rs
@@ -1,11 +1,13 @@
use num_traits::{Num, PrimInt, Unsigned};
use core::fmt::Debug;

use crate::internal::BeBytes;
use num_traits::{PrimInt, Unsigned};

use crate::internal::{BeBytes, LeBytes};

/// Register identifier for target registers.
///
/// These identifiers are used by GDB for single register operations.
pub trait RegId: Sized {
pub trait RegId: Sized + Debug {
/// Map raw GDB register number corresponding `RegId` and register size.
///
/// Returns `None` if the register is not available.
Expand All @@ -31,6 +33,7 @@ pub trait RegId: Sized {
///
/// It bears repeating: if you end up implementing the `read/write_register`
/// methods using `RawRegId`, please consider upstreaming your implementation!
#[derive(Debug)]
pub struct RawRegId(pub usize);

impl RegId for RawRegId {
Expand All @@ -48,9 +51,7 @@ impl RegId for RawRegId {
/// e.g: for ARM:
/// github.com/bminor/binutils-gdb/blob/master/gdb/features/arm/arm-core.xml
// TODO: add way to de/serialize arbitrary "missing"/"uncollected" registers.
// TODO: add (optional?) trait methods for reading/writing specific register
// (via it's GDB index)
pub trait Registers: Default {
pub trait Registers: Default + Debug + Clone + PartialEq {
/// Register identifier for addressing single registers.
type RegId: RegId;

Expand All @@ -65,9 +66,14 @@ pub trait Registers: Default {

/// Encodes architecture-specific information, such as pointer size, register
/// layout, etc...
pub trait Arch: Eq + PartialEq {
///
/// Types implementing `Arch` should be
/// [Zero-variant Enums](https://doc.rust-lang.org/reference/items/enumerations.html#zero-variant-enums),
/// as they are only ever used at the type level, and should never be
/// instantiated.
pub trait Arch {
/// The architecture's pointer size (e.g: `u32` on a 32-bit system).
type Usize: Num + PrimInt + Unsigned + BeBytes;
type Usize: PrimInt + Unsigned + BeBytes + LeBytes;

/// The architecture's register file
type Registers: Registers;
Expand Down
18 changes: 9 additions & 9 deletions src/arch/x86/mod.rs
@@ -1,14 +1,14 @@
//! Implementations for x86
//! Implementations for various x86 architectures.

use crate::arch::Arch;

pub mod reg;

/// Implements `Arch` for 64-bit x86
#[derive(Eq, PartialEq)]
pub struct X86_64;
/// Implements `Arch` for 64-bit x86 + SSE Extensions
#[allow(non_camel_case_types)]
pub enum X86_64_SSE {}

impl Arch for X86_64 {
impl Arch for X86_64_SSE {
type Usize = u64;
type Registers = reg::X86_64CoreRegs;

Expand All @@ -19,11 +19,11 @@ impl Arch for X86_64 {
}
}

/// Implements `Arch` for 32-bit x86
#[derive(Eq, PartialEq)]
pub struct X86;
/// Implements `Arch` for 32-bit x86 + SSE Extensions
#[allow(non_camel_case_types)]
pub enum X86_SSE {}

impl Arch for X86 {
impl Arch for X86_SSE {
type Usize = u32;
type Registers = reg::X86CoreRegs;

Expand Down
2 changes: 1 addition & 1 deletion src/arch/x86/reg/core32.rs
Expand Up @@ -8,7 +8,7 @@ use crate::arch::Registers;
///
/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/i386/32bit-core.xml
/// Additionally: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/i386/32bit-sse.xml
#[derive(Default)]
#[derive(Debug, Default, Clone, PartialEq)]
pub struct X86CoreRegs {
/// Accumulator
pub eax: u32,
Expand Down
2 changes: 1 addition & 1 deletion src/arch/x86/reg/core64.rs
Expand Up @@ -8,7 +8,7 @@ use crate::arch::Registers;
///
/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/i386/64bit-core.xml
/// Additionally: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/i386/64bit-sse.xml
#[derive(Default)]
#[derive(Debug, Default, Clone, PartialEq)]
pub struct X86_64CoreRegs {
/// RAX, RBX, RCX, RDX, RSI, RDI, RBP, RSP, r8-r15
pub regs: [u64; 16],
Expand Down
4 changes: 2 additions & 2 deletions src/arch/x86/reg/mod.rs
@@ -1,4 +1,4 @@
//! `GdbRegister` structs for x86 architectures.
//! `Register` structs for x86 architectures.

use core::convert::TryInto;

Expand All @@ -15,7 +15,7 @@ pub use core64::X86_64CoreRegs;
pub type F80 = [u8; 10];

/// FPU registers
#[derive(Default)]
#[derive(Debug, Default, Clone, PartialEq)]
pub struct X87FpuInternalRegs {
/// Floating-point control register
pub fctrl: u32,
Expand Down
6 changes: 6 additions & 0 deletions src/target/base/multithread.rs
Expand Up @@ -249,6 +249,12 @@ pub struct Actions<'a> {
inner: &'a mut dyn Iterator<Item = (TidSelector, ResumeAction)>,
}

impl core::fmt::Debug for Actions<'_> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "Actions {{ .. }}")
}
}

impl Actions<'_> {
pub(crate) fn new(iter: &mut dyn Iterator<Item = (TidSelector, ResumeAction)>) -> Actions<'_> {
Actions { inner: iter }
Expand Down

0 comments on commit 2dfe132

Please sign in to comment.