base: Make the BaseRemoteGDB class able to handle multiple TCs.

Only one is set up corrent, the one passed in from the constructor.
Others can be added with addThreadContext.

The inconsistency of adding one ThreadContext through the constructor
and others through addThreadContext isn't great, but this way we can
ensure that there is always at least one ThreadContext. I'm not sure
what the GDB stub should do if there aren't any threads. I don't think
that the protocol can actually handle that, judging from the
documentation I can find.

Change-Id: I9160c3701ce78dcbbe99de1a6fe2a13e7e69404e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44611
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>

src/arch/null/remote_gdb.hh

@@ -38,6 +38,8 @@
 #ifndef __ARCH_NULL_REMOTE_GDB_HH__
 #define __ARCH_NULL_REMOTE_GDB_HH__
 
+#include "base/types.hh"
+
 class ThreadContext;
 
 class BaseRemoteGDB
@@ -47,7 +49,7 @@ class BaseRemoteGDB
 
     bool breakpoint() { return false; }
     void replaceThreadContext(ThreadContext *tc) {}
-    bool trap(int type) { return true; }
+    bool trap(ContextID id, int type) { return true; }
 
     virtual ~BaseRemoteGDB() {}
 };

src/base/remote_gdb.cc

@@ -132,9 +132,11 @@
 #include <sys/signal.h>
 #include <unistd.h>
 
+#include <cassert>
 #include <csignal>
 #include <cstdint>
 #include <cstdio>
+#include <iterator>
 #include <sstream>
 #include <string>
 #include <utility>
@@ -182,7 +184,7 @@ class HardBreakpoint : public PCEvent
         DPRINTF(GDBMisc, "handling hardware breakpoint at %#x\n", pc());
 
         if (tc == gdb->tc)
-            gdb->trap(SIGTRAP);
+            gdb->trap(tc->contextId(), SIGTRAP);
     }
 };
 
@@ -351,9 +353,10 @@ std::map<Addr, HardBreakpoint *> hardBreakMap;
 
 BaseRemoteGDB::BaseRemoteGDB(System *_system, ThreadContext *c, int _port) :
         connectEvent(nullptr), dataEvent(nullptr), _port(_port), fd(-1),
-        active(false), attached(false), sys(_system), tc(c),
+        active(false), attached(false), sys(_system),
         trapEvent(this), singleStepEvent(*this)
 {
+    addThreadContext(c);
 }
 
 BaseRemoteGDB::~BaseRemoteGDB()
@@ -437,12 +440,37 @@ BaseRemoteGDB::detach()
     DPRINTFN("remote gdb detached\n");
 }
 
+void
+BaseRemoteGDB::addThreadContext(ThreadContext *_tc)
+{
+    M5_VAR_USED auto it_success = threads.insert({_tc->contextId(), _tc});
+    assert(it_success.second);
+    // If no ThreadContext is current selected, select this one.
+    if (!tc)
+        assert(selectThreadContext(_tc->contextId()));
+}
+
 void
 BaseRemoteGDB::replaceThreadContext(ThreadContext *_tc)
 {
-    ContextID id = _tc->contextId();
-    panic_if(id != tc->contextId(), "No context with ID %d found.", id);
-    tc = _tc;
+    auto it = threads.find(_tc->contextId());
+    panic_if(it == threads.end(), "No context with ID %d found.",
+            _tc->contextId());
+    it->second = _tc;
+}
+
+bool
+BaseRemoteGDB::selectThreadContext(ContextID id)
+{
+    auto it = threads.find(id);
+    if (it == threads.end())
+        return false;
+
+    tc = it->second;
+    // Update the register cache for the new thread context, if there is one.
+    if (regCachePtr)
+        regCachePtr->getRegs(tc);
+    return true;
 }
 
 // This function does all command processing for interfacing to a
@@ -451,12 +479,16 @@ BaseRemoteGDB::replaceThreadContext(ThreadContext *_tc)
 // makes sense to use POSIX errno values, because that is what the
 // gdb/remote.c functions want to return.
 bool
