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 <vanekt@fbl.cz>
Signed-off-by: Matthias Welwarsky <matthias.welwarsky@sysgo.com>
Reviewed-on: http://openocd.zylin.com/4432
Tested-by: jenkins
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index cbf1ca9f43611e05f3975a4650fafe92c0ba37e8..a15c5bb810499ce0c285924e3de9ef2429fa9ef0 100644 (file)
--- 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 14a2da6c1a51697f3ecea014c0397d4de90d4732..0630ffb9bd00a0abc5f9fc64c629c197476e122a 100644 (file)
--- 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 9bbc6e32c92358fc054750e3f9158403aa456661..cf844e1f64c3734a0f5b76978688c52048e52b1b 100644 (file)
--- 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"
        }
 }