M68K: signedness of displacements

[csoren]

I have discovered a few disassembly issues with M68K. While fixing these, I have noticed that m68k_op_mem.in_disp and .out_disp are uint32_t. However, M68K displacements are signed.

M68K displacements can be 8, 16 or 32 bit. The 8 bit displacement is placed in .disp, which is int16_t, so this is correct. The 16 and 32 bit displacements are placed in .in_disp and .out_disp depending on their meaning in the EA. Since .in_disp and .out_disp are unsigned, and the 16 bit displacement is not sign-extended, this is the source of one of the issues I have discovered. The sign bit in the displacement fields is incorrect with 16 bit displacements. The unsignedness causes a great deal of confusion and guesswork, since it does not seem to be documented. It also means there's a bit of two's complement magic is implemented in several places to work around this, for instance objdump-m68k.py.

As part of my upcoming PR, would it be possible to for me to change the signedness of .in_disp and .out_disp? Unfortunately it also means updating several python files, which I am not comfortable with. Working around this unfortunate typing will introduce further two's complement magic.

[Rot127]

As part of my upcoming PR, would it be possible to for me to change the signedness of .in_disp and .out_disp?

Sure! Please go ahead. Just make sure everything is nicely tested.

The tests are in tests/details/m68k.yaml. Just add tests cases like this at the end of the file.

  -
    input:
      bytes: [ 0xf0, 0x10, 0xf0, 0x00 ]
      arch: "m68k"
      options: [ CS_OPT_DETAIL, CS_MODE_BIG_ENDIAN, CS_MODE_M68K_040 ]
      address: 0x1000
    expected:
      insns:
      -
        asm_text: "fmovem #$0, (a0)"
        details:
          regs_read: [ a0 ]
          m68k:
            operands:
              -
                type: M68K_OP_REG_BITS
                register_bits: 0x0
              -
                type: M68K_OP_MEM
                address_mode: M68K_AM_REGI_ADDR

To ensure everything works fine you'd also need to change

capstone/suite/cstest/include/test_detail_m68k.h

Lines 17 to 18 in e1c78c6

	uint32_t in_disp;
	uint32_t out_disp;

capstone/suite/cstest/include/test_detail_m68k.h

Lines 39 to 42 in e1c78c6

	CYAML_FIELD_UINT("in_disp", CYAML_FLAG_OPTIONAL, TestDetailM68KOpMem,
	in_disp),
	CYAML_FIELD_UINT("out_disp", CYAML_FLAG_OPTIONAL, TestDetailM68KOpMem,
	out_disp),

capstone/suite/cstest/src/test_detail_m68k.c

Lines 193 to 200 in e1c78c6

	if (eop->mem->in_disp) {
	compare_uint32_ret(op->mem.in_disp,
	eop->mem->in_disp, false);
	}
	if (eop->mem->out_disp) {
	compare_uint32_ret(op->mem.out_disp,
	eop->mem->out_disp, false);
	}

Unfortunately it also means updating several python files, which I am not comfortable with.

Please remind me in the PR. I can do the changes.