-BaseRemoteGDB::trap(int type)
+BaseRemoteGDB::trap(ContextID id, int type)
 {
-
     if (!attached)
         return false;
 
+    if (tc->contextId() != id) {
+        if (!selectThreadContext(id))
+            return false;
+    }
+
     DPRINTF(GDBMisc, "trap: PC=%s\n", tc->pcState());
 
     clearSingleStep();
@@ -534,6 +566,7 @@ BaseRemoteGDB::incomingData(int revent)
 
     if (revent & POLLIN) {
         trapEvent.type(SIGILL);
+        trapEvent.id(tc->contextId());
         scheduleInstCommitEvent(&trapEvent, 0);
     } else if (revent & POLLNVAL) {
         descheduleInstCommitEvent(&trapEvent);
@@ -688,7 +721,7 @@ BaseRemoteGDB::singleStep()
 {
     if (!singleStepEvent.scheduled())
         scheduleInstCommitEvent(&singleStepEvent, 1);
-    trap(SIGTRAP);
+    trap(tc->contextId(), SIGTRAP);
 }
 
 void
@@ -927,11 +960,13 @@ BaseRemoteGDB::cmdSetThread(GdbCommand::Context &ctx)
         // should complain.
         if (all)
             throw CmdError("E03");
-        // If GDB cares which thread we're using and wants a different one, we
-        // don't support that right now.
-        if (!any && tid != tc->contextId())
-            throw CmdError("E04");
-        // Since we weren't asked to do anything, we succeeded.
+
+        // If GDB doesn't care which thread we're using, keep using the
+        // current one, otherwise switch.
+        if (!any && tid != tc->contextId()) {
+            if (!selectThreadContext(tid))
+                throw CmdError("E04");
+        }
     } else {
         throw CmdError("E05");
     }
@@ -1075,13 +1110,21 @@ BaseRemoteGDB::queryXfer(QuerySetCommand::Context &ctx)
 void
 BaseRemoteGDB::queryFThreadInfo(QuerySetCommand::Context &ctx)
 {
-    send("m%x", encodeThreadId(tc->contextId()));
+    threadInfoIdx = 0;
+    querySThreadInfo(ctx);
 }
 
 void
 BaseRemoteGDB::querySThreadInfo(QuerySetCommand::Context &ctx)
 {
-    send("l");
+    if (threadInfoIdx >= threads.size()) {
+        threadInfoIdx = 0;
+        send("l");
+    } else {
+        auto it = threads.begin();
+        std::advance(it, threadInfoIdx++);
+        send("m%x", encodeThreadId(it->second->contextId()));
+    }
 }
 
 bool

src/base/remote_gdb.hh

@@ -163,9 +163,11 @@ class BaseRemoteGDB
     void detach();
     bool isAttached() { return attached; }
 
+    void addThreadContext(ThreadContext *_tc);
     void replaceThreadContext(ThreadContext *_tc);
+    bool selectThreadContext(ContextID id);
 
-    bool trap(int type);
+    bool trap(ContextID id, int type);
 
     /** @} */ // end of api_remote_gdb
 
@@ -227,14 +229,17 @@ class BaseRemoteGDB
     bool attached;
 
     System *sys;
-    ThreadContext *tc;
 
-    BaseGdbRegCache *regCachePtr;
+    std::map<ContextID, ThreadContext *> threads;
+    ThreadContext *tc = nullptr;
+
+    BaseGdbRegCache *regCachePtr = nullptr;
 
     class TrapEvent : public Event
     {
       protected:
         int _type;
+        ContextID _id;
         BaseRemoteGDB *gdb;
 
       public:
@@ -242,7 +247,8 @@ class BaseRemoteGDB
         {}
 
         void type(int t) { _type = t; }
-        void process() { gdb->trap(_type); }
+        void id(ContextID id) { _id = id; }
+        void process() { gdb->trap(_id, _type); }
     } trapEvent;
 
     /*
@@ -340,6 +346,8 @@ class BaseRemoteGDB
     void queryC(QuerySetCommand::Context &ctx);
     void querySupported(QuerySetCommand::Context &ctx);
     void queryXfer(QuerySetCommand::Context &ctx);
+
+    size_t threadInfoIdx = 0;
     void queryFThreadInfo(QuerySetCommand::Context &ctx);
     void querySThreadInfo(QuerySetCommand::Context &ctx);
 

src/sim/system.cc

@@ -523,7 +523,7 @@ System::trapToGdb(int signal, ContextID ctx_id) const
     auto *gdb = threads.thread(ctx_id).gdb;
     if (!gdb)
         return false;
-    gdb->trap(signal);
+    gdb->trap(ctx_id, signal);
     return true;
 }