Re: top vs. array sorted_state's out of bounds indexing (and related issues)

On Sun, Jun 18, 2023 at 09:23:06PM -0700, Mark Millard wrote:
> usr.bin/top/machine.c (from main) has:
> 
> /*
> . . . The process states are ordered as follows (from least
>  *      to most important):  WAIT, zombie, sleep, stop, start, run.  The
>  *      array declaration below maps a process state index into a number
>  *      that reflects this ordering.
>  */
> 
> static int sorted_state[] = {
>         0,      /* not used             */
>         3,      /* sleep                */
>         1,      /* ABANDONED (WAIT)     */
>         6,      /* run                  */
>         5,      /* start                */
>         2,      /* zombie               */
>         4       /* stop                 */
> };
> 
> Note the elements are 0..6, so 7 elements.
> 
> It is accessed via code like:
> 
> sorted_state[(unsigned char)(b)->ki_stat]
> 
> But ->ki_stat has the values listed in
> sys/sys/proc.h :
> 
> /*
>  * These were process status values (p_stat), now they are only used in
>  * legacy conversion code.
>  */
> #define SIDL    1               /* Process being created by fork. */
> #define SRUN    2               /* Currently runnable. */
> #define SSLEEP  3               /* Sleeping on an address. */
> #define SSTOP   4               /* Process debugging or suspension. */
> #define SZOMB   5               /* Awaiting collection by parent. */
> #define SWAIT   6               /* Waiting for interrupt. */
> #define SLOCK   7               /* Blocked on a lock. */
> 
> This leads to element indexes (including the
> "unused" 0) of 0..7 so spanning 8 elements.
> 
> Thus there is a out of bounds error involved when
> ->ki_stat == SLOCK .
> 
> (I later show the ki_stat assignments.)
> 
> There is also the issue that:
> 
> SIDL   is misidentified as: sleep
> SRUN   is misidentified as: ABANDONED (WAIT)
> SSLEEP is misidentified as: run
> SZOMB  is misidentified as: start
> SWAIT  is misidentified as: zombie
> SLOCK  is out of bounds (as indicated above).
> 
> So the sort order in top is not as documented.
> 
> 
> For reference, from sys/kern/kern_proc.c :
> 
>         if (p->p_state == PRS_NORMAL) { /* approximate. */
>                 if (TD_ON_RUNQ(td) ||
>                     TD_CAN_RUN(td) ||
>                     TD_IS_RUNNING(td)) {
>                         kp->ki_stat = SRUN;
>                 } else if (P_SHOULDSTOP(p)) {
>                         kp->ki_stat = SSTOP;
>                 } else if (TD_IS_SLEEPING(td)) {
>                         kp->ki_stat = SSLEEP;
>                 } else if (TD_ON_LOCK(td)) {
>                         kp->ki_stat = SLOCK;
>                 } else {
>                         kp->ki_stat = SWAIT;
>                 }
>         } else if (p->p_state == PRS_ZOMBIE) {
>                 kp->ki_stat = SZOMB;
>         } else {
>                 kp->ki_stat = SIDL;
>         }
https://reviews.freebsd.org/D40607