Merge #18620 #19296 #19504 #19506

18620: core: add core_mutex_debug to aid debugging deadlocks r=maribu a=maribu

### Contribution description

Adding `USEMODULE += core_mutex_debug` to your `Makefile` results in
on log messages such as

    [mutex] waiting for thread 1 (pc = 0x800024d)

being added whenever `mutex_lock()` blocks. This makes tracing down
deadlocks easier.

### Testing procedure

Run e.g.

```sh
USEMODULE=core_mutex_debug BOARD=nucleo-f767zi make -C tests/mutex_cancel flash test
```

which should provide output such as

```
Welcome to pyterm!
Type '/exit' to exit.
READY
s
[mutex] waiting for thread 1 (pc = 0x8000f35)
START
main(): This is RIOT! (Version: 2022.10-devel-841-g5cc02-core/mutex/debug)
Test Application for mutex_cancel / mutex_lock_cancelable
=========================================================

Test without cancellation: OK
Test early cancellation: OK
Verify no side effects on subsequent calls: [mutex] waiting for thread 1 (pc = 0x800024d)
OK
Test late cancellation: [mutex] waiting for thread 1 (pc = 0x0)
OK
TEST PASSED
```

```sh
$ arm-none-eabi-addr2line -a 0x800024d -e tests/mutex_cancel/bin/nucleo-f767zi/tests_mutex_cancel.elf 
0x0800024d
/home/maribu/Repos/software/RIOT/tests/mutex_cancel/main.c:51
```

### Issues/PRs references

Depends on and includes #18619

19296: nanocoap: allow to define CoAP resources as XFA r=maribu a=benpicco



19504: cpu/cc26xx_cc13xx: Fix bogus array-bound warning r=maribu a=maribu

### Contribution description

GCC 12 create a bogus array out of bounds warning as it assumes that because there is special handling for `uart == 0` and `uart == 1`, `uart` can indeed be `1`. There is an `assert(uart < UART_NUMOF)` above that would blow up prior to any out of bounds access.

In any case, optimizing out the special handling of `uart == 1` for when `UART_NUMOF == 1` likely improves the generated code and fixes the warning.

    /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:88:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds]
       88 |     ctx[uart].rx_cb = rx_cb;
          |     ~~~^~~~~~
    /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx'
       52 | static uart_isr_ctx_t ctx[UART_NUMOF];
          |                       ^~~
    /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:89:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds]
       89 |     ctx[uart].arg = arg;
          |     ~~~^~~~~~
    /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx'
       52 | static uart_isr_ctx_t ctx[UART_NUMOF];
          |                       ^~~

### Testing procedure

The actual change is a pretty obvious one-liner, so that code review and a green CI should be sufficient. If not, running any UART example app without regression should do.

### Issues/PRs references

None

19506: tools/openocd: Fix handling of OPENOCD_CMD_RESET_HALT r=maribu a=maribu

### Contribution description

The OPENOCD_CMD_RESET_HALT was not longer correctly passed to the script. This fixes the issue.

### Testing procedure

Flashing of e.g. the `cc2650-launchpad` with upstream OpenOCD should work again.

### Issues/PRs references

The change was added to #19050 after testing the PR and before merging. I'm not sure if the fix never worked because of this, or if behavior of `target-export-variables` or GNU Make changed.

Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>

core/Kconfig

@@ -39,6 +39,9 @@ config MODULE_CORE_MSG_BUS
     help
         Messaging Bus API for inter process message broadcast.
 
+config MODULE_CORE_MUTEX_DEBUG
+    bool "Aid debugging deadlocks by printing on whom mutex_lock() is waiting"
+
 config MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
     bool "Use priority inheritance to mitigate priority inversion for mutexes"
 

core/include/mutex.h

@@ -97,6 +97,25 @@
  *       `MUTEX_LOCK`.
  *     - The scheduler is run, so that if the unblocked waiting thread can
  *       run now, in case it has a higher priority than the running thread.
