[gem5-dev] Change in gem5/gem5[develop]: base: Correct checkBpLen naming with checkBpKind

2021-12-16 Thread Yu-hsin Wang (Gerrit) via gem5-dev
Yu-hsin Wang has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54123 )


Change subject: base: Correct checkBpLen naming with checkBpKind
..

base: Correct checkBpLen naming with checkBpKind

In gdb document*1, the second parameter of checkpoint command(Z0, Z1) is
named after kind. Although underlying implementation probably considers
it as length*2, it's still good to follow the name described in gdb
document for avoiding any confusion.

Refs:
1. https://sourceware.org/gdb/onlinedocs/gdb/Packets.html
2. https://github.com/bminor/binutils-gdb/blob/master/gdb/arch-utils.h#L41

Change-Id: Ib4b585613b8018970b16355f96cdff2ce9d5bae6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54123
Reviewed-by: Daniel Carvalho 
Maintainer: Daniel Carvalho 
Tested-by: kokoro 
Reviewed-by: Giacomo Travaglini 
---
M src/arch/riscv/remote_gdb.hh
M src/arch/arm/remote_gdb.cc
M src/arch/arm/remote_gdb.hh
M src/arch/x86/remote_gdb.hh
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
6 files changed, 61 insertions(+), 36 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved
  Daniel Carvalho: Looks good to me, but someone else must approve; Looks  
good to me, approved

  kokoro: Regressions pass




diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc
index 2efa82f..215c532 100644
--- a/src/arch/arm/remote_gdb.cc
+++ b/src/arch/arm/remote_gdb.cc
@@ -362,10 +362,10 @@
 }

 bool
-RemoteGDB::checkBpLen(size_t len)
+RemoteGDB::checkBpKind(size_t kind)
 {
 // 2 for Thumb ISA, 4 for ARM ISA.
-return len == 2 || len == 4;
+return kind == 2 || kind == 4;
 }

 } // namespace gem5
diff --git a/src/arch/arm/remote_gdb.hh b/src/arch/arm/remote_gdb.hh
index cff6d4a..8e512a4 100644
--- a/src/arch/arm/remote_gdb.hh
+++ b/src/arch/arm/remote_gdb.hh
@@ -120,7 +120,7 @@
   public:
 RemoteGDB(System *_system, int _port);
 BaseGdbRegCache *gdbRegs() override;
-bool checkBpLen(size_t len) override;
+bool checkBpKind(size_t kind) override;
 std::vector
 availableFeatures() const override
 {
diff --git a/src/arch/riscv/remote_gdb.hh b/src/arch/riscv/remote_gdb.hh
index 40fe821..753859f 100644
--- a/src/arch/riscv/remote_gdb.hh
+++ b/src/arch/riscv/remote_gdb.hh
@@ -56,7 +56,7 @@

 bool acc(Addr addr, size_t len) override;
 // A breakpoint will be 2 bytes if it is compressed and 4 if not
-bool checkBpLen(size_t len) override { return len == 2 || len == 4; }
+bool checkBpKind(size_t kind) override { return kind == 2 || kind ==  
4; }


 class RiscvGdbRegCache : public BaseGdbRegCache
 {
diff --git a/src/arch/x86/remote_gdb.hh b/src/arch/x86/remote_gdb.hh
index 62176a5..dfa9177 100644
--- a/src/arch/x86/remote_gdb.hh
+++ b/src/arch/x86/remote_gdb.hh
@@ -58,7 +58,7 @@
 {
   protected:
 bool acc(Addr addr, size_t len);
-bool checkBpLen(size_t len) { return len == 1; }
+bool checkBpKind(size_t kind) { return kind == 1; }
 class X86GdbRegCache : public BaseGdbRegCache
 {
   using BaseGdbRegCache::BaseGdbRegCache;
diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 1437b75..798d09f 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -784,28 +784,28 @@
 }

 void
-BaseRemoteGDB::insertSoftBreak(Addr addr, size_t len)
+BaseRemoteGDB::insertSoftBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

-return insertHardBreak(addr, len);
+return insertHardBreak(addr, kind);
 }

 void
-BaseRemoteGDB::removeSoftBreak(Addr addr, size_t len)
+BaseRemoteGDB::removeSoftBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length.\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

-return removeHardBreak(addr, len);
+return removeHardBreak(addr, kind);
 }

 void
-BaseRemoteGDB::insertHardBreak(Addr addr, size_t len)
+BaseRemoteGDB::insertHardBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

 DPRINTF(GDBMisc, "Inserting hardware breakpoint at %#x\n", addr);

@@ -817,10 +817,10 @@
 }

 void
-BaseRemoteGDB::removeHardBreak(Addr addr, size_t len)
+BaseRemoteGDB::removeHardBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

 DPRINTF(GDBMisc, "Removing hardware breakpoint at %#x\n", addr);

@@ -917,7 +917,7 @@
 };

 bool
-BaseRemoteGDB::checkBpLen(size_t len)
+BaseRemoteGDB::checkBpKind(size_t kind)
 {
 return true;
 }
@@ -1302,17 +1302,17 @@
 

