core/thread: fix thread_measure_stack_free()

`thread_measure_stack_free()` previously assumed that reading past the
stack is safe. When the stack was indeed part of a thread, the
`thread_t` structure is put after the stack, increasing the odds of
this assumption to hold. However, `thread_measure_stack_free()` could
also be used on the ISR stack, which may be allocated at the end of
SRAM.

A second parameter had to be added to indicate the stack size, so that
reading past the stack can now be prevented.

This also makes valgrind happy on `native`/`native64`.

core/include/thread.h

@@ -450,15 +450,20 @@ static inline const char *thread_getname(kernel_pid_t pid)
 #endif
 
 /**
- * @brief Measures the stack usage of a stack
+ * @brief       Measures the stack usage of a stack
+ * @internal    Should not be used externally
  *
- * Only works if the thread was created with the flag THREAD_CREATE_STACKTEST.
+ * Only works if the stack is filled with canaries
+ * (`*((uintptr_t *)ptr) == (uintptr_t)ptr` for naturally aligned `ptr` within
+ * the stack).
  *
- * @param[in] stack the stack you want to measure. Try `thread_get_stackstart(thread_get_active())`
+ * @param[in] stack     the stack you want to measure. Try
+ *                      `thread_get_stackstart(thread_get_active())`
+ * @param[in] size      size of @p stack in bytes
  *
- * @return          the amount of unused space of the thread's stack
+ * @return              the amount of unused space of the thread's stack
  */
-uintptr_t thread_measure_stack_free(const char *stack);
+uintptr_t measure_stack_free_internal(const char *stack, size_t size);
 
 /**
  * @brief   Get the number of bytes used on the ISR stack
@@ -621,6 +626,24 @@ static inline const char *thread_get_name(const thread_t *thread)
 #endif
 }
 
+/**
+ * @brief       Measures the stack usage of a stack
+ *
+ * @pre         Only works if the thread was created with the flag
+ *              `THREAD_CREATE_STACKTEST`.
+ *
+ * @param[in] thread    The thread to measure the stack of
+ *
+ * @return              the amount of unused space of the thread's stack
+ */
+static inline uintptr_t thread_measure_stack_free(const thread_t *thread)
+{
+    /* explicitly casting void pointers is bad code style, but needed for C++
+     * compatibility */
+    return measure_stack_free_internal((const char *)thread_get_stackstart(thread),
+                                       thread_get_stacksize(thread));
+}
+
 #ifdef __cplusplus
 }
 #endif

core/thread.c

@@ -171,15 +171,18 @@ void thread_add_to_list(list_node_t *list, thread_t *thread)
     list->next = new_node;
 }
 
-uintptr_t thread_measure_stack_free(const char *stack)
+uintptr_t measure_stack_free_internal(const char *stack, size_t size)
 {
     /* Alignment of stack has been fixed (if needed) by thread_create(), so
      * we can silence -Wcast-align here */
     uintptr_t *stackp = (uintptr_t *)(uintptr_t)stack;
+    uintptr_t end = (uintptr_t)stack + size;
+
+    /* better be safe than sorry: align end of stack just in case */
+    end &= (sizeof(uintptr_t) - 1);
 
-    /* assume that the comparison fails before or after end of stack */
     /* assume that the stack grows "downwards" */
-    while (*stackp == (uintptr_t)stackp) {
+    while (((uintptr_t)stackp < end) && (*stackp == (uintptr_t)stackp)) {
         stackp++;
     }
 

cpu/esp_common/freertos/task.c

@@ -197,7 +197,7 @@ UBaseType_t uxTaskGetStackHighWaterMark(TaskHandle_t xTask)
     thread_t *thread = thread_get((xTask == NULL) ? (uint32_t)thread_getpid()
                                                   : (uint32_t)xTask);
     assert(thread != NULL);
-    return thread_measure_stack_free(thread->stack_start);
+    return thread_measure_stack_free(thread);
 }
 
 void *pvTaskGetThreadLocalStoragePointer(TaskHandle_t xTaskToQuery,

cpu/esp_common/thread_arch.c

@@ -92,10 +92,9 @@ void thread_isr_stack_init(void)
 
 int thread_isr_stack_usage(void)
 {
-    /* cppcheck-suppress comparePointers
+    size_t stack_size = (uintptr_t)&port_IntStackTop - (uintptr_t)&port_IntStack;
-     * (reason: comes from ESP-SDK, so should be valid) */
+    return stack_size -
-    return &port_IntStackTop - &port_IntStack -
+           measure_stack_free_internal((char *)&port_IntStack, stack_size);
-           thread_measure_stack_free((char*)&port_IntStack);
 }
 
 void *thread_isr_stack_pointer(void)

sys/ps/ps.c

@@ -98,7 +98,7 @@ void ps(void)
 #ifdef DEVELHELP
             int stacksz = thread_get_stacksize(p);                          /* get stack size */
             overall_stacksz += stacksz;
-            int stack_free = thread_measure_stack_free(thread_get_stackstart(p));
+            int stack_free = thread_measure_stack_free(p);
             stacksz -= stack_free;
             overall_used += stacksz;
 #endif

sys/test_utils/print_stack_usage/print_stack_usage.c

@@ -34,7 +34,7 @@
 
 void print_stack_usage_metric(const char *name, void *stack, unsigned max_size)
 {
-    unsigned free = thread_measure_stack_free(stack);
+    unsigned free = measure_stack_free_internal(stack, max_size);
 
     if ((LOG_LEVEL >= LOG_INFO) &&
         (thread_get_stacksize(thread_get_active()) >= MIN_SIZE)) {