Skip to content

Commit 83e5b4f

Browse files
jurahuljurahul
committed
Fix for incorrect compilation with gcc/expensive checks.
Add wrapper structs to discriminate between decode tables that use 2 vs 3 byte for NumToSkip, and overload the `decodeInstruction` function based on this type. This ensures that if we have a mix of 2 and 3 byte coode, it works correctly. Without this, 2 different compilation units may generate 2 different versions of `decodeInstruction` and can cause problems.
1 parent 9de5ff5 commit 83e5b4f

File tree

10 files changed

+143
-104
lines changed

10 files changed

+143
-104
lines changed

‎llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,9 @@ DecodeStatus AArch64Disassembler::getInstruction(MCInst &MI, uint64_t &Size,
244244
uint32_t Insn =
245245
(Bytes[3] << 24) | (Bytes[2] << 16) | (Bytes[1] << 8) | (Bytes[0] << 0);
246246

247-
const uint8_t *Tables[] = {DecoderTable32, DecoderTableFallback32};
247+
const DecoderTable3Bytes Tables[] = {DecoderTable32, DecoderTableFallback32};
248248

249-
for (const auto *Table : Tables) {
249+
for (const auto Table : Tables) {
250250
DecodeStatus Result =
251251
decodeInstruction(Table, MI, Insn, Address, this, STI);
252252

‎llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp‎

Lines changed: 72 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,47 @@ static DecodeStatus decodeVersionImm(MCInst &Inst, unsigned Imm,
487487
//
488488
//===----------------------------------------------------------------------===//
489489

490+
template <typename InsnType>
491+
static DecodeStatus tryDecodeInst(DecoderTable2Bytes Table, MCInst &MI,
492+
InsnType Inst, uint64_t Address,
493+
raw_ostream &Comments,
494+
const AMDGPUDisassembler *Asm) {
495+
assert(MI.getOpcode() == 0);
496+
assert(MI.getNumOperands() == 0);
497+
MCInst TmpInst;
498+
Asm->setHasLiteral(false);
499+
const auto SavedBytes = Asm->getBytes();
500+
501+
SmallString<64> LocalComments;
502+
raw_svector_ostream LocalCommentStream(LocalComments);
503+
Asm->CommentStream = &LocalCommentStream;
504+
505+
DecodeStatus Res = decodeInstruction(Table, TmpInst, Inst, Address, Asm,
506+
Asm->getSubtargetInfo());
507+
508+
Asm->CommentStream = nullptr;
509+
510+
if (Res != MCDisassembler::Fail) {
511+
MI = TmpInst;
512+
Comments << LocalComments;
513+
return MCDisassembler::Success;
514+
}
515+
Asm->setBytes(SavedBytes);
516+
return MCDisassembler::Fail;
517+
}
518+
519+
template <typename InsnType>
520+
static DecodeStatus
521+
tryDecodeInst(DecoderTable2Bytes Table1, DecoderTable2Bytes Table2, MCInst &MI,
522+
InsnType Inst, uint64_t Address, raw_ostream &Comments,
523+
const AMDGPUDisassembler *Asm) {
524+
for (DecoderTable2Bytes T : {Table1, Table2}) {
525+
if (DecodeStatus Res = tryDecodeInst(T, MI, Inst, Address, Comments, Asm))
526+
return Res;
527+
}
528+
return MCDisassembler::Fail;
529+
}
530+
490531
template <typename T> static inline T eatBytes(ArrayRef<uint8_t>& Bytes) {
491532
assert(Bytes.size() >= sizeof(T));
492533
const auto Res =
@@ -539,16 +580,16 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
539580

540581
if (isGFX11() &&
541582
tryDecodeInst(DecoderTableGFX1196, DecoderTableGFX11_FAKE1696, MI,
542-
DecW, Address, CS))
583+
DecW, Address, CS, this))
543584
break;
544585

545586
if (isGFX12() &&
546587
tryDecodeInst(DecoderTableGFX1296, DecoderTableGFX12_FAKE1696, MI,
547-
DecW, Address, CS))
588+
DecW, Address, CS, this))
548589
break;
549590

550591
if (isGFX12() &&
551-
tryDecodeInst(DecoderTableGFX12W6496, MI, DecW, Address, CS))
592+
tryDecodeInst(DecoderTableGFX12W6496, MI, DecW, Address, CS, this))
552593
break;
553594

