capstone arm: Add ARM_OP

ATTENTION: THIS CHANGE BREAKS COMPATIBILITY

I found the assumption that address is stored as ARM_OP_IMM is actually already broken with #762. However, it is no longer possible to put it to imm since address can be greater than INT32_MAX. Doing so can result in anything, especially faults in arithmetic operations.

Moreover, the old binding in Ocaml ignores bit 31 since the length of int in the language is 31 bits. It means addresses should be treated differently in the binding.

Though the series of changes is a bug fix, breaking compatibility is inevitable.

See also #765.

Sep 08 '16 00:09 akihikodaki

please rebase

Apr 14 '17 08:04 radare

ok to break compat for me unless it introduces regressions

Apr 14 '17 10:04 radare

Can you please generate a new PR on libcapstone?

Mar 04 '21 07:03 pranith

Rebased to commit 3f46b83045049901585688f6a53501e5e032449e with commit 2e6575f20246651cdd9b11f3386ac983f4d9b460, and it is mirrored to libcapstone/libcapstone#3.

Mar 05 '21 06:03 akihikodaki

@aquynh LGTM.

Mar 05 '21 08:03 pranith

Relevant to the ARM auto-sync work https://github.com/capstone-engine/capstone/pull/1949 and https://github.com/capstone-engine/capstone/pull/2045

Jun 16 '23 10:06 XVilka

Indeed it is. But I would not add ARM_OP_ADDR as additional operand type. The underlying problem is more that the immediate type is int32_t and not uint64_t + some information how to interpret it.

The new operand mapping tables contain the type of each operand. (although they seem to be off. E.g. the address is signed. Maybe we need to fix the td files for this:

Here an example:

{ /* ARM_VLD1LNdAsm_16 (384) - ARM_INS_VLD1 - vld1${p}.16	$list, $addr */
{
  { CS_OP_REG, CS_AC_READ, { CS_DATA_TYPE_f64, CS_DATA_TYPE_v8i8, CS_DATA_TYPE_v4i16, CS_DATA_TYPE_v2i32, CS_DATA_TYPE_v1i64, CS_DATA_TYPE_v2f32, CS_DATA_TYPE_v4f16, CS_DATA_TYPE_v4bf16, CS_DATA_TYPE_LAST } }, /* list - DPR */
  { CS_OP_IMM, CS_AC_READ, { CS_DATA_TYPE_i32, CS_DATA_TYPE_LAST } }, /* list - i32imm */
  { CS_OP_MEM | CS_OP_REG, CS_AC_READ, { CS_DATA_TYPE_i32, CS_DATA_TYPE_LAST } }, /* addr - GPR */
  { CS_OP_MEM | CS_OP_IMM, CS_AC_READ, { CS_DATA_TYPE_i32, CS_DATA_TYPE_LAST } }, /* addr - i32imm */
  { CS_OP_IMM, CS_AC_READ, { CS_DATA_TYPE_i32, CS_DATA_TYPE_LAST } }, /* p - i32imm */
  { CS_OP_IMM, CS_AC_READ, { CS_DATA_TYPE_i32, CS_DATA_TYPE_LAST } }, /* p - i32imm */
  { 0 }
}},

So I would propose to solve this problem by making arm_op->imm an uint64_t and add a helper function like

typedef union {
  uint32_t u32;
  int32_t s32;
  float f32;
  ...
} cast_result;

/// Cast the given @imm to its data type specified by LLVM.
/// It returns the cs_data_type the @imm was casted to.
/// The corresponding field in @result is set to the casted value.
cs_data_type map_cast_operand(uint64_t imm, cast_result *result);

Jun 16 '23 12:06 Rot127

I'm closing this pull request. As @Rot127 says, the sign of the immediate should be determined by the instruction. ARM_OP_ADDR clarifies an immediate is unsigned if it's PC-relative, but it cannot tell the sign of the immediate otherwise. There should be some common mechanism to determine the sign of the immediate, and such a mechanism can be implemented easily with a table @Rot127 suggests.

However I suggest to leave imm 32-bit instead of converting it to uint64_t. There is no need for extending it to 64-bit.

Jun 21 '23 06:06 akihikodaki

@akihikodaki Would you mind open an issue about it? Just so we do not forget it and we can assign it to a milestone.

However I suggest to leave imm 32-bit instead of converting it to uint64_t. There is no need for extending it to 64-bit.

Will include this suggestion in https://github.com/capstone-engine/capstone/pull/1949

Jun 21 '23 09:06 Rot127

@akihikodaki Would you mind open an issue about it? Just so we do not forget it and we can assign it to a milestone.

Done: https://github.com/capstone-engine/capstone/issues/2056

Jun 22 '23 08:06 akihikodaki

arm: Add ARM_OP_ADDR