From c50af597a05cfbc9b68f71b7c5cbd0378c7fb3c3 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Mon, 12 Apr 2021 07:24:51 -0700 Subject: [PATCH] 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 Reviewed-by: Daniel Carvalho Maintainer: Gabe Black Tested-by: kokoro --- src/arch/null/remote_gdb.hh | 4 +- src/base/remote_gdb.cc | 73 +++++++++++++++++++++++++++++-------- src/base/remote_gdb.hh | 16 ++++++-- src/sim/system.cc | 2 +- 4 files changed, 74 insertions(+), 21 deletions(-) diff --git a/src/arch/null/remote_gdb.hh b/src/arch/null/remote_gdb.hh index c47ca9bc55..cef2338b30 100644 --- a/src/arch/null/remote_gdb.hh +++ b/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() {} }; diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc index 98686cf56e..8839f81f7c 100644 --- a/src/base/remote_gdb.cc +++ b/src/base/remote_gdb.cc @@ -132,9 +132,11 @@ #include #include +#include #include #include #include +#include #include #include #include @@ -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 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 diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh index ba274e6893..bfaaddfeea 100644 --- a/src/base/remote_gdb.hh +++ b/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 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); diff --git a/src/sim/system.cc b/src/sim/system.cc index 1bbda6b145..492bf63c61 100644 --- a/src/sim/system.cc +++ b/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; }