This is an automated email from Gerrit. "Evgeniy Naydanov <[email protected]>" just uploaded a new patch set to Gerrit, which you can find at https://review.openocd.org/c/openocd/+/9177
-- gerrit commit c4e746075e71467c38db03c7c0fe8bfb0eb23444 Author: Tim Newsome <[email protected]> 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 <[email protected]> diff --git a/src/rtos/hwthread.c b/src/rtos/hwthread.c index b2a45ade9f..f7617ead7a 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 { @@ -446,3 +448,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 2ccccf1b0d..dc2b83ee82 100644 --- a/src/rtos/rtos.c +++ b/src/rtos/rtos.c @@ -730,3 +730,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 dbaa7e8ce8..c0a39771aa 100644 --- a/src/rtos/rtos.h +++ b/src/rtos/rtos.h @@ -77,6 +77,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 { @@ -135,6 +142,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 080e3360ae..0cef8a8ba5 100644 --- a/src/server/gdb_server.c +++ b/src/server/gdb_server.c @@ -3075,8 +3075,21 @@ 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$ + * */ target->rtos->gdb_target_for_threadid(connection, thread_id, &ct); @@ -3084,8 +3097,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] == ';') { --
