[ARM][Thumb2] Fix ADD/SUB invalid writes to SP

Summary:
This patch fixes pr23772  [ARM] r226200 can emit illegal thumb2 instruction: "sub sp, r12, #80".
The violation was that SUB and ADD (reg, immediate) instructions can only write to SP if the source register is also SP. So the above instructions was unpredictable.
To enforce that the instruction t2(ADD|SUB)ri does not write to SP we now enforce the destination register to be rGPR (That exclude PC and SP).
Different than the ARM specification, that defines one instruction that can read from SP, and one that can't, here we inserted one that can't write to SP, and other that can only write to SP as to reuse most of the hard-coded size optimizations.
When performing this change, it uncovered that emitting Thumb2 Reg plus Immediate could not emit all variants of ADD SP, SP #imm instructions before so it was refactored to be able to. (see test/CodeGen/Thumb2/mve-stacksplot.mir where we use a subw sp, sp, Imm12 variant )
It also uncovered a disassembly issue of adr.w instructions, that were only written as SUBW instructions (see llvm/test/MC/Disassembler/ARM/thumb2.txt).

Reviewers: eli.friedman, dmgreen, carwil, olista01, efriedma

Reviewed By: efriedma

Subscribers: john.brawn, efriedma, ostannard, kristof.beyls, hiraditya, dmgreen, llvm-commits

Tags: #llvm

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

1 files changed, 69 insertions, 4 deletions

diff --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
index eabc26d05f4..97a28fa233d 100644
--- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
+++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
@@ -204,6 +204,9 @@ static DecodeStatus DecoderGPRRegisterClass(MCInst &Inst, unsigned RegNo,
                                    uint64_t Address, const void *Decoder);
 static DecodeStatus DecodeGPRPairRegisterClass(MCInst &Inst, unsigned RegNo,
                                    uint64_t Address, const void *Decoder);
+static DecodeStatus DecodeGPRspRegisterClass(MCInst &Inst, unsigned RegNo,
+                                             uint64_t Address,
+                                             const void *Decoder);
 static DecodeStatus DecodeHPRRegisterClass(MCInst &Inst, unsigned RegNo,
                                    uint64_t Address, const void *Decoder);
 static DecodeStatus DecodeSPRRegisterClass(MCInst &Inst, unsigned RegNo,
@@ -566,6 +569,9 @@ static DecodeStatus DecodeMVEVPNOT(MCInst &Inst, unsigned Insn,
 static DecodeStatus DecodeMVEOverlappingLongShift(MCInst &Inst, unsigned Insn,
                                                   uint64_t Address,
                                                   const void *Decoder);
+static DecodeStatus DecodeT2AddSubSPImm(MCInst &Inst, unsigned Insn,
+                                        uint64_t Address, const void *Decoder);
+
 #include "ARMGenDisassemblerTables.inc"
 
 static MCDisassembler *createARMDisassembler(const Target &T,
@@ -1231,6 +1237,17 @@ static DecodeStatus DecodeGPRPairRegisterClass(MCInst &Inst, unsigned RegNo,
   return S;
 }
 
+static DecodeStatus DecodeGPRspRegisterClass(MCInst &Inst, unsigned RegNo,
+                                             uint64_t Address,
+                                             const void *Decoder) {
+  if (RegNo != 13)
+    return MCDisassembler::Fail;
+
+  unsigned Register = GPRDecoderTable[RegNo];
+  Inst.addOperand(MCOperand::createReg(Register));
+  return MCDisassembler::Success;
+}
+
 static DecodeStatus DecodetcGPRRegisterClass(MCInst &Inst, unsigned RegNo,
                                    uint64_t Address, const void *Decoder) {
   unsigned Register = 0;
@@ -5588,14 +5605,25 @@ static DecodeStatus DecodeT2Adr(MCInst &Inst, uint32_t Insn,
   unsigned sign1 = fieldFromInstruction(Insn, 21, 1);
   unsigned sign2 = fieldFromInstruction(Insn, 23, 1);
   if (sign1 != sign2) return MCDisassembler::Fail;
+  const unsigned Rd = fieldFromInstruction(Insn, 8, 4);
+  assert(Inst.getNumOperands() == 0 && "We should receive an empty Inst");
+  DecodeStatus S = DecoderGPRRegisterClass(Inst, Rd, Address, Decoder);
 
   unsigned Val = fieldFromInstruction(Insn, 0, 8);
   Val |= fieldFromInstruction(Insn, 12, 3) << 8;
   Val |= fieldFromInstruction(Insn, 26, 1) << 11;
-  Val |= sign1 << 12;
-  Inst.addOperand(MCOperand::createImm(SignExtend32<13>(Val)));
-
-  return MCDisassembler::Success;
+  // If sign, then it is decreasing the address.
+  if (sign1) {
+    // Following ARMv7 Architecture Manual, when the offset
+    // is zero, it is decoded as a subw, not as a adr.w
+    if (!Val) {
+      Inst.setOpcode(ARM::t2SUBri12);
+      Inst.addOperand(MCOperand::createReg(ARM::PC));
+    } else
+      Val = -Val;
+  }
+  Inst.addOperand(MCOperand::createImm(Val));
+  return S;
 }
 
 static DecodeStatus DecodeT2ShifterImmOperand(MCInst &Inst, uint32_t Val,
@@ -6595,3 +6623,40 @@ static DecodeStatus DecodeMVEVPNOT(MCInst &Inst, unsigned Insn, uint64_t Address
   Inst.addOperand(MCOperand::createReg(ARM::VPR));
   return S;
 }
+
+static DecodeStatus DecodeT2AddSubSPImm(MCInst &Inst, unsigned Insn,
+                                        uint64_t Address, const void *Decoder) {
+  const unsigned Rd = fieldFromInstruction(Insn, 8, 4);
+  const unsigned Rn = fieldFromInstruction(Insn, 16, 4);
+  const unsigned Imm12 = fieldFromInstruction(Insn, 26, 1) << 11 |
+                         fieldFromInstruction(Insn, 12, 3) << 8 |
+                         fieldFromInstruction(Insn, 0, 8);
+  const unsigned TypeT3 = fieldFromInstruction(Insn, 25, 1);
+  unsigned sign1 = fieldFromInstruction(Insn, 21, 1);
+  unsigned sign2 = fieldFromInstruction(Insn, 23, 1);
+  unsigned S = fieldFromInstruction(Insn, 20, 1);
+  if (sign1 != sign2)
+    return MCDisassembler::Fail;
+
+  // T3 does a zext of imm12, where T2 does a ThumbExpandImm (T2SOImm)
+  DecodeStatus DS = MCDisassembler::Success;
+  if ((!Check(DS,
+              DecodeGPRspRegisterClass(Inst, Rd, Address, Decoder))) || // dst
+      (!Check(DS, DecodeGPRspRegisterClass(Inst, Rn, Address, Decoder))))
+    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;
+
+  Inst.addOperand(MCOperand::createReg(0)); // pred
+
+  return DS;
+}