Introduce an owned insn type (#123)

* Remove unused `#[must_use]` attribute

* Introduce a OwnedInsn type

- Use `ptr::read` instead of `MaybeUninit` + `ptr::copy`
- Implement Deref for &OwnedInsn to &Insn
- Make `Instruction` an unsafe trait
- Add documentation to new items

* Formatting + Clippy

capstone-rs/src/arch/evm.rs

@@ -36,15 +36,9 @@ impl_PartialEq_repr_fields!(EvmInsnDetail<'a> [ 'a ];
 );
 
 /// EVM has no operands, so this is a zero-size type.
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq, Default)]
 pub struct EvmOperand(());
 
-impl Default for EvmOperand {
-    fn default() -> Self {
-        EvmOperand(())
-    }
-}
-
 // Do not use def_arch_details_struct! since EVM does not have operands
 
 /// Iterates over instruction operands

capstone-rs/src/error.rs

@@ -72,7 +72,6 @@ capstone_error_def!(
     => UnsupportedX86Masm = CS_ERR_X86_MASM;
 );
 
-#[must_use]
 pub type CsResult<T> = result::Result<T, Error>;
 
 impl fmt::Display for Error {

capstone-rs/src/instruction.rs

@@ -1,3 +1,4 @@
+use alloc::{self, boxed::Box};
 use core::convert::{TryFrom, TryInto};
 use core::fmt::{self, Debug, Display, Error, Formatter};
 use core::marker::PhantomData;
@@ -202,6 +203,7 @@ pub struct Insn<'a> {
 ///
 pub struct InsnDetail<'a>(pub(crate) &'a cs_detail, pub(crate) Arch);
 
+#[allow(clippy::len_without_is_empty)]
 impl<'a> Insn<'a> {
     /// Create an `Insn` from a raw pointer to a [`capstone_sys::cs_insn`].
     ///
@@ -214,9 +216,11 @@ impl<'a> Insn<'a> {
     /// no-way actually tied to the cs_insn itself. It is the responsibility of the caller to
     /// ensure that the resulting `Insn` lives only as long as the `cs_insn`. This function
     /// assumes that the pointer passed is non-null and a valid `cs_insn` pointer.
+    ///
+    /// The caller is fully responsible for the backing allocations lifetime, including freeing.
     pub unsafe fn from_raw(insn: *const cs_insn) -> Self {
         Self {
-            insn: *insn,
+            insn: core::ptr::read(insn),
             _marker: PhantomData,
         }
     }
@@ -237,7 +241,7 @@ impl<'a> Insn<'a> {
     }
 
     /// Size of instruction (in bytes)
-    fn len(&self) -> usize {
+    pub fn len(&self) -> usize {
         self.insn.size as usize
     }
 
@@ -256,11 +260,58 @@ impl<'a> Insn<'a> {
     ///
     /// Be careful this is still in early stages and largely untested with various `cs_option` and
     /// architecture matrices
+    ///
+    /// # Safety
+    /// The [`cs_insn::detail`] pointer must be valid and non-null.
     pub(crate) unsafe fn detail(&self, arch: Arch) -> InsnDetail {
         InsnDetail(&*self.insn.detail, arch)
     }
 }
 
+impl<'a> From<&Insn<'_>> for OwnedInsn<'a> {
+    // SAFETY: assumes that `cs_detail` struct transitively only contains owned
+    // types and no pointers, including the union over the architecture-specific
+    // types.
+    fn from(insn: &Insn<'_>) -> Self {
+        let mut new = unsafe { <*const cs_insn>::read(&insn.insn as _) };
+        new.detail = if new.detail.is_null() {
+            new.detail
+        } else {
+            unsafe {
+                let new_detail = Box::new(*new.detail);
+                Box::into_raw(new_detail)
+            }
+        };
+        Self {
+            insn: new,
+            _marker: PhantomData,
+        }
+    }
+}
+
+/// SAFETY:
+/// 1. [`OwnedInsn`] and [`Insn`] must be `#repr(transparent)` of [`cs_insn`]
+/// 2. all [`Insn`] methods must be safe to perform for an [`OwnedInsn`]
+impl<'a> Deref for OwnedInsn<'a> {
+    type Target = Insn<'a>;
+    fn deref(&self) -> &Self::Target {
+        unsafe { &*(&self.insn as *const cs_insn as *const Insn) }
+    }
+}
+
+/// A single disassembled CPU instruction that lives on the Rust heap.
+///
+/// # Detail
+///
+/// To learn how to get more instruction details, see [`InsnDetail`].
+pub struct OwnedInsn<'a> {
+    /// Inner cs_insn
+    pub(crate) insn: cs_insn,
+
+    /// Adds lifetime
+    pub(crate) _marker: PhantomData<&'a InsnDetail<'a>>,
+}
+
 impl<'a> Debug for Insn<'a> {
     fn fmt(&self, fmt: &mut Formatter) -> Result<(), Error> {
         fmt.debug_struct("Insn")
@@ -272,7 +323,6 @@ impl<'a> Debug for Insn<'a> {
             .finish()
     }
 }
-
 impl<'a> Display for Insn<'a> {
     fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
         write!(fmt, "{:#x}: ", self.address())?;
@@ -286,6 +336,25 @@ impl<'a> Display for Insn<'a> {
     }
 }
 
+impl<'a> Drop for OwnedInsn<'a> {
+    fn drop(&mut self) {
+        if let Some(ptr) = core::ptr::NonNull::new(self.insn.detail) {
+            unsafe { drop(Box::from_raw(ptr.as_ptr())) }
+        }
+    }
+}
+
+impl<'a> Debug for OwnedInsn<'a> {
+    fn fmt(&self, fmt: &mut Formatter) -> Result<(), Error> {
+        Debug::fmt(&self.deref(), fmt)
+    }
+}
+impl<'a> Display for OwnedInsn<'a> {
+    fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
+        Display::fmt(&self.deref(), fmt)
+    }
+}
+
 /// Iterator over instruction group ids
 #[derive(Debug, Clone)]
 pub struct InsnGroupIter<'a>(slice::Iter<'a, InsnGroupIdInt>);

