keystone-engine / keystone

Keystone assembler framework: Core (Arm, Arm64, Hexagon, Mips, PowerPC, Sparc, SystemZ & X86) + bindings
http://www.keystone-engine.org
GNU General Public License v2.0
2.26k stars 453 forks source link

keystone can not encode thumb2 bcc instructions correctlly #451

Open 4B5F5F4B opened 4 years ago

4B5F5F4B commented 4 years ago

Hi all,

I found keystone can not encode thumb2 bcc instructions correctlly.For example:

kstool thumb "bne 0x15f0" 0x1248
bne  0x15f0 = [ 41 f0 f8 82 ]
cstool thumb "\x41\xf0\xf8\x82" 0x1248
1248  41 f0 f8 82  bne.w    #0x283c

I invistigated this issue and foud out the following code my be wrong

//llvm/lib/Target/ARM/ARMGenMCCodeEmitter.inc
case ARM::t2Bcc: {
      // op: p
      op = getMachineOpValue(MI, MI.getOperand(1), Fixups, STI);
      Value |= (op & UINT64_C(15)) << 22;
      // op: target

     /*
        hey, keystone, what are you dong?
     */
      op = getBranchTargetOpValue(MI, 0, Fixups, STI);
      Value |= (op & UINT64_C(1048576)) << 6;
      Value |= (op & UINT64_C(258048)) << 4;
      Value |= (op & UINT64_C(262144)) >> 5;
      Value |= (op & UINT64_C(524288)) >> 8;
      Value |= (op & UINT64_C(4094)) >> 1;
      break;
 }

I pathced the code in this way

case ARM::t2Bcc: {
      // op: p
     op = getMachineOpValue(MI, MI.getOperand(1), Fixups, STI);
      Value |= (op & UINT64_C(15)) << 22;
      // op: target

      op = getThumbBCCTargetOpValue(MI, 0, Fixups, STI);
      op = (op << 1);

      Value |= ((op << (26 - 20)) & 0x04000000u) |           // Shift bit 20 to 26, "S".
          ((op >> (19 - 11)) & 0x00000800u) |                // Shift bit 19 to 13, "J1".
          ((op >> (18 - 13)) & 0x00002000u) |                // Shift bit 18 to 11, "J2".
          ((op << (16 - 12)) & 0x003f0000u) |                // Shift bits 12-17 to 16-25, "imm6".
          ((op >> (1 - 0)) & 0x000007ffu);                   // Shift bits 1-12 to 0-11, "imm11".

      break;
 }

and it worked well then.

kstool thumb "bne  0x15f0" 0x1248
bne  0x15f0 = [ 40 f0 d2 81 ]
cstool thumb "\x40\xf0\xd2\x81" 0x1248
1248  40 f0 d2 81  bne.w    #0x15f0
NyaMisty commented 4 years ago

I can comfirm that the problem exists :(

NyaMisty commented 4 years ago

And in fact changing the

/// getBranchTargetOpValue - Helper function to get the branch target operand,
/// which is either an immediate or requires a fixup.
static uint32_t getBranchTargetOpValue(const MCInst &MI, unsigned OpIdx,
                                       unsigned FixupKind,
                                       SmallVectorImpl<MCFixup> &Fixups,
                                       const MCSubtargetInfo &STI) {
  const MCOperand &MO = MI.getOperand(OpIdx);

  // If the destination is an immediate, we have nothing to do.
  if (MO.isImm()) return MO.getImm() - MI.getAddress() - 4;
  assert(MO.isExpr() && "Unexpected branch target type!");
  const MCExpr *Expr = MO.getExpr();
  MCFixupKind Kind = MCFixupKind(FixupKind);
  Fixups.push_back(MCFixup::create(0, Expr, Kind, MI.getLoc()));

  // All of the information is in the fixup.
  return 0;
}

should be an more elegant way. (Also tested)