misc: Generalize GDB single stepping.

The new single stepping implementation for x86 doesn't rely on any ISA
specific properties or functionality. This change pulls out the per ISA
implementation of those functions and promotes the X86 implementation to the
base class.

One drawback of that implementation is that the CPU might stop on an
instruction twice if it's affected by both breakpoints and single stepping.
While that might be a little surprising, it's harmless and would only happen
under somewhat unlikely circumstances.

diff --git a/src/arch/alpha/remote_gdb.cc b/src/arch/alpha/remote_gdb.cc
index 951a20982b7340cb804c28ad4ae26ace401d3cf0..a3fcf613641d059ad56de59b10296e8e513da4f9 100644 (file)
--- a/src/arch/alpha/remote_gdb.cc
+++ b/src/arch/alpha/remote_gdb.cc
@@ -255,46 +255,6 @@ RemoteGDB::setregs()
     context->pcState(gdbregs.regs64[KGDB_REG_PC]);
 }
 
-void
-RemoteGDB::clearSingleStep()
-{
-    DPRINTF(GDBMisc, "clearSingleStep bt_addr=%#x nt_addr=%#x\n",
-            takenBkpt, notTakenBkpt);
-
-    if (takenBkpt != 0)
-        clearTempBreakpoint(takenBkpt);
-
-    if (notTakenBkpt != 0)
-        clearTempBreakpoint(notTakenBkpt);
-}
-
-void
-RemoteGDB::setSingleStep()
-{
-    PCState pc = context->pcState();
-    PCState bpc;
-    bool set_bt = false;
-
-    // User was stopped at pc, e.g. the instruction at pc was not
-    // executed.
-    MachInst inst = read<MachInst>(pc.pc());
-    StaticInstPtr si = context->getDecoderPtr()->decode(inst, pc.pc());
-    if (si->hasBranchTarget(pc, context, bpc)) {
-        // Don't bother setting a breakpoint on the taken branch if it
-        // is the same as the next pc
-        if (bpc.pc() != pc.npc())
-            set_bt = true;
-    }
-
-    DPRINTF(GDBMisc, "setSingleStep bt_addr=%#x nt_addr=%#x\n",
-            takenBkpt, notTakenBkpt);
-
-    setTempBreakpoint(notTakenBkpt = pc.npc());
-
-    if (set_bt)
-        setTempBreakpoint(takenBkpt = bpc.pc());
-}
-
 // Write bytes to kernel address space for debugger.
 bool
 RemoteGDB::write(Addr vaddr, size_t size, const char *data)

diff --git a/src/arch/alpha/remote_gdb.hh b/src/arch/alpha/remote_gdb.hh
index d9c124c7239842a6d4db6adf5ebaaf2e48ba767d..33994653d9e0cc3d77603109cbbd752898245fcf 100644 (file)
--- a/src/arch/alpha/remote_gdb.hh
+++ b/src/arch/alpha/remote_gdb.hh
@@ -47,22 +47,15 @@ namespace AlphaISA {
 
 class RemoteGDB : public BaseRemoteGDB
 {
-  protected:
-    Addr notTakenBkpt;
-    Addr takenBkpt;
-
   protected:
     void getregs();
     void setregs();
 
-    void clearSingleStep();
-    void setSingleStep();
-
     // Machine memory
     bool acc(Addr addr, size_t len);
     bool write(Addr addr, size_t size, const char *data);
 
-    virtual bool insertHardBreak(Addr addr, size_t len);
+    bool insertHardBreak(Addr addr, size_t len);
 
   public:
     RemoteGDB(System *system, ThreadContext *context);

diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc
index fce060311346ac76acede757475a2ec7e205d2e3..1686cab397ed3f7f58687fbf621218ffae667d91 100644 (file)
--- a/src/arch/arm/remote_gdb.cc
+++ b/src/arch/arm/remote_gdb.cc
@@ -161,8 +161,7 @@ using namespace std;
 using namespace ArmISA;
 
 RemoteGDB::RemoteGDB(System *_system, ThreadContext *tc)
-    : BaseRemoteGDB(_system, tc, GDB_REG_BYTES),
-      notTakenBkpt(0), takenBkpt(0)
+    : BaseRemoteGDB(_system, tc, GDB_REG_BYTES)
 {
 }
 
@@ -314,46 +313,6 @@ RemoteGDB::setregs()
     }
 }
 
