Don't fake step for hwthread rtos. (#393)

Fake step is a hack introduced to make things work with real RTOSs that
have a concept of a current thread. The hwthread rtos always has access
to all threads, so doesn't need it.

This fixes a bug when running my MulticoreRegTest against HiFive
Unleashed where OpenOCD would return the registers of the wrong thread
after gdb stepped a hart.

Change-Id: I64f538a133fb078c05a0c6b8121388b0b9d7f1b8

4 files changed, 26 insertions, 11 deletions

diff --git a/src/rtos/hwthread.c b/src/rtos/hwthread.c
index 90ba958..61dd338 100644
--- a/src/rtos/hwthread.c
+++ b/src/rtos/hwthread.c
@@ -38,6 +38,7 @@ static int hwthread_get_thread_reg_list(struct rtos *rtos, int64_t thread_id,
 static int hwthread_get_symbol_list_to_lookup(symbol_table_elem_t *symbol_list[]);
 static int hwthread_smp_init(struct target *target);
 int hwthread_set_reg(struct rtos *rtos, uint32_t reg_num, uint8_t *reg_value);
+bool hwthread_needs_fake_step(struct target *target, int64_t thread_id);
 
 #define HW_THREAD_NAME_STR_SIZE (32)
 
@@ -58,6 +59,7 @@ const struct rtos_type hwthread_rtos = {
 	.get_symbol_list_to_lookup = hwthread_get_symbol_list_to_lookup,
 	.smp_init = hwthread_smp_init,
 	.set_reg = hwthread_set_reg,
+	.needs_fake_step = hwthread_needs_fake_step
 };
 
 struct hwthread_params {
@@ -353,7 +355,6 @@ static int hwthread_thread_packet(struct connection *connection, const char *pac
 	int64_t current_threadid;
 
 	if (packet[0] == 'H' && packet[1] == 'g') {
-		/* Never reached, because this case is handled by rtos_thread_packet(). */
 		sscanf(packet, "Hg%16" SCNx64, &current_threadid);
 
 		if (current_threadid > 0) {
@@ -387,3 +388,8 @@ static int hwthread_create(struct target *target)
 	target->rtos->gdb_thread_packet = hwthread_thread_packet;
 	return 0;
 }
+
+bool hwthread_needs_fake_step(struct target *target, int64_t thread_id)
+{
+	return false;
+}
diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c
index d4eff5b..96dc99b 100644
--- a/src/rtos/rtos.c
+++ b/src/rtos/rtos.c
@@ -673,3 +673,10 @@ void rtos_free_threadlist(struct rtos *rtos)
 		rtos->current_thread = 0;
 	}
 }
+
+bool rtos_needs_fake_step(struct target *target, int64_t thread_id)
+{
+	if (target->rtos->type->needs_fake_step)
+		return target->rtos->type->needs_fake_step(target, thread_id);
+	return target->rtos->current_thread != thread_id;
+}
diff --git a/src/rtos/rtos.h b/src/rtos/rtos.h
index 81829b1..35420b1 100644
--- a/src/rtos/rtos.h
+++ b/src/rtos/rtos.h
@@ -83,6 +83,13 @@ struct rtos_type {
 	int (*clean)(struct target *target);
 	char * (*ps_command)(struct target *target);
 	int (*set_reg)(struct rtos *rtos, uint32_t reg_num, uint8_t *reg_value);
+	/**
+	 * Possibly 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. If an RTOS can do this, override
+	 * needs_fake_step(). */
+	bool (*needs_fake_step)(struct target *target, int64_t thread_id);
 };
 
 struct stack_register_offset {
@@ -128,5 +135,6 @@ void rtos_free_threadlist(struct rtos *rtos);
 int rtos_smp_init(struct target *target);
 /*  function for handling symbol access */
 int rtos_qsymbol(struct connection *connection, char const *packet, int packet_size);
+bool rtos_needs_fake_step(struct target *target, int64_t thread_id);
 
 #endif /* OPENOCD_RTOS_RTOS_H */
diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index c9c8aae..56da6ed 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -2808,8 +2808,7 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
 				 * 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;
+				fake_step = rtos_needs_fake_step(target, thread_id);
 			}
 
 			if (parse[0] == ';') {
@@ -2849,15 +2848,10 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
 			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) {
+				/* 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. */
 				int sig_reply_len;
 				char sig_reply[128];