From 38f4f5b9508cab958c7ebf46f3fe3e05d1aa3031 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Wed, 24 Feb 2021 19:26:54 +0100 Subject: [PATCH 1/5] Improve check for unimplemented vCont actions --- vp/src/core/common/gdb-mc/handler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vp/src/core/common/gdb-mc/handler.cpp b/vp/src/core/common/gdb-mc/handler.cpp index 2ba72980a..fb7217858 100644 --- a/vp/src/core/common/gdb-mc/handler.cpp +++ b/vp/src/core/common/gdb-mc/handler.cpp @@ -217,7 +217,7 @@ void GDBServer::vCont(int conn, gdb_command_t *cmd) { bool single = false; if (vcont->action == 's') single = true; - else if (vcont->action == 'S') + else if (vcont->action != 'c') throw std::invalid_argument("Unimplemented vCont action"); /* TODO */ std::vector selected_harts; From 613ec0439b1c05934698dbe77f3c513232f9cd43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Thu, 25 Feb 2021 10:58:21 +0100 Subject: [PATCH 2/5] libgdb: return beginning of vCont list in gdbf_vcont Previously, this function incorrectly returned the end of the list, thereby causing vCont packets with multiple actions to be parsed incorrectly. This commit also updates the integration tests submodules to include test cases for the vCont packet. --- vp/src/core/common/gdb-mc/libgdb/parser2.c | 2 +- vp/tests | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vp/src/core/common/gdb-mc/libgdb/parser2.c b/vp/src/core/common/gdb-mc/libgdb/parser2.c index fe9568c6b..dd9abf83a 100644 --- a/vp/src/core/common/gdb-mc/libgdb/parser2.c +++ b/vp/src/core/common/gdb-mc/libgdb/parser2.c @@ -223,7 +223,7 @@ gdbf_vcont(int n, mpc_val_t **xs) vcont->next = (gdb_vcont_t *)xs[i]; vcont->next = NULL; - return vcont; + return (gdb_vcont_t *)xs[0]; } gdbf_fold(vcont, GDB_ARG_VCONT, GDBF_ARG_VCONT) diff --git a/vp/tests b/vp/tests index 1251c5684..b5d387645 160000 --- a/vp/tests +++ b/vp/tests @@ -1 +1 @@ -Subproject commit 1251c56841ec45814554092831e641dcf0389502 +Subproject commit b5d3876459e2f3a85a54f8e6c20f703a13f485b5 From fb232faa1a03095cbb62ace6a839516f9fc303e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Thu, 25 Feb 2021 02:02:38 +0100 Subject: [PATCH 3/5] gdb-mc: pass vector of threads to run to GDBServer::run_threads --- vp/src/core/common/gdb-mc/gdb_server.cpp | 6 ++++-- vp/src/core/common/gdb-mc/gdb_server.h | 2 +- vp/src/core/common/gdb-mc/handler.cpp | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/vp/src/core/common/gdb-mc/gdb_server.cpp b/vp/src/core/common/gdb-mc/gdb_server.cpp index e6fc1e637..c8a32b7f5 100644 --- a/vp/src/core/common/gdb-mc/gdb_server.cpp +++ b/vp/src/core/common/gdb-mc/gdb_server.cpp @@ -137,9 +137,11 @@ void GDBServer::exec_thread(thread_func fn, char op) { fn(thread); } -std::vector GDBServer::run_threads(int id, bool single) { +std::vector GDBServer::run_threads(std::vector hartsrun, bool single) { + if (hartsrun.empty()) { + return hartsrun; + } this->single_run = single; - std::vector hartsrun = get_threads(id); /* invoke all selected harts */ sc_core::sc_event_or_list allharts; diff --git a/vp/src/core/common/gdb-mc/gdb_server.h b/vp/src/core/common/gdb-mc/gdb_server.h index eaf26c448..a8b5461af 100644 --- a/vp/src/core/common/gdb-mc/gdb_server.h +++ b/vp/src/core/common/gdb-mc/gdb_server.h @@ -87,7 +87,7 @@ SC_MODULE(GDBServer) { std::vector get_threads(int); uint64_t translate_addr(debug_target_if *, uint64_t, MemoryAccessType type); void exec_thread(thread_func, char = 'g'); - std::vector run_threads(int, bool = false); + std::vector run_threads(std::vector, bool = false); void writeall(int, char *, size_t); void send_packet(int, const char *, gdb_kind_t = GDB_KIND_PACKET); void retransmit(int); diff --git a/vp/src/core/common/gdb-mc/handler.cpp b/vp/src/core/common/gdb-mc/handler.cpp index fb7217858..649f74a71 100644 --- a/vp/src/core/common/gdb-mc/handler.cpp +++ b/vp/src/core/common/gdb-mc/handler.cpp @@ -222,7 +222,8 @@ void GDBServer::vCont(int conn, gdb_command_t *cmd) { std::vector selected_harts; try { - selected_harts = run_threads(vcont->thread.tid, single); + auto run = get_threads(vcont->thread.tid); + selected_harts = run_threads(run, single); } catch (const std::out_of_range&) { send_packet(conn, "E01"); return; From e24fc6556ce3e58f6ce252df053ece20267b86d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= Date: Thu, 25 Feb 2021 02:17:14 +0100 Subject: [PATCH 4/5] gdb-mc: Only execute leftmost action with matching thread id in vCont MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consider a vCont input command such as: vCont;s:1;c. Without this patch applied, gdb-mc would single-step the first thread and afterwards continue all threads (including the first). However, according to the specification the stub should only single step the first thread and continue all threads except the first one afterwards (“the leftmost action with a matching thread-id is applied”). The changes implemented in this commit implement this part of the specification. Fixes https://github.com/agra-uni-bremen/riscv-vp/issues/7 --- vp/src/core/common/gdb-mc/handler.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/vp/src/core/common/gdb-mc/handler.cpp b/vp/src/core/common/gdb-mc/handler.cpp index 649f74a71..cc8c46cbb 100644 --- a/vp/src/core/common/gdb-mc/handler.cpp +++ b/vp/src/core/common/gdb-mc/handler.cpp @@ -208,6 +208,7 @@ void GDBServer::vCont(int conn, gdb_command_t *cmd) { gdb_vcont_t *vcont; int stopped_thread = -1; const char *stop_reason = NULL; + std::map matched; /* This handler attempts to implement the all-stop mode. * See: https://sourceware.org/gdb/onlinedocs/gdb/All_002dStop-Mode.html */ @@ -223,6 +224,14 @@ void GDBServer::vCont(int conn, gdb_command_t *cmd) { std::vector selected_harts; try { auto run = get_threads(vcont->thread.tid); + for (auto i = run.begin(); i != run.end();) { + debug_target_if *hart = *i; + if (matched.count(hart)) + i = run.erase(i); /* already matched */ + else + i++; + } + selected_harts = run_threads(run, single); } catch (const std::out_of_range&) { send_packet(conn, "E01"); @@ -247,6 +256,16 @@ void GDBServer::vCont(int conn, gdb_command_t *cmd) { continue; } } + + /* The vCont specification mandates that only the leftmost action with + * a matching thread-id is applied. Unfourtunatly, the specification + * is a bit unclear in regards to handling two actions with no thread + * id (i.e. GDB_THREAD_ALL). */ + if (vcont->thread.tid > 0) { + auto threads = get_threads(vcont->thread.tid); + assert(threads.size() == 1); + matched[threads.front()] = true; + } } assert(stop_reason && stopped_thread >= 1); From a4bfecfde8905962529303acd1a8813f72aca979 Mon Sep 17 00:00:00 2001 From: Pascal Pieper Date: Thu, 25 Feb 2021 11:33:52 +0100 Subject: [PATCH 5/5] Beep boop, I'm a robot --- vp/src/core/common/gdb-mc/handler.cpp | 2 +- vp/tests | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vp/src/core/common/gdb-mc/handler.cpp b/vp/src/core/common/gdb-mc/handler.cpp index cc8c46cbb..c55d88f35 100644 --- a/vp/src/core/common/gdb-mc/handler.cpp +++ b/vp/src/core/common/gdb-mc/handler.cpp @@ -258,7 +258,7 @@ void GDBServer::vCont(int conn, gdb_command_t *cmd) { } /* The vCont specification mandates that only the leftmost action with - * a matching thread-id is applied. Unfourtunatly, the specification + * a matching thread-id is applied. Unfortunately, the specification * is a bit unclear in regards to handling two actions with no thread * id (i.e. GDB_THREAD_ALL). */ if (vcont->thread.tid > 0) { diff --git a/vp/tests b/vp/tests index b5d387645..1251c5684 160000 --- a/vp/tests +++ b/vp/tests @@ -1 +1 @@ -Subproject commit b5d3876459e2f3a85a54f8e6c20f703a13f485b5 +Subproject commit 1251c56841ec45814554092831e641dcf0389502