-void
-RemoteGDB::clearSingleStep()
-{
-    DPRINTF(GDBMisc, "clearSingleStep bt_addr=%#x nt_addr=%#x\n",
-            takenBkpt, notTakenBkpt);
-
-    if (takenBkpt != 0)
-        clearTempBreakpoint(takenBkpt);
-
-    if (notTakenBkpt != 0)
-        clearTempBreakpoint(notTakenBkpt);
-}
-
-void
-RemoteGDB::setSingleStep()
-{
-    PCState pc = context->pcState();
-    PCState bpc;
-    bool set_bt = false;
-
-    // User was stopped at pc, e.g. the instruction at pc was not
-    // executed.
-    MachInst inst = read<MachInst>(pc.pc());
-    StaticInstPtr si = context->getDecoderPtr()->decode(inst, pc.pc());
-    if (si->hasBranchTarget(pc, context, bpc)) {
-        // Don't bother setting a breakpoint on the taken branch if it
-        // is the same as the next pc
-        if (bpc.pc() != pc.npc())
-            set_bt = true;
-    }
-
-    DPRINTF(GDBMisc, "setSingleStep bt_addr=%#x nt_addr=%#x\n",
-            takenBkpt, notTakenBkpt);
-
-    setTempBreakpoint(notTakenBkpt = pc.npc());
-
-    if (set_bt)
-        setTempBreakpoint(takenBkpt = bpc.pc());
-}
-
 // Write bytes to kernel address space for debugger.
 bool
 RemoteGDB::write(Addr vaddr, size_t size, const char *data)

diff --git a/src/arch/arm/remote_gdb.hh b/src/arch/arm/remote_gdb.hh
index 423ba9d02fcf9064b9e656308513248dbbf019e8..80a0bf1b12f629f343a407503628e01ea4233db4 100644 (file)
--- a/src/arch/arm/remote_gdb.hh
+++ b/src/arch/arm/remote_gdb.hh
@@ -80,23 +80,15 @@ const int GDB_REG_BYTES = std::max(GDB64_NUMREGS * sizeof(uint64_t),
 
 class RemoteGDB : public BaseRemoteGDB
 {
+  protected:
+    bool acc(Addr addr, size_t len);
+    bool write(Addr addr, size_t size, const char *data);
 
-protected:
-  Addr notTakenBkpt;
-  Addr takenBkpt;
+    void getregs();
+    void setregs();
 
-protected:
-  bool acc(Addr addr, size_t len);
-  bool write(Addr addr, size_t size, const char *data);
-
-  void getregs();
-  void setregs();
-
-  void clearSingleStep();
-  void setSingleStep();
-
-public:
-  RemoteGDB(System *_system, ThreadContext *tc);
+  public:
+    RemoteGDB(System *_system, ThreadContext *tc);
 };
 } // namespace ArmISA
 

diff --git a/src/arch/mips/remote_gdb.cc b/src/arch/mips/remote_gdb.cc
index 9c944fc59b4e7f0eb542e827696170475b1028ad..a7bde8ba609f70206baf5e5c9b73faacd487ca5d 100644 (file)
--- a/src/arch/mips/remote_gdb.cc
+++ b/src/arch/mips/remote_gdb.cc
@@ -235,47 +235,3 @@ RemoteGDB::setregs()
     context->setFloatRegBits(FLOATREG_FIR,
         gdbregs.regs32[GdbIntRegs + GdbFloatArchRegs + 1]);
 }
-
-void
-RemoteGDB::clearSingleStep()
-{
-    DPRINTF(GDBMisc, "clearSingleStep bt_addr=%#x nt_addr=%#x\n",
-            takenBkpt, notTakenBkpt);
-
-    if (takenBkpt != 0)
-        clearTempBreakpoint(takenBkpt);
-
-    if (notTakenBkpt != 0)
-        clearTempBreakpoint(notTakenBkpt);
-}
-
-void
-RemoteGDB::setSingleStep()
-{
-    PCState pc = context->pcState();
-    PCState bpc;
-    bool set_bt = false;
-
-    // User was stopped at pc, e.g. the instruction at pc was not
-    // executed.
-    MachInst inst = read<MachInst>(pc.pc());
-    StaticInstPtr si = context->getDecoderPtr()->decode(inst, pc.pc());
-    if (si->hasBranchTarget(pc, context, bpc)) {
-        // Don't bother setting a breakpoint on the taken branch if it
-        // is the same as the next npc
-        if (bpc.npc() != pc.nnpc())
-            set_bt = true;
-    }
-
-    DPRINTF(GDBMisc, "setSingleStep bt_addr=%#x nt_addr=%#x\n",
-            takenBkpt, notTakenBkpt);
-
-    notTakenBkpt = pc.nnpc();
-    setTempBreakpoint(notTakenBkpt);
-
-    if (set_bt) {
-        takenBkpt = bpc.npc();
-        setTempBreakpoint(takenBkpt);
-    }
-}
-