554595
// Reinitialize Bytes
@@ -557,7 +598,7 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
557598
} else if (Bytes.size() >= 16 &&
558599
STI.hasFeature(AMDGPU::FeatureGFX950Insts)) {
559600
DecoderUInt128 DecW = eat16Bytes(Bytes);
560-
if (tryDecodeInst(DecoderTableGFX940128, MI, DecW, Address, CS))
601+
if (tryDecodeInst(DecoderTableGFX940128, MI, DecW, Address, CS, this))
561602
break;
562603

563604
// Reinitialize Bytes
@@ -568,58 +609,61 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
568609
const uint64_t QW = eatBytes<uint64_t>(Bytes);
569610

570611
if (STI.hasFeature(AMDGPU::FeatureGFX10_BEncoding) &&
571-
tryDecodeInst(DecoderTableGFX10_B64, MI, QW, Address, CS))
612+
tryDecodeInst(DecoderTableGFX10_B64, MI, QW, Address, CS, this))
572613
break;
573614

574615
if (STI.hasFeature(AMDGPU::FeatureUnpackedD16VMem) &&
575-
tryDecodeInst(DecoderTableGFX80_UNPACKED64, MI, QW, Address, CS))
616+
tryDecodeInst(DecoderTableGFX80_UNPACKED64, MI, QW, Address, CS,
617+
this))
576618
break;
577619

578620
if (STI.hasFeature(AMDGPU::FeatureGFX950Insts) &&
579-
tryDecodeInst(DecoderTableGFX95064, MI, QW, Address, CS))
621+
tryDecodeInst(DecoderTableGFX95064, MI, QW, Address, CS, this))
580622
break;
581623

582624
// Some GFX9 subtargets repurposed the v_mad_mix_f32, v_mad_mixlo_f16 and
583625
// v_mad_mixhi_f16 for FMA variants. Try to decode using this special
584626
// table first so we print the correct name.
585627
if (STI.hasFeature(AMDGPU::FeatureFmaMixInsts) &&
586-
tryDecodeInst(DecoderTableGFX9_DL64, MI, QW, Address, CS))
628+
tryDecodeInst(DecoderTableGFX9_DL64, MI, QW, Address, CS, this))
587629
break;
588630

589631
if (STI.hasFeature(AMDGPU::FeatureGFX940Insts) &&
590-
tryDecodeInst(DecoderTableGFX94064, MI, QW, Address, CS))
632+
tryDecodeInst(DecoderTableGFX94064, MI, QW, Address, CS, this))
591633
break;
592634

593635
if (STI.hasFeature(AMDGPU::FeatureGFX90AInsts) &&
594-
tryDecodeInst(DecoderTableGFX90A64, MI, QW, Address, CS))
636+
tryDecodeInst(DecoderTableGFX90A64, MI, QW, Address, CS, this))
595637
break;
596638

597639
if ((isVI() || isGFX9()) &&
598-
tryDecodeInst(DecoderTableGFX864, MI, QW, Address, CS))
640+
tryDecodeInst(DecoderTableGFX864, MI, QW, Address, CS, this))
599641
break;
600642

601-
if (isGFX9() && tryDecodeInst(DecoderTableGFX964, MI, QW, Address, CS))
643+
if (isGFX9() &&
644+
tryDecodeInst(DecoderTableGFX964, MI, QW, Address, CS, this))
602645
break;
603646

