touchtypes.c:195:9: warning: array index 8 is past the end of the array

[dlmiles]

While building

Source line affected:
https://github.com/RTimothyEdwards/magic/blob/master/utils/touchtypes.c#L195
Line:
TTMaskSetType(&(parms->tfp_types),TT_SUBCELL);

clang18 warning:
touchtypes.c:195:9: warning: array index 8 is past the end of the array (that has type 'unsigned int[8]') [-Warray-bounds]

There is a special note near definition of TT_SUBCELL about it being a pseudo-value but it is being applied as-is.

When building with clang18

Macro/Preprocessor/Mathematical Working:

#define TT_MASKWORDS    ((TT_MAXTYPES + TT_BPW - 1) / TT_BPW)

                        // (256 + 32 - 1) / 32 = 8 as truncated integer

typedef struct
{
    unsigned int        tt_words[TT_MASKWORDS];
} TileTypeBitMask;


typedef struct touchingfuncparms
{
    Point               tfp_point;
    TileTypeBitMask     tfp_types;
} TouchingFuncParms;

TouchingFuncParms *parms = (TouchingFuncParms *) cdarg;


#define TT_MAXTYPES             256
/* Pseudo type signifying unexpanded subcells.  Never painted.  -  Only
   used in a few places, e.g.  TouchingTypes() and mzrouter spacing arrays.
 */
#define TT_SUBCELL      TT_MAXTYPES

-DSIZEOF_UNSIGNED_INT=4  // sizeof(unsigned int) == 4

#define TT_BPW          (8 * sizeof (unsigned int))
#define TT_WORDMASK     (TT_BPW - 1)

#define TT_WORDSHIFT    5                       /* LOG2(TT_BPW) */

#define ttBit(t)        ((t) & TT_WORDMASK)
#define ttMask(t)       ((unsigned int)1 << ttBit(t))

#define ttWord(t)       ((t) >> TT_WORDSHIFT)

#define TTMaskSetType(m, t)     ((m)->tt_words[ttWord(t)] |= ttMask(t))

utils/touchtypes.c:195
        TTMaskSetType(&(parms->tfp_types),TT_SUBCELL);

based on the above info some working:
        ((m)->tt_words[(256) >> TT_WORDSHIFT] |= ((unsigned int)1 << ((256) & TT_WORDMASK))

        ((m)->tt_words[(256) >> 5] |= ((unsigned int)1 << ((256) & 31))

        ((m)->tt_words[8] |= ((unsigned int)1 << (0))

// So yes clang18 can see this access beyond end of array and cause the warning

So I'm agreeing with clang18 ((m)->tt_words[8] |= ((unsigned int)1 << (0))

Was the original programming intention to do nothing ?
(which is what happens now, since it doesn't modify data within array bounds.
assume setting bit0 of the 32bit little-endian after the end of tt_words[]
this is the same as doing nothing, since it didn't actively edit the memory
it intended. resolution comment out the line

Or maybe the programming intention was to set bit0 of the index 0 of tt_word[0] as in
tt_word[0] |= 0x1. resolution might be to use TT_SUBNET & TT_WORDMASK

[RTimothyEdwards]

I think that the point of TT_SUBCELL was to add an extra bit to the type mask for use with the interactive router; this is consistent with uses elsewhere in the irouter and mzrouter code (e.g., mzrouter.h line 67 int rt_spacing[TT_MAXTYPES + 1]; is defining an array that is one larger than the number of tile types, and then using TT_SUBCELL to index it.

I'm inclined to suggest that the proper solution is to use one of the indexes that is lower than TT_TECHDEPBASE for TT_SUBCELL, since those type numbers should never be exercised in the irouter and mzrouter code; e.g.,

#define TT_SUBCELL  TT_CHECKSUBCELL

seems like it ought to work; then all the arrays defined with TT_MAXTYPES + 1 can be reduced to TT_MAXTYPES and there will be no issues with the bitmask.