[AMDGPU][MC] Fix a crash when invalid SDWA encoding is used - #215140
Open
shiltian wants to merge 1 commit into
Open
[AMDGPU][MC] Fix a crash when invalid SDWA encoding is used#215140shiltian wants to merge 1 commit into
shiltian wants to merge 1 commit into
Conversation
Contributor
Author
|
This stack of pull requests is managed by sgh. |
|
@llvm/pr-subscribers-backend-amdgpu Author: Shilei Tian (shiltian) ChangesFixes #215006. Full diff: https://github.com/llvm/llvm-project/pull/215140.diff 3 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index d05997cacba0a..9c70d01abe225 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -453,6 +453,29 @@ DECODE_SDWA(Src32)
DECODE_SDWA(Src16)
DECODE_SDWA(VopcDst)
+// The 3-bit SDWA sel fields only define values up to DWORD; 7 is reserved.
+static DecodeStatus decodeSDWASel(MCInst &Inst, unsigned Imm,
+ uint64_t /* Addr */,
+ const MCDisassembler * /* Decoder */) {
+ using namespace AMDGPU::SDWA;
+
+ if (Imm > SdwaSel::DWORD)
+ return MCDisassembler::Fail;
+ return addOperand(Inst, MCOperand::createImm(Imm));
+}
+
+// The 2-bit SDWA dst_unused field only defines values up to UNUSED_PRESERVE;
+// 3 is reserved.
+static DecodeStatus decodeSDWADstUnused(MCInst &Inst, unsigned Imm,
+ uint64_t /* Addr */,
+ const MCDisassembler * /* Decoder */) {
+ using namespace AMDGPU::SDWA;
+
+ if (Imm > DstUnused::UNUSED_PRESERVE)
+ return MCDisassembler::Fail;
+ return addOperand(Inst, MCOperand::createImm(Imm));
+}
+
static DecodeStatus decodeVersionImm(MCInst &Inst, unsigned Imm,
uint64_t /* Addr */,
const MCDisassembler *Decoder) {
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.td b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
index 5baf52dd12407..1f05bf8961f3b 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
@@ -1247,6 +1247,7 @@ class SDWAOperand<string Id, string Name = NAME>
let ParserMethod =
"[this](OperandVector &Operands) -> ParseStatus { "#
"return parseSDWASel(Operands, \""#Id#"\", AMDGPUOperand::"#ImmTy#"); }";
+ let DecoderMethod = "decodeSDWASel";
}
class ArrayOperand0<string Id, string Name = NAME>
@@ -1312,7 +1313,9 @@ def Dim : CustomOperand</*optional=*/1, type=i8>;
def dst_sel : SDWAOperand<"dst_sel", "SDWADstSel">;
def src0_sel : SDWAOperand<"src0_sel", "SDWASrc0Sel">;
def src1_sel : SDWAOperand<"src1_sel", "SDWASrc1Sel">;
-def dst_unused : CustomOperand<1, "SDWADstUnused">;
+def dst_unused : CustomOperand<1, "SDWADstUnused"> {
+ let DecoderMethod = "decodeSDWADstUnused";
+}
def op_sel0 : ArrayOperand0<"op_sel", "OpSel">;
def op_sel_hi0 : ArrayOperand0<"op_sel_hi", "OpSelHi">;
diff --git a/llvm/test/MC/Disassembler/AMDGPU/decode-err.txt b/llvm/test/MC/Disassembler/AMDGPU/decode-err.txt
index de9bf7bac9548..b4b65d029ce19 100644
--- a/llvm/test/MC/Disassembler/AMDGPU/decode-err.txt
+++ b/llvm/test/MC/Disassembler/AMDGPU/decode-err.txt
@@ -61,6 +61,20 @@
# GFX1250-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
0xfc,0x0e,0x80,0xbe
+# These are v_add_f32_sdwa with a reserved value in one of the SDWA fields:
+# 7 in a 3-bit sel field and 3 in the 2-bit dst_unused field.
+# GCN-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
+0xf9,0x04,0x00,0x02,0x01,0x07,0x06,0x06
+
+# GCN-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
+0xf9,0x04,0x00,0x02,0x01,0x1e,0x06,0x06
+
+# GCN-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
+0xf9,0x04,0x00,0x02,0x01,0x06,0x07,0x06
+
+# GCN-ERR: [[@LINE+1]]:1: warning: invalid instruction encoding
+0xf9,0x04,0x00,0x02,0x01,0x06,0x06,0x07
+
# W32: v_dual_add_f32 v5, 0xaf123456, v2 :: v_dual_fmaak_f32 v6, v3, v1, 0xaf123456 ; encoding: [0xff,0x04,0x02,0xc9,0x03,0x03,0x06,0x05,0x56,0x34,0x12,0xaf]
# W64: [[@LINE+1]]:1: warning: invalid instruction encoding
0xff,0x04,0x02,0xc9,0x03,0x03,0x06,0x05,0x56,0x34,0x12,0xaf
|
arsenm
reviewed
Aug 9, 2026
| const MCDisassembler * /* Decoder */) { | ||
| using namespace AMDGPU::SDWA; | ||
|
|
||
| if (Imm > SdwaSel::DWORD) |
Contributor
There was a problem hiding this comment.
Seems like this kind of check should have been autogenerated
Contributor
Author
There was a problem hiding this comment.
I thought about that, but I don't have any better idea off the top of my mind. Any suggestions?
Contributor
Author
There was a problem hiding this comment.
I can only think about doing some macro to avoid some duplication if that's what you want.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #215006.
This PR was assisted by AI but I reviewed all the changes.