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

capstone: add cs_regs_access #77

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
23 changes: 22 additions & 1 deletion capstone-rs/src/capstone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ use capstone_sys::*;
use crate::constants::{Arch, Endian, ExtraMode, Mode, OptValue, Syntax};
use crate::error::*;
use crate::ffi::str_from_cstr_ptr;
use crate::instruction::{Insn, InsnDetail, InsnGroupId, InsnId, Instructions, RegId};
use crate::instruction::{
Insn, InsnDetail, InsnGroupId, InsnId, InsnRegsAccess, Instructions, RegId,
};


/// An instance of the capstone disassembler
Expand Down Expand Up @@ -378,6 +380,25 @@ impl Capstone {
}
}

/// Returns `RegsAccess` structure for a given instruction
///
/// Requires:
///
/// 1. Instruction was created with detail enabled
/// 2. Skipdata is disabled
/// 3. Capstone was not compiled in diet mode
pub fn insn_regs_access<'s, 'i: 's>(&'s self, insn: &'i Insn) -> CsResult<InsnRegsAccess> {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of trying to do all of the error handling here, check the returned integer and convert that to a CsResult.
You can move that logic into
That way, we don't have to manually track all possible errors that capstone should be handling anyway.

For examples, see disasm(). Also, From<capstone_sys::cs_err::Type> is implemented for Error, so you can use Error::from(capstone_sys_rc)` to do the error parsing for you.

if !self.detail_enabled {
Err(Error::DetailOff)
} else if insn.id().0 == 0 {
Err(Error::IrrelevantDataInSkipData)
} else if Self::is_diet() {
Err(Error::IrrelevantDataInDiet)
} else {
Ok(unsafe { insn.regs_access(self.csh()) })
}
}

/// Returns a tuple (major, minor) indicating the version of the capstone C library.
pub fn lib_version() -> (u32, u32) {
let mut major: c_int = 0;
Expand Down
75 changes: 75 additions & 0 deletions capstone-rs/src/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ pub struct Insn<'a> {
/// `ArchDetail` enum.
pub struct InsnDetail<'a>(pub(crate) &'a cs_detail, pub(crate) Arch);

/// Contains information about registers accessed by an instruction, either explicitly or implicitly
pub struct InsnRegsAccess {
Copy link
Member

Choose a reason for hiding this comment

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

Add #[derive(Debug, Clone, PartialEq, Eq)]

Copy link
Author

Choose a reason for hiding this comment

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

Debug, PartialEq, Eq can't be derived because of the array with size >32, I added a comment

Copy link
Member

Choose a reason for hiding this comment

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

Could you manually impl PartialEq, Eq?

pub(crate) regs_read: cs_regs,
pub(crate) regs_read_count: u8,
pub(crate) regs_write: cs_regs,
pub(crate) regs_write_count: u8,
}

impl<'a> Insn<'a> {
/// The mnemonic for the instruction
pub fn mnemonic(&self) -> Option<&str> {
Expand Down Expand Up @@ -184,6 +192,15 @@ impl<'a> Insn<'a> {
pub(crate) unsafe fn detail(&self, arch: Arch) -> InsnDetail {
InsnDetail(&*self.insn.detail, arch)
}

/// Returns the `RegsAccess` object, if there is one. It is up to the caller to determine
/// the pre-conditions are satisfied.
///
/// Be careful this is still in early stages and largely untested with various `cs_option` and
/// architecture matrices
pub(crate) unsafe fn regs_access(&self, cs: csh) -> InsnRegsAccess {
InsnRegsAccess::new(cs, &self.insn)
}
}

impl<'a> Debug for Insn<'a> {
Expand Down Expand Up @@ -328,6 +345,64 @@ impl<'a> InsnDetail<'a> {
}
}

impl<'a> InsnRegsAccess {
fn new(cs: csh, ins: &cs_insn) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

Any function that can cause undefined behavior should be marked unsafe.

However, once you move the error checking (checking return value), then this shouldn't need to be marked unsafe.

Copy link
Member

Choose a reason for hiding this comment

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

Have this return a CsResult<Self> instead

let mut regs_read = [0u16; 64];
Copy link
Member

Choose a reason for hiding this comment

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

Use cs_regs type directly instead of manually declaring an array.

Copy link
Author

Choose a reason for hiding this comment

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

Can you give an example how to initialize a variable with a type?

Copy link
Member

Choose a reason for hiding this comment

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

It looks like cs_regs is a type alias:

pub type cs_regs = [u16; 64usize];

Just add an explicit type instead of using type inference:

let mut regs_read: cs_regs = [0u16; 64];

let mut regs_read_count = 0u8;
let mut regs_write = [0u16; 64];
let mut regs_write_count = 0u8;

unsafe {
cs_regs_access(
Copy link
Member

Choose a reason for hiding this comment

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

check this return value to create a CsResult

cs,
ins,
regs_read.as_mut_ptr(),
&mut regs_read_count,
regs_write.as_mut_ptr(),
&mut regs_write_count,
);
}

Self {
regs_read,
regs_read_count,
regs_write,
regs_write_count,
}
}

/// Returns the explicit and implicit accessed registers
pub fn regs_read(&self) -> RegsIter<RegIdInt> {
RegsIter(self.regs_read[..self.regs_read_count as usize].iter())
}

/// Returns the number of explicit and implicit read registers
pub fn regs_read_count(&self) -> u8 {
self.regs_read_count
}

/// Returns the explicit and implicit write registers
pub fn regs_write(&self) -> RegsIter<RegIdInt> {
RegsIter(self.regs_write[..self.regs_write_count as usize].iter())
}

/// Returns the number of explicit and implicit write registers
pub fn regs_write_count(&self) -> u8 {
self.regs_write_count
}
}

impl Debug for InsnRegsAccess {
tmfink marked this conversation as resolved.
Show resolved Hide resolved
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
fmt.debug_struct("RegsAccess")
.field("regs_read", &self.regs_read())
.field("regs_read_count", &self.regs_read_count())
.field("regs_write", &self.regs_write())
.field("regs_write_count", &self.regs_write_count())
.finish()
}
}

impl<'a> Debug for InsnDetail<'a> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
fmt.debug_struct("Detail")
Expand Down
2 changes: 2 additions & 0 deletions capstone-rs/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ fn test_detail_false_fail() {

assert_eq!(cs.insn_detail(&insns[0]).unwrap_err(), Error::DetailOff);
assert_eq!(cs.insn_detail(&insns[1]).unwrap_err(), Error::DetailOff);
assert_eq!(cs.insn_regs_access(&insns[0]).unwrap_err(), Error::DetailOff);
assert_eq!(cs.insn_regs_access(&insns[1]).unwrap_err(), Error::DetailOff);
}

#[test]
Expand Down