diff --git a/src/arch/mips/remote_gdb.hh b/src/arch/mips/remote_gdb.hh
index 157276dcf535cfe4b2f048ba57397c590b74dd5c..8d113eb99e9dcef1d984a2fedd625b5f14de28a6 100644 (file)
--- a/src/arch/mips/remote_gdb.hh
+++ b/src/arch/mips/remote_gdb.hh
@@ -54,10 +54,6 @@ namespace MipsISA
 
     class RemoteGDB : public BaseRemoteGDB
     {
-      protected:
-        Addr notTakenBkpt;
-        Addr takenBkpt;
-
       public:
         RemoteGDB(System *_system, ThreadContext *tc);
 
@@ -66,9 +62,6 @@ namespace MipsISA
 
         void getregs();
         void setregs();
-
-        void clearSingleStep();
-        void setSingleStep();
     };
 }
 

diff --git a/src/arch/power/remote_gdb.hh b/src/arch/power/remote_gdb.hh
index b37c3171332c65b971bddef4c1e420b951a44343..fa82af95bb627e3bb537209e450788793f74f33e 100644 (file)
--- a/src/arch/power/remote_gdb.hh
+++ b/src/arch/power/remote_gdb.hh
@@ -65,18 +65,6 @@ class RemoteGDB : public BaseRemoteGDB
     {
         panic("setregs not implemented for POWER!");
     }
-
-    void
-    clearSingleStep()
-    {
-        panic("clearSingleStep not implemented for POWER!");
-    }
-
-    void
-    setSingleStep()
-    {
-        panic("setSingleStep not implemented for POWER!");
-    }
 };
 
 } // namespace PowerISA

diff --git a/src/arch/sparc/remote_gdb.cc b/src/arch/sparc/remote_gdb.cc
index 778c207312bb780b70c5bbccac0d856228770ba6..e654741b6cf67e5e3117036bfdf7a8ed179842c2 100644 (file)
--- a/src/arch/sparc/remote_gdb.cc
+++ b/src/arch/sparc/remote_gdb.cc
@@ -144,7 +144,7 @@ using namespace std;
 using namespace SparcISA;
 
 RemoteGDB::RemoteGDB(System *_system, ThreadContext *c)
-    : BaseRemoteGDB(_system, c, NumGDBRegs * sizeof(uint64_t)), nextBkpt(0)
+    : BaseRemoteGDB(_system, c, NumGDBRegs * sizeof(uint64_t))
 {}
 
 ///////////////////////////////////////////////////////////
@@ -241,17 +241,3 @@ RemoteGDB::setregs()
         context->setIntReg(x - RegG0, gdbregs.regs64[x]);
     // Only the integer registers, pc and npc are set in netbsd
 }
-
-void
-RemoteGDB::clearSingleStep()
-{
-   if (nextBkpt)
-       clearTempBreakpoint(nextBkpt);
-}
-
-void
-RemoteGDB::setSingleStep()
-{
-    nextBkpt = context->pcState().npc();
-    setTempBreakpoint(nextBkpt);
-}

diff --git a/src/arch/sparc/remote_gdb.hh b/src/arch/sparc/remote_gdb.hh
index 0176fd3232b4c796f4b5d313b1d89b83f711746b..46aa03a931864bc68a469c06fdcda56721b63632 100644 (file)
--- a/src/arch/sparc/remote_gdb.hh
+++ b/src/arch/sparc/remote_gdb.hh
@@ -66,11 +66,6 @@ class RemoteGDB : public BaseRemoteGDB
   protected:
     void getregs();
     void setregs();
-
-    void clearSingleStep();
-    void setSingleStep();
-
-    Addr nextBkpt;
 };
 
 }

diff --git a/src/arch/x86/remote_gdb.cc b/src/arch/x86/remote_gdb.cc
index b30bf573926d990ed73c64b7f71a1662cc6979a1..dd96037e0c67d9fd6ac25d6ff7551a029b610bb5 100644 (file)
--- a/src/arch/x86/remote_gdb.cc
+++ b/src/arch/x86/remote_gdb.cc
@@ -61,7 +61,7 @@ using namespace std;
 using namespace X86ISA;
 
 RemoteGDB::RemoteGDB(System *_system, ThreadContext *c) :
-    BaseRemoteGDB(_system, c, GDB_REG_BYTES), singleStepEvent(this)
+    BaseRemoteGDB(_system, c, GDB_REG_BYTES)
 {}
 
 bool
@@ -88,14 +88,6 @@ RemoteGDB::acc(Addr va, size_t len)
     }
 }
 
