-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Reland #158161 with cmake fix #159842
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
Reland #158161 with cmake fix #159842
Conversation
This reverts commit 19bc0d6.
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-lldb Author: None (barsolo2000) ChangesInitial PR was reverted due failing test since the buildbots don't include RISCV. Full diff: https://github.com/llvm/llvm-project/pull/159842.diff 5 Files Affected:
diff --git a/lldb/include/lldb/Core/Opcode.h b/lldb/include/lldb/Core/Opcode.h
index 7bbd73d039f99..7e756d3f15d22 100644
--- a/lldb/include/lldb/Core/Opcode.h
+++ b/lldb/include/lldb/Core/Opcode.h
@@ -223,7 +223,9 @@ class Opcode {
int Dump(Stream *s, uint32_t min_byte_width) const;
const void *GetOpcodeBytes() const {
- return ((m_type == Opcode::eTypeBytes) ? m_data.inst.bytes : nullptr);
+ return ((m_type == Opcode::eTypeBytes || m_type == Opcode::eType16_32Tuples)
+ ? 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 5e429a92613ce..20661290ca4c6 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -33,6 +33,10 @@ 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>
@@ -108,6 +112,16 @@ 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;
@@ -230,10 +244,34 @@ Load(EmulateInstructionRISCV &emulator, I inst, uint64_t (*extend)(E)) {
auto addr = LoadStoreAddr(emulator, inst);
if (!addr)
return false;
- return transformOptional(
- emulator.ReadMem<T>(*addr),
- [&](T t) { return inst.rd.Write(emulator, extend(E(t))); })
- .value_or(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))));
}
template <typename I, typename T>
@@ -242,9 +280,35 @@ Store(EmulateInstructionRISCV &emulator, I inst) {
auto addr = LoadStoreAddr(emulator, inst);
if (!addr)
return false;
- return transformOptional(
- inst.rs2.Read(emulator),
- [&](uint64_t rs2) { return emulator.WriteMem<T>(*addr, rs2); })
+
+ // 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));
+ })
.value_or(false);
}
@@ -737,11 +801,44 @@ 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) {
- return inst.rd.Write(
- m_emu, rs1 + int64_t(SignExt(inst.imm)));
- })
+ 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);
+ })
.value_or(false);
}
bool operator()(SLTI inst) {
@@ -1745,6 +1842,61 @@ 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 3578a4ab03053..c196a9bb9ce82 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
@@ -61,6 +61,7 @@ class EmulateInstructionRISCV : public EmulateInstruction {
case eInstructionTypePCModifying:
return true;
case eInstructionTypePrologueEpilogue:
+ return true;
case eInstructionTypeAll:
return false;
}
@@ -85,6 +86,7 @@ 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; }
@@ -94,6 +96,8 @@ 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 10385377923ba..f5b442464cac5 100644
--- a/lldb/unittests/Instruction/CMakeLists.txt
+++ b/lldb/unittests/Instruction/CMakeLists.txt
@@ -2,9 +2,10 @@ add_lldb_unittest(EmulatorTests
ARM64/TestAArch64Emulator.cpp
LoongArch/TestLoongArchEmulator.cpp
RISCV/TestRISCVEmulator.cpp
-
+
LINK_COMPONENTS
Support
+ ${LLVM_TARGETS_TO_BUILD}
LINK_LIBS
lldbCore
lldbSymbol
@@ -12,4 +13,23 @@ add_lldb_unittest(EmulatorTests
lldbPluginInstructionARM64
lldbPluginInstructionLoongArch
lldbPluginInstructionRISCV
+ lldbPluginProcessUtility
)
+
+if ("RISCV" IN_LIST LLVM_TARGETS_TO_BUILD)
+ add_lldb_unittest(RISCVEmulatorTests
+ RISCV/TestRiscvInstEmulation.cpp
+
+ LINK_COMPONENTS
+ Support
+ ${LLVM_TARGETS_TO_BUILD}
+ LINK_LIBS
+ lldbCore
+ lldbSymbol
+ lldbTarget
+ lldbPluginInstructionRISCV
+ lldbPluginDisassemblerLLVMC
+ lldbPluginUnwindAssemblyInstEmulation
+ lldbPluginProcessUtility
+ )
+endif()
diff --git a/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp b/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp
new file mode 100644
index 0000000000000..009b9cdc79607
--- /dev/null
+++ b/lldb/unittests/Instruction/RISCV/TestRiscvInstEmulation.cpp
@@ -0,0 +1,188 @@
+//===-- 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());
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the title to something like "Reland #158161 with cmake fix"
Please update the commit message with a description of why the original commit was failing and what you did to fix it. It's also good to explicitly reference the original PR from the commit message to have a solid link back.
LoongArch/TestLoongArchEmulator.cpp | ||
RISCV/TestRISCVEmulator.cpp | ||
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There shouldn't be any changes to this unittest target now that we are splitting it out into its own test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me !
return transformOptional( | ||
inst.rs1.ReadI64(m_emu), | ||
[&](int64_t rs1) { | ||
int64_t result = rs1 + int64_t(SignExt(inst.imm)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to be uint64_t? I'm seeing this failure w/ ubsan: lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp:807:40: runtime error: signed integer overflow: -9223372036854775808 + -16 cannot be represented in type 'int64_t' (aka 'long')
This is fine: uint64_t result = rs1 + uint64_t(SignExt(inst.imm));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@barsolo2000 can you take a look at this failure?
In RISC-V, the same instruction is used for both signed and unsigned additions. Signed integer overflow is undefined behavior in C++, but signed integer overflow is defined behavior when using ADDI in RISC-V. As we are emulating the RISC-V behavior we should be using uint. This fix the failure with ubsan introduced by #159842: lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp:807:40: runtime error: signed integer overflow: -9223372036854775808 + -16 cannot be represented in type 'int64_t' (aka 'long')
In RISC-V, the same instruction is used for both signed and unsigned additions. Signed integer overflow is undefined behavior in C++, but signed integer overflow is defined behavior when using ADDI in RISC-V. As we are emulating the RISC-V behavior we should be using uint. This fix the failure with ubsan introduced by llvm#159842: lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp:807:40: runtime error: signed integer overflow: -9223372036854775808 + -16 cannot be represented in type 'int64_t' (aka 'long')
Initial PR was reverted due failing test since the buildbots don't include the RISCV backend.