capstone icon indicating copy to clipboard operation
capstone copied to clipboard

arm64: cs_arm64_op sys field can be arm64_sys_op OR arm64_reg but is defined as arm64_sys_op

Open tmfink opened this issue 3 years ago • 1 comments

The cs_arm64_op field sys can be either arm64_sysreg OR arm64_reg but the field is defined as arm64_sys_op.

https://github.com/capstone-engine/capstone/blob/fe3e7ab6716d0ba249b6779dadd4a3a2eb842f21/include/capstone/arm64.h#L1356

The current state makes it harder to:

  • Use the C interface
    • In fact, cstool does not even print out the "SYS" operands correctly
  • Write bindings in other languages (https://github.com/capstone-rust/capstone-rs/pull/129).

This may also be related to #1760.

arm64_sys_op

$ ./cstool -d arm64 "00 78 08 d5"
 0  00 78 08 d5  at     s1e1r, x0
        ID: 948 (at)
        op_count: 2
                operands[0].type: SYS = 0x4f
                operands[1].type: REG = x0

1st operand "s1e1r" is arm64_sys_op value ARM64_AT_S1E1R.

arm64_reg

$ ./cstool -d arm64 "090038d5"
 0  09 00 38 d5  mrs    x9, midr_el1
        ID: 495 (mrs)
        op_count: 2
                operands[0].type: REG = x9
                operands[0].access: READ | WRITE
                operands[1].type: SYS = 0xc000
                operands[1].access: READ | WRITE
        Registers read: x9
        Registers modified: x9
        Groups: privilege 

The 2nd operand "midr_el1" is arm64_sysreg value ARM64_SYSREG_MIDR_EL1.

tmfink avatar May 10 '22 09:05 tmfink

It looks like for MRS and MSR, the operand type should really be ARM64_OP_REG_MRS and ARM64_OP_REG_MSR, not ARM64_OP_SYS.

I think the confusion arises from the "SYS" instruction (for which AT / DC / IC / TLBI are aliases) and MSR / MRS refer to "system registers".

This is a regression since 4.x, check out old printMRSSystemRegister, which sets the type as ARM64_OP_REG_MRS:

https://github.com/capstone-engine/capstone/blob/master/arch/AArch64/AArch64InstPrinter.c#L1603

but now sets the type to ARM64_OP_SYS:

https://github.com/capstone-engine/capstone/blob/next/arch/AArch64/AArch64InstPrinter.c#L2026

adamjseitz avatar May 10 '22 12:05 adamjseitz

@adamjseitz nice analysis. Here are the permalinks:

Old way (v4 branch): https://github.com/capstone-engine/capstone/blob/0efa3cc530ea188c0e03c945ab884ee19dd16342/arch/AArch64/AArch64InstPrinter.c#L1612

New way (next branch currently): https://github.com/capstone-engine/capstone/blob/ba2199042700e6bda2e0069e924370a7b2530ac0/arch/AArch64/AArch64InstPrinter.c#L2152

Currently in the next branch, if the operand is ARM64_SYSREG_DBGDTRRX_EL0, ARM64_SYSREG_VSCTLR_EL2, or a readable register, then the type is set to ARM64_OP_SYS. Otherwise, the type is set to ARM64_OP_REG_MRS.

tmfink avatar Oct 16 '22 18:10 tmfink

@Rot127 take a look at this one too please

XVilka avatar Jun 29 '23 04:06 XVilka

Will be fixed with https://github.com/capstone-engine/capstone/pull/2026

System operands are described in way more detail (just as LLVM defines them).

typedef enum {
    [...]

    // Different system operands.
    AArch64_OP_SVCR = CS_OP_SPECIAL + 4,
    AArch64_OP_AT = CS_OP_SPECIAL + 5,
    AArch64_OP_DB = CS_OP_SPECIAL + 6,
    AArch64_OP_DC = CS_OP_SPECIAL + 7,
    AArch64_OP_ISB = CS_OP_SPECIAL + 8,
    AArch64_OP_TSB = CS_OP_SPECIAL + 9,
    AArch64_OP_PRFM = CS_OP_SPECIAL + 10,
    AArch64_OP_SVEPRFM = CS_OP_SPECIAL + 11,
    AArch64_OP_RPRFM = CS_OP_SPECIAL + 12,
    AArch64_OP_PSTATEIMM0_15 = CS_OP_SPECIAL + 13,
    AArch64_OP_PSTATEIMM0_1 = CS_OP_SPECIAL + 14,
    AArch64_OP_PSB = CS_OP_SPECIAL + 15,
    AArch64_OP_BTI = CS_OP_SPECIAL + 16,
    AArch64_OP_SVEPREDPAT = CS_OP_SPECIAL + 17,
    AArch64_OP_SVEVECLENSPECIFIER = CS_OP_SPECIAL + 18,
} aarch64_op_type;

typedef union {
    aarch64_sysreg sysreg;
    aarch64_tlbi tlbi;
    aarch64_ic ic;
} aarch64_sysop_reg;


typedef union {
    aarch64_dbnxs dbnxs;
    aarch64_exactfpimm exactfpimm;
} aarch64_sysop_imm;


typedef union {
    aarch64_svcr svcr;
    aarch64_at at;
    aarch64_db db;
    aarch64_dc dc;
    aarch64_isb isb;
    aarch64_tsb tsb;
    aarch64_prfm prfm;
    aarch64_sveprfm sveprfm;
    aarch64_rprfm rprfm;
    aarch64_pstateimm0_15 pstateimm0_15;
    aarch64_pstateimm0_1 pstateimm0_1;
    aarch64_psb psb;
    aarch64_bti bti;
    aarch64_svepredpat svepredpat;
    aarch64_sveveclenspecifier sveveclenspecifier;
} aarch64_sysop_alias;


typedef union {
    aarch64_sysop_reg reg;
    aarch64_sysop_imm imm;
    aarch64_sysop_alias alias;
} aarch64_sysop;

Rot127 avatar Jun 29 '23 06:06 Rot127