capstone ints and enums

[mothran]

I ran into a problem with matching instruction id's to their enum counter parts defined in capstone-sys. Because rust does not have a provide a simple/safe way to cast a u32 to an enum value matching something like instruction id's to a list of possible instructions seems to get tricky.

This applies to instruction ID's as well as reg_ids, op_types and all the other capstone-sys enums.

For example it would be nice to have code as clean as:

fn is_valid_ret(insn: &Insn) -> bool {
    match insn.id().0 {
        x86_insn::X86_INS_RET => true,
        _ => false
    }
}

Maybe the bindgen could be changed for capstone-sys to output constants instead of enums for these types of values? Or am I missing something defined in capstone-sys?

[mothran]

I was curious about what the process would be like so for consideration: https://github.com/mothran/capstone-sys/commit/daf261f6b2b64ec025a886288d8a7ce88fe33ef5 and https://github.com/mothran/capstone-rs/commit/55e67d8c713c58a22b32c6aa0740c9d2184d47f6

I did not update the documentation to clearly reflect the changes, and there are still a number of arch specific enums that would need to be converted to work cleanly in the model above. It was more a PoC to show myself if it would be a fair way to tackle the above issue.

[tmfink]

Thanks for filing the issue!

I see what you are saying, Insn::id() returns an InsnId, which just contains an integer value. However, the architecture-specific instruction enums like X86Insn cannot be created from integers, which makes matching more tedious.

Possible solutions:

1. More constants: (Your recommendation) change the arch instructions enums to constified modules
   • This reduces "type safety" since nothing would prevent you from matching Arm instruction ids values to X86 instructions.
2. More enums: change the Insn type to be an enum over architecture types. For example, there would be a X86(X86Insn) variant.
   • This would incur some runtime overhead to "unwrap" a Insn to a known variant such as X86Insn
3. More enums & generic Capstone: to avoid runtime overhead as with (2), we could change the Capstone struct to be generic over architecture types.
   • For example, we would know at compile-time that a Capstone<X86>::disasm() generates X86Insn.
4. X86Insn::from_insn_id(): Add a function to X86Insn (and other archs) to create an X86Insn from an InsnId.
   • We would need to be able to iterate over capstone_sys::x86_insn at compile-time.
   • We would need to add bindgen functionality OR post-process the generated Rust code in the capstone-sys build.rs.

For the short-term, I think we can go with (1) is the easiest. The other methods require more work. It should be easy enough to do.

Please feel free to file PRs!

[tmfink]

Also, to do what you wanted, with the current capstone-rs, you would do something like:

fn is_valid_ret(insn: Insn) -> bool {
    match insn.id().0 {
        x if x == arch::x86::X86Insn::X86_INS_RET as InsnIdInt => true,
        _ => false
    }
}

[mothran]

Thanks for the ideas! I also worry about the type safety but I think most people would be familiar with that specific usage pattern from capstone and other bindings.

I would be happy to clean up my existing const branch (option 1) and make pull requests on both this project and -sys.

[io12]

I am also having this issue.