From ffb2b347ffbdc667169af4f4627cfdf7d64be6a0 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 27 Jun 2019 12:41:07 +0000 Subject: [PATCH] [ARM] Fix handling of zero offsets in LOB instructions. The BF and WLS/WLSTP instructions have various branch-offset fields occupying different positions and lengths in the instruction encoding, and all of them were decoded at disassembly time by the function DecodeBFLabelOffset() which returned SoftFail if the offset was zero. In fact, it's perfectly fine and not even a SoftFail for most of those offset fields to be zero. The only one that can't be zero is the 4-bit field labelled `boff` in the architecture spec, occupying bits {26-23} of the BF instruction family. If that one is zero, the encoding overlaps other instructions (WLS, DLS, LETP, VCTP), so it ought to be a full Fail. Fixed by adding an extra template parameter to DecodeBFLabelOffset which controls whether a zero offset is accepted or rejected. Adjusted existing tests (only in error messages for bad disassemblies); added extra tests to demonstrate zero offsets being accepted in all the right places, and a few demonstrating rejection of zero `boff`. Reviewers: DavidSpickett, ostannard Subscribers: javed.absar, kristof.beyls, hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D63864 llvm-svn: 364533 --- llvm/lib/Target/ARM/ARMInstrThumb2.td | 17 +++---- .../ARM/Disassembler/ARMDisassembler.cpp | 16 +++---- llvm/test/MC/ARM/mve-misc.s | 4 ++ llvm/test/MC/ARM/thumbv8.1m.s | 20 ++++++++ llvm/test/MC/Disassembler/ARM/mve-misc.txt | 46 ++++++++++--------- .../MC/Disassembler/ARM/mve-qdest-rsrc.txt | 8 ++-- .../test/MC/Disassembler/ARM/thumb2-v8.1m.txt | 18 ++++++++ 7 files changed, 88 insertions(+), 41 deletions(-) diff --git a/llvm/lib/Target/ARM/ARMInstrThumb2.td b/llvm/lib/Target/ARM/ARMInstrThumb2.td index cad35d3e4f8f..1525cff080ab 100644 --- a/llvm/lib/Target/ARM/ARMInstrThumb2.td +++ b/llvm/lib/Target/ARM/ARMInstrThumb2.td @@ -360,18 +360,19 @@ def addrmode_tbh : MemOperand { // Define ARMv8.1-M specific addressing modes. // Label operands for BF/BFL/WLS/DLS/LE -class BFLabelOp +class BFLabelOp : Operand { let EncoderMethod = !strconcat("getBFTargetOpValue<", isNeg, ", ", fixup, ">"); let OperandType = "OPERAND_PCREL"; let DecoderMethod = !strconcat("DecodeBFLabelOperand<", signed, ", ", - isNeg, ", ", size, ">"); + isNeg, ", ", zeroPermitted, ", ", size, ">"); } -def bflabel_u4 : BFLabelOp<"false", "false", "4", "ARM::fixup_bf_branch">; -def bflabel_s12 : BFLabelOp<"true", "false", "12", "ARM::fixup_bfc_target">; -def bflabel_s16 : BFLabelOp<"true", "false", "16", "ARM::fixup_bf_target">; -def bflabel_s18 : BFLabelOp<"true", "false", "18", "ARM::fixup_bfl_target">; +def bflabel_u4 : BFLabelOp<"false", "false", "false", "4", "ARM::fixup_bf_branch">; +def bflabel_s12 : BFLabelOp<"true", "false", "true", "12", "ARM::fixup_bfc_target">; +def bflabel_s16 : BFLabelOp<"true", "false", "true", "16", "ARM::fixup_bf_target">; +def bflabel_s18 : BFLabelOp<"true", "false", "true", "18", "ARM::fixup_bfl_target">; def wlslabel_u11_asmoperand : AsmOperandClass { let Name = "WLSLabel"; @@ -380,7 +381,7 @@ def wlslabel_u11_asmoperand : AsmOperandClass { let DiagnosticString = "loop end is out of range or not a positive multiple of 2"; } -def wlslabel_u11 : BFLabelOp<"false", "false", "11", "ARM::fixup_wls"> { +def wlslabel_u11 : BFLabelOp<"false", "false", "true", "11", "ARM::fixup_wls"> { let ParserMatchClass = wlslabel_u11_asmoperand; } def lelabel_u11_asmoperand : AsmOperandClass { @@ -390,7 +391,7 @@ def lelabel_u11_asmoperand : AsmOperandClass { let DiagnosticString = "loop start is out of range or not a negative multiple of 2"; } -def lelabel_u11 : BFLabelOp<"false", "true", "11", "ARM::fixup_le"> { +def lelabel_u11 : BFLabelOp<"false", "true", "true", "11", "ARM::fixup_le"> { let ParserMatchClass = lelabel_u11_asmoperand; } diff --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp index 548fb10fb3fc..9fb2fa65fddd 100644 --- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp +++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp @@ -488,7 +488,7 @@ static DecodeStatus DecoderForMRRC2AndMCRR2(MCInst &Inst, unsigned Val, static DecodeStatus DecodeForVMRSandVMSR(MCInst &Inst, unsigned Val, uint64_t Address, const void *Decoder); -template +template static DecodeStatus DecodeBFLabelOperand(MCInst &Inst, unsigned val, uint64_t Address, const void *Decoder); static DecodeStatus DecodeBFAfterTargetOperand(MCInst &Inst, unsigned val, @@ -5908,13 +5908,13 @@ static DecodeStatus DecodeForVMRSandVMSR(MCInst &Inst, unsigned Val, return S; } -template +template static DecodeStatus DecodeBFLabelOperand(MCInst &Inst, unsigned Val, uint64_t Address, const void *Decoder) { DecodeStatus S = MCDisassembler::Success; - if (Val == 0) - S = MCDisassembler::SoftFail; + if (Val == 0 && !zeroPermitted) + S = MCDisassembler::Fail; uint64_t DecVal; if (isSigned) @@ -5965,8 +5965,8 @@ static DecodeStatus DecodeLOLoop(MCInst &Inst, unsigned Insn, uint64_t Address, Inst.addOperand(MCOperand::createReg(ARM::LR)); LLVM_FALLTHROUGH; case ARM::t2LE: - if (!Check(S, DecodeBFLabelOperand(Inst, Imm, Address, - Decoder))) + if (!Check(S, DecodeBFLabelOperand( + Inst, Imm, Address, Decoder))) return MCDisassembler::Fail; break; case ARM::t2WLS: @@ -5978,8 +5978,8 @@ static DecodeStatus DecodeLOLoop(MCInst &Inst, unsigned Insn, uint64_t Address, if (!Check(S, DecoderGPRRegisterClass(Inst, fieldFromInstruction(Insn, 16, 4), Address, Decoder)) || - !Check(S, DecodeBFLabelOperand(Inst, Imm, Address, - Decoder))) + !Check(S, DecodeBFLabelOperand( + Inst, Imm, Address, Decoder))) return MCDisassembler::Fail; break; case ARM::t2DLS: diff --git a/llvm/test/MC/ARM/mve-misc.s b/llvm/test/MC/ARM/mve-misc.s index f7893914865a..f3af9e0afe64 100644 --- a/llvm/test/MC/ARM/mve-misc.s +++ b/llvm/test/MC/ARM/mve-misc.s @@ -102,6 +102,10 @@ wlstp.32 lr, r3, .Lendloop # ERROR-NOMVE: [[@LINE+1]]:1: error: invalid instruction wlstp.64 lr, r5, .Lendloop +# CHECK: wlstp.64 lr, r5, #0 @ encoding: [0x35,0xf0,0x01,0xc0] +# ERROR-NOMVE: [[@LINE+1]]:1: error: invalid instruction +wlstp.64 lr, r5, #0 + # CHECK: dlstp.8 lr, r5 @ encoding: [0x05,0xf0,0x01,0xe0] # ERROR-NOMVE: [[@LINE+1]]:1: error: invalid instruction dlstp.8 lr, r5 diff --git a/llvm/test/MC/ARM/thumbv8.1m.s b/llvm/test/MC/ARM/thumbv8.1m.s index 9bb89c71204a..b24f998a1f0c 100644 --- a/llvm/test/MC/ARM/thumbv8.1m.s +++ b/llvm/test/MC/ARM/thumbv8.1m.s @@ -92,6 +92,11 @@ bf #4, #65536 // ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: bf #4, #-65538 +// CHECK: bf #4, #0 +// CHECK-FP: bf #4, #0 +// ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: +bf #4, #0 + // ERROR: :[[@LINE+3]]:{{[0-9]+}}: error: branch target out of range or not a multiple of 2 // ERROR-FP: :[[@LINE+2]]:{{[0-9]+}}: error: branch target out of range or not a multiple of 2 // ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: @@ -102,6 +107,11 @@ bfl #4, #262144 // ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: bfl #4, #-262146 +// CHECK: bfl #4, #0 +// CHECK-FP: bfl #4, #0 +// ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: +bfl #4, #0 + // ERROR: :[[@LINE+3]]:{{[0-9]+}}: error: branch location out of range or not a multiple of 2 // ERROR-FP: :[[@LINE+2]]:{{[0-9]+}}: error: branch location out of range or not a multiple of 2 // ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: @@ -132,6 +142,11 @@ bfcsel #4, #-65538, #8, eq // ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: bfcsel #4, #65534, #10, eq +// CHECK: bfcsel #4, #0, #8, eq +// CHECK-FP: bfcsel #4, #0, #8, eq +// ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: +bfcsel #4, #0, #8, eq + // CHECK: bf .Lbranch, .Ltarget @ encoding: [0x40'B',0xf0'B',0x01'B',0xe0'B'] // CHECK-FP: bf .Lbranch, .Ltarget @ encoding: [0x40'B',0xf0'B',0x01'B',0xe0'B'] // ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: instruction requires: lob @@ -162,6 +177,11 @@ bflx .Lbranch, r7 // ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: instruction requires: lob wls lr, r2, .Lend +// CHECK: wls lr, r2, #0 +// CHECK-FP: wls lr, r2, #0 +// ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: instruction requires: lob +wls lr, r2, #0 + // CHECK: dls lr, r2 @ encoding: [0x42,0xf0,0x01,0xe0] // CHECK-FP: dls lr, r2 @ encoding: [0x42,0xf0,0x01,0xe0] // ERROR-NOLOB: :[[@LINE+1]]:{{[0-9]+}}: error: instruction requires: lob diff --git a/llvm/test/MC/Disassembler/ARM/mve-misc.txt b/llvm/test/MC/Disassembler/ARM/mve-misc.txt index c97dec447d2c..ecb3ee1576d5 100644 --- a/llvm/test/MC/Disassembler/ARM/mve-misc.txt +++ b/llvm/test/MC/Disassembler/ARM/mve-misc.txt @@ -12,87 +12,91 @@ [0x31,0xfe,0x4d,0x0f] # CHECK: wlstp.8 lr, r0, #1668 @ encoding: [0x00,0xf0,0x43,0xc3] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x00,0xf0,0x43,0xc3] # CHECK: wlstp.16 lr, r0, #1668 @ encoding: [0x10,0xf0,0x43,0xc3] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x10,0xf0,0x43,0xc3] # CHECK: wlstp.32 lr, r4, #2706 @ encoding: [0x24,0xf0,0x49,0xcd] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x24,0xf0,0x49,0xcd] # CHECK: wlstp.64 lr, lr, #3026 @ encoding: [0x3e,0xf0,0xe9,0xcd] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x3e,0xf0,0xe9,0xcd] # CHECK: wlstp.8 lr, r5, #3436 @ encoding: [0x05,0xf0,0xb7,0xc6] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x05,0xf0,0xb7,0xc6] # CHECK: wlstp.16 lr, r1, #1060 @ encoding: [0x11,0xf0,0x13,0xc2] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x11,0xf0,0x13,0xc2] # CHECK: wlstp.32 lr, r7, #4036 @ encoding: [0x27,0xf0,0xe3,0xc7] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x27,0xf0,0xe3,0xc7] # CHECK: wlstp.8 lr, r1, #538 @ encoding: [0x01,0xf0,0x0d,0xc9] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x01,0xf0,0x0d,0xc9] # CHECK: wlstp.8 lr, r10, #1404 @ encoding: [0x0a,0xf0,0xbf,0xc2] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x0a,0xf0,0xbf,0xc2] # CHECK: wlstp.8 lr, r10, #1408 @ encoding: [0x0a,0xf0,0xc1,0xc2] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x0a,0xf0,0xc1,0xc2] # CHECK: wlstp.8 lr, r10, #2358 @ encoding: [0x0a,0xf0,0x9b,0xcc] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x0a,0xf0,0x9b,0xcc] # CHECK: wlstp.8 lr, r10, #4086 @ encoding: [0x0a,0xf0,0xfb,0xcf] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x0a,0xf0,0xfb,0xcf] # CHECK: wlstp.8 lr, r11, #1442 @ encoding: [0x0b,0xf0,0xd1,0xca] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x0b,0xf0,0xd1,0xca] +# CHECK: wlstp.64 lr, r5, #0 @ encoding: [0x35,0xf0,0x01,0xc0] +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding +[0x35,0xf0,0x01,0xc0] + # CHECK: dlstp.8 lr, r5 @ encoding: [0x05,0xf0,0x01,0xe0] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x05,0xf0,0x01,0xe0] # CHECK: dlstp.16 lr, r5 @ encoding: [0x15,0xf0,0x01,0xe0] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x15,0xf0,0x01,0xe0] # CHECK: dlstp.32 lr, r7 @ encoding: [0x27,0xf0,0x01,0xe0] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x27,0xf0,0x01,0xe0] # CHECK: dlstp.64 lr, r2 @ encoding: [0x32,0xf0,0x01,0xe0] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x32,0xf0,0x01,0xe0] # CHECK: letp lr, #-2 @ encoding: [0x1f,0xf0,0x01,0xc8] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x1f,0xf0,0x01,0xc8] # CHECK: letp lr, #-8 @ encoding: [0x1f,0xf0,0x05,0xc0] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x1f,0xf0,0x05,0xc0] # CHECK: letp lr, #-4094 @ encoding: [0x1f,0xf0,0xff,0xcf] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x1f,0xf0,0xff,0xcf] # CHECK: lctp @ encoding: [0x0f,0xf0,0x01,0xe0] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x0f,0xf0,0x01,0xe0] # ERROR: [[@LINE+1]]:2: warning: potentially undefined instruction encoding diff --git a/llvm/test/MC/Disassembler/ARM/mve-qdest-rsrc.txt b/llvm/test/MC/Disassembler/ARM/mve-qdest-rsrc.txt index f21a7ee99b24..1730666995a3 100644 --- a/llvm/test/MC/Disassembler/ARM/mve-qdest-rsrc.txt +++ b/llvm/test/MC/Disassembler/ARM/mve-qdest-rsrc.txt @@ -515,17 +515,17 @@ [0x21,0xee,0xef,0x5f] # CHECK: vctp.8 lr @ encoding: [0x0e,0xf0,0x01,0xe8] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x0e,0xf0,0x01,0xe8] # CHECK: vctp.16 r0 @ encoding: [0x10,0xf0,0x01,0xe8] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x10,0xf0,0x01,0xe8] # CHECK: vctp.32 r10 @ encoding: [0x2a,0xf0,0x01,0xe8] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x2a,0xf0,0x01,0xe8] # CHECK: vctp.64 r1 @ encoding: [0x31,0xf0,0x01,0xe8] -# CHECK-NOMVE: [[@LINE+1]]:2: warning: potentially undefined instruction encoding +# CHECK-NOMVE: [[@LINE+1]]:2: warning: invalid instruction encoding [0x31,0xf0,0x01,0xe8] diff --git a/llvm/test/MC/Disassembler/ARM/thumb2-v8.1m.txt b/llvm/test/MC/Disassembler/ARM/thumb2-v8.1m.txt index 2f5ccb2b3cc3..a32c495abf1b 100644 --- a/llvm/test/MC/Disassembler/ARM/thumb2-v8.1m.txt +++ b/llvm/test/MC/Disassembler/ARM/thumb2-v8.1m.txt @@ -1156,3 +1156,21 @@ # ERROR: [[@LINE+1]]:2: warning: invalid instruction encoding [0x50,0xea,0xf0,0x80] + +# CHECK: bf #4, #0 @ encoding: [0x40,0xf1,0x01,0xe0] +[0x40,0xf1,0x01,0xe0] + +# CHECK: bfl #4, #0 @ encoding: [0x00,0xf1,0x01,0xc0] +[0x00,0xf1,0x01,0xc0] + +# CHECK: bfcsel #4, #0, #8, eq @ encoding: [0x02,0xf1,0x01,0xe0] +[0x02,0xf1,0x01,0xe0] + +# CHECK: wls lr, r2, #0 @ encoding: [0x42,0xf0,0x01,0xc0] +[0x42,0xf0,0x01,0xc0] + +# ERROR: [[@LINE+1]]:2: warning: invalid instruction encoding +[0x60,0xf0,0x01,0xe0] + +# ERROR: [[@LINE+1]]:2: warning: invalid instruction encoding +[0x70,0xf0,0x01,0xe0]