server: rtos: don't fake step for hwthread rtos.

This is a cherry-pick of:
Link: efce094b40

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
Signed-off-by: Tim Newsome <tim@sifive.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/9177
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
Tested-by: jenkins

src/rtos/hwthread.c

@@ -28,6 +28,7 @@ static int hwthread_read_buffer(struct rtos *rtos, target_addr_t address,
 		uint32_t size, uint8_t *buffer);
 static int hwthread_write_buffer(struct rtos *rtos, target_addr_t address,
 		uint32_t size, const uint8_t *buffer);
+static bool hwthread_needs_fake_step(struct target *target, int64_t thread_id);
 
 #define HW_THREAD_NAME_STR_SIZE (32)
 
@@ -59,6 +60,7 @@ const struct rtos_type hwthread_rtos = {
 	.set_reg = hwthread_set_reg,
 	.read_buffer = hwthread_read_buffer,
 	.write_buffer = hwthread_write_buffer,
+	.needs_fake_step = hwthread_needs_fake_step
 };
 
 struct hwthread_params {
@@ -449,3 +451,8 @@ static int hwthread_write_buffer(struct rtos *rtos, target_addr_t address,
 
 	return target_write_buffer(curr, address, size, buffer);
 }
+
+bool hwthread_needs_fake_step(struct target *target, int64_t thread_id)
+{
+	return false;
+}

src/rtos/rtos.c

@@ -755,3 +755,10 @@ int rtos_write_buffer(struct target *target, target_addr_t address,
 		return target->rtos->type->write_buffer(target->rtos, address, size, buffer);
 	return ERROR_NOT_IMPLEMENTED;
 }
+
+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;
+}

src/rtos/rtos.h

@@ -79,6 +79,13 @@ struct rtos_type {
 			uint8_t *buffer);
 	int (*write_buffer)(struct rtos *rtos, target_addr_t address, uint32_t size,
 			const uint8_t *buffer);
+	/**
+	 * 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 {
@@ -137,6 +144,7 @@ int rtos_read_buffer(struct target *target, target_addr_t address,
 		uint32_t size, uint8_t *buffer);
 int rtos_write_buffer(struct target *target, target_addr_t address,
 		uint32_t size, const uint8_t *buffer);
+bool rtos_needs_fake_step(struct target *target, int64_t thread_id);
 
 // Keep in alphabetic order this list of rtos
 extern const struct rtos_type chibios_rtos;

src/server/gdb_server.c

@@ -3080,8 +3080,22 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
 		}
 
 		if (target->rtos) {
-			/* FIXME: why is this necessary? rtos state should be up-to-date here already! */
-			rtos_update_threads(target);
+			/* Sometimes this results in picking a different thread than
+			 * gdb just requested to step. Then we fake it, and now there's
+			 * a different thread selected than gdb expects, so register
+			 * accesses go to the wrong one!
+			 * E.g.:
+			 * Hg1$
+			 * P8=72101ce197869329$		# write r8 on thread 1
+			 * g$
+			 * vCont?$
+			 * vCont;s:1;c$				# rtos_update_threads changes to other thread
+			 * g$
+			 * qXfer:threads:read::0,fff$
+			 * P8=cc060607eb89ca7f$		# write r8 on other thread
+			 * g$
+			 */
+			 /* rtos_update_threads(target); */
 
 			target->rtos->gdb_target_for_threadid(connection, thread_id, &ct);
 
@@ -3089,8 +3103,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] == ';') {