This is an automated email from Gerrit. "Marian Buschsieweke <marian.buschsiew...@ovgu.de>" just uploaded a new patch set to Gerrit, which you can find at https://review.openocd.org/c/openocd/+/7133
-- gerrit commit 0211a0e720204238298dda8792700f5ecccd5c94 Author: Marian Buschsieweke <marian.buschsiew...@ovgu.de> Date: Wed Aug 17 13:31:50 2022 +0200 rtos/riot: read in thread state names at runtime Starting with https://github.com/RIOT-OS/RIOT/pull/18460 RIOT provides thread state names so that they can be read out at runtime. This makes use of this feature to populate the thread state descriptions at run-time. If a pre-#18460 RIOT is used, it falls back to the current builtin thread state lookup table, so that OpenOCD can still provide thread state descriptions for older RIOT based firmwares. It seems that RIOT is a bit more volatile in this regard than the typical bare metal RTOS. Hence, reading this at runtime provides some robustness despite occasional changes. Signed-off-by: Marian Buschsieweke <marian.buschsiew...@ovgu.de> Change-Id: I729fd9d387aea45ec182b4e905c6b28036b77d98 diff --git a/src/rtos/riot.c b/src/rtos/riot.c index 6717085d59..d8aa5548f1 100644 --- a/src/rtos/riot.c +++ b/src/rtos/riot.c @@ -27,27 +27,28 @@ static int riot_get_thread_reg_list(struct rtos *rtos, int64_t thread_id, static int riot_get_symbol_list_to_lookup(struct symbol_table_elem *symbol_list[]); struct riot_thread_state { - int value; const char *desc; }; -/* refer RIOT sched.h */ -static const struct riot_thread_state riot_thread_states[] = { - { 0, "Stopped" }, - { 1, "Zombie" }, - { 2, "Sleeping" }, - { 3, "Blocked mutex" }, - { 4, "Blocked receive" }, - { 5, "Blocked send" }, - { 6, "Blocked reply" }, - { 7, "Blocked any flag" }, - { 8, "Blocked all flags" }, - { 9, "Blocked mbox" }, - { 10, "Blocked condition" }, - { 11, "Running" }, - { 12, "Pending" }, +/* Older versions of RIOT do not provide thread states in ELF for OpenOCD. For them, + * this lookup table is used as fallback: */ +static const struct riot_thread_state riot_thread_states_legacy[] = { + { "Stopped" }, + { "Zombie" }, + { "Sleeping" }, + { "Blocked mutex" }, + { "Blocked receive" }, + { "Blocked send" }, + { "Blocked reply" }, + { "Blocked any flag" }, + { "Blocked all flags" }, + { "Blocked mbox" }, + { "Blocked condition" }, + { "Running" }, + { "Pending" }, }; -#define RIOT_NUM_STATES ARRAY_SIZE(riot_thread_states) +static const struct riot_thread_state *riot_thread_states = riot_thread_states_legacy; +static size_t riot_thread_states_numof = ARRAY_SIZE(riot_thread_states_legacy); struct riot_params { const char *target_name; @@ -78,6 +79,8 @@ enum riot_symbol_values { RIOT_ACTIVE_PID, RIOT_MAX_THREADS, RIOT_NAME_OFFSET, + RIOT_THREAD_STATE_NAMES, + RIOT_THREAD_STATE_NAMES_NUMOF, }; struct riot_symbol { @@ -92,6 +95,8 @@ static struct riot_symbol const riot_symbol_list[] = { {"sched_active_pid", false}, {"max_threads", false}, {"_tcb_name_offset", true}, + {"thread_state_names", true}, + {"thread_state_names_numof", true}, {NULL, false} }; @@ -237,20 +242,13 @@ static int riot_update_threads(struct rtos *rtos) goto error; } - /* Search for state */ - unsigned int k; - for (k = 0; k < RIOT_NUM_STATES; k++) { - if (riot_thread_states[k].value == status) - break; - } - /* Copy state string */ - if (k >= RIOT_NUM_STATES) { + if (status < riot_thread_states_numof) { rtos->thread_details[tasks_found].extra_info_str = - strdup("unknown state"); + strdup(riot_thread_states[status].desc); } else { rtos->thread_details[tasks_found].extra_info_str = - strdup(riot_thread_states[k].desc); + strdup("unknown state"); } if (!rtos->thread_details[tasks_found].extra_info_str) { @@ -375,11 +373,110 @@ static int riot_get_symbol_list_to_lookup(struct symbol_table_elem *symbol_list[ return ERROR_OK; } +static void free_thread_state_names(struct riot_thread_state *names, size_t numof) +{ + for (size_t i = 0; i < numof; i++) { + if (names[i].desc) { + free((void *)names[i].desc); + names[i].desc = NULL; + } + } + + free(names); +} + +static int update_thread_state_names(struct rtos *rtos) +{ + int retval; + + if (rtos->symbols[RIOT_THREAD_STATE_NAMES_NUMOF].address == 0) { + /* older version of RIOT, keep using builtin thread state table */ + LOG_INFO("Old version of RIOT is used, using builtin thread names"); + return ERROR_OK; + } + + uint8_t numof; + retval = target_read_u8(rtos->target, + rtos->symbols[RIOT_THREAD_STATE_NAMES_NUMOF].address, &numof); + if (retval != ERROR_OK) { + LOG_ERROR("Failed to read number of thread state names: " + "Cannot read from symbol thread_state_names_numof"); + return retval; + } + + struct riot_thread_state *states = calloc(sizeof(struct riot_thread_state), numof); + if (!states) { + LOG_ERROR("Failed to allocate memory for thread state names"); + return ERROR_FAIL; + } + + uint32_t array_ptr; + retval = target_read_u32(rtos->target, rtos->symbols[RIOT_THREAD_STATE_NAMES].address, + &array_ptr); + + if (retval != ERROR_OK) { + LOG_ERROR("Failed to read number of thread state names: " + "Cannot read from symbol thread_state_names"); + free(states); + return retval; + } + + for (size_t i = 0; i < numof; i++, array_ptr += sizeof(uint32_t)) { + /* chances are good that thread state descriptions fit into a 128 byte buffer :) */ + uint8_t desc[128]; + uint32_t desc_ptr; + size_t desc_len; + retval = target_read_u32(rtos->target, array_ptr, &desc_ptr); + if (retval != ERROR_OK) { + LOG_ERROR("Failed to read number of thread state names: " + "Cannot read address of state name %zu", i); + free_thread_state_names(states, numof); + return retval; + } + + desc_len = 0; + while (1) { + retval = target_read_u8(rtos->target, desc_ptr, &desc[desc_len]); + if (retval != ERROR_OK) { + LOG_ERROR("Failed to read number of thread state names: " + "Cannot read character %zu from state name %zu at address 0x%" PRIx32, + desc_len, i, desc_ptr); + free_thread_state_names(states, numof); + return retval; + } + + if (desc[desc_len] == '\0') + break; + + desc_ptr++; + desc_len++; + + if (desc_len >= sizeof(desc)) { + LOG_ERROR("Thread state name longer than 128 bytes, ELF corrupted?!?"); + free_thread_state_names(states, numof); + return ERROR_FAIL; + } + } + + states[i].desc = strdup((char *)desc); + if (states[i].desc == NULL) { + LOG_ERROR("Failed to allocate memory for thread state name"); + free_thread_state_names(states, numof); + return ERROR_FAIL; + } + } + + riot_thread_states = states; + riot_thread_states_numof = numof; + return ERROR_OK; +} + static bool riot_detect_rtos(struct target *target) { if ((target->rtos->symbols) && (target->rtos->symbols[RIOT_THREADS_BASE].address != 0)) { /* looks like RIOT */ + update_thread_state_names(target->rtos); return true; } return false; --