-void
-RemoteGDB::SingleStepEvent::process()
-{
-    if (!gdb->singleStepEvent.scheduled())
-        gdb->scheduleInstCommitEvent(&gdb->singleStepEvent, 1);
-    gdb->trap(SIGTRAP);
-}
-
 void
 RemoteGDB::getregs()
 {
@@ -231,16 +223,3 @@ RemoteGDB::setregs()
         }
     }
 }
-
-void
-RemoteGDB::clearSingleStep()
-{
-    descheduleInstCommitEvent(&singleStepEvent);
-}
-
-void
-RemoteGDB::setSingleStep()
-{
-    if (!singleStepEvent.scheduled())
-        scheduleInstCommitEvent(&singleStepEvent, 1);
-}

diff --git a/src/arch/x86/remote_gdb.hh b/src/arch/x86/remote_gdb.hh
index b654fc2f8441995c23bd20c832cc4d04e40c341d..f09d1e012d5b9f1d1c61a50c8846d785b70fb3d9 100644 (file)
--- a/src/arch/x86/remote_gdb.hh
+++ b/src/arch/x86/remote_gdb.hh
@@ -116,26 +116,9 @@ class RemoteGDB : public BaseRemoteGDB
     bool acc(Addr addr, size_t len);
 
   protected:
-    class SingleStepEvent : public Event
-    {
-      protected:
-        RemoteGDB *gdb;
-
-      public:
-        SingleStepEvent(RemoteGDB *g) : gdb(g)
-        {}
-
-        void process();
-    };
-
-    SingleStepEvent singleStepEvent;
-
     void getregs();
     void setregs();
 
-    void clearSingleStep();
-    void setSingleStep();
-
     bool checkBpLen(size_t len) { return len == 1; }
 };
 

diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 97d76380363a9feab8f3b1f2370e40dce35555a4..e603fb90f06dfef7f9cc93d36cacfeb77fc64efe 100644 (file)
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -262,11 +262,18 @@ BaseRemoteGDB::TrapEvent::process()
     gdb->trap(_type);
 }
 
-BaseRemoteGDB::BaseRemoteGDB(System *_system, ThreadContext *c, size_t cacheSize)
-    : inputEvent(NULL), trapEvent(this), listener(NULL), number(-1), fd(-1),
-      active(false), attached(false),
-      system(_system), context(c),
-      gdbregs(cacheSize)
+void
+BaseRemoteGDB::SingleStepEvent::process()
+{
+    if (!gdb->singleStepEvent.scheduled())
+        gdb->scheduleInstCommitEvent(&gdb->singleStepEvent, 1);
+    gdb->trap(SIGTRAP);
+}
+
+BaseRemoteGDB::BaseRemoteGDB(System *_system, ThreadContext *c,
+        size_t cacheSize) : inputEvent(NULL), trapEvent(this), listener(NULL),
+        number(-1), fd(-1), active(false), attached(false), system(_system),
+        context(c), gdbregs(cacheSize), singleStepEvent(this)
 {
     memset(gdbregs.regs, 0, gdbregs.bytes());
 }
@@ -525,6 +532,19 @@ BaseRemoteGDB::write(Addr vaddr, size_t size, const char *data)
     return true;
 }
 
+void
+BaseRemoteGDB::clearSingleStep()
+{
+    descheduleInstCommitEvent(&singleStepEvent);
+}
+
+void
+BaseRemoteGDB::setSingleStep()
+{
+    if (!singleStepEvent.scheduled())
+        scheduleInstCommitEvent(&singleStepEvent, 1);
+}
+
 PCEventQueue *BaseRemoteGDB::getPcEventQueue()
 {
     return &system->pcEventQueue;

diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 62f98f29a7db910da2360ebb59f3745bd74423ea..6cca485e3a46ef65bf335804be4769acaa1dcb08 100644 (file)
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -209,11 +209,25 @@ class BaseRemoteGDB
     }
 
   protected:
+    class SingleStepEvent : public Event
+    {
+      protected:
+        BaseRemoteGDB *gdb;
+
+      public:
+        SingleStepEvent(BaseRemoteGDB *g) : gdb(g)
+        {}
+
+        void process();
+    };
+
+    SingleStepEvent singleStepEvent;
+
     virtual void getregs() = 0;
     virtual void setregs() = 0;
 
-    virtual void clearSingleStep() = 0;
-    virtual void setSingleStep() = 0;
+    void clearSingleStep();
+    void setSingleStep();
 
     PCEventQueue *getPcEventQueue();
     EventQueue *getComInstEventQueue();