[gem5-dev] Change in gem5/gem5[develop]: base: Correct checkBpLen naming with checkBpKind

2021-12-13 Thread Yu-hsin Wang (Gerrit) via gem5-dev
Yu-hsin Wang has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54123 )



Change subject: base: Correct checkBpLen naming with checkBpKind
..

base: Correct checkBpLen naming with checkBpKind

In gdb document*1, the second parameter of checkpoint command(Z0, Z1) is
named after kind. Although underlying implementation probably considers
it as length*2, it's still good to follow the name described in gdb
document for avoiding any confusion.

Refs:
1. https://sourceware.org/gdb/onlinedocs/gdb/Packets.html
2. https://github.com/bminor/binutils-gdb/blob/master/gdb/arch-utils.h#L41

Change-Id: Ib4b585613b8018970b16355f96cdff2ce9d5bae6
---
M src/arch/riscv/remote_gdb.hh
M src/arch/arm/remote_gdb.cc
M src/arch/arm/remote_gdb.hh
M src/arch/x86/remote_gdb.hh
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
6 files changed, 43 insertions(+), 25 deletions(-)



diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc
index 2efa82f..215c532 100644
--- a/src/arch/arm/remote_gdb.cc
+++ b/src/arch/arm/remote_gdb.cc
@@ -362,10 +362,10 @@
 }

 bool
-RemoteGDB::checkBpLen(size_t len)
+RemoteGDB::checkBpKind(size_t kind)
 {
 // 2 for Thumb ISA, 4 for ARM ISA.
-return len == 2 || len == 4;
+return kind == 2 || kind == 4;
 }

 } // namespace gem5
diff --git a/src/arch/arm/remote_gdb.hh b/src/arch/arm/remote_gdb.hh
index cff6d4a..8e512a4 100644
--- a/src/arch/arm/remote_gdb.hh
+++ b/src/arch/arm/remote_gdb.hh
@@ -120,7 +120,7 @@
   public:
 RemoteGDB(System *_system, int _port);
 BaseGdbRegCache *gdbRegs() override;
-bool checkBpLen(size_t len) override;
+bool checkBpKind(size_t kind) override;
 std::vector
 availableFeatures() const override
 {
diff --git a/src/arch/riscv/remote_gdb.hh b/src/arch/riscv/remote_gdb.hh
index 40fe821..753859f 100644
--- a/src/arch/riscv/remote_gdb.hh
+++ b/src/arch/riscv/remote_gdb.hh
@@ -56,7 +56,7 @@

 bool acc(Addr addr, size_t len) override;
 // A breakpoint will be 2 bytes if it is compressed and 4 if not
-bool checkBpLen(size_t len) override { return len == 2 || len == 4; }
+bool checkBpKind(size_t kind) override { return kind == 2 || kind ==  
4; }


 class RiscvGdbRegCache : public BaseGdbRegCache
 {
diff --git a/src/arch/x86/remote_gdb.hh b/src/arch/x86/remote_gdb.hh
index 62176a5..dfa9177 100644
--- a/src/arch/x86/remote_gdb.hh
+++ b/src/arch/x86/remote_gdb.hh
@@ -58,7 +58,7 @@
 {
   protected:
 bool acc(Addr addr, size_t len);
-bool checkBpLen(size_t len) { return len == 1; }
+bool checkBpKind(size_t kind) { return kind == 1; }
 class X86GdbRegCache : public BaseGdbRegCache
 {
   using BaseGdbRegCache::BaseGdbRegCache;
diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 1437b75..64e5e74 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -784,28 +784,28 @@
 }

 void
-BaseRemoteGDB::insertSoftBreak(Addr addr, size_t len)
+BaseRemoteGDB::insertSoftBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

-return insertHardBreak(addr, len);
+return insertHardBreak(addr, kind);
 }

 void
-BaseRemoteGDB::removeSoftBreak(Addr addr, size_t len)
+BaseRemoteGDB::removeSoftBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length.\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

-return removeHardBreak(addr, len);
+return removeHardBreak(addr, kind);
 }

 void
-BaseRemoteGDB::insertHardBreak(Addr addr, size_t len)
+BaseRemoteGDB::insertHardBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

 DPRINTF(GDBMisc, "Inserting hardware breakpoint at %#x\n", addr);

@@ -817,10 +817,10 @@
 }

 void
-BaseRemoteGDB::removeHardBreak(Addr addr, size_t len)
+BaseRemoteGDB::removeHardBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

 DPRINTF(GDBMisc, "Removing hardware breakpoint at %#x\n", addr);

@@ -917,7 +917,7 @@
 };

 bool
-BaseRemoteGDB::checkBpLen(size_t len)
+BaseRemoteGDB::checkBpKind(size_t kind)
 {
 return true;
 }
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 59bccc5..30800d6 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -313,10 +313,10 @@
 void descheduleInstCommitEvent(Event *ev);

 // Breakpoints.
-void insertSoftBreak(Addr addr, size_t len);
-void removeSoftBreak(Addr addr, size_t len);
-void insertHardBreak(Addr addr, size_t len);