From 40bc0f3ba8b181391f09d720e80c3f378a6c7a4d Mon Sep 17 00:00:00 2001 From: Oliver Stannard Date: Tue, 10 Oct 2017 12:38:22 +0000 Subject: [PATCH] [ARM, Asm] Harden GNU LDRD/STRD aliases against invalid inputs Previously, the code that implemented the GNU assembler aliases for the LDRD and STRD instructions (where the second register is omitted) assumed that the input was a valid instruction. This caused assertion failures for every example in ldrd-strd-gnu-bad-inst.s. This improves this code so that it bails out if the instruction is not in the expected format, the check bails out, and the asm parser is run on the unmodified instruction. It also relaxes the alias on thumb targets, so that unaligned pairs of registers can be used. The restriction that Rt must be even-numbered only applies to the ARM versions of these instructions. Differential revision: https://reviews.llvm.org/D36732 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@315305 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 68 ++++++++++++++++------ test/MC/ARM/ldrd-strd-gnu-arm-bad-regs.s | 19 ++++++ test/MC/ARM/ldrd-strd-gnu-bad-inst.s | 29 +++++++++ test/MC/ARM/ldrd-strd-gnu-sp.s | 26 +++++++-- test/MC/ARM/ldrd-strd-gnu-thumb-bad-regs.s | 7 +-- test/MC/ARM/ldrd-strd-gnu-thumb.s | 15 +++++ 6 files changed, 136 insertions(+), 28 deletions(-) create mode 100644 test/MC/ARM/ldrd-strd-gnu-arm-bad-regs.s create mode 100644 test/MC/ARM/ldrd-strd-gnu-bad-inst.s diff --git a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp index f4089482e4a..3792e10e3eb 100644 --- a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp +++ b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp @@ -564,6 +564,7 @@ class ARMAsmParser : public MCTargetAsmParser { bool shouldOmitCCOutOperand(StringRef Mnemonic, OperandVector &Operands); bool shouldOmitPredicateOperand(StringRef Mnemonic, OperandVector &Operands); bool isITBlockTerminator(MCInst &Inst) const; + void fixupGNULDRDAlias(StringRef Mnemonic, OperandVector &Operands); public: enum ARMMatchResultTy { @@ -5865,6 +5866,52 @@ static bool RequiresVFPRegListValidation(StringRef Inst, return false; } +// The GNU assembler has aliases of ldrd and strd with the second register +// omitted. We don't have a way to do that in tablegen, so fix it up here. +// +// We have to be careful to not emit an invalid Rt2 here, because the rest of +// the assmebly parser could then generate confusing diagnostics refering to +// it. If we do find anything that prevents us from doing the transformation we +// bail out, and let the assembly parser report an error on the instruction as +// it is written. +void ARMAsmParser::fixupGNULDRDAlias(StringRef Mnemonic, + OperandVector &Operands) { + if (Mnemonic != "ldrd" && Mnemonic != "strd") + return; + if (Operands.size() < 4) + return; + + ARMOperand &Op2 = static_cast(*Operands[2]); + ARMOperand &Op3 = static_cast(*Operands[3]); + + if (!Op2.isReg()) + return; + if (!Op3.isMem()) + return; + + const MCRegisterClass &GPR = MRI->getRegClass(ARM::GPRRegClassID); + if (!GPR.contains(Op2.getReg())) + return; + + unsigned RtEncoding = MRI->getEncodingValue(Op2.getReg()); + if (!isThumb() && (RtEncoding & 1)) { + // In ARM mode, the registers must be from an aligned pair, this + // restriction does not apply in Thumb mode. + return; + } + if (Op2.getReg() == ARM::PC) + return; + unsigned PairedReg = GPR.getRegister(RtEncoding + 1); + if (!PairedReg || PairedReg == ARM::PC || + (PairedReg == ARM::SP && !hasV8Ops())) + return; + + Operands.insert( + Operands.begin() + 3, + ARMOperand::CreateReg(PairedReg, Op2.getStartLoc(), Op2.getEndLoc())); + return; +} + /// Parse an arm instruction mnemonic followed by its operands. bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name, SMLoc NameLoc, OperandVector &Operands) { @@ -6107,25 +6154,8 @@ bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name, } } - // GNU Assembler extension (compatibility) - if ((Mnemonic == "ldrd" || Mnemonic == "strd")) { - ARMOperand &Op2 = static_cast(*Operands[2]); - ARMOperand &Op3 = static_cast(*Operands[3]); - if (Op3.isMem()) { - assert(Op2.isReg() && "expected register argument"); - - unsigned SuperReg = MRI->getMatchingSuperReg( - Op2.getReg(), ARM::gsub_0, &MRI->getRegClass(ARM::GPRPairRegClassID)); - - assert(SuperReg && "expected register pair"); - - unsigned PairedReg = MRI->getSubReg(SuperReg, ARM::gsub_1); - - Operands.insert( - Operands.begin() + 3, - ARMOperand::CreateReg(PairedReg, Op2.getStartLoc(), Op2.getEndLoc())); - } - } + // GNU Assembler extension (compatibility). + fixupGNULDRDAlias(Mnemonic, Operands); // FIXME: As said above, this is all a pretty gross hack. This instruction // does not fit with other "subs" and tblgen. diff --git a/test/MC/ARM/ldrd-strd-gnu-arm-bad-regs.s b/test/MC/ARM/ldrd-strd-gnu-arm-bad-regs.s new file mode 100644 index 00000000000..bb30bde49af --- /dev/null +++ b/test/MC/ARM/ldrd-strd-gnu-arm-bad-regs.s @@ -0,0 +1,19 @@ +@ RUN: not llvm-mc -triple=armv7-linux-gnueabi %s 2>&1 | FileCheck %s + +.text +.arm +@ CHECK: error: invalid instruction +@ CHECK: ldrd r12, [r0, #512] + ldrd r12, [r0, #512] + +@ CHECK: error: invalid instruction +@ CHECK: strd r12, [r0, #512] + strd r12, [r0, #512] + +@ CHECK: error: invalid instruction +@ CHECK: ldrd r1, [r0, #512] + ldrd r1, [r0, #512] + +@ CHECK: error: invalid instruction +@ CHECK: strd r1, [r0, #512] + strd r1, [r0, #512] diff --git a/test/MC/ARM/ldrd-strd-gnu-bad-inst.s b/test/MC/ARM/ldrd-strd-gnu-bad-inst.s new file mode 100644 index 00000000000..49c7eb12a91 --- /dev/null +++ b/test/MC/ARM/ldrd-strd-gnu-bad-inst.s @@ -0,0 +1,29 @@ +@ RUN: not llvm-mc -triple=armv7-linux-gnueabi %s 2>&1 | FileCheck %s + + .text + .thumb +@ CHECK: error: invalid instruction + strd +@ CHECK: error: invalid instruction + ldrd +@ CHECK: error: invalid instruction + strd r0 +@ CHECK: error: invalid instruction + ldrd r0 +@ CHECK: error: invalid instruction + strd s0, [r0] +@ CHECK: error: invalid instruction + ldrd s0, [r0] + .arm +@ CHECK: error: invalid instruction + strd +@ CHECK: error: invalid instruction + ldrd +@ CHECK: error: invalid instruction + strd r0 +@ CHECK: error: invalid instruction + ldrd r0 +@ CHECK: error: invalid instruction + strd s0, [r0] +@ CHECK: error: invalid instruction + ldrd s0, [r0] diff --git a/test/MC/ARM/ldrd-strd-gnu-sp.s b/test/MC/ARM/ldrd-strd-gnu-sp.s index 21efae98525..3d6db3bf422 100644 --- a/test/MC/ARM/ldrd-strd-gnu-sp.s +++ b/test/MC/ARM/ldrd-strd-gnu-sp.s @@ -1,9 +1,27 @@ // PR19320 -// RUN: llvm-mc -triple=armv7-linux-gnueabi -show-encoding < %s | FileCheck %s -.text +// RUN: not llvm-mc -triple=armv7a-linux-gnueabi -show-encoding < %s 2>&1 | FileCheck %s --check-prefix=V7 +// RUN: llvm-mc -triple=armv8a-linux-gnueabi -show-encoding < %s 2>&1 | FileCheck %s --check-prefix=V8 + .text -// CHECK: ldrd r12, sp, [r0, #32] @ encoding: [0xd0,0xc2,0xc0,0xe1] +// This tries to use the GNU ldrd/strd alias to create an ldrd/strd instruction +// using the sp register. This is valid for V8, but not earlier architectures. + + .arm + +// V7: error: invalid instruction +// V8: ldrd r12, sp, [r0, #32] @ encoding: [0xd0,0xc2,0xc0,0xe1] + ldrd r12, [r0, #32] + +// V7: error: invalid instruction +// V8: strd r12, sp, [r0, #32] @ encoding: [0xf0,0xc2,0xc0,0xe1] + strd r12, [r0, #32] + + .thumb + +// V7: error: invalid instruction +// V8: ldrd r12, sp, [r0, #32] @ encoding: [0xd0,0xe9,0x08,0xcd] ldrd r12, [r0, #32] -// CHECK: strd r12, sp, [r0, #32] @ encoding: [0xf0,0xc2,0xc0,0xe1] +// V7: error: invalid instruction +// V8: strd r12, sp, [r0, #32] @ encoding: [0xc0,0xe9,0x08,0xcd] strd r12, [r0, #32] diff --git a/test/MC/ARM/ldrd-strd-gnu-thumb-bad-regs.s b/test/MC/ARM/ldrd-strd-gnu-thumb-bad-regs.s index 3785815f41c..93e2db1cb0c 100644 --- a/test/MC/ARM/ldrd-strd-gnu-thumb-bad-regs.s +++ b/test/MC/ARM/ldrd-strd-gnu-thumb-bad-regs.s @@ -1,14 +1,11 @@ @ RUN: not llvm-mc -triple=armv7-linux-gnueabi %s 2>&1 | FileCheck %s -@ FIXME: These errors are inaccurate because the error is being reported on the -@ implicit r13 operand added after r12. - .text .thumb -@ CHECK: error: operand must be a register in range [r0, r12] or r14 +@ CHECK: error: invalid instruction @ CHECK: ldrd r12, [r0, #512] ldrd r12, [r0, #512] -@ CHECK: error: operand must be a register in range [r0, r12] or r14 +@ CHECK: error: invalid instruction @ CHECK: strd r12, [r0, #512] strd r12, [r0, #512] diff --git a/test/MC/ARM/ldrd-strd-gnu-thumb.s b/test/MC/ARM/ldrd-strd-gnu-thumb.s index 67d2aa7f548..2536c1ef2f9 100644 --- a/test/MC/ARM/ldrd-strd-gnu-thumb.s +++ b/test/MC/ARM/ldrd-strd-gnu-thumb.s @@ -18,3 +18,18 @@ strd r0, [r10, #512]! strd r0, [r10], #512 strd r0, [r10, #512] + +@ Rt is allowed to be odd for Thumb (but not ARM) +@ CHECK: ldrd r1, r2, [r10, #512]! @ encoding: [0xfa,0xe9,0x80,0x12] +@ CHECK: ldrd r1, r2, [r10], #512 @ encoding: [0xfa,0xe8,0x80,0x12] +@ CHECK: ldrd r1, r2, [r10, #512] @ encoding: [0xda,0xe9,0x80,0x12] + ldrd r1, [r10, #512]! + ldrd r1, [r10], #512 + ldrd r1, [r10, #512] + +@ CHECK: strd r1, r2, [r10, #512]! @ encoding: [0xea,0xe9,0x80,0x12] +@ CHECK: strd r1, r2, [r10], #512 @ encoding: [0xea,0xe8,0x80,0x12] +@ CHECK: strd r1, r2, [r10, #512] @ encoding: [0xca,0xe9,0x80,0x12] + strd r1, [r10, #512]! + strd r1, [r10], #512 + strd r1, [r10, #512] -- 2.40.0