From f8ea29f91571e4420e4b97be0f90d8ca8789c1e6 Mon Sep 17 00:00:00 2001 From: Sergei Barannikov Date: Sun, 7 Sep 2025 22:29:07 +0300 Subject: [PATCH 1/2] [ARM] Fix a few disassembler bugs --- .../ARM/Disassembler/ARMDisassembler.cpp | 56 +++++++------------ llvm/test/MC/Disassembler/ARM/arm-tests.txt | 2 +- 2 files changed, 22 insertions(+), 36 deletions(-) diff --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp index 41d554f2cece9..9792d95716892 100644 --- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp +++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp @@ -1365,24 +1365,6 @@ static DecodeStatus DecodeRFEInstruction(MCInst &Inst, unsigned Insn, DecodeStatus S = MCDisassembler::Success; unsigned Rn = fieldFromInstruction(Insn, 16, 4); - unsigned mode = fieldFromInstruction(Insn, 23, 2); - - switch (mode) { - case 0: - mode = ARM_AM::da; - break; - case 1: - mode = ARM_AM::ia; - break; - case 2: - mode = ARM_AM::db; - break; - case 3: - mode = ARM_AM::ib; - break; - } - - Inst.addOperand(MCOperand::createImm(mode)); if (!Check(S, DecodeGPRRegisterClass(Inst, Rn, Address, Decoder))) return MCDisassembler::Fail; @@ -2779,10 +2761,6 @@ static DecodeStatus DecodeMVEModImmInstruction(MCInst &Inst, unsigned Insn, Inst.addOperand(MCOperand::createImm(imm)); - Inst.addOperand(MCOperand::createImm(ARMVCC::None)); - Inst.addOperand(MCOperand::createReg(0)); - Inst.addOperand(MCOperand::createImm(0)); - return S; } @@ -2807,7 +2785,6 @@ static DecodeStatus DecodeMVEVADCInstruction(MCInst &Inst, unsigned Insn, return MCDisassembler::Fail; if (!fieldFromInstruction(Insn, 12, 1)) // I bit clear => need input FPSCR Inst.addOperand(MCOperand::createReg(ARM::FPSCR_NZCV)); - Inst.addOperand(MCOperand::createImm(Qd)); return S; } @@ -5956,10 +5933,6 @@ static DecodeStatus DecodeMVEVCMP(MCInst &Inst, unsigned Insn, uint64_t Address, if (!Check(S, predicate_decoder(Inst, fc, Address, Decoder))) return MCDisassembler::Fail; - Inst.addOperand(MCOperand::createImm(ARMVCC::None)); - Inst.addOperand(MCOperand::createReg(0)); - Inst.addOperand(MCOperand::createImm(0)); - return S; } @@ -6103,9 +6076,23 @@ DecodeStatus ARMDisassembler::getInstruction(MCInst &MI, uint64_t &Size, ArrayRef Bytes, uint64_t Address, raw_ostream &CS) const { + DecodeStatus S; if (STI.hasFeature(ARM::ModeThumb)) - return getThumbInstruction(MI, Size, Bytes, Address, CS); - return getARMInstruction(MI, Size, Bytes, Address, CS); + S = getThumbInstruction(MI, Size, Bytes, Address, CS); + else + S = getARMInstruction(MI, Size, Bytes, Address, CS); + if (S == DecodeStatus::Fail) + return S; + + // Verify that the decoded instruction has the correct number of operands. + const MCInstrDesc &MCID = MCII->get(MI.getOpcode()); + if (!MCID.isVariadic() && MI.getNumOperands() != MCID.getNumOperands()) { + reportFatalInternalError(MCII->getName(MI.getOpcode()) + ": expected " + + Twine(MCID.getNumOperands()) + " operands, got " + + Twine(MI.getNumOperands()) + "\n"); + } + + return S; } DecodeStatus ARMDisassembler::getARMInstruction(MCInst &MI, uint64_t &Size, @@ -6144,7 +6131,7 @@ DecodeStatus ARMDisassembler::getARMInstruction(MCInst &MI, uint64_t &Size, const DecodeTable Tables[] = { {DecoderTableVFP32, false}, {DecoderTableVFPV832, false}, {DecoderTableNEONData32, true}, {DecoderTableNEONLoadStore32, true}, - {DecoderTableNEONDup32, true}, {DecoderTablev8NEON32, false}, + {DecoderTableNEONDup32, false}, {DecoderTablev8NEON32, false}, {DecoderTablev8Crypto32, false}, }; @@ -6154,8 +6141,10 @@ DecodeStatus ARMDisassembler::getARMInstruction(MCInst &MI, uint64_t &Size, Size = 4; // Add a fake predicate operand, because we share these instruction // definitions with Thumb2 where these instructions are predicable. - if (Table.DecodePred && !DecodePredicateOperand(MI, 0xE, Address, this)) - return MCDisassembler::Fail; + if (Table.DecodePred && MCII->get(MI.getOpcode()).isPredicable()) { + MI.addOperand(MCOperand::createImm(ARMCC::AL)); + MI.addOperand(MCOperand::createReg(ARM::NoRegister)); + } return Result; } } @@ -6189,8 +6178,6 @@ void ARMDisassembler::AddThumb1SBit(MCInst &MI, bool InITBlock) const { return; } } - - MI.insert(I, MCOperand::createReg(InITBlock ? ARM::NoRegister : ARM::CPSR)); } bool ARMDisassembler::isVectorPredicable(const MCInst &MI) const { @@ -6491,7 +6478,6 @@ DecodeStatus ARMDisassembler::getThumbInstruction(MCInst &MI, uint64_t &Size, STI); if (Result != MCDisassembler::Fail) { Size = 4; - Check(Result, AddThumbPredicate(MI)); return Result; } } diff --git a/llvm/test/MC/Disassembler/ARM/arm-tests.txt b/llvm/test/MC/Disassembler/ARM/arm-tests.txt index 008bb1154e57f..a1016cdb5c8cc 100644 --- a/llvm/test/MC/Disassembler/ARM/arm-tests.txt +++ b/llvm/test/MC/Disassembler/ARM/arm-tests.txt @@ -354,7 +354,7 @@ # CHECK: strheq r0, [r0, -r0] 0xb0 0x00 0x00 0x01 -# CHECK: rfedb #4! +# CHECK: rfedb r2! 0x14 0x0 0x32 0xf9 # CHECK: stc2l p0, c0, [r2], #-96 From f52e2bd57e7193e52f6a48963cf9e370331c4abb Mon Sep 17 00:00:00 2001 From: Sergei Barannikov Date: Mon, 8 Sep 2025 02:02:26 +0300 Subject: [PATCH 2/2] Use UpdateThumbPredicate --- llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp index 9792d95716892..5deffba9aa76b 100644 --- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp +++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp @@ -152,7 +152,7 @@ class ARMDisassembler : public MCDisassembler { void AddThumb1SBit(MCInst &MI, bool InITBlock) const; bool isVectorPredicable(const MCInst &MI) const; DecodeStatus AddThumbPredicate(MCInst&) const; - void UpdateThumbVFPPredicate(DecodeStatus &, MCInst&) const; + void UpdateThumbPredicate(DecodeStatus &S, MCInst &MI) const; llvm::endianness InstructionEndianness; }; @@ -6308,13 +6308,12 @@ ARMDisassembler::AddThumbPredicate(MCInst &MI) const { return S; } -// Thumb VFP instructions are a special case. Because we share their -// encodings between ARM and Thumb modes, and they are predicable in ARM +// Thumb VFP and some NEON instructions are a special case. Because we share +// their encodings between ARM and Thumb modes, and they are predicable in ARM // mode, the auto-generated decoder will give them an (incorrect) // predicate operand. We need to rewrite these operands based on the IT // context as a post-pass. -void ARMDisassembler::UpdateThumbVFPPredicate( - DecodeStatus &S, MCInst &MI) const { +void ARMDisassembler::UpdateThumbPredicate(DecodeStatus &S, MCInst &MI) const { unsigned CC; CC = ITBlock.getITCC(); if (CC == 0xF) @@ -6461,7 +6460,7 @@ DecodeStatus ARMDisassembler::getThumbInstruction(MCInst &MI, uint64_t &Size, decodeInstruction(DecoderTableVFP32, MI, Insn32, Address, this, STI); if (Result != MCDisassembler::Fail) { Size = 4; - UpdateThumbVFPPredicate(Result, MI); + UpdateThumbPredicate(Result, MI); return Result; } } @@ -6478,6 +6477,7 @@ DecodeStatus ARMDisassembler::getThumbInstruction(MCInst &MI, uint64_t &Size, STI); if (Result != MCDisassembler::Fail) { Size = 4; + UpdateThumbPredicate(Result, MI); return Result; } }