capstone: add cs_regs_access #77

SWW13 · 2019-10-08T18:17:37Z

Adds support for cs_regs_access, fixes #63.

I'm not sure how to implement the tests yet, the instructions_* tests are a bit complicated on first sight.

codecov · 2019-10-08T18:42:38Z

Codecov Report

Attention: Patch coverage is 71.75573% with 37 lines in your changes missing coverage. Please review.

Project coverage is 94.17%. Comparing base (efdddcb) to head (0475878).
Report is 140 commits behind head on master.

Files with missing lines	Patch %	Lines
capstone-rs/src/instruction.rs	48.61%	37 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #77      +/-   ##
==========================================
+ Coverage   93.81%   94.17%   +0.35%     
==========================================
  Files          20       20              
  Lines        2701     3983    +1282     
==========================================
+ Hits         2534     3751    +1217     
- Misses        167      232      +65

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tmfink

Overall, this looks really great.

Once you implement these changes, also add some tests in src/tests.rs. These should basically be an example program that shows how to use the new API, but also calls assert_eq!(...) to verify that you get the expected output.

tmfink · 2019-10-09T03:03:01Z

capstone-rs/src/instruction.rs

@@ -328,6 +345,64 @@ impl<'a> InsnDetail<'a> {
    }
 }

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


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.

Have this return a CsResult<Self> instead

tmfink · 2019-10-09T03:19:01Z

capstone-rs/src/capstone.rs

+    /// 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> {


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.

tmfink · 2019-10-09T03:19:35Z

capstone-rs/src/instruction.rs

+        let mut regs_write_count = 0u8;
+
+        unsafe {
+            cs_regs_access(


check this return value to create a CsResult

tmfink · 2019-10-09T03:21:22Z

capstone-rs/src/instruction.rs

@@ -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 {


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

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

Could you manually impl PartialEq, Eq?

capstone-rs/src/instruction.rs

tmfink · 2019-10-09T03:29:25Z

capstone-rs/src/instruction.rs

@@ -328,6 +345,64 @@ impl<'a> InsnDetail<'a> {
    }
 }

+impl<'a> InsnRegsAccess {
+    fn new(cs: csh, ins: &cs_insn) -> Self {
+        let mut regs_read = [0u16; 64];


Use cs_regs type directly instead of manually declaring an array.

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

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];

tmfink · 2019-10-09T03:42:16Z

@SWW13 Thanks for the pull-request! This is great work.

If you have any questions related to review comments or capstone, feel free to reach out.

tmfink · 2019-12-05T02:42:11Z

capstone-rs/src/instruction.rs

@@ -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 {


Could you manually impl PartialEq, Eq?

tmfink · 2019-12-05T02:47:28Z

capstone-rs/src/instruction.rs

@@ -328,6 +345,64 @@ impl<'a> InsnDetail<'a> {
    }
 }

+impl<'a> InsnRegsAccess {
+    fn new(cs: csh, ins: &cs_insn) -> Self {
+        let mut regs_read = [0u16; 64];


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];

tmfink · 2025-04-20T15:51:43Z

Addresses #84

tmfink · 2025-04-20T16:25:01Z

Thanks for the PR, but I decided to go with #149 as a starting point.

SWW13 added 2 commits October 8, 2019 20:04

added cs_regs_access support

c8e130a

fixed detail_enabled check

35ad1ec

tmfink requested changes Oct 9, 2019

View reviewed changes

tmfink self-assigned this Oct 9, 2019

tmfink added the enhancement label Oct 9, 2019

SWW13 added 2 commits October 9, 2019 15:54

added changes

4791dab

cargo fmt

67a4617

SWW13 changed the title ~~WIP: capstone: add cs_regs_access~~ capstone: add cs_regs_access Oct 9, 2019

SWW13 added 2 commits October 9, 2019 16:53

added InsnRegsAccess to prelude

ada788e

added test for regs_read_count and regs_write_count

0475878

tmfink requested changes Dec 5, 2019

View reviewed changes

tmfink closed this Apr 20, 2025

capstone: add cs_regs_access #77

capstone: add cs_regs_access #77

Uh oh!

Conversation

SWW13 commented Oct 8, 2019

Uh oh!

codecov bot commented Oct 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tmfink left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmfink commented Oct 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmfink commented Apr 20, 2025

Uh oh!

tmfink commented Apr 20, 2025

Uh oh!

Uh oh!

codecov bot commented Oct 8, 2019 •

edited

Loading