From 935f0c5cc28a3a2cc627ff25b30d65d8d82ad8c4 Mon Sep 17 00:00:00 2001
From: Matthias Welwarsky <matthias.welwarsky@sysgo.com>
Date: Mon, 5 Mar 2018 14:50:51 +0100
Subject: [PATCH] gdb_server: fake step if thread is not current rtos thread

gdb assumes that a rtos can make any thread active at will in response
to a 'Hg' packet. It further assumes that it needs to step-over after
setting a breakpoint on frame #0 of any non-current thread. Both
assumptions are not valid for an actual rtos. We fake the step-over to
not trigger an internal error in gdb. See
https://sourceware.org/bugzilla/show_bug.cgi?id=22925 for details.

Change-Id: Ida60cd134033c1d58ada77b87fe664a58f61e2c0
Signed-off-by: Matthias Welwarsky <matthias.welwarsky@sysgo.com>
Reviewed-on: http://openocd.zylin.com/4448
Tested-by: jenkins
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>
---
 src/server/gdb_server.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index 712c39341..74074a1f1 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -2695,6 +2695,8 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
 
 	/* single-step or step-over-breakpoint */
 	if (parse[0] == 's') {
+		bool fake_step = false;
+
 		if (strncmp(parse, "s:", 2) == 0) {
 			struct target *ct = target;
 			int current_pc = 1;
@@ -2710,9 +2712,20 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
 				parse = endp;
 			}
 
-			if (target->rtos != NULL)
+			if (target->rtos != NULL) {
+				/* FIXME: why is this necessary? rtos state should be up-to-date here already! */
+				rtos_update_threads(target);
+
 				target->rtos->gdb_target_for_threadid(connection, thread_id, &ct);
 
+				/*
+				 * check if the thread to be stepped is the current rtos thread
+				 * if not, we must fake the step
+				 */
+				if (target->rtos->current_thread != thread_id)
+					fake_step = true;
+			}
+
 			if (parse[0] == ';') {
 				++parse;
 				--packet_size;
@@ -2746,10 +2759,33 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
 				}
 			}
 
-			LOG_DEBUG("target %s single-step thread %"PRId64, target_name(ct), thread_id);
+			LOG_DEBUG("target %s single-step thread %"PRIx64, target_name(ct), thread_id);
 			log_add_callback(gdb_log_callback, connection);
 			target_call_event_callbacks(ct, TARGET_EVENT_GDB_START);
 
+			/*
+			 * work around an annoying gdb behaviour: when the current thread
+			 * is changed in gdb, it assumes that the target can follow and also
+			 * make the thread current. This is an assumption that cannot hold
+			 * for a real target running a multi-threading OS. We just fake
+			 * the step to not trigger an internal error in gdb. See
+			 * https://sourceware.org/bugzilla/show_bug.cgi?id=22925 for details
+			 */
+			if (fake_step) {
+				int sig_reply_len;
+				char sig_reply[128];
+
+				LOG_DEBUG("fake step thread %"PRIx64, thread_id);
+
+				sig_reply_len = snprintf(sig_reply, sizeof(sig_reply),
+										 "T05thread:%016"PRIx64";", thread_id);
+
+				gdb_put_packet(connection, sig_reply, sig_reply_len);
+				log_remove_callback(gdb_log_callback, connection);
+
+				return true;
+			}
+
 			/* support for gdb_sync command */
 			if (gdb_connection->sync) {
 				gdb_connection->sync = false;