604-
if (isGFX10() && tryDecodeInst(DecoderTableGFX1064, MI, QW, Address, CS))
647+
if (isGFX10() &&
648+
tryDecodeInst(DecoderTableGFX1064, MI, QW, Address, CS, this))
605649
break;
606650

607651
if (isGFX12() &&
608652
tryDecodeInst(DecoderTableGFX1264, DecoderTableGFX12_FAKE1664, MI, QW,
609-
Address, CS))
653+
Address, CS, this))
610654
break;
611655

612656
if (isGFX11() &&
613657
tryDecodeInst(DecoderTableGFX1164, DecoderTableGFX11_FAKE1664, MI, QW,
614-
Address, CS))
658+
Address, CS, this))
615659
break;
616660

617661
if (isGFX11() &&
618-
tryDecodeInst(DecoderTableGFX11W6464, MI, QW, Address, CS))
662+
tryDecodeInst(DecoderTableGFX11W6464, MI, QW, Address, CS, this))
619663
break;
620664

621665
if (isGFX12() &&
622-
tryDecodeInst(DecoderTableGFX12W6464, MI, QW, Address, CS))
666+
tryDecodeInst(DecoderTableGFX12W6464, MI, QW, Address, CS, this))
623667
break;
624668

625669
// Reinitialize Bytes
@@ -631,38 +675,40 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
631675
const uint32_t DW = eatBytes<uint32_t>(Bytes);
632676

633677
if ((isVI() || isGFX9()) &&
634-
tryDecodeInst(DecoderTableGFX832, MI, DW, Address, CS))
678+
tryDecodeInst(DecoderTableGFX832, MI, DW, Address, CS, this))
635679
break;
636680

637-
if (tryDecodeInst(DecoderTableAMDGPU32, MI, DW, Address, CS))
681+
if (tryDecodeInst(DecoderTableAMDGPU32, MI, DW, Address, CS, this))
638682
break;
639683

640-
if (isGFX9() && tryDecodeInst(DecoderTableGFX932, MI, DW, Address, CS))
684+
if (isGFX9() &&
685+
tryDecodeInst(DecoderTableGFX932, MI, DW, Address, CS, this))
641686
break;
642687

643688
if (STI.hasFeature(AMDGPU::FeatureGFX950Insts) &&
644-
tryDecodeInst(DecoderTableGFX95032, MI, DW, Address, CS))
689+
tryDecodeInst(DecoderTableGFX95032, MI, DW, Address, CS, this))
645690
break;
646691

647692
if (STI.hasFeature(AMDGPU::FeatureGFX90AInsts) &&
648-
tryDecodeInst(DecoderTableGFX90A32, MI, DW, Address, CS))
693+
tryDecodeInst(DecoderTableGFX90A32, MI, DW, Address, CS, this))
649694
break;
650695

651696
if (STI.hasFeature(AMDGPU::FeatureGFX10_BEncoding) &&
652-
tryDecodeInst(DecoderTableGFX10_B32, MI, DW, Address, CS))
697+
tryDecodeInst(DecoderTableGFX10_B32, MI, DW, Address, CS, this))
653698
break;
654699

655-
if (isGFX10() && tryDecodeInst(DecoderTableGFX1032, MI, DW, Address, CS))
700+
if (isGFX10() &&
701+
tryDecodeInst(DecoderTableGFX1032, MI, DW, Address, CS, this))
656702
break;
657703

658704
if (isGFX11() &&
659705
tryDecodeInst(DecoderTableGFX1132, DecoderTableGFX11_FAKE1632, MI, DW,
660-
Address, CS))
706+
Address, CS, this))
661707
break;
662708

663709
if (isGFX12() &&
664710
tryDecodeInst(DecoderTableGFX1232, DecoderTableGFX12_FAKE1632, MI, DW,
665-
Address, CS))
711+
Address, CS, this))
666712
break;
667713
}
668714

‎llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h‎

