From bae76053dc515252dc5c8235b9a848e461080c66 Mon Sep 17 00:00:00 2001 From: Tomas Vanek Date: Fri, 23 Feb 2018 19:27:28 +0100 Subject: [PATCH] gdb_server: run control fixes for vCont this patch contains several changes to run control and state handling together with gdb: - graceful handling of target/gdb desync on resume, step and halt - a default gdb-attach event executing the "halt" command, to meet gdb expectation of target state when it attaches - call target_poll() after Ctrl-C command from gdb - call target_poll() after resume and step through a vCont packet - fix log message forwarding on vCont stepping, also move an aarch64 log message from INFO to DEBUG level to prevent messing up the gdb console during source-line stepping - fix oversight in vCont support that messes up breakpoint handling during stepping Change-Id: Ic79db7c2b798a35283ff752e9b12475486a1f31a Fixes: d301d8b42f0bfe67d76d6f340db6570cc71c876e Signed-off-by: Tomas Vanek Signed-off-by: Matthias Welwarsky Reviewed-on: http://openocd.zylin.com/4432 Tested-by: jenkins Reviewed-by: Matthias Welwarsky --- src/server/gdb_server.c | 78 +++++++++++++++++++++++++++++++++++------ src/target/aarch64.c | 4 +-- src/target/startup.tcl | 1 + 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index cbf1ca9f4..a15c5bb81 100644 --- a/src/server/gdb_server.c +++ b/src/server/gdb_server.c @@ -2599,18 +2599,32 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p LOG_DEBUG("target %s continue", target_name(target)); log_add_callback(gdb_log_callback, connection); retval = target_resume(target, 1, 0, 0, 0); - if (retval == ERROR_OK) { - gdb_connection->frontend_state = TARGET_RUNNING; - target_call_event_callbacks(target, TARGET_EVENT_GDB_START); + if (retval == ERROR_TARGET_NOT_HALTED) + LOG_INFO("target %s was not halted when resume was requested", target_name(target)); + + /* poll target in an attempt to make its internal state consistent */ + if (retval != ERROR_OK) { + retval = target_poll(target); + if (retval != ERROR_OK) + LOG_DEBUG("error polling target %s after failed resume", target_name(target)); } + + /* + * We don't report errors to gdb here, move frontend_state to + * TARGET_RUNNING to stay in sync with gdb's expectation of the + * target state + */ + gdb_connection->frontend_state = TARGET_RUNNING; + target_call_event_callbacks(target, TARGET_EVENT_GDB_START); + return true; } /* single-step or step-over-breakpoint */ if (parse[0] == 's') { if (strncmp(parse, "s:", 2) == 0) { - int handle_breakpoint = 1; struct target *ct = target; + int current_pc = 1; int64_t thread_id; char *endp; @@ -2634,21 +2648,63 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p parse += 1; packet_size -= 1; - handle_breakpoint = 0; + /* check if thread-id follows */ + if (parse[0] == ':') { + int64_t tid; + parse += 1; + packet_size -= 1; + + tid = strtoll(parse, &endp, 16); + if (tid == thread_id) { + /* + * Special case: only step a single thread (core), + * keep the other threads halted. Currently, only + * aarch64 target understands it. Other target types don't + * care (nobody checks the actual value of 'current') + * and it doesn't really matter. This deserves + * a symbolic constant and a formal interface documentation + * at a later time. + */ + LOG_DEBUG("request to step current core only"); + /* uncomment after checking that indeed other targets are safe */ + /*current_pc = 2;*/ + } + } } } LOG_DEBUG("target %s single-step thread %"PRId64, target_name(ct), thread_id); - retval = target_step(ct, 1, 0, handle_breakpoint); + log_add_callback(gdb_log_callback, connection); + target_call_event_callbacks(ct, TARGET_EVENT_GDB_START); + + /* support for gdb_sync command */ + if (gdb_connection->sync) { + gdb_connection->sync = false; + if (ct->state == TARGET_HALTED) { + LOG_WARNING("stepi ignored. GDB will now fetch the register state " \ + "from the target."); + gdb_sig_halted(connection); + log_remove_callback(gdb_log_callback, connection); + } else + gdb_connection->frontend_state = TARGET_RUNNING; + return true; + } + + retval = target_step(ct, current_pc, 0, 0); + if (retval == ERROR_TARGET_NOT_HALTED) + LOG_INFO("target %s was not halted when step was requested", target_name(ct)); + + /* if step was successful send a reply back to gdb */ if (retval == ERROR_OK) { - gdb_signal_reply(target, connection); + retval = target_poll(ct); + if (retval != ERROR_OK) + LOG_DEBUG("error polling target %s after successful step", target_name(ct)); + /* send back signal information */ + gdb_signal_reply(ct, connection); /* stop forwarding log packets! */ log_remove_callback(gdb_log_callback, connection); } else - if (retval == ERROR_TARGET_TIMEOUT) { gdb_connection->frontend_state = TARGET_RUNNING; - target_call_event_callbacks(ct, TARGET_EVENT_GDB_START); - } } else { LOG_ERROR("Unknown vCont packet"); return false; @@ -3124,6 +3180,8 @@ static int gdb_input_inner(struct connection *connection) if (target->rtos) target->rtos->gdb_target_for_threadid(connection, target->rtos->current_threadid, &t); retval = target_halt(t); + if (retval == ERROR_OK) + retval = target_poll(t); if (retval != ERROR_OK) target_call_event_callbacks(target, TARGET_EVENT_GDB_HALT); gdb_con->ctrl_c = 0; diff --git a/src/target/aarch64.c b/src/target/aarch64.c index 14a2da6c1..0630ffb9b 100644 --- a/src/target/aarch64.c +++ b/src/target/aarch64.c @@ -452,7 +452,7 @@ static int update_halt_gdb(struct target *target, enum target_debug_reason debug struct target *curr; if (debug_reason == DBG_REASON_NOTHALTED) { - LOG_INFO("Halting remaining targets in SMP group"); + LOG_DEBUG("Halting remaining targets in SMP group"); aarch64_halt_smp(target, true); } @@ -1086,7 +1086,7 @@ static int aarch64_step(struct target *target, int current, target_addr_t addres if (retval != ERROR_OK) return retval; - if (target->smp && !handle_breakpoints) { + if (target->smp && (current == 1)) { /* * isolate current target so that it doesn't get resumed * together with the others diff --git a/src/target/startup.tcl b/src/target/startup.tcl index 9bbc6e32c..cf844e1f6 100644 --- a/src/target/startup.tcl +++ b/src/target/startup.tcl @@ -203,6 +203,7 @@ proc init_target_events {} { foreach t $targets { set_default_target_event $t gdb-flash-erase-start "reset init" set_default_target_event $t gdb-flash-write-end "reset halt" + set_default_target_event $t gdb-attach "halt" } }