std::array assertion in doSetTile_char for non-optimized builds

[ChrisJohnsen]

(This is more detail on a cause of the assertion that prompted #5864.)

The combination of a local, non-optimized build of DFHack (not production
builds) made with GCC 15 and the recent change to using std::array
highlighted a potential out-of-bounds indexing in doSetTile_char. Just
starting gui/launcher is sufficient to trigger a hard crash that does not
apparently produce any logs (not on the DFHack console, not in the stderr.log,
nor in the errorlog.txt). This is not the only trigger for this assertion.

Stack Trace

Asking GDB for a backtrace shows these as the uppermost frames (excluding the
internals above the assertion and below do_render):

std::__glibcxx_assert_fail
std::array<std::array<unsigned char, 3ul>, 16ul>::operator[](unsigned long)
doSetTile_char(DFHack::Screen::Pen const&, int, int, bool)
doSetTile_default(DFHack::Screen::Pen const&, int, int, bool, int* df::graphic_viewportst::*)
doSetTile(DFHack::Screen::Pen const&, int, int, bool, int* df::graphic_viewportst::*)
DFHack::Screen::paintTile(DFHack::Screen::Pen const&, int, int, bool, int* df::graphic_viewportst::*)
screen_paintTile(lua_State*)
luaD_precall(lua_State*, lua_TValue*, int)
luaV_execute(lua_State*)
luaD_call(lua_State*, lua_TValue*, int)
luaD_callnoyield(lua_State*, lua_TValue*, int)
lua_callk(lua_State*, int, int, long, int (*)(lua_State*, int, long))
DFHack::dfhack_lua_viewscreen::do_render(lua_State*)

The std::array<std::array<unsigned char, 3>, 16>::operator[] looked
suspiciously like the indexing of gps->uccolor.

Dropping a debug print into doSetTile_char just before uccolor indexing
shows that bg was 255 in my crash scenario (this happens even in optimized
GCC 12 builds).

Source of This Particular Crash

The TextAreaComponent used in the launcher's input field sets a default bg of
COLOR_RESET (-1). It isn't overridden in the case of the launcher, so it makes
it into the Pen instance as a bg data member with values -1. When used as
a uint8_t (in doSetTile_char) the value is 255 and is OOB for the
16-element gps->uccolor array.

Possible Fixes

COLOR_RESET makes sense in the context of consoles (e.g., where it can be "SGR
0" on ANSI consoles). But maybe it doesn't make as much sense as a color
specification when drawing into the DF buffers.

The non-console uses of COLOR_RESET could be changed to use specific colors,
but doSetTile_char should also probably make sure the values are not out of
range however they arise.

For Lua-originating Pens, maybe the preliminary Pen-handling could check for
COLOR_RESET (and otherwise make sure values are in bound)? decode_pen,
Lua::CheckPen, adjust_pen, and dfhack_pen_newindex all look like they are
involved with setting Pen fg, bg, tile_fg, and tile_bg.

It might be nice to handle the COLOR_RESET/-1/255 value specially, so that it
can yield different values for foreground and background use (use the Pen
"defaults" of grey and black?). Other out of range values could probably just
be masked off to keep them in bounds.

"Protecting" Pen from OOB values on the C++ side is harder since all the
relevant members are public. Maybe it is enough to rely on simple masking done
in doSetTile_char?

Non-Optimized Build? GCC 15?

I had been using GCC 15.3.0 since GCC 12 was not conveniently available. I have
since installed a local build of GCC 12 to use the officially specified
toolchain.

The assertions do not trigger in any GCC 12 builds that I have tried (empty,
Release, or RelWithDebInfo). The CI and release builds do not trigger the
assertions. So the production builds do not suffer the crash I found (they do
still presumably potentially do the OOB read; though I haven't looked at a
disassembly to verify that the OOB read actually makes it past the optimizer).

The non-optimized build was due to an unintentional CMake configuration with an
unset/empty CMAKE_BUILD_TYPE (CMake 4.4.2). Specifying Release or
RelWithDebInfo both enable optimization (and Release adds NDEBUG, which
should also completely disable the assertions?).

[ab9rf]

#5864 prevents the out of bound access but doesn't address the underlying cause

my initial research suggests that #4995 is at least one of the sources of this behavior, in that that PR introduced code that set Pen.bg to COLOR_RESET, which is invalid. I do not know what @wiktor-obrebski had in mind when they did this and would appreciate feedback from them or from anyone else familiar with this code to explain what purpose setting the bg of a Pen to COLOR_RESET is intended to serve here