capstone-rs/src/test.rs

@@ -3215,3 +3215,19 @@ fn test_insn_from_raw() {
         assert_eq!(format!("{:?}", from_raw_insn), format!("{:?}", insn));
     }
 }
+
+#[test]
+fn test_owned_insn() {
+    let cs = Capstone::new()
+        .x86()
+        .mode(x86::ArchMode::Mode64)
+        .detail(true)
+        .build()
+        .unwrap();
+
+    let insns = cs.disasm_all(X86_CODE, START_TEST_ADDR).unwrap();
+    let owned: Vec<OwnedInsn> = insns.iter().map(|i| i.into()).collect();
+    for (insn, owned) in insns.iter().zip(&owned) {
+        assert_eq!(format!("{:?}", insn), format!("{:?}", owned));
+    }
+}

capstone-sys/build.rs

@@ -256,7 +256,8 @@ fn write_bindgen_bindings(
         .impl_debug(true)
         .constified_enum_module("cs_err|cs_group_type|cs_opt_value")
         .bitfield_enum("cs_mode|cs_ac_type")
-        .rustified_enum(".*");
+        .rustified_enum(".*")
+        .no_copy("cs_insn");
 
     // Whitelist cs_.* functions and types
     let pattern = String::from("cs_.*");

capstone-sys/pre_generated/capstone.rs

@@ -1,4 +1,4 @@
-/* automatically generated by rust-bindgen 0.59.1 */
+/* automatically generated by rust-bindgen 0.59.2 */
 
 pub type va_list = __builtin_va_list;
 pub type __int8_t = libc::c_schar;
@@ -15609,7 +15609,6 @@ impl ::core::fmt::Debug for cs_detail {
 }
 #[doc = " Detail information of disassembled instruction"]
 #[repr(C)]
-#[derive(Copy)]
 pub struct cs_insn {
     #[doc = " Instruction ID (basically a numeric ID for the instruction mnemonic)"]
     #[doc = " Find the instruction id in the '[ARCH]_insn' enum in the header file"]
@@ -15642,11 +15641,6 @@ pub struct cs_insn {
     #[doc = "     is not NULL, its content is still irrelevant."]
     pub detail: *mut cs_detail,
 }
-impl Clone for cs_insn {
-    fn clone(&self) -> Self {
-        *self
-    }
-}
 impl ::core::fmt::Debug for cs_insn {
     fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
         write ! (f , "cs_insn {{ id: {:?}, address: {:?}, size: {:?}, bytes: {:?}, mnemonic: [...], op_str: [...], detail: {:?} }}" , self . id , self . address , self . size , self . bytes , self . detail)

cstool/src/bin/cstool.rs

@@ -274,7 +274,7 @@ fn main() {
         file.read_to_end(&mut buf).expect_exit();
         buf
     } else if let Some(code) = matches.value_of(CODE_ARG) {
-        code.as_bytes().iter().copied().collect()
+        code.as_bytes().to_vec()
     } else {
         let mut buf = Vec::with_capacity(DEFAULT_CAPACITY);
         let stdin = std::io::stdin();