nametabletest.cpp: fix out-of-bounds access on endianness conversion

[trofi]

Initially observed as a nametabletest test failre on today's gcc-13. -fsanitize=undefined detected off-by-one error accessing one element past NameRecord array:

$ ./tests/nametabletest/nametabletest
tests/nametabletest/nametabletest.cpp:142:79: runtime error: index 5 out of bounds for type 'NameRecord [5]'
...
tests/nametabletest/nametabletest.cpp:142:79: runtime error: index 7 out of bounds for type 'NameRecord [7]'
...

This happens because test record are slightly inconsistent: when name array has N entries N+1 is specified as count, and local toBigEndian helper uses this count to iterate over local array.

The rest of code (like NameTable::getLanguageId) seems to assume that table count is off-by-one.

The change fixes test failure on this week's gcc-13.

[tim-eves]

Thanks for spotting this bug. Looking into this further the real source of the bug is the use of sizeof(TtfUtil::Sfnt::FontNames) in calculating the nameheader count values initialised into testA and testBdeclarations at the start of the file. The underlying structure (FontName) includes a C++ spec compliant version of a flexible member array at the end, but due to C++ this cannot be 0, so it is 1 (these structures are designed for memory mapped reading of TTF files, rather than generation and writing). Several compilers permit 0 length arrays in that position, but that is due a compiler specific language extension, and we're trying to keep code like this as spec compliant as possible. That is where the +1 is coming from.

We do however include a non-entry for that first record embedded in the struct FontName , which the code in NameTable::getLanguageId() does process thus the +1 count is correct there. But as you identified we don't processs entries after that first one in the testcase and therefore the code in toBigEndian requires require the adjusted value.

[tim-eves]

@trofi This lead down quite the rabbit hole as it turns out nametabletest needs a bit of attention in places. It seems the test "worked" without your fix and started failing for the ksw-MM and mnw-MM language ID test cases with it. Your fix is undoubtedly a good fix, for an actual bug, but it seems the overrun you identified was silently byte swapping NameTestB::m_langTagCount into the correct byte order. This was supposed to be done in toBigEndian1(...), but that function wasn't being called at all. Once your fix is applied it uncovers this other bug. I'll commit a fix for both (and a couple of other discovered during fixing these), credit you in the commit and then close the PR with a comment once the fix is in the default branch.