[llvm] [ARM] Verify that disassembled instruction is correct (PR #157360)
Sergei Barannikov via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 19 10:05:59 PDT 2025
https://github.com/s-barannikov updated https://github.com/llvm/llvm-project/pull/157360
>From f8ea29f91571e4420e4b97be0f90d8ca8789c1e6 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
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<uint8_t> 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 <barannikov88 at gmail.com>
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;
}
}
More information about the llvm-commits
mailing list