[llvm] fc86e6d - [ARM][disassembler] Fix incorrect number of MCOperands generated by the disassembler

[Min-Yih Hsu via llvm-commits]

Author: Min-Yih Hsu
Date: 2021-04-25T11:55:10-07:00
New Revision: fc86e6d188c38e2cee221fae4960c3307367f387

URL: https://github.com/llvm/llvm-project/commit/fc86e6d188c38e2cee221fae4960c3307367f387
DIFF: https://github.com/llvm/llvm-project/commit/fc86e6d188c38e2cee221fae4960c3307367f387.diff

LOG: [ARM][disassembler] Fix incorrect number of MCOperands generated by the disassembler

Try to fix bug 49974.

This patch fixes two issues:

 1. BL does not use predicate (BL_pred is the predicate version of BL),
    so we shouldn't add predicate operands in DecodeBranchImmInstruction.
 2. Inside DecodeT2AddSubSPImm, we shouldn't add predicate operands into
    the MCInst because ARMDisassembler::AddThumbPredicate will do that for us.
    However, we should handle CC-out operand for t2SUBspImm and t2AddspImm.

Differential Revision: https://reviews.llvm.org/D100585

Added: 
    llvm/test/MC/Disassembler/ARM/bl-arm.txt
    llvm/test/MC/Disassembler/ARM/sub-sp-imm-thumb2.txt

Modified: 
    llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
index 8ea323a9ced54..51fd450345345 100644
--- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
+++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
@@ -2676,8 +2676,12 @@ DecodeBranchImmInstruction(MCInst &Inst, unsigned Insn,
   if (!tryAddingSymbolicOperand(Address, Address + SignExtend32<26>(imm) + 8,
                                 true, 4, Inst, Decoder))
     Inst.addOperand(MCOperand::createImm(SignExtend32<26>(imm)));
-  if (!Check(S, DecodePredicateOperand(Inst, pred, Address, Decoder)))
-    return MCDisassembler::Fail;
+
+  // We already have BL_pred for BL w/ predicate, no need to add addition
+  // predicate opreands for BL
+  if (Inst.getOpcode() != ARM::BL)
+    if (!Check(S, DecodePredicateOperand(Inst, pred, Address, Decoder)))
+      return MCDisassembler::Fail;
 
   return S;
 }
@@ -6670,17 +6674,14 @@ static DecodeStatus DecodeT2AddSubSPImm(MCInst &Inst, unsigned Insn,
     return MCDisassembler::Fail;
   if (TypeT3) {
     Inst.setOpcode(sign1 ? ARM::t2SUBspImm12 : ARM::t2ADDspImm12);
-    S = 0;
     Inst.addOperand(MCOperand::createImm(Imm12)); // zext imm12
   } else {
     Inst.setOpcode(sign1 ? ARM::t2SUBspImm : ARM::t2ADDspImm);
     if (!Check(DS, DecodeT2SOImm(Inst, Imm12, Address, Decoder))) // imm12
       return MCDisassembler::Fail;
+    if (!Check(DS, DecodeCCOutOperand(Inst, S, Address, Decoder))) // cc_out
+      return MCDisassembler::Fail;
   }
-  if (!Check(DS, DecodeCCOutOperand(Inst, S, Address, Decoder))) // cc_out
-    return MCDisassembler::Fail;
-
-  Inst.addOperand(MCOperand::createReg(0)); // pred
 
   return DS;
 }

diff  --git a/llvm/test/MC/Disassembler/ARM/bl-arm.txt b/llvm/test/MC/Disassembler/ARM/bl-arm.txt
new file mode 100644
index 0000000000000..63394c37b1a2e
--- /dev/null
+++ b/llvm/test/MC/Disassembler/ARM/bl-arm.txt
@@ -0,0 +1,11 @@
+# RUN: llvm-mc -triple=arm -disassemble -show-inst < %s | FileCheck %s
+
+# https://bugs.llvm.org/show_bug.cgi?id=49974
+# Redundant (predicate) operands were inserted to the
+# disassembled MCInst.
+
+# CHECK:      bl  #152
+# CHECK-SAME: <MCInst #{{[0-9]+}} BL
+# CHECK-NEXT: <MCOperand Imm:152>>
+
+0x26 0x00 0x00 0xeb

diff  --git a/llvm/test/MC/Disassembler/ARM/sub-sp-imm-thumb2.txt b/llvm/test/MC/Disassembler/ARM/sub-sp-imm-thumb2.txt
new file mode 100644
index 0000000000000..d8a7cf140f6b3
--- /dev/null
+++ b/llvm/test/MC/Disassembler/ARM/sub-sp-imm-thumb2.txt
@@ -0,0 +1,37 @@
+# RUN: llvm-mc -triple=thumbv7 -disassemble -show-inst < %s | FileCheck %s
+
+# https://bugs.llvm.org/show_bug.cgi?id=49974
+# Incorrect number of predicate operands were inserted to the
+# disassembled MCInst.
+
+# CHECK:      subw sp, sp, #1148
+# CHECK-SAME: <MCInst #{{[0-9]+}} t2SUBspImm12
+# CHECK-NEXT: <MCOperand Reg:15>
+# CHECK-NEXT: <MCOperand Reg:15>
+# CHECK-NEXT: <MCOperand Imm:1148>
+# CHECK-NEXT: <MCOperand Imm:14>
+# CHECK-NEXT: <MCOperand Reg:0>>
+
+0xad 0xf2 0x7c 0x4d
+
+# CHECK:      sub.w   sp, sp, #1024
+# CHECK-SAME: <MCInst #{{[0-9]+}} t2SUBspImm
+# CHECK-NEXT: <MCOperand Reg:15>
+# CHECK-NEXT: <MCOperand Reg:15>
+# CHECK-NEXT: <MCOperand Imm:1024>
+# CHECK-NEXT: <MCOperand Imm:14>
+# CHECK-NEXT: <MCOperand Reg:0>
+# CHECK-NEXT: <MCOperand Reg:0>>
+
+0xad,0xf5,0x80,0x6d
+
+# CHECK:      subs.w  sp, sp, #1024
+# CHECK-SAME: <MCInst #{{[0-9]+}} t2SUBspImm
+# CHECK-NEXT: <MCOperand Reg:15>
+# CHECK-NEXT: <MCOperand Reg:15>
+# CHECK-NEXT: <MCOperand Imm:1024>
+# CHECK-NEXT: <MCOperand Imm:14>
+# CHECK-NEXT: <MCOperand Reg:0>
+# CHECK-NEXT: <MCOperand Reg:3>>
+
+0xbd,0xf5,0x80,0x6d