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>

3 files changed, 71 insertions, 12 deletions

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index cbf1ca9..a15c5bb 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 14a2da6..0630ffb 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 9bbc6e3..cf844e1 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"
 	}
 }