+ *
+ * Debugging deadlocks
+ * -------------------
+ *
+ * The module `core_mutex_debug` can be used to print on whom `mutex_lock()`
+ * is waiting. This information includes the thread ID of the owner and the
+ * program counter (PC) from where `mutex_lock()` was called. Note that the
+ * information is only valid if:
+ *
+ * - The mutex was locked by a thread, and not e.g. by `MUTEX_INIT_LOCKED`
+ * - The function `cpu_get_caller_pc()` is implemented for the target
+ *   architecture. (The thread ID will remain valid, though.)
+ * - The caller PC is briefly 0 when the current owner passes over ownership
+ *   to the next thread, but that thread didn't get CPU time yet to write its
+ *   PC into the data structure. Even worse, on architectures where an aligned
+ *   function-pointer-sized write is not atomic, the value may briefly be
+ *   bogus. Chances are close to zero this ever hits and since this only
+ *   effects debug output, the ostrich algorithm was chosen here.
+ *
  * @{
  *
  * @file
@@ -112,6 +131,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 
+#include "architecture.h"
 #include "kernel_defines.h"
 #include "list.h"
 #include "thread.h"
@@ -130,7 +150,8 @@ typedef struct {
      * @internal
      */
     list_node_t queue;
-#if defined(DOXYGEN) || defined(MODULE_CORE_MUTEX_PRIORITY_INHERITANCE)
+#if defined(DOXYGEN) || defined(MODULE_CORE_MUTEX_PRIORITY_INHERITANCE) \
+    || defined(MODULE_CORE_MUTEX_DEBUG)
     /**
      * @brief   The current owner of the mutex or `NULL`
      * @note    Only available if module core_mutex_priority_inheritance
@@ -141,6 +162,18 @@ typedef struct {
      * this will have the value of `NULL`.
      */
     kernel_pid_t owner;
+#endif
+#if defined(DOXYGEN) || defined(MODULE_CORE_MUTEX_DEBUG)
+    /**
+     * @brief   Program counter of the call to @ref mutex_lock that most
+     *          recently acquired this mutex
+     *
+     * This is used when the module `core_mutex_debug` is used to debug
+     * deadlocks and is non-existing otherwise
+     */
+    uinttxtptr_t owner_calling_pc;
+#endif
+#if defined(DOXYGEN) || defined(MODULE_CORE_MUTEX_PRIORITY_INHERITANCE)
     /**
      * @brief   Original priority of the owner
      * @note    Only available if module core_mutex_priority_inheritance

core/mutex.c

@@ -47,9 +47,18 @@
  * function into both @ref mutex_lock and @ref mutex_lock_cancelable is,
  * therefore, beneficial for the majority of applications.
  */
-static inline __attribute__((always_inline)) void _block(mutex_t *mutex,
-                                                         unsigned irq_state)
+static inline __attribute__((always_inline))
+void _block(mutex_t *mutex,
+            unsigned irq_state,
+            uinttxtptr_t pc)
 {
+    /* pc is only used when MODULE_CORE_MUTEX_DEBUG */
+    (void)pc;
+#if IS_USED(MODULE_CORE_MUTEX_DEBUG)
+    printf("[mutex] waiting for thread %" PRIkernel_pid " (pc = 0x%" PRIxTXTPTR
+           ")\n",
+           mutex->owner, mutex->owner_calling_pc);
+#endif
     thread_t *me = thread_get_active();
 
     /* Fail visibly even if a blocking action is called from somewhere where
@@ -80,10 +89,17 @@ static inline __attribute__((always_inline)) void _block(mutex_t *mutex,
     irq_restore(irq_state);
     thread_yield_higher();
     /* We were woken up by scheduler. Waker removed us from queue. */
+#if IS_USED(MODULE_CORE_MUTEX_DEBUG)
+    mutex->owner_calling_pc = pc;
+#endif
 }
 
 bool mutex_lock_internal(mutex_t *mutex, bool block)
 {
+    uinttxtptr_t pc = 0;
+#if IS_USED(MODULE_CORE_MUTEX_DEBUG)
+    pc = cpu_get_caller_pc();
+#endif
     unsigned irq_state = irq_disable();
 
     DEBUG("PID[%" PRIkernel_pid "] mutex_lock_internal(block=%u).\n",
@@ -92,9 +108,15 @@ bool mutex_lock_internal(mutex_t *mutex, bool block)
     if (mutex->queue.next == NULL) {
         /* mutex is unlocked. */
         mutex->queue.next = MUTEX_LOCKED;
-#ifdef MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
+#if IS_USED(MODULE_CORE_MUTEX_PRIORITY_INHERITANCE) \
+        || IS_USED(MODULE_CORE_MUTEX_DEBUG)
         thread_t *me = thread_get_active();
         mutex->owner = me->pid;
+#endif
+#if IS_USED(MODULE_CORE_MUTEX_DEBUG)
+        mutex->owner_calling_pc = pc;
+#endif
+#if IS_USED(MODULE_CORE_MUTEX_PRIORITY_INHERITANCE)
         mutex->owner_original_priority = me->priority;
 #endif
         DEBUG("PID[%" PRIkernel_pid "] mutex_lock(): early out.\n",
@@ -106,14 +128,18 @@ bool mutex_lock_internal(mutex_t *mutex, bool block)
             irq_restore(irq_state);
             return false;
         }
-        _block(mutex, irq_state);
+        _block(mutex, irq_state, pc);
     }
 
     return true;
 }
 
 int mutex_lock_cancelable(mutex_cancel_t *mc)
 {
+    uinttxtptr_t pc = 0;
+#if IS_USED(MODULE_CORE_MUTEX_DEBUG)
+    pc = cpu_get_caller_pc();
+#endif
     unsigned irq_state = irq_disable();
 
     DEBUG("PID[%" PRIkernel_pid "] mutex_lock_cancelable()\n",
@@ -131,9 +157,15 @@ int mutex_lock_cancelable(mutex_cancel_t *mc)
     if (mutex->queue.next == NULL) {
         /* mutex is unlocked. */
         mutex->queue.next = MUTEX_LOCKED;
-#ifdef MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
+#if IS_USED(MODULE_CORE_MUTEX_PRIORITY_INHERITANCE) \
+        || IS_USED(MODULE_CORE_MUTEX_DEBUG)
         thread_t *me = thread_get_active();
         mutex->owner = me->pid;
+#endif
+#if IS_USED(MODULE_CORE_MUTEX_DEBUG)
+        mutex->owner_calling_pc = pc;
+#endif
+#if IS_USED(MODULE_CORE_MUTEX_PRIORITY_INHERITANCE)
         mutex->owner_original_priority = me->priority;
 #endif
         DEBUG("PID[%" PRIkernel_pid "] mutex_lock_cancelable() early out.\n",
@@ -142,7 +174,7 @@ int mutex_lock_cancelable(mutex_cancel_t *mc)
         return 0;
     }
     else {
-        _block(mutex, irq_state);
+        _block(mutex, irq_state, pc);
         if (mc->cancelled) {
             DEBUG("PID[%" PRIkernel_pid "] mutex_lock_cancelable() "
                   "cancelled.\n", thread_getpid());
@@ -185,7 +217,7 @@ void mutex_unlock(mutex_t *mutex)
 
     uint16_t process_priority = process->priority;
 
-#ifdef MODULE_CORE_MUTEX_PRIORITY_INHERITANCE
+#if IS_USED(MODULE_CORE_MUTEX_PRIORITY_INHERITANCE)
     thread_t *owner = thread_get(mutex->owner);
     if ((owner) && (owner->priority != mutex->owner_original_priority)) {
         DEBUG("PID[%" PRIkernel_pid "] prio %u --> %u\n",
@@ -194,6 +226,9 @@ void mutex_unlock(mutex_t *mutex)
         sched_change_priority(owner, mutex->owner_original_priority);
     }
 #endif
+#if IS_USED(MODULE_CORE_MUTEX_DEBUG)
+    mutex->owner_calling_pc = 0;
+#endif
 
     irq_restore(irqstate);
     sched_switch(process_priority);

cpu/cc26xx_cc13xx/periph/uart.c

@@ -64,7 +64,7 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
     int cts_pin = uart_config[uart].cts_pin;
 #endif
 
-    if (uart == 0) {
+    if ((UART_NUMOF == 1) || (uart == 0)) {
         /* UART0 requires serial domain to be enabled */
         if (!power_is_domain_enabled(POWER_DOMAIN_SERIAL)) {
             power_enable_domain(POWER_DOMAIN_SERIAL);

examples/nanocoap_server/Makefile

@@ -18,6 +18,7 @@ USEMODULE += sock_udp
 USEMODULE += gnrc_icmpv6_echo
 
 USEMODULE += nanocoap_sock
+USEMODULE += nanocoap_resources
 
 USEMODULE += xtimer
 
@@ -42,6 +43,23 @@ ifneq (,$(filter $(BOARD),$(LOW_MEMORY_BOARDS)))
   USEMODULE += prng_minstd
 endif
 
+# Enable fileserver for boards with plenty of memory
+HIGH_MEMORY_BOARDS := native same54-xpro mcb2388
+
+ifneq (,$(filter $(BOARD),$(HIGH_MEMORY_BOARDS)))
+  USEMODULE += gcoap_fileserver
+  USEMODULE += vfs_default
+
+  # we don't want to run GCoAP
+  CFLAGS += -DCONFIG_GCOAP_NO_AUTO_INIT=1
+  CFLAGS += -DCONFIG_NANOCOAP_SERVER_WELL_KNOWN_CORE=1
+
+  # always enable auto-format for native
+  ifeq ($(BOARD),native)
+    USEMODULE += vfs_auto_format
+  endif
+endif
+
 # Change this to 0 show compiler invocation lines by default:
 QUIET ?= 1
 

examples/nanocoap_server/coap_handler.c

@@ -166,14 +166,38 @@ ssize_t _sha256_handler(coap_pkt_t* pkt, uint8_t *buf, size_t len, coap_request_
     return pkt_pos - (uint8_t*)pkt->hdr;
 }
 
-/* must be sorted by path (ASCII order) */
-const coap_resource_t coap_resources[] = {
-    COAP_WELL_KNOWN_CORE_DEFAULT_HANDLER,
-    { "/echo/", COAP_GET | COAP_MATCH_SUBTREE, _echo_handler, NULL },
-    { "/riot/board", COAP_GET, _riot_board_handler, NULL },
-    { "/riot/value", COAP_GET | COAP_PUT | COAP_POST, _riot_value_handler, NULL },
-    { "/riot/ver", COAP_GET, _riot_block2_handler, NULL },
-    { "/sha256", COAP_POST, _sha256_handler, NULL },
+NANOCOAP_RESOURCE(echo) {
+    .path = "/echo/", .methods = COAP_GET | COAP_MATCH_SUBTREE, .handler = _echo_handler
+};
+NANOCOAP_RESOURCE(board) {
+    .path = "/riot/board", .methods = COAP_GET, .handler = _riot_board_handler
+};
+NANOCOAP_RESOURCE(value) {
+    .path = "/riot/value", .methods = COAP_GET | COAP_PUT | COAP_POST, .handler = _riot_value_handler
+};
+NANOCOAP_RESOURCE(ver) {
+    .path = "/riot/ver", .methods = COAP_GET, .handler = _riot_block2_handler
+};
+NANOCOAP_RESOURCE(sha256) {
+    .path = "/sha256", .methods = COAP_POST, .handler = _sha256_handler
 };
 
-const unsigned coap_resources_numof = ARRAY_SIZE(coap_resources);
+/* we can also include the fileserver module */
+#ifdef MODULE_GCOAP_FILESERVER
+#include "net/gcoap/fileserver.h"
+#include "vfs_default.h"
+
+NANOCOAP_RESOURCE(fileserver) {
+    .path = "/vfs",
+    .methods = COAP_GET
+#if IS_USED(MODULE_GCOAP_FILESERVER_PUT)
+      | COAP_PUT
+#endif
+#if IS_USED(MODULE_GCOAP_FILESERVER_DELETE)
+      | COAP_DELETE
+#endif
+      | COAP_MATCH_SUBTREE,
+    .handler = gcoap_fileserver_handler,
+    .context = VFS_DEFAULT_DATA
+};
+#endif

makefiles/tools/openocd.inc.mk

@@ -40,7 +40,8 @@ endif
 
 ifneq (,$(OPENOCD_CMD_RESET_HALT))
   # Export OPENOCD_CMD_RESET_HALT only to the flash targets
-  $(call target-export-variables,flash%,OPENOCD_CMD_RESET_HALT)
+  $(call target-export-variables,flash,OPENOCD_CMD_RESET_HALT)
+  $(call target-export-variables,flash-only,OPENOCD_CMD_RESET_HALT)
 endif
 
 OPENOCD_DEBUG_TARGETS = debug debugr debug-server

sys/Makefile.dep

@@ -754,6 +754,15 @@ ifneq (,$(filter nanocoap_dtls,$(USEMODULE)))
   USEPKG += tinydtls
 endif
 
+ifneq (,$(filter nanocoap_server_auto_init,$(USEMODULE)))
+  USEMODULE += nanocoap_server
+endif
+
+ifneq (,$(filter nanocoap_server,$(USEMODULE)))
+  USEMODULE += nanocoap_resources
+  USEMODULE += nanocoap_sock
+endif
+
 ifneq (,$(filter nanocoap_sock,$(USEMODULE)))
   USEMODULE += sock_udp
   USEMODULE += sock_util

sys/auto_init/auto_init.c

@@ -160,6 +160,10 @@ extern void gcoap_init(void);
 AUTO_INIT(gcoap_init,
           AUTO_INIT_PRIO_MOD_GCOAP);
 #endif
+#if IS_USED(MODULE_NANOCOAP_SERVER_AUTO_INIT)
+extern void auto_init_nanocoap_server(void);
+AUTO_INIT(auto_init_nanocoap_server, AUTO_INIT_PRIO_MOD_NANOCOAP);
+#endif
 #if IS_USED(MODULE_DEVFS)
 extern void auto_init_devfs(void);
 AUTO_INIT(auto_init_devfs,

sys/auto_init/include/auto_init_priorities.h

@@ -174,6 +174,12 @@ extern "C" {
 #define AUTO_INIT_PRIO_MOD_UWB_CORE                     1230
 #endif
 #ifndef AUTO_INIT_PRIO_MOD_GCOAP
+/**
+ * @brief   nanoCoAP server priority
+ */
+#define AUTO_INIT_PRIO_MOD_NANOCOAP                     1235
+#endif
+#ifndef AUTO_INIT_PRIO_MOD_GCOAP
 /**
  * @brief   GCoAP priority
  */

sys/include/net/nanocoap.h

@@ -90,6 +90,7 @@
 #include "bitfield.h"
 #include "byteorder.h"
 #include "iolist.h"
+#include "macros/utils.h"
 #include "net/coap.h"
 #else
 #include "coap.h"
@@ -102,6 +103,10 @@
 typedef void sock_udp_ep_t;
 #endif
 
+#if defined(MODULE_NANOCOAP_RESOURCES)
+#include "xfa.h"
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -402,15 +407,29 @@ typedef struct {
     uint8_t *opt;                   /**< Pointer to the placed option       */
 } coap_block_slicer_t;
 
+#if defined(MODULE_NANOCOAP_RESOURCES) || DOXYGEN
+/**
+ * @brief   CoAP XFA resource entry
+ *
+ * @param name  internal name of the resource entry, must be unique
+ */
+#define NANOCOAP_RESOURCE(name) \
+    XFA_CONST(coap_resources_xfa, 0) coap_resource_t CONCAT(coap_resource_, name) =
+#else
 /**
  * @brief   Global CoAP resource list
+ * @deprecated Use @ref NANOCOAP_RESOURCE instead.
+ *             The function will be removed after the 2024.01 release.
  */
 extern const coap_resource_t coap_resources[];
 
 /**
  * @brief   Number of entries in global CoAP resource list
+ * @deprecated Use @ref NANOCOAP_RESOURCE instead.
+ *             The function will be removed after the 2024.01 release.
  */
 extern const unsigned coap_resources_numof;
+#endif
 
 /**
  * @name    Functions -- Header Read/Write
@@ -2048,6 +2067,13 @@ extern ssize_t coap_well_known_core_default_handler(coap_pkt_t *pkt, \
                                                     coap_request_ctx_t *context);
 /**@}*/
 
+/**
+ * @brief   Respond to `/.well-known/core` to list all resources on the server
+ */
+#ifndef CONFIG_NANOCOAP_SERVER_WELL_KNOWN_CORE
+#define CONFIG_NANOCOAP_SERVER_WELL_KNOWN_CORE !IS_USED(MODULE_GCOAP)
+#endif
+
 /**
  * @brief   Checks if a CoAP resource path matches a given URI
  *