[llvm] [llvm][SystemZ] Set comment stream in SystemZDisassembler::getInstruction (PR #148614)
David Spickett via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 14 04:44:46 PDT 2025
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/148614
>From 7eb7635dd6138c315d5a0a1022265890756e27dd Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 14 Jul 2025 11:30:27 +0000
Subject: [PATCH 1/2] [llvm][SystemZ] Set comment stream in
SystemZDisassembler::getInstruction
This is done by other backends at the start of this function, for example
AArch64Disassembler::getInstruction. Not setting it means you hit asserts
in MCDisassembler::tryAddingSymbolicOperand and MCDisassembler::tryAddingPcLoadReferenceComment
when there is a symbolizer set.
Which I found while debugging a SystemZ program using LLDB.
As the only good way to hit this path is from C++, I've copied
X86's disassembler unit tests and added just enough to hit
an assert, were it not for the changes in this PR.
---
.../Disassembler/SystemZDisassembler.cpp | 2 +
llvm/unittests/MC/SystemZ/CMakeLists.txt | 4 +-
.../MC/SystemZ/SystemZMCDisassemblerTest.cpp | 103 ++++++++++++++++++
3 files changed, 108 insertions(+), 1 deletion(-)
create mode 100644 llvm/unittests/MC/SystemZ/SystemZMCDisassemblerTest.cpp
diff --git a/llvm/lib/Target/SystemZ/Disassembler/SystemZDisassembler.cpp b/llvm/lib/Target/SystemZ/Disassembler/SystemZDisassembler.cpp
index 6ae529e974186..31b4f1196392c 100644
--- a/llvm/lib/Target/SystemZ/Disassembler/SystemZDisassembler.cpp
+++ b/llvm/lib/Target/SystemZ/Disassembler/SystemZDisassembler.cpp
@@ -327,6 +327,8 @@ DecodeStatus SystemZDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
ArrayRef<uint8_t> Bytes,
uint64_t Address,
raw_ostream &CS) const {
+ CommentStream = &CS;
+
// Get the first two bytes of the instruction.
Size = 0;
if (Bytes.size() < 2)
diff --git a/llvm/unittests/MC/SystemZ/CMakeLists.txt b/llvm/unittests/MC/SystemZ/CMakeLists.txt
index 3b7af4a3bbea3..a972ef27ddd8a 100644
--- a/llvm/unittests/MC/SystemZ/CMakeLists.txt
+++ b/llvm/unittests/MC/SystemZ/CMakeLists.txt
@@ -10,6 +10,8 @@ set(LLVM_LINK_COMPONENTS
TargetParser
)
-add_llvm_unittest(SystemZAsmLexerTests
+add_llvm_unittest(SystemZMCTests
SystemZAsmLexerTest.cpp
+ SystemZMCDisassemblerTest.cpp
)
+
diff --git a/llvm/unittests/MC/SystemZ/SystemZMCDisassemblerTest.cpp b/llvm/unittests/MC/SystemZ/SystemZMCDisassemblerTest.cpp
new file mode 100644
index 0000000000000..df59fcb402e21
--- /dev/null
+++ b/llvm/unittests/MC/SystemZ/SystemZMCDisassemblerTest.cpp
@@ -0,0 +1,103 @@
+//===- SystemZMCDisassemblerTest.cpp - Tests for SystemZ MCDisassembler ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/MC/MCAsmInfo.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCDisassembler/MCDisassembler.h"
+#include "llvm/MC/MCDisassembler/MCSymbolizer.h"
+#include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/MC/MCTargetOptions.h"
+#include "llvm/MC/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+struct Context {
+ const char *TripleName = "systemz-unknown";
+ std::unique_ptr<MCRegisterInfo> MRI;
+ std::unique_ptr<MCAsmInfo> MAI;
+ std::unique_ptr<MCContext> Ctx;
+ std::unique_ptr<MCSubtargetInfo> STI;
+ std::unique_ptr<MCDisassembler> DisAsm;
+
+ Context() {
+ LLVMInitializeSystemZTargetInfo();
+ LLVMInitializeSystemZTargetMC();
+ LLVMInitializeSystemZDisassembler();
+
+ // If we didn't build SystemZ, do not run the test.
+ std::string Error;
+ const Target *TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
+ if (!TheTarget)
+ return;
+
+ MRI.reset(TheTarget->createMCRegInfo(TripleName));
+ MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCTargetOptions()));
+ STI.reset(TheTarget->createMCSubtargetInfo(TripleName, "", ""));
+ Ctx = std::make_unique<MCContext>(Triple(TripleName), MAI.get(), MRI.get(),
+ STI.get());
+
+ DisAsm.reset(TheTarget->createMCDisassembler(*STI, *Ctx));
+ }
+
+ operator MCContext &() { return *Ctx; };
+};
+
+Context &getContext() {
+ static Context Ctxt;
+ return Ctxt;
+}
+
+class SystemZMCSymbolizerTest : public MCSymbolizer {
+public:
+ SystemZMCSymbolizerTest(MCContext &MC) : MCSymbolizer(MC, nullptr) {}
+ ~SystemZMCSymbolizerTest() {}
+
+ bool tryAddingSymbolicOperand([[maybe_unused]] MCInst &Inst,
+ [[maybe_unused]] raw_ostream &CStream,
+ [[maybe_unused]] int64_t Value,
+ [[maybe_unused]] uint64_t Address,
+ [[maybe_unused]] bool IsBranch,
+ [[maybe_unused]] uint64_t Offset,
+ [[maybe_unused]] uint64_t OpSize,
+ [[maybe_unused]] uint64_t InstSize) override {
+ return true;
+ }
+
+ void
+ tryAddingPcLoadReferenceComment([[maybe_unused]] raw_ostream &cStream,
+ [[maybe_unused]] int64_t Value,
+ [[maybe_unused]] uint64_t Address) override {}
+};
+
+} // namespace
+
+TEST(SystemZDisassembler, SystemZMCSymbolizerTest) {
+ SystemZMCSymbolizerTest *TestSymbolizer =
+ new SystemZMCSymbolizerTest(getContext());
+ getContext().DisAsm->setSymbolizer(
+ std::unique_ptr<MCSymbolizer>(TestSymbolizer));
+
+ MCInst Inst;
+ uint64_t InstSize;
+
+ // Check that the SystemZ disassembler sets the comment stream before calling
+ // MCDisassembler::tryAddingSymbolicOperand. This will fail an assert if it
+ // does not do that.
+ MCDisassembler::DecodeStatus Status = getContext().DisAsm->getInstruction(
+ Inst, InstSize,
+ // lgrl %r1, 0x1234
+ {0xc4, 0x18, 0x00, 0x00, 0x9a, 0x1a}, 0, nulls());
+ ASSERT_TRUE(Status == MCDisassembler::Success);
+ EXPECT_EQ(InstSize, uint64_t{6});
+}
>From 0a06c7874b73e981b571bc11645ed15044f06e30 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 14 Jul 2025 11:44:31 +0000
Subject: [PATCH 2/2] stray newline
---
llvm/unittests/MC/SystemZ/CMakeLists.txt | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/unittests/MC/SystemZ/CMakeLists.txt b/llvm/unittests/MC/SystemZ/CMakeLists.txt
index a972ef27ddd8a..4af12efa7984d 100644
--- a/llvm/unittests/MC/SystemZ/CMakeLists.txt
+++ b/llvm/unittests/MC/SystemZ/CMakeLists.txt
@@ -14,4 +14,3 @@ add_llvm_unittest(SystemZMCTests
SystemZAsmLexerTest.cpp
SystemZMCDisassemblerTest.cpp
)
-
More information about the llvm-commits
mailing list