https://github.com/DavidSpickett created
https://github.com/llvm/llvm-project/pull/159790
Reverts llvm/llvm-project#158161
Due to reported failures on remote Linux and Swift buildbots.
>From 9907ef2d116b893e0c3cce2cbf228ce52a82a285 Mon Sep 17 00:00:00 2001
From: David Spickett <spickettda...@googlemail.com>
Date: Fri, 19 Sep 2025 16:19:19 +0100
Subject: [PATCH] Revert "RISCV enable assembly unwinding (#158161)"
This reverts commit 1c95d80ba68efd2ca9a0336529ea5fb7dc871417.
---
lldb/include/lldb/Core/Opcode.h | 4 +-
.../RISCV/EmulateInstructionRISCV.cpp | 176 ++--------------
.../RISCV/EmulateInstructionRISCV.h | 4 -
lldb/unittests/Instruction/CMakeLists.txt | 5 -
.../RISCV/TestRiscvInstEmulation.cpp | 188 ------------------
5 files changed, 13 insertions(+), 364 deletions(-)
delete mode 100644 lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp
diff --git a/lldb/include/lldb/Core/Opcode.h b/lldb/include/lldb/Core/Opcode.h
index 7e756d3f15d22..7bbd73d039f99 100644
--- a/lldb/include/lldb/Core/Opcode.h
+++ b/lldb/include/lldb/Core/Opcode.h
@@ -223,9 +223,7 @@ class Opcode {
int Dump(Stream *s, uint32_t min_byte_width) const;
const void *GetOpcodeBytes() const {
- return ((m_type == Opcode::eTypeBytes || m_type ==
Opcode::eType16_32Tuples)
- ? m_data.inst.bytes
- : nullptr);
+ return ((m_type == Opcode::eTypeBytes) ? m_data.inst.bytes : nullptr);
}
uint32_t GetByteSize() const {
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index 20661290ca4c6..5e429a92613ce 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -33,10 +33,6 @@ LLDB_PLUGIN_DEFINE_ADV(EmulateInstructionRISCV,
InstructionRISCV)
namespace lldb_private {
-// RISC-V General Purpose Register numbers
-static constexpr uint32_t RISCV_GPR_SP = 2; // x2 is the stack pointer
-static constexpr uint32_t RISCV_GPR_FP = 8; // x8 is the frame pointer
-
/// Returns all values wrapped in Optional, or std::nullopt if any of the
values
/// is std::nullopt.
template <typename... Ts>
@@ -112,16 +108,6 @@ static uint32_t FPREncodingToLLDB(uint32_t reg_encode) {
return LLDB_INVALID_REGNUM;
}
-// Helper function to get register info from GPR encoding
-static std::optional<RegisterInfo>
-GPREncodingToRegisterInfo(EmulateInstructionRISCV &emulator,
- uint32_t reg_encode) {
- uint32_t lldb_reg = GPREncodingToLLDB(reg_encode);
- if (lldb_reg == LLDB_INVALID_REGNUM)
- return std::nullopt;
- return emulator.GetRegisterInfo(eRegisterKindLLDB, lldb_reg);
-}
-
bool Rd::Write(EmulateInstructionRISCV &emulator, uint64_t value) {
uint32_t lldb_reg = GPREncodingToLLDB(rd);
EmulateInstruction::Context ctx;
@@ -244,34 +230,10 @@ Load(EmulateInstructionRISCV &emulator, I inst, uint64_t
(*extend)(E)) {
auto addr = LoadStoreAddr(emulator, inst);
if (!addr)
return false;
-
- // Set up context for the load operation, similar to ARM64.
- EmulateInstructionRISCV::Context context;
-
- // Get register info for base register
- std::optional<RegisterInfo> reg_info_rs1 =
- GPREncodingToRegisterInfo(emulator, inst.rs1.rs);
-
- if (!reg_info_rs1)
- return false;
-
- // Set context type based on whether this is a stack-based load.
- if (inst.rs1.rs == RISCV_GPR_SP)
- context.type = EmulateInstruction::eContextPopRegisterOffStack;
- else
- context.type = EmulateInstruction::eContextRegisterLoad;
-
- // Set the context address information
- context.SetAddress(*addr);
-
- // Read from memory with context and write to register.
- bool success = false;
- uint64_t value =
- emulator.ReadMemoryUnsigned(context, *addr, sizeof(T), 0, &success);
- if (!success)
- return false;
-
- return inst.rd.Write(emulator, extend(E(T(value))));
+ return transformOptional(
+ emulator.ReadMem<T>(*addr),
+ [&](T t) { return inst.rd.Write(emulator, extend(E(t))); })
+ .value_or(false);
}
template <typename I, typename T>
@@ -280,35 +242,9 @@ Store(EmulateInstructionRISCV &emulator, I inst) {
auto addr = LoadStoreAddr(emulator, inst);
if (!addr)
return false;
-
- // Set up context for the store operation, similar to ARM64.
- EmulateInstructionRISCV::Context context;
-
- // Get register info for source and base registers.
- std::optional<RegisterInfo> reg_info_rs1 =
- GPREncodingToRegisterInfo(emulator, inst.rs1.rs);
- std::optional<RegisterInfo> reg_info_rs2 =
- GPREncodingToRegisterInfo(emulator, inst.rs2.rs);
-
- if (!reg_info_rs1 || !reg_info_rs2)
- return false;
-
- // Set context type based on whether this is a stack-based store.
- if (inst.rs1.rs == RISCV_GPR_SP)
- context.type = EmulateInstruction::eContextPushRegisterOnStack;
- else
- context.type = EmulateInstruction::eContextRegisterStore;
-
- // Set the context to show which register is being stored to which base
- // register + offset.
- context.SetRegisterToRegisterPlusOffset(*reg_info_rs2, *reg_info_rs1,
- SignExt(inst.imm));
-
- return transformOptional(inst.rs2.Read(emulator),
- [&](uint64_t rs2) {
- return emulator.WriteMemoryUnsigned(
- context, *addr, rs2, sizeof(T));
- })
+ return transformOptional(
+ inst.rs2.Read(emulator),
+ [&](uint64_t rs2) { return emulator.WriteMem<T>(*addr, rs2); })
.value_or(false);
}
@@ -801,44 +737,11 @@ class Executor {
bool operator()(SH inst) { return Store<SH, uint16_t>(m_emu, inst); }
bool operator()(SW inst) { return Store<SW, uint32_t>(m_emu, inst); }
bool operator()(ADDI inst) {
- return transformOptional(
- inst.rs1.ReadI64(m_emu),
- [&](int64_t rs1) {
- int64_t result = rs1 + int64_t(SignExt(inst.imm));
- // Check if this is a stack pointer adjustment.
- if (inst.rd.rd == RISCV_GPR_SP &&
- inst.rs1.rs == RISCV_GPR_SP) {
- EmulateInstruction::Context context;
- context.type =
- EmulateInstruction::eContextAdjustStackPointer;
- context.SetImmediateSigned(SignExt(inst.imm));
- uint32_t sp_lldb_reg = GPREncodingToLLDB(RISCV_GPR_SP);
- RegisterValue registerValue;
- registerValue.SetUInt64(result);
- return m_emu.WriteRegister(context, eRegisterKindLLDB,
- sp_lldb_reg, registerValue);
- }
- // Check if this is setting up the frame pointer.
- // addi fp, sp, imm -> fp = sp + imm (frame pointer setup).
- if (inst.rd.rd == RISCV_GPR_FP &&
- inst.rs1.rs == RISCV_GPR_SP) {
- EmulateInstruction::Context context;
- context.type = EmulateInstruction::eContextSetFramePointer;
- auto sp_reg_info = m_emu.GetRegisterInfo(
- eRegisterKindLLDB, GPREncodingToLLDB(RISCV_GPR_SP));
- if (sp_reg_info) {
- context.SetRegisterPlusOffset(*sp_reg_info,
- SignExt(inst.imm));
- }
- uint32_t fp_lldb_reg = GPREncodingToLLDB(RISCV_GPR_FP);
- RegisterValue registerValue;
- registerValue.SetUInt64(result);
- return m_emu.WriteRegister(context, eRegisterKindLLDB,
- fp_lldb_reg, registerValue);
- }
- // Regular ADDI instruction.
- return inst.rd.Write(m_emu, result);
- })
+ return transformOptional(inst.rs1.ReadI64(m_emu),
+ [&](int64_t rs1) {
+ return inst.rd.Write(
+ m_emu, rs1 + int64_t(SignExt(inst.imm)));
+ })
.value_or(false);
}
bool operator()(SLTI inst) {
@@ -1842,61 +1745,6 @@ EmulateInstructionRISCV::GetRegisterInfo(RegisterKind
reg_kind,
return array[reg_index];
}
-bool EmulateInstructionRISCV::SetInstruction(const Opcode &opcode,
- const Address &inst_addr,
- Target *target) {
- // Call the base class implementation.
- if (!EmulateInstruction::SetInstruction(opcode, inst_addr, target))
- return false;
-
- // Extract instruction data from the opcode.
- uint32_t inst_data = 0;
- const void *opcode_data = m_opcode.GetOpcodeBytes();
- if (!opcode_data)
- return false;
-
- if (m_opcode.GetByteSize() == 2) {
- // 16-bit compressed instruction.
- const uint16_t *data = static_cast<const uint16_t *>(opcode_data);
- inst_data = *data;
- } else if (m_opcode.GetByteSize() == 4) {
- // 32-bit instruction.
- const uint32_t *data = static_cast<const uint32_t *>(opcode_data);
- inst_data = *data;
- } else {
- return false;
- }
-
- // Decode the instruction.
- auto decoded_inst = Decode(inst_data);
- if (!decoded_inst)
- return false;
-
- // Store the decoded result.
- m_decoded = *decoded_inst;
- return true;
-}
-
-bool EmulateInstructionRISCV::CreateFunctionEntryUnwind(
- UnwindPlan &unwind_plan) {
- unwind_plan.Clear();
- unwind_plan.SetRegisterKind(eRegisterKindLLDB);
-
- UnwindPlan::Row row;
-
- row.GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_riscv, 0);
- row.SetRegisterLocationToSame(gpr_ra_riscv, /*must_replace=*/false);
- row.SetRegisterLocationToSame(gpr_fp_riscv, /*must_replace=*/false);
-
- unwind_plan.AppendRow(std::move(row));
- unwind_plan.SetSourceName("EmulateInstructionRISCV");
- unwind_plan.SetSourcedFromCompiler(eLazyBoolNo);
- unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolYes);
- unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
- unwind_plan.SetReturnAddressRegister(gpr_ra_riscv);
- return true;
-}
-
bool EmulateInstructionRISCV::SetTargetTriple(const ArchSpec &arch) {
return SupportsThisArch(arch);
}
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
index c196a9bb9ce82..3578a4ab03053 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
@@ -61,7 +61,6 @@ class EmulateInstructionRISCV : public EmulateInstruction {
case eInstructionTypePCModifying:
return true;
case eInstructionTypePrologueEpilogue:
- return true;
case eInstructionTypeAll:
return false;
}
@@ -86,7 +85,6 @@ class EmulateInstructionRISCV : public EmulateInstruction {
return SupportsThisInstructionType(inst_type);
}
- bool CreateFunctionEntryUnwind(UnwindPlan &unwind_plan) override;
bool SetTargetTriple(const ArchSpec &arch) override;
bool ReadInstruction() override;
std::optional<uint32_t> GetLastInstrSize() override { return m_last_size; }
@@ -96,8 +94,6 @@ class EmulateInstructionRISCV : public EmulateInstruction {
std::optional<RegisterInfo> GetRegisterInfo(lldb::RegisterKind reg_kind,
uint32_t reg_num) override;
- bool SetInstruction(const Opcode &opcode, const Address &inst_addr,
- Target *target) override;
std::optional<DecodeResult> ReadInstructionAt(lldb::addr_t addr);
std::optional<DecodeResult> Decode(uint32_t inst);
bool Execute(DecodeResult inst, bool ignore_cond);
diff --git a/lldb/unittests/Instruction/CMakeLists.txt
b/lldb/unittests/Instruction/CMakeLists.txt
index 6a35b1c5b02d6..10385377923ba 100644
--- a/lldb/unittests/Instruction/CMakeLists.txt
+++ b/lldb/unittests/Instruction/CMakeLists.txt
@@ -2,11 +2,9 @@ add_lldb_unittest(EmulatorTests
ARM64/TestAArch64Emulator.cpp
LoongArch/TestLoongArchEmulator.cpp
RISCV/TestRISCVEmulator.cpp
- RISCV/TestRiscvInstEmulation.cpp
LINK_COMPONENTS
Support
- ${LLVM_TARGETS_TO_BUILD}
LINK_LIBS
lldbCore
lldbSymbol
@@ -14,7 +12,4 @@ add_lldb_unittest(EmulatorTests
lldbPluginInstructionARM64
lldbPluginInstructionLoongArch
lldbPluginInstructionRISCV
- lldbPluginDisassemblerLLVMC
- lldbPluginUnwindAssemblyInstEmulation
- lldbPluginProcessUtility
)
diff --git a/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp
b/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp
deleted file mode 100644
index 009b9cdc79607..0000000000000
--- a/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp
+++ /dev/null
@@ -1,188 +0,0 @@
-//===-- TestRiscvInstEmulation.cpp
----------------------------------------===//
-//
-// 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 "gtest/gtest.h"
-
-#include "Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h"
-
-#include "lldb/Core/AddressRange.h"
-#include "lldb/Symbol/UnwindPlan.h"
-#include "lldb/Utility/ArchSpec.h"
-
-#include "Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h"
-#include "Plugins/Instruction/RISCV/EmulateInstructionRISCV.h"
-#include "Plugins/Process/Utility/lldb-riscv-register-enums.h"
-#include "llvm/Support/TargetSelect.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-class TestRiscvInstEmulation : public testing::Test {
-public:
- static void SetUpTestCase();
- static void TearDownTestCase();
-
-protected:
-};
-
-void TestRiscvInstEmulation::SetUpTestCase() {
- llvm::InitializeAllTargets();
- llvm::InitializeAllAsmPrinters();
- llvm::InitializeAllTargetMCs();
- llvm::InitializeAllDisassemblers();
- DisassemblerLLVMC::Initialize();
- EmulateInstructionRISCV::Initialize();
-}
-
-void TestRiscvInstEmulation::TearDownTestCase() {
- DisassemblerLLVMC::Terminate();
- EmulateInstructionRISCV::Terminate();
-}
-
-TEST_F(TestRiscvInstEmulation, TestSimpleRiscvFunction) {
- ArchSpec arch("riscv64-unknown-linux-gnu");
- // Enable compressed instruction support (RVC extension).
- arch.SetFlags(ArchSpec::eRISCV_rvc);
- std::unique_ptr<UnwindAssemblyInstEmulation> engine(
- static_cast<UnwindAssemblyInstEmulation *>(
- UnwindAssemblyInstEmulation::CreateInstance(arch)));
- ASSERT_NE(nullptr, engine);
-
- // RISC-V function with compressed and uncompressed instructions
- // 0x0000: 1141 addi sp, sp, -0x10
- // 0x0002: e406 sd ra, 0x8(sp)
- // 0x0004: e022 sd s0, 0x0(sp)
- // 0x0006: 0800 addi s0, sp, 0x10
- // 0x0008: 00000537 lui a0, 0x0
- // 0x000C: 00050513 mv a0, a0
- // 0x0010: 00000097 auipc ra, 0x0
- // 0x0014: 000080e7 jalr ra <main+0x10>
- // 0x0018: 4501 li a0, 0x0
- // 0x001A: ff040113 addi sp, s0, -0x10
- // 0x001E: 60a2 ld ra, 0x8(sp)
- // 0x0020: 6402 ld s0, 0x0(sp)
- // 0x0022: 0141 addi sp, sp, 0x10
- // 0x0024: 8082 ret
- uint8_t data[] = {// 0x0000: 1141 addi sp, sp, -0x10
- 0x41, 0x11,
- // 0x0002: e406 sd ra, 0x8(sp)
- 0x06, 0xE4,
- // 0x0004: e022 sd s0, 0x0(sp)
- 0x22, 0xE0,
- // 0x0006: 0800 addi s0, sp, 0x10
- 0x00, 0x08,
- // 0x0008: 00000537 lui a0, 0x0
- 0x37, 0x05, 0x00, 0x00,
- // 0x000C: 00050513 mv a0, a0
- 0x13, 0x05, 0x05, 0x00,
- // 0x0010: 00000097 auipc ra, 0x0
- 0x97, 0x00, 0x00, 0x00,
- // 0x0014: 000080e7 jalr ra <main+0x10>
- 0xE7, 0x80, 0x00, 0x00,
- // 0x0018: 4501 li a0, 0x0
- 0x01, 0x45,
- // 0x001A: ff040113 addi sp, s0, -0x10
- 0x13, 0x01, 0x04, 0xFF,
- // 0x001E: 60a2 ld ra, 0x8(sp)
- 0xA2, 0x60,
- // 0x0020: 6402 ld s0, 0x0(sp)
- 0x02, 0x64,
- // 0x0022: 0141 addi sp, sp, 0x10
- 0x41, 0x01,
- // 0x0024: 8082 ret
- 0x82, 0x80};
-
- // Expected UnwindPlan (prologue only - emulation stops after frame setup):
- // row[0]: 0: CFA=sp+0 => fp= <same> ra= <same>
- // row[1]: 2: CFA=sp+16 => fp= <same> ra= <same> (after
stack
- // allocation) row[2]: 4: CFA=sp+16 => fp= <same> ra=[CFA-8]
- // (after saving ra) row[3]: 6: CFA=sp+16 => fp=[CFA-16]
ra=[CFA-8]
- // (after saving s0/fp) row[4]: 8: CFA=s0+0 => fp=[CFA-16] ra=[CFA-8]
- // (after setting frame pointer: s0=sp+16)
-
- const UnwindPlan::Row *row;
- AddressRange sample_range;
- UnwindPlan unwind_plan(eRegisterKindLLDB);
- UnwindPlan::Row::AbstractRegisterLocation regloc;
- sample_range = AddressRange(0x1000, sizeof(data));
-
- EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
- sample_range, data, sizeof(data), unwind_plan));
-
- // CFA=sp+0 => fp=<same> ra=<same>.
- row = unwind_plan.GetRowForFunctionOffset(0);
- EXPECT_EQ(0, row->GetOffset());
- EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_riscv);
- EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
- EXPECT_EQ(0, row->GetCFAValue().GetOffset());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
- EXPECT_TRUE(regloc.IsSame());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
- EXPECT_TRUE(regloc.IsSame());
-
- // CFA=sp+16 => fp=<same> ra=<same>.
- row = unwind_plan.GetRowForFunctionOffset(2);
- EXPECT_EQ(2, row->GetOffset());
- EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_riscv);
- EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
- EXPECT_EQ(16, row->GetCFAValue().GetOffset());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
- EXPECT_TRUE(regloc.IsSame());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
- EXPECT_TRUE(regloc.IsSame());
-
- // CFA=sp+16 => fp=<same> ra=[CFA-8].
- row = unwind_plan.GetRowForFunctionOffset(4);
- EXPECT_EQ(4, row->GetOffset());
- EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_riscv);
- EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
- EXPECT_EQ(16, row->GetCFAValue().GetOffset());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
- EXPECT_TRUE(regloc.IsSame());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
- EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
- EXPECT_EQ(-8, regloc.GetOffset());
-
- // CFA=sp+16 => fp=[CFA-16] ra=[CFA-8]
- row = unwind_plan.GetRowForFunctionOffset(6);
- EXPECT_EQ(6, row->GetOffset());
- EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_riscv);
- EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
- EXPECT_EQ(16, row->GetCFAValue().GetOffset());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
- EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
- EXPECT_EQ(-16, regloc.GetOffset());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
- EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
- EXPECT_EQ(-8, regloc.GetOffset());
-
- // CFA=s0+0 => fp=[CFA-16] ra=[CFA-8]
- // s0 = sp + 16, so switching CFA to s0 does not change the effective
- // locations.
- row = unwind_plan.GetRowForFunctionOffset(8);
- EXPECT_EQ(8, row->GetOffset());
- EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_fp_riscv);
- EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
- EXPECT_EQ(0, row->GetCFAValue().GetOffset());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_riscv, regloc));
- EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
- EXPECT_EQ(-16, regloc.GetOffset());
-
- EXPECT_TRUE(row->GetRegisterInfo(gpr_ra_riscv, regloc));
- EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
- EXPECT_EQ(-8, regloc.GetOffset());
-}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
