From b0d81ec8a2bcece1a510b20d40bd47314b24a89e Mon Sep 17 00:00:00 2001
From: Yangyu Chen <cyy@cyyself.name>
Date: Tue, 20 Aug 2024 01:25:39 +0800
Subject: [PATCH] arch-riscv: fix GDB breakpoint issue for RV32 (#1470)

Since PR #1316, we use sign-extend for all address generation, including
PC, to match the ISA specification for modifiable XLEN. However, when we
set a breakpoint using remote GDB, our address is not sign-extended.
This causes the breakpoint to be set at the wrong address, as specified
in Issue #1463. This PR fixes the issue by sign-extending the address
when setting a breakpoint. This also matches the RISC-V ISA
Specification that "must sign-extend results to fill the entire widest
supported XLEN in the destination register."

Change-Id: I9b493bf8ad5b1ef45a9728bb40fc5e38250fe9c3

Signed-off-by: Yangyu Chen <cyy@cyyself.name>
---
 src/arch/riscv/remote_gdb.cc | 14 ++++++++++++++
 src/arch/riscv/remote_gdb.hh |  2 ++
 src/base/remote_gdb.hh       |  8 ++++----
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/arch/riscv/remote_gdb.cc b/src/arch/riscv/remote_gdb.cc
index ef76451b17..111af8e62f 100644
--- a/src/arch/riscv/remote_gdb.cc
+++ b/src/arch/riscv/remote_gdb.cc
@@ -233,6 +233,20 @@ RemoteGDB::acc(Addr va, size_t len)
     return context()->getProcessPtr()->pTable->lookup(va) != nullptr;
 }
 
+void
+RemoteGDB::insertHardBreak(Addr addr, size_t kind)
+{
+    Addr realAddr = getRvType(context()) == RV64 ? addr : sext(addr, 32);
+    BaseRemoteGDB::insertHardBreak(realAddr, kind);
+}
+
+void
+RemoteGDB::removeHardBreak(Addr addr, size_t kind)
+{
+    Addr realAddr = getRvType(context()) == RV64 ? addr : sext(addr, 32);
+    BaseRemoteGDB::removeHardBreak(realAddr, kind);
+}
+
 void
 RemoteGDB::Riscv32GdbRegCache::getRegs(ThreadContext *context)
 {
diff --git a/src/arch/riscv/remote_gdb.hh b/src/arch/riscv/remote_gdb.hh
index b2f90c32be..a01ebe08f0 100644
--- a/src/arch/riscv/remote_gdb.hh
+++ b/src/arch/riscv/remote_gdb.hh
@@ -57,6 +57,8 @@ class RemoteGDB : public BaseRemoteGDB
     bool acc(Addr addr, size_t len) override;
     // A breakpoint will be 2 bytes if it is compressed and 4 if not
     bool checkBpKind(size_t kind) override { return kind == 2 || kind == 4; }
+    void insertHardBreak(Addr addr, size_t kind) override;
+    void removeHardBreak(Addr addr, size_t kind) override;
 
     class Riscv32GdbRegCache : public BaseGdbRegCache
     {
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 02802e7a85..8735f6feaf 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -321,10 +321,10 @@ class BaseRemoteGDB
     void descheduleInstCommitEvent(Event *ev);
 
     // Breakpoints.
-    void insertSoftBreak(Addr addr, size_t kind);
-    void removeSoftBreak(Addr addr, size_t kind);
-    void insertHardBreak(Addr addr, size_t kind);
-    void removeHardBreak(Addr addr, size_t kind);
+    virtual void insertSoftBreak(Addr addr, size_t kind);
+    virtual void removeSoftBreak(Addr addr, size_t kind);
+    virtual void insertHardBreak(Addr addr, size_t kind);
+    virtual void removeHardBreak(Addr addr, size_t kind);
 
     void sendTPacket(GDBSignal sig, ContextID id,
       const std::string& stopReason);