[Enhancement] Add additional instruction groups

[luser]

I'm working on swapping out Breakpad's local copy of libdisasm with capstone, and it's generally going well, except that libdisasm has a richer set of instruction groups as well as instruction types:
https://chromium.googlesource.com/breakpad/breakpad/+/4f512397f5d7fcff4681b2e6b651c28d881bfb34/src/third_party/libdisasm/libdis.h#279

And the Breakpad code uses those groups/types a lot:
https://chromium.googlesource.com/breakpad/breakpad/+/4f512397f5d7fcff4681b2e6b651c28d881bfb34/src/processor/disassembler_x86.cc#86

If I wrote a patch to add some of these groups, would you be amenable to taking it?

[aquynh]

this is definitely interesting. for the record, could you please list all the new groups here for discussion?

thanks.

[luser]

The full list of groups from libdisasm is:

enum x86_insn_group {
    insn_none = 0,      /* invalid instruction */
        insn_controlflow = 1,
        insn_arithmetic = 2,
        insn_logic = 3,
        insn_stack = 4,
        insn_comparison = 5,
        insn_move = 6,
        insn_string = 7,
        insn_bit_manip = 8,
        insn_flag_manip = 9,
        insn_fpu = 10,
        insn_interrupt = 13,
        insn_system = 14,
        insn_other = 15
};

The full list of types is:

enum x86_insn_type {
    insn_invalid = 0,   /* invalid instruction */
        /* insn_controlflow */
        insn_jmp = 0x1001,
        insn_jcc = 0x1002,
        insn_call = 0x1003,
        insn_callcc = 0x1004,
        insn_return = 0x1005,
        /* insn_arithmetic */
        insn_add = 0x2001,
        insn_sub = 0x2002,
        insn_mul = 0x2003,
        insn_div = 0x2004,
        insn_inc = 0x2005,
        insn_dec = 0x2006,
        insn_shl = 0x2007,
        insn_shr = 0x2008,
        insn_rol = 0x2009,
        insn_ror = 0x200A,
        /* insn_logic */
        insn_and = 0x3001,
        insn_or = 0x3002,
        insn_xor = 0x3003,
        insn_not = 0x3004,
        insn_neg = 0x3005,
        /* insn_stack */
        insn_push = 0x4001,
        insn_pop = 0x4002,
        insn_pushregs = 0x4003,
        insn_popregs = 0x4004,
        insn_pushflags = 0x4005,
        insn_popflags = 0x4006,
        insn_enter = 0x4007,
        insn_leave = 0x4008,
        /* insn_comparison */
        insn_test = 0x5001,
        insn_cmp = 0x5002,
        /* insn_move */
        insn_mov = 0x6001,      /* move */
        insn_movcc = 0x6002,    /* conditional move */
        insn_xchg = 0x6003,     /* exchange */
        insn_xchgcc = 0x6004,   /* conditional exchange */
        /* insn_string */
        insn_strcmp = 0x7001,
        insn_strload = 0x7002,
        insn_strmov = 0x7003,
        insn_strstore = 0x7004,
        insn_translate = 0x7005,        /* xlat */
        /* insn_bit_manip */
        insn_bittest = 0x8001,
        insn_bitset = 0x8002,
        insn_bitclear = 0x8003,
        /* insn_flag_manip */
        insn_clear_carry = 0x9001,
        insn_clear_zero = 0x9002,
        insn_clear_oflow = 0x9003,
        insn_clear_dir = 0x9004,
        insn_clear_sign = 0x9005,
        insn_clear_parity = 0x9006,
        insn_set_carry = 0x9007,
        insn_set_zero = 0x9008,
        insn_set_oflow = 0x9009,
        insn_set_dir = 0x900A,
        insn_set_sign = 0x900B,
        insn_set_parity = 0x900C,
        insn_tog_carry = 0x9010,
        insn_tog_zero = 0x9020,
        insn_tog_oflow = 0x9030,
        insn_tog_dir = 0x9040,
        insn_tog_sign = 0x9050,
        insn_tog_parity = 0x9060,
        /* insn_fpu */
        insn_fmov = 0xA001,
        insn_fmovcc = 0xA002,
        insn_fneg = 0xA003,
        insn_fabs = 0xA004,
        insn_fadd = 0xA005,
        insn_fsub = 0xA006,
        insn_fmul = 0xA007,
        insn_fdiv = 0xA008,
        insn_fsqrt = 0xA009,
        insn_fcmp = 0xA00A,
        insn_fcos = 0xA00C,
        insn_fldpi = 0xA00D,
        insn_fldz = 0xA00E,
        insn_ftan = 0xA00F,
        insn_fsine = 0xA010,
        insn_fsys = 0xA020,
        /* insn_interrupt */
        insn_int = 0xD001,
        insn_intcc = 0xD002,    /* not present in x86 ISA */
        insn_iret = 0xD003,
        insn_bound = 0xD004,
        insn_debug = 0xD005,
        insn_trace = 0xD006,
        insn_invalid_op = 0xD007,
        insn_oflow = 0xD008,
        /* insn_system */
        insn_halt = 0xE001,
        insn_in = 0xE002,       /* input from port/bus */
        insn_out = 0xE003,      /* output to port/bus */
        insn_cpuid = 0xE004,
        /* insn_other */
        insn_nop = 0xF001,
        insn_bcdconv = 0xF002,  /* convert to or from BCD */
        insn_szconv = 0xF003    /* change size of operand */
};

These are handy because insn_mov, for example, covers mov, movd, movq. The list of types are subsets of each group from the list of groups.

[aquynh]

yes, this sounds very helpful! please go ahead adding these instruction groups to enum x86_insn_group in x86.h, and send a PR.

thanks

[luser]

Just one minor question: do you take PRs against master or next?

[aquynh]

"next" branch, please!

[aquynh]

any update on this proposal?

thanks

[luser]

I'm still interested in doing this work, I've just been tied up in other stuff. The other Breakpad developers would like to have this change in before taking my change to use capstone in Breakpad, so I will find the time to do this soon.

[kabeor]

Close this issue for now because of legacy, we are preparing to release Capstone 5.0, plz feel free to open a new issue if that still has this issue.
thx :)