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

[AsmWriter] Ensure getMnemonic doesn't return invalid pointers #75783

Merged
merged 4 commits into from
Dec 20, 2023

Conversation

pratlucas
Copy link
Contributor

For instructions that don't map to a mnemonic string, the implementation
of MCInstPrinter::getMnemonic would return an invalid pointer due to the
result of the calculation of the instruction's position in the AsmStrs
table. This patch fixes the issue by ensuring those cases return a
nullptr value instead.

Fixes #74177.

For instructions that don't map to a mnemonic string, the implementation
of MCInstPrinter::getMnemonic would return an invalid pointer due to the
result of the calculation of the instruction's position in the `AsmStrs`
table. This patch fixes the issue by ensuring those cases return a
`nullptr` value instead.

Fixes llvm#74177.
@llvmbot llvmbot added the mc Machine (object) code label Dec 18, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 18, 2023

@llvm/pr-subscribers-mc

Author: Lucas Duarte Prates (pratlucas)

Changes

For instructions that don't map to a mnemonic string, the implementation
of MCInstPrinter::getMnemonic would return an invalid pointer due to the
result of the calculation of the instruction's position in the AsmStrs
table. This patch fixes the issue by ensuring those cases return a
nullptr value instead.

Fixes #74177.


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

2 Files Affected:

  • (modified) llvm/lib/MC/MCAsmStreamer.cpp (+4-1)
  • (modified) llvm/utils/TableGen/AsmWriterEmitter.cpp (+4)
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index 9e1d108ac14dc5..532ac89bf9ff76 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -154,7 +154,10 @@ class MCAsmStreamer final : public MCStreamer {
   void emitGNUAttribute(unsigned Tag, unsigned Value) override;
 
   StringRef getMnemonic(MCInst &MI) override {
-    return InstPrinter->getMnemonic(&MI).first;
+    std::pair<const char *, uint64_t> M = InstPrinter->getMnemonic(&MI);
+    assert((M.second != 0 || M.first == nullptr) &&
+           "Invalid char pointer for instruction with no mnemonic");
+    return M.first;
   }
 
   void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
diff --git a/llvm/utils/TableGen/AsmWriterEmitter.cpp b/llvm/utils/TableGen/AsmWriterEmitter.cpp
index 0220927295cf78..e0cd5fad3254de 100644
--- a/llvm/utils/TableGen/AsmWriterEmitter.cpp
+++ b/llvm/utils/TableGen/AsmWriterEmitter.cpp
@@ -438,6 +438,10 @@ void AsmWriterEmitter::EmitGetMnemonic(
   O << "  // Emit the opcode for the instruction.\n";
   O << BitsString;
 
+  // Make sure we don't return an invalid pointer if bits is 0
+  O << "  if (Bits == 0)\n"
+       "    return {nullptr, Bits};\n";
+
   // Return mnemonic string and bits.
   O << "  return {AsmStrs+(Bits & " << (1 << AsmStrBits) - 1
     << ")-1, Bits};\n\n";

Copy link
Contributor

@tmatheson-arm tmatheson-arm left a comment

Choose a reason for hiding this comment

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

LGMT, surprised this didn't blow up earlier.

@pratlucas pratlucas merged commit b652674 into llvm:main Dec 20, 2023
3 of 4 checks passed
Algunenano pushed a commit to ClickHouse/llvm-project that referenced this pull request Jan 18, 2024
…75783)

For instructions that don't map to a mnemonic string, the implementation
of MCInstPrinter::getMnemonic would return an invalid pointer due to the
result of the calculation of the instruction's position in the `AsmStrs`
table. This patch fixes the issue by ensuring those cases return a
`nullptr` value instead.

Fixes llvm#74177.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#74163 reverted #73777 and followup fixes
4 participants