arraydel() reads one slot past the array allocation when the array is full

[mgrossmann]

arraydel() reads one element past the end of the array allocation when the
array is full. The value is discarded immediately afterwards, so this is an
ASAN-class finding rather than a measured fault — filed because the routine is
on considerably more live paths than was assumed when it was last looked at.

Found while harvesting the §6 footnotes of #108 before closing it.

The read

src/clib/@@ardel.c:22-23:

if ((index > 0) && (index <= array->count)) {
    index--;
    vitem = (*carray)[index];
    /* now shift everything left */
    for( ; index < array->count; index++ ) {
        (*carray)[index] = (*carray)[index+1];     /* <- index+1 == count */
    }
    array->count--;
    (*carray)[array->count] = NULL;
}

The loop runs while index < count, so its last iteration reads
(*carray)[count]. The allocation, from @@aradd.c, is
(ARRAY_SIZE + size) * sizeof(void *) bytes with ARRAY_SIZE slots of header
ahead of *carray — so slots 0 .. size-1 are in bounds and (*carray)[size]
is the first one out. When the array is full (count == size) the last
iteration reads exactly that slot.

The bound the loop wants is count - 1.

Why it does not currently misbehave

The out-of-bounds value is written into slot count-1, and the two statements
after the loop set count-- and then (*carray)[count] = NULL — which is that
same slot. So whatever was read is overwritten before anything can observe it,
and the array is left correct.

That makes this benign on the target today. It is still a read outside the
allocation, it would be flagged the moment any of this is exercised under ASAN
on the host, and it depends on the two lines after the loop staying exactly as
they are.

Why it is worth fixing anyway

The #108 review recorded this footnote with the note that array_del was
#if 0'd out of the jesjob path. That is true of the jesjob path only —
arraydel() has around eighteen live callers elsewhere, several on hot paths:

src/thdmgr/@@cmwdel.c:66     worker table
src/thdmgr/@@cmqdel.c:27     work queue
src/thdmgr/@@cminit.c:389    work queue (dequeue)
src/clib/@@sodel.c:26        socket table
src/clib/@@reopen.c:81       FILE table
src/clib/fclose.c:60         FILE table
src/clib/@@ctcrtx.c:182      mutex table
src/clib/mtxunlk.c:23        mutex table
src/clib/@@ctpush.c:22       CRT push stack
src/clib/@@ctpop.c:25-26     CRT push stack / args
src/clib/atexit.c:19         exit list
src/clib/on@exit.c:20        exit list
src/clib/@@ctdel.c:39        thread table
src/clib/@@crtres.c:31       PPA CRT list

Proposed fix

Stop the shift one element earlier:

for( ; index + 1 < array->count; index++ ) {
    (*carray)[index] = (*carray)[index+1];
}

Host-testable: fill an array to exactly size, delete an element, assert the
contents and count. Worth adding either way, since there is no test over
arraydel() at full occupancy today.