Various improvements to the Rust bindings (#401)

* Fix Rust bindings

* Various small improvements for the Rust bindings

* Remove unused dependencies

* Use the cmake crate to compile keystone

* Add Windows support to Rust bindings

* Cleanup doc a bit

* Fix symlink path

* Link to C++ standard library

* Build all keystone targets

bindings/rust/Cargo.toml

 [package]
 name = "keystone"
-version = "0.9.1"
+version = "0.10.0"
 authors = [
   "Remco Verhoef <remco.verhoef@dutchcoders.io>",
   "Tasuku SUENAGA a.k.a. gunyarakun <tasuku-s-github@titech.ac>"
@@ -10,17 +10,17 @@ license = "GPL-2.0"
 readme = "README.md"
 repository = "https://github.com/keystone-engine/keystone"
 include = [
-  "src/*", "tests/*", "Cargo.toml", "COPYING", "README.md",
-  "keystone-sys/*"
+  "src/*", "tests/*", "Cargo.toml", "COPYING", "README.md"
 ]
 
 [dependencies]
-bitflags = "1.0"
 libc = "0.2"
-keystone-sys = { path = "keystone-sys", version = "0.9.1" }
+keystone-sys = { path = "keystone-sys", version = "0.10" }
 
 [features]
 default = []
 
 use_system_keystone = ["keystone-sys/use_system_keystone"]
 build_keystone_cmake = ["keystone-sys/build_keystone_cmake"]
+
+[workspace]

bindings/rust/Makefile

@@ -9,15 +9,18 @@ package: keystone-sys/keystone
 	cd keystone-sys && cargo package -vv
 	cargo package -vv
 
+# For packaging we need to embed the keystone source in the crate
 keystone-sys/keystone:
 	rsync -a ../.. keystone-sys/keystone --exclude bindings --filter ":- ../../.gitignore"
 
 clean:
-	rm -rf target/ keystone-sys/target/ keystone-sys/keystone/
+	rm -rf keystone-sys/keystone/
+	cargo clean
 
 check:
-	cargo test
+# 	Make sure to only use one test thread as keystone isn't thread-safe
+	cargo test -- --test-threads=1
 
 gen_const:
-	cd .. && python const_generator.py rust
+	cd .. && python2 const_generator.py rust
 	cargo fmt

bindings/rust/README.md

@@ -39,6 +39,7 @@ If you want to use keystone already installed in the system, specify `use_system
 ```
 [dependencies.keystone]
 version = "0.10.0"
+default-features = false
 features = ["use_system_keystone"]
 ```
 

bindings/rust/keystone-sys/Cargo.toml

 [package]
 name = "keystone-sys"
-version = "0.9.0"
+version = "0.10.0"
 authors = [
   "Remco Verhoef <remco.verhoef@dutchcoders.io>",
   "Tasuku SUENAGA a.k.a. gunyarakun <tasuku-s-github@titech.ac>"
@@ -10,21 +10,17 @@ repository = "https://github.com/keystone-engine/keystone"
 documentation = "https://docs.rs/keystone/"
 license = "GPL-2.0"
 build = "build.rs"
-exclude = [
-  "keystone/build/**"
-]
 
 [build-dependencies]
-build-helper = "0.1"
-os_type = "2.0"
 pkg-config = { optional = true, version = "0.3" }
+cmake = { optional = true, version = "0.1" }
 
 [dependencies]
 bitflags = "1.0"
 libc = "0.2"
 
 [features]
-default = []
+default = ["build_keystone_cmake"]
 
 use_system_keystone = ["pkg-config"]
-build_keystone_cmake = []
+build_keystone_cmake = ["cmake"]

bindings/rust/keystone-sys/build.rs

+#[cfg(feature = "build_keystone_cmake")]
+extern crate cmake;
 #[cfg(feature = "use_system_keystone")]
 extern crate pkg_config;
 
-use std::env;
-use std::path::PathBuf;
-use std::process::Command;
+#[cfg(all(not(windows), feature = "build_keystone_cmake"))]
+use std::os::unix::fs::symlink;
+#[cfg(all(windows, feature = "build_keystone_cmake"))]
+use std::os::windows::fs::symlink_dir as symlink;
 
-fn build_with_cmake() {
-    let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
-    let cmake_dir = PathBuf::from("keystone");
-    let build_dir = cmake_dir.join("build");
+#[cfg(feature = "build_keystone_cmake")]
+use std::path::Path;
 
-    if !cmake_dir.exists() {
-        run(Command::new("ln").arg("-s").arg("../../..").arg("keystone"));
+#[cfg(feature = "build_keystone_cmake")]
+fn build_with_cmake() {
+    if !Path::new("keystone").exists() {
+        // This only happens when using the crate via a `git` reference as the
+        // published version already embeds keystone's source.
+        let pwd = std::env::current_dir().unwrap();
+        let keystone_dir = pwd.ancestors().skip(3).next().unwrap();
+        symlink(keystone_dir, "keystone").expect("failed to symlink keystone");
     }
 
-    run(Command::new("mkdir")
-        .current_dir(&cmake_dir)
-        .arg("-p")
-        .arg("build"));
-
-    run(Command::new("../make-share.sh").current_dir(&build_dir));
+    let dest = cmake::Config::new("keystone")
+        .define("BUILD_LIBS_ONLY", "1")
+        .define("BUILD_SHARED_LIBS", "OFF")
+        .define("LLVM_TARGETS_TO_BUILD", "all")
+        // Prevent python from leaving behind `.pyc` files which break `cargo package`
+        .env("PYTHONDONTWRITEBYTECODE", "1")
+        .build();
 
-    run(Command::new("cmake").current_dir(&build_dir).args(&[
-        &format!("-DCMAKE_INSTALL_PREFIX={}", out_dir.display()),
-        "-DCMAKE_BUILD_TYPE=Release",
-        "-DBUILD_LIBS_ONLY=1",
-        "-DCMAKE_OSX_ARCHITECTURES=",
-        "-DBUILD_SHARED_LIBS=ON",
-        "-DLLVM_TARGET_ARCH=host",
-        "-G",
-        "Unix Makefiles",
-        "..",
-    ]));
-
-    run(Command::new("make").current_dir(&build_dir).arg("install"));
-
-    println!("cargo:rustc-link-search=native={}/lib", out_dir.display());
+    println!("cargo:rustc-link-search=native={}/lib", dest.display());
     println!("cargo:rustc-link-lib=keystone");
+
+    let target = std::env::var("TARGET").unwrap();
+    if target.contains("apple") {
+        println!("cargo:rustc-link-lib=dylib=c++");
+    } else if target.contains("linux") {
+        println!("cargo:rustc-link-lib=dylib=stdc++");
+    } else if target.contains("windows") {
+        println!("cargo:rustc-link-lib=dylib=shell32");
+    }
 }
 
 fn main() {
@@ -44,24 +47,7 @@ fn main() {
         #[cfg(feature = "use_system_keystone")]
         pkg_config::find_library("keystone").expect("Could not find system keystone");
     } else {
+        #[cfg(feature = "build_keystone_cmake")]
         build_with_cmake();
     }
 }
-
-fn run(cmd: &mut Command) {
-    println!("run: {:?}", cmd);
-    let status = match cmd.status() {
-        Ok(s) => s,
-        Err(ref e) => fail(&format!("failed to execute command: {}", e)),
-    };
-    if !status.success() {
-        fail(&format!(
-            "command did not execute successfully, got: {}",
-            status
-        ));
-    }
-}
-
-fn fail(s: &str) -> ! {
-    panic!("\n{}\n\nbuild script failed, must exit now", s);
-}

bindings/rust/keystone-sys/src/lib.rs

@@ -9,17 +9,16 @@ extern crate libc;
 
 pub mod keystone_const;
 
-use std::fmt;
+use keystone_const::{Arch, Error, Mode, OptionType, OptionValue};
 use std::ffi::CStr;
+use std::fmt;
 use std::os::raw::c_char;
-use keystone_const::{Arch, Error, Mode, OptionType, OptionValue};
 
 #[allow(non_camel_case_types)]
 pub type ks_handle = libc::size_t;
 
-#[link(name = "keystone")]
 extern "C" {
-    pub fn ks_version(major: *const u32, minor: *const u32) -> u32;
+    pub fn ks_version(major: *mut u32, minor: *mut u32) -> u32;
     pub fn ks_arch_supported(arch: Arch) -> bool;
     pub fn ks_open(arch: Arch, mode: Mode, engine: *mut ks_handle) -> Error;
     pub fn ks_asm(
@@ -38,8 +37,8 @@ extern "C" {
 }
 
 impl Error {
-    pub fn msg(&self) -> String {
-        error_msg(*self)
+    pub fn msg(self) -> String {
+        error_msg(self)
     }
 }
 

bindings/rust/src/lib.rs

-//! Keystone Assembler Engine (www.keystone-engine.org) */
-//! By Nguyen Anh Quynh <aquynh@gmail.com>, 2016 */
-//! Rust bindings by Remco Verhoef <remco@dutchcoders.io>, 2016 */
+//! Keystone Assembler Engine (www.keystone-engine.org) \
+//! By Nguyen Anh Quynh <aquynh@gmail.com>, 2016 \
+//! Rust bindings by Remco Verhoef <remco@dutchcoders.io>, 2016
 //!
 //! ```rust
 //! extern crate keystone;
@@ -16,12 +16,10 @@
 //! }
 //! ```
 
-#![doc(html_root_url = "https://keystone/doc/here/v1")]
-
 extern crate keystone_sys as ffi;
 extern crate libc;
 
-use std::ffi::CString;
+use std::ffi::{CStr, CString};
 use std::fmt;
 
 pub use ffi::keystone_const::*;
@@ -66,7 +64,7 @@ pub fn arch_supported(arch: Arch) -> bool {
 
 pub fn error_msg(error: Error) -> String {
     unsafe {
-        CStr::from_ptr(ffi::ks_strerror(error.bits()))
+        CStr::from_ptr(ffi::ks_strerror(error))
             .to_string_lossy()
             .into_owned()
     }