[OpenOCD-commit] Main OpenOCD repository branch, master, updated. v0.12.0-1280-g68b0f7bdf

[openocd-gerrit <ope...@us...>]

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Main OpenOCD repository".

The branch, master has been updated
       via  68b0f7bdff8a20f3280b47e28b5215a5ae2522b6 (commit)
      from  52ac91a73e19759a74bf4b834225fba7c6a95084 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 68b0f7bdff8a20f3280b47e28b5215a5ae2522b6
Author: Tim Newsome <ti...@si...>
Date:   Wed Aug 14 11:56:44 2019 -0700

    server: rtos: don't fake step for hwthread rtos.
    
    This is a cherry-pick of:
    Link: https://github.com/riscv-collab/riscv-openocd/commit/efce094b409179acbaa7726c112a10fcf8343503
    
    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 <ti...@si...>
    Reviewed-on: https://review.openocd.org/c/openocd/+/9177
    Reviewed-by: Tomas Vanek <va...@fb...>
    Tested-by: jenkins

diff --git a/src/rtos/hwthread.c b/src/rtos/hwthread.c
index 3f0041e0b..d4fc880f6 100644
--- a/src/rtos/hwthread.c
+++ b/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;
+}
diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c
index 163d882a4..7957a5b6b 100644
--- a/src/rtos/rtos.c
+++ b/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;
+}
diff --git a/src/rtos/rtos.h b/src/rtos/rtos.h
index ca6d7518d..bbd1d1a1d 100644
--- a/src/rtos/rtos.h
+++ b/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;
diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index d18d48a3a..486080bbd 100644
--- a/src/server/gdb_server.c
+++ b/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] == ';') {

-----------------------------------------------------------------------

Summary of changes:
 src/rtos/hwthread.c     |  7 +++++++
 src/rtos/rtos.c         |  7 +++++++
 src/rtos/rtos.h         |  8 ++++++++
 src/server/gdb_server.c | 21 +++++++++++++++++----
 4 files changed, 39 insertions(+), 4 deletions(-)


hooks/post-receive
-- 
Main OpenOCD repository