sched/semaphore: Keep a negative task id out of the mutex holder.

A mutex records its holder as a task id in the low 31 bits of a word
whose top bit means "someone is blocked on this".  The id was stored
without masking, so an id with its top bit set became a holder with the
blocking bit raised.

Task ids are normally small and positive, but not always.
nxsched_gettid() reports -ESRCH for a context that no longer maps to a
running task, and there is a window where that is exactly what the
running context is: nxtask_exit() marks the next task ready to run
while the dying task is still executing on its own stack, and only then
releases the TCB.  Freeing the group inside that release takes and
drops the group's mutexes, so the lock stores 0xfffffffd and the unlock
compares 0x7ffffffd, which are not equal.

With assertions enabled the unlock trips its holder check, and every
exit of a process that frees memory panics.  In a kernel build that is
every exit, so no program could be run twice, and running one at all
took the shell down with it.  Without assertions the failure is silent:
the accidental blocking bit sends the unlock looking for a waiter that
never existed.

Encode the id the same way everywhere it is stored or compared, so that
a lock and an unlock from one context agree whatever the id's sign.
The masked forms of -1 and -2 would alias the "no holder" and "reset"
values, but nxsched_gettid() yields only valid ids and -ESRCH.

mm_lock() already sidesteps this window with a note that gettid() may
return -ESRCH during a context switch; this gives the generic mutex the
same footing rather than a second special case.

Test case, on the EIC7700 EVB, which is a kernel build with assertions:

  nsh> hello
  Hello, World!!

Before, that printed and then panicked in sem_post, taking the shell
with it, every time.  After, five runs in a row complete and the shell
survives.  ps over telnet still completes.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>

include/nuttx/semaphore.h

@@ -74,6 +74,22 @@
 
 #define NXSEM_MHOLDER(s)      ((FAR atomic_t *)&((s)->val.mholder))
 
+/* Make a mutex holder value from a task id.  The holder field keeps it in
+ * the low 31 bits because bit 31 is the blocking flag, and a task id can
+ * be negative here: nxsched_gettid() reports -ESRCH for a context that no
+ * longer maps to a task, which is exactly the state a task is in while
+ * nxtask_exit() tears it down and its group's mutexes get their final
+ * lock and unlock.  Storing such an id unmasked would raise the blocking
+ * flag by accident, and an unlock would then try to wake waiters that do
+ * not exist.  Masking keeps a lock and unlock from the same context
+ * consistent with each other, whatever the id's sign.
+ *
+ * The ids -1 and -2 would alias NXSEM_MRESET and NXSEM_NO_MHOLDER, but
+ * nxsched_gettid() produces only valid ids and -ESRCH, which is -3.
+ */
+
+#define NXSEM_MAKE_MHOLDER(tid) ((uint32_t)(tid) & ~NXSEM_MBLOCKING_BIT)
+
 /* Check if holder value (TID) is not NO_HOLDER or RESET */
 
 #define NXSEM_MACQUIRED(h)    (((h) & NXSEM_NO_MHOLDER) != NXSEM_NO_MHOLDER)

libs/libc/semaphore/sem_trywait.c

@@ -152,7 +152,7 @@ int nxsem_trywait(FAR sem_t *sem)
               return -EAGAIN;
             }
 
-          new = _SCHED_GETTID();
+          new = NXSEM_MAKE_MHOLDER(_SCHED_GETTID());
         }
       else
         {

libs/libc/semaphore/sem_wait.c

@@ -180,7 +180,7 @@ int nxsem_wait(FAR sem_t *sem)
               break;
             }
 
-          new = _SCHED_GETTID();
+          new = NXSEM_MAKE_MHOLDER(_SCHED_GETTID());
         }
       else
         {

sched/semaphore/sem_post.c

@@ -99,11 +99,14 @@ int nxsem_post_slow(FAR sem_t *sem)
       mholder = atomic_fetch_or(NXSEM_MHOLDER(sem), NXSEM_MBLOCKING_BIT);
 
       /* Mutex post from another thread is not allowed, unless
-       * called from nxsem_reset
+       * called from nxsem_reset.  The comparison uses the same encoding
+       * as the lock side so that a context whose id is -ESRCH, which is
+       * what a task being torn down reports, still matches its own lock.
        */
 
       DEBUGASSERT(mholder == (NXSEM_MBLOCKING_BIT | NXSEM_MRESET) ||
-                  (mholder & (~NXSEM_MBLOCKING_BIT)) == nxsched_gettid());
+                  (mholder & (~NXSEM_MBLOCKING_BIT)) ==
+                  NXSEM_MAKE_MHOLDER(nxsched_gettid()));
 
       blocking = NXSEM_MBLOCKING(mholder);
 

sched/semaphore/sem_trywait.c

@@ -88,7 +88,7 @@ int nxsem_trywait_slow(FAR sem_t *sem)
               break;
             }
 
-          new = nxsched_gettid();
+          new = NXSEM_MAKE_MHOLDER(nxsched_gettid());
         }
       else
         {

sched/semaphore/sem_wait.c

@@ -119,9 +119,13 @@ int nxsem_wait_slow(FAR sem_t *sem)
 
       mholder = atomic_fetch_or(NXSEM_MHOLDER(sem), NXSEM_MBLOCKING_BIT);
 
-      /* Avoid mutex recursion, which is not allowed. */
+      /* Avoid mutex recursion, which is not allowed.  The comparison uses
+       * the lock side's encoding so that ids of either sign compare the
+       * way they were stored.
+       */
 
-      DEBUGASSERT((mholder & (~NXSEM_MBLOCKING_BIT)) != nxsched_gettid());
+      DEBUGASSERT((mholder & (~NXSEM_MBLOCKING_BIT)) !=
+                  NXSEM_MAKE_MHOLDER(nxsched_gettid()));
 
       if (NXSEM_MACQUIRED(mholder))
         {