capstone4: powerpc sign bug (works fine in cs5 but could be better)

[trufae]

I'm finally switching to capstone5 in r2, and i'm finding some differences in the analysis and disassemblies of some architectures.

-|       .-> 0x100012c8      3928febc       addi r9, r8, 0xfebc
+|       .-> 0x100012c8      3928febc       addi r9, r8, -0x144

I think that integer values should be disassembled as decimal and not hexadecimal. Also, this exposes a bug in the capstone4 disassembler for powerpc.

the 3rd argument is a signed 2 byte integer as specified in the cpu documentation: Specifies 16-bit signed integer for operation.

[richardlford]

@trufae, this bug was introduced by commit 4948fd1, by @aquynh. That commit was intended to just change the format of printing of signed 16 bit immediates for PowerPC, but it as can be seen from the following code (at that commit)

static void printS16ImmOperand(MCInst *MI, unsigned OpNo, SStream *O)
{
	if (MCOperand_isImm(MCInst_getOperand(MI, OpNo))) {
		unsigned short Imm = (unsigned short)MCOperand_getImm(MCInst_getOperand(MI, OpNo));
        	if (Imm > HEX_THRESHOLD)
        	    SStream_concat(O, "0x%x", Imm);
        	else
        	    SStream_concat(O, "%u", Imm);

	        if (MI->csh->detail) {
			MI->flat_insn->detail->ppc.operands[MI->flat_insn->detail->ppc.op_count].type = PPC_OP_IMM;
			MI->flat_insn->detail->ppc.operands[MI->flat_insn->detail->ppc.op_count].imm = Imm;
			MI->flat_insn->detail->ppc.op_count++;
		}
	} else
		printOperand(MI, OpNo, O);
}

it also had the effect of storing the unsigned value into the ppc.operands[MI->flat_insn->detail->ppc.op_count].imm field, which is definitely incorrect.

This bug is present in the latest released version (as of this writing), 4.0.2. However, it was fixed in the commit 11206de (titled ppc: sync with llvm 7.0.1), also by @aquynh, on 4/30/2019, where this code becomes:

static void printS16ImmOperand(MCInst *MI, unsigned OpNo, SStream *O)
{
	if (MCOperand_isImm(MCInst_getOperand(MI, OpNo))) {
		short Imm = (short)MCOperand_getImm(MCInst_getOperand(MI, OpNo));
		printInt32(O, Imm);

		if (MI->csh->detail) {
			if (MI->csh->doing_mem) {
                MI->flat_insn->detail->ppc.operands[MI->flat_insn->detail->ppc.op_count].mem.disp = Imm;
			} else {
                MI->flat_insn->detail->ppc.operands[MI->flat_insn->detail->ppc.op_count].type = PPC_OP_IMM;
                MI->flat_insn->detail->ppc.operands[MI->flat_insn->detail->ppc.op_count].imm = Imm;
                MI->flat_insn->detail->ppc.op_count++;
            }
		}
	} else
		printOperand(MI, OpNo, O);
}

As can be seen, the immediate is also no longer printed in hex.

I'm currently working with RetDec which is using an older version of Capstone. But that older version of Capstone does not support the endbr32 and endbr64 instructions, so I'm trying to update to a new Capstone, but this bug is causing a problem. It would be nice if it could be back-ported to 4.0.2. For now I'll try the 11206de version with RetDec and see if that works.

[Rot127]

Closed with #2013