Lines changed: 4 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -128,44 +128,6 @@ class AMDGPUDisassembler : public MCDisassembler {
128128

129129
MCOperand errOperand(unsigned V, const Twine& ErrMsg) const;
130130

131-
template <typename InsnType>
132-
DecodeStatus tryDecodeInst(const uint8_t *Table, MCInst &MI, InsnType Inst,
133-
uint64_t Address, raw_ostream &Comments) const {
134-
assert(MI.getOpcode() == 0);
135-
assert(MI.getNumOperands() == 0);
136-
MCInst TmpInst;
137-
HasLiteral = false;
138-
const auto SavedBytes = Bytes;
139-
140-
SmallString<64> LocalComments;
141-
raw_svector_ostream LocalCommentStream(LocalComments);
142-
CommentStream = &LocalCommentStream;
143-
144-
DecodeStatus Res =
145-
decodeInstruction(Table, TmpInst, Inst, Address, this, STI);
146-
147-
CommentStream = nullptr;
148-
149-
if (Res != Fail) {
150-
MI = TmpInst;
151-
Comments << LocalComments;
152-
return MCDisassembler::Success;
153-
}
154-
Bytes = SavedBytes;
155-
return MCDisassembler::Fail;
156-
}
157-
158-
template <typename InsnType>
159-
DecodeStatus tryDecodeInst(const uint8_t *Table1, const uint8_t *Table2,
160-
MCInst &MI, InsnType Inst, uint64_t Address,
161-
raw_ostream &Comments) const {
162-
for (const uint8_t *T : {Table1, Table2}) {
163-
if (DecodeStatus Res = tryDecodeInst(T, MI, Inst, Address, Comments))
164-
return Res;
165-
}
166-
return MCDisassembler::Fail;
167-
}
168-
169131
Expected<bool> onSymbolStart(SymbolInfoTy &Symbol, uint64_t &Size,
170132
ArrayRef<uint8_t> Bytes,
171133
uint64_t Address) const override;
@@ -294,6 +256,10 @@ class AMDGPUDisassembler : public MCDisassembler {
294256
bool hasKernargPreload() const;
295257

296258
bool isMacDPP(MCInst &MI) const;
259+
260+
void setHasLiteral(bool Val) const { HasLiteral = Val; }
261+
void setBytes(ArrayRef<uint8_t> Val) const { Bytes = Val; }
262+
ArrayRef<uint8_t> getBytes() const { return Bytes; }
297263
};
298264

299265
//===----------------------------------------------------------------------===//

‎llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -809,25 +809,20 @@ DecodeStatus ARMDisassembler::getARMInstruction(MCInst &MI, uint64_t &Size,
809809
return checkDecodedInstruction(MI, Size, Address, CS, Insn, Result);
810810
}
811811

812-
struct DecodeTable {
813-
const uint8_t *P;
814-
bool DecodePred;
815-
};
816-
817-
const DecodeTable Tables[] = {
812+
constexpr std::pair<DecoderTable2Bytes, bool> Tables[] = {
818813
{DecoderTableVFP32, false}, {DecoderTableVFPV832, false},
819814
{DecoderTableNEONData32, true}, {DecoderTableNEONLoadStore32, true},
820815
{DecoderTableNEONDup32, true}, {DecoderTablev8NEON32, false},
821816
{DecoderTablev8Crypto32, false},
822817
};
823818

824-
for (auto Table : Tables) {
825-
Result = decodeInstruction(Table.P, MI, Insn, Address, this, STI);
819+
for (auto [Table, DecodePred] : Tables) {
820+
Result = decodeInstruction(Table, MI, Insn, Address, this, STI);
826821
if (Result != MCDisassembler::Fail) {
827822
Size = 4;
828823
// Add a fake predicate operand, because we share these instruction
829824
// definitions with Thumb2 where these instructions are predicable.
830-
if (Table.DecodePred && !DecodePredicateOperand(MI, 0xE, Address, this))
825+
if (DecodePred && !DecodePredicateOperand(MI, 0xE, Address, this))
831826
return MCDisassembler::Fail;
832827
return Result;
833828
}
@@ -1254,9 +1249,9 @@ DecodeStatus ARMDisassembler::getThumbInstruction(MCInst &MI, uint64_t &Size,
12541249
}
12551250

12561251
uint32_t Coproc = fieldFromInstruction(Insn32, 8, 4);
1257-
const uint8_t *DecoderTable = ARM::isCDECoproc(Coproc, STI)
1258-
? DecoderTableThumb2CDE32
1259-
: DecoderTableThumb2CoProc32;
1252+
const auto DecoderTable = ARM::isCDECoproc(Coproc, STI)
1253+
? DecoderTableThumb2CDE32
1254+
: DecoderTableThumb2CoProc32;
12601255
Result =
12611256
decodeInstruction(DecoderTable, MI, Insn32, Address, this, STI);
12621257
if (Result != MCDisassembler::Fail) {

‎llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,8 +458,7 @@ static DecodeStatus readInstruction32(ArrayRef<uint8_t> Bytes, uint64_t Address,
458458
return MCDisassembler::Success;
459459
}
460460

461-
static const uint8_t *getDecoderTable(uint64_t Size) {
462-
461+
static DecoderTable2Bytes getDecoderTable(uint64_t Size) {
463462
switch (Size) {
464463
case 2:
465464
return DecoderTable16;

‎llvm/lib/Target/Hexagon/Disassembler/HexagonDisassembler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ DecodeStatus HexagonDisassembler::getSingleInstruction(MCInst &MI, MCInst &MCB,
327327
if ((Instruction & HexagonII::INST_PARSE_MASK) ==
328328
HexagonII::INST_PARSE_DUPLEX) {
329329
unsigned duplexIClass;
330-
uint8_t const *DecodeLow, *DecodeHigh;
330+
DecoderTable2Bytes DecodeLow, DecodeHigh;
331331
duplexIClass = ((Instruction >> 28) & 0xe) | ((Instruction >> 13) & 0x1);
332332
switch (duplexIClass) {
333333
default:

‎llvm/lib/Target/MSP430/Disassembler/MSP430Disassembler.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ static AddrMode DecodeDstAddrMode(unsigned Insn) {
201201
return Ad ? amIndexed : amRegister;
202202
}
203203

204-
static const uint8_t *getDecoderTable(AddrMode SrcAM, unsigned Words) {
204+
static DecoderTable2Bytes getDecoderTable(AddrMode SrcAM, unsigned Words) {
205205
assert(0 < Words && Words < 4 && "Incorrect number of words");
206206
switch (SrcAM) {
207207
default:
@@ -308,7 +308,8 @@ DecodeStatus MSP430Disassembler::getInstructionII(MCInst &MI, uint64_t &Size,
308308
break;
309309
}
310310

311-
const uint8_t *DecoderTable = Words == 2 ? DecoderTable32 : DecoderTable16;
311+
DecoderTable2Bytes DecoderTable =
312+
Words == 2 ? DecoderTable32 : DecoderTable16;
312313
DecodeStatus Result = decodeInstruction(DecoderTable, MI, Insn, Address,
313314
this, STI);
314315
if (Result != MCDisassembler::Fail) {

‎llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ void RISCVDisassembler::addSPOperands(MCInst &MI) const {
675675
namespace {
676676

677677
struct DecoderListEntry {
678-
const uint8_t *Table;
678+
DecoderTable2Bytes Table;
679679
FeatureBitset ContainedFeatures;
680680
const char *Desc;
681681

‎llvm/lib/Target/SystemZ/Disassembler/SystemZDisassembler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ DecodeStatus SystemZDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
331331
return MCDisassembler::Fail;
332332

333333
// The top 2 bits of the first byte specify the size.
334-
const uint8_t *Table;
334+
DecoderTable2Bytes Table;
335335
if (Bytes[0] < 0x40) {
336336
Size = 2;
337337
Table = DecoderTable16;

0 commit comments

Comments
 (0)