Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TableGen] Fix a potential crash when operand doesn't appear in the instruction pattern #87663

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

shiltian
Copy link
Contributor

@shiltian shiltian commented Apr 4, 2024

We have a check of whether an operand is in the instruction pattern, and emit an
error if it is not, but we simply continue execution, including directly
dereferencing a point-like object InVal, which will be just created when
accessing the map. It contains a nullptr so dereferencing it causes crash.
This is a very trivial fix.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 4, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Shilei Tian (shiltian)

Changes

We have a check of whether an operand is in the instruction pattern, and emit an
error if it is not, but we simply continue execution, including directly
dereferencing a point-like object InVal, which causes crash. This is a very
trivial fix.


Full diff: https://github.com/llvm/llvm-project/pull/87663.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/VOP3PInstructions.td (+2-2)
  • (modified) llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp (+1)
diff --git a/llvm/lib/Target/AMDGPU/VOP3PInstructions.td b/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
index a7d63fdb2e04c8..c929506601c067 100644
--- a/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
@@ -564,9 +564,9 @@ class VOPProfileSMFMAC<VOPProfile P, RegisterOperand _DstRC,
   : VOPProfileMAI<P, _DstRC, _DstRC, _SrcARC> {
   let Src1RC64 = _SrcBRC;
   let Src2VT = DstVT;
-  let Asm64 = " $vdst, $src0, $src1, $idx$cbsz$abid";
+  let Asm64 = " $vdst, $src0, $src1, $idx$cbsz$abid$clamp";
   let Outs64 = (outs DstRC:$vdst);
-  let Ins64 = (ins Src0RC64:$src0, Src1RC64:$src1, VRegSrc_32:$idx, CBSZ:$cbsz, ABID:$abid, Src2RC64:$src2);
+  let Ins64 = (ins Src0RC64:$src0, Src1RC64:$src1, VRegSrc_32:$idx, CBSZ:$cbsz, ABID:$abid, clampmod:$clamp, Src2RC64:$src2);
 }
 
 def VOPProfileMAI_F32_F32_X4    : VOPProfileMAI<VOP_V4F32_F32_F32_V4F32,       AISrc_128_f32,  ADst_128>;
diff --git a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
index 076d0427a85971..eeb3d6873e9915 100644
--- a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
@@ -3872,6 +3872,7 @@ void CodeGenDAGPatterns::parseInstructionPattern(CodeGenInstruction &CGI,
       }
       I.error("Operand $" + OpName +
               " does not appear in the instruction pattern");
+      return;
     }
     TreePatternNodePtr InVal = InstInputs[OpName];
     InstInputs.erase(OpName); // It occurred, remove from map.

Copy link
Collaborator

@kosarev kosarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

…nstruction pattern

We have a check of whether an operand is in the instruction pattern, and emit an
error if it is not, but we simply continue execution, including directly
dereferencing a point-like object `InVal`, which will be just created when
accessing the map. It contains a `nullptr` so dereferencing it causes crash.
This is a very trivial fix.
@shiltian shiltian merged commit cfadf3f into llvm:main Apr 5, 2024
3 of 4 checks passed
@shiltian shiltian deleted the tbgen-crash branch April 5, 2024 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants