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;

-- 

Reply via email to