From 681ef2e0002b077abe1c3eb4369fafccd264b5ea Mon Sep 17 00:00:00 2001 From: Daniel Prilik Date: Sat, 12 Sep 2020 23:05:38 -0400 Subject: [PATCH] docs + #[derive] sweep 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`, `Eq`, and `PartialEq`. 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. --- src/arch/arm/mod.rs | 3 +-- src/arch/arm/reg/arm_core.rs | 4 ++-- src/arch/arm/reg/mod.rs | 2 +- src/arch/mips/mod.rs | 6 ++---- src/arch/mips/reg/mips.rs | 10 +++++----- src/arch/mips/reg/mod.rs | 2 +- src/arch/mod.rs | 5 +++++ src/arch/msp430/mod.rs | 3 +-- src/arch/msp430/reg/mod.rs | 2 +- src/arch/msp430/reg/msp430.rs | 2 +- src/arch/ppc/mod.rs | 3 +-- src/arch/ppc/reg/common.rs | 10 +++++----- src/arch/ppc/reg/mod.rs | 2 +- src/arch/riscv/mod.rs | 8 +++----- src/arch/riscv/reg/mod.rs | 2 +- src/arch/riscv/reg/riscv.rs | 6 +++--- src/arch/traits.rs | 22 ++++++++++++++-------- src/arch/x86/mod.rs | 18 +++++++++--------- src/arch/x86/reg/core32.rs | 2 +- src/arch/x86/reg/core64.rs | 2 +- src/arch/x86/reg/mod.rs | 4 ++-- src/target/base/multithread.rs | 6 ++++++ 22 files changed, 67 insertions(+), 57 deletions(-) diff --git a/src/arch/arm/mod.rs b/src/arch/arm/mod.rs index d1617652..59d2393c 100644 --- a/src/arch/arm/mod.rs +++ b/src/arch/arm/mod.rs @@ -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; diff --git a/src/arch/arm/reg/arm_core.rs b/src/arch/arm/reg/arm_core.rs index 98d62a3a..0c566fcb 100644 --- a/src/arch/arm/reg/arm_core.rs +++ b/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), @@ -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], diff --git a/src/arch/arm/reg/mod.rs b/src/arch/arm/reg/mod.rs index 67c3aea6..0ad0f9b5 100644 --- a/src/arch/arm/reg/mod.rs +++ b/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; diff --git a/src/arch/mips/mod.rs b/src/arch/mips/mod.rs index 4eb48e50..f993c38d 100644 --- a/src/arch/mips/mod.rs +++ b/src/arch/mips/mod.rs @@ -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; diff --git a/src/arch/mips/reg/mips.rs b/src/arch/mips/reg/mips.rs index 2e8f64d5..3014ef16 100644 --- a/src/arch/mips/reg/mips.rs +++ b/src/arch/mips/reg/mips.rs @@ -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 `` 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 { /// General purpose registers (R0-R32) pub r: [U; 32], @@ -28,7 +28,7 @@ pub struct MipsCoreRegs { /// 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 { /// Status register (regnum 32) pub status: U, @@ -41,7 +41,7 @@ pub struct MipsCp0Regs { /// 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 { /// FP registers (F0-F32) starting at regnum 38 pub r: [U; 32], @@ -53,7 +53,7 @@ pub struct MipsFpuRegs { impl Registers for MipsCoreRegs where - U: PrimInt + LeBytes + Default, + U: PrimInt + LeBytes + Default + core::fmt::Debug, { type RegId = RawRegId; diff --git a/src/arch/mips/reg/mod.rs b/src/arch/mips/reg/mod.rs index ee270d46..a85c74ed 100644 --- a/src/arch/mips/reg/mod.rs +++ b/src/arch/mips/reg/mod.rs @@ -1,4 +1,4 @@ -//! `GdbRegister` structs for MIPS architectures. +//! `Register` structs for MIPS architectures. mod mips; diff --git a/src/arch/mod.rs b/src/arch/mod.rs index 9773dc85..da280791 100644 --- a/src/arch/mod.rs +++ b/src/arch/mod.rs @@ -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; diff --git a/src/arch/msp430/mod.rs b/src/arch/msp430/mod.rs index 18d83c7a..7c125e65 100644 --- a/src/arch/msp430/mod.rs +++ b/src/arch/msp430/mod.rs @@ -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; diff --git a/src/arch/msp430/reg/mod.rs b/src/arch/msp430/reg/mod.rs index ffe2f899..f4c87eab 100644 --- a/src/arch/msp430/reg/mod.rs +++ b/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; diff --git a/src/arch/msp430/reg/msp430.rs b/src/arch/msp430/reg/msp430.rs index fce94d8d..94dc777f 100644 --- a/src/arch/msp430/reg/msp430.rs +++ b/src/arch/msp430/reg/msp430.rs @@ -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, diff --git a/src/arch/ppc/mod.rs b/src/arch/ppc/mod.rs index f2b16a7b..7cd1f098 100644 --- a/src/arch/ppc/mod.rs +++ b/src/arch/ppc/mod.rs @@ -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; diff --git a/src/arch/ppc/reg/common.rs b/src/arch/ppc/reg/common.rs index a036f5b6..4e3df625 100644 --- a/src/arch/ppc/reg/common.rs +++ b/src/arch/ppc/reg/common.rs @@ -11,12 +11,12 @@ 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, Eq, PartialEq)] pub struct PowerPcCommonRegs { /// General purpose registers pub r: [u32; 32], - /// Float registers - pub f: [f64; 32], + /// Floating Point registers + pub f: [u64; 32], /// Program counter pub pc: u32, /// Machine state @@ -92,7 +92,7 @@ impl Registers for PowerPcCommonRegs { let mut regs = bytes[0x80..0x180] .chunks_exact(8) - .map(|x| f64::from_be_bytes(x.try_into().unwrap())); + .map(|x| u64::from_be_bytes(x.try_into().unwrap())); for reg in &mut self.f { *reg = regs.next().ok_or(())?; @@ -140,7 +140,7 @@ mod tests { ctr: 6, xer: 7, fpscr: 8, - f: [9.0; 32], + f: [9.0 as u64; 32], vr: [52; 32], vrsave: 10, vscr: 11, diff --git a/src/arch/ppc/reg/mod.rs b/src/arch/ppc/reg/mod.rs index cb48e8d6..b98e7bd7 100644 --- a/src/arch/ppc/reg/mod.rs +++ b/src/arch/ppc/reg/mod.rs @@ -1,4 +1,4 @@ -//! Registers for PowerPC architectures +//! `Register` structs for PowerPC architectures mod common; diff --git a/src/arch/riscv/mod.rs b/src/arch/riscv/mod.rs index a5a4f780..5eff0374 100644 --- a/src/arch/riscv/mod.rs +++ b/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. @@ -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; diff --git a/src/arch/riscv/reg/mod.rs b/src/arch/riscv/reg/mod.rs index b35409a0..a0b4de46 100644 --- a/src/arch/riscv/reg/mod.rs +++ b/src/arch/riscv/reg/mod.rs @@ -1,4 +1,4 @@ -//! `GdbRegister` structs for RISC-V architectures. +//! `Register` structs for RISC-V architectures. mod riscv; diff --git a/src/arch/riscv/reg/riscv.rs b/src/arch/riscv/reg/riscv.rs index 7c9e9dcf..d4deb372 100644 --- a/src/arch/riscv/reg/riscv.rs +++ b/src/arch/riscv/reg/riscv.rs @@ -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), @@ -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, Eq, PartialEq)] pub struct RiscvCoreRegs { /// General purpose registers (x0-x31) pub x: [U; 32], @@ -48,7 +48,7 @@ pub struct RiscvCoreRegs { impl Registers for RiscvCoreRegs where - U: PrimInt + LeBytes + Default, + U: PrimInt + LeBytes + Default + core::fmt::Debug, { type RegId = RiscvRegId; diff --git a/src/arch/traits.rs b/src/arch/traits.rs index 52ea62f0..826cc752 100644 --- a/src/arch/traits.rs +++ b/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. @@ -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 { @@ -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 { /// Register identifier for addressing single registers. type RegId: RegId; @@ -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; diff --git a/src/arch/x86/mod.rs b/src/arch/x86/mod.rs index a36f8a65..8f1881ee 100644 --- a/src/arch/x86/mod.rs +++ b/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; @@ -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; diff --git a/src/arch/x86/reg/core32.rs b/src/arch/x86/reg/core32.rs index 25766aa2..da6b2a14 100644 --- a/src/arch/x86/reg/core32.rs +++ b/src/arch/x86/reg/core32.rs @@ -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, Eq, PartialEq)] pub struct X86CoreRegs { /// Accumulator pub eax: u32, diff --git a/src/arch/x86/reg/core64.rs b/src/arch/x86/reg/core64.rs index 4246120f..788d84f8 100644 --- a/src/arch/x86/reg/core64.rs +++ b/src/arch/x86/reg/core64.rs @@ -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, Eq, PartialEq)] pub struct X86_64CoreRegs { /// RAX, RBX, RCX, RDX, RSI, RDI, RBP, RSP, r8-r15 pub regs: [u64; 16], diff --git a/src/arch/x86/reg/mod.rs b/src/arch/x86/reg/mod.rs index d0759a85..a8b9e7fe 100644 --- a/src/arch/x86/reg/mod.rs +++ b/src/arch/x86/reg/mod.rs @@ -1,4 +1,4 @@ -//! `GdbRegister` structs for x86 architectures. +//! `Register` structs for x86 architectures. use core::convert::TryInto; @@ -15,7 +15,7 @@ pub use core64::X86_64CoreRegs; pub type F80 = [u8; 10]; /// FPU registers -#[derive(Default)] +#[derive(Debug, Default, Clone, Eq, PartialEq)] pub struct X87FpuInternalRegs { /// Floating-point control register pub fctrl: u32, diff --git a/src/target/base/multithread.rs b/src/target/base/multithread.rs index 901161df..d0ffd4f7 100644 --- a/src/target/base/multithread.rs +++ b/src/target/base/multithread.rs @@ -249,6 +249,12 @@ pub struct Actions<'a> { inner: &'a mut dyn Iterator, } +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) -> Actions<'_> { Actions { inner: iter }