fabiangreffrath / woof

Woof! is a continuation of the Boom/MBF bloodline of Doom source ports.
GNU General Public License v2.0
216 stars 37 forks source link

crash when entering a savegame name with "grave accent" (ASCII 96) #2032

Open fabiangreffrath opened 2 hours ago

fabiangreffrath commented 2 hours ago

Try to enter a savegame name containing a grave accent character. On my keyboard, this is the key to the left of the '1' key in the top-left corner. The crash happens, because there is no such glyph in Doom's HUD font.

First crash in WriteText():

Thread 1 "woof" received signal SIGSEGV, Segmentation fault.
0x00005555555c6fb0 in WriteText (x=80, y=26, string=0x5555557694d0 <savegamestrings> "`") at /home/fabian/Source/woof/src/mn_menu.c:3621
3621            w = SHORT(hu_font[c]->width);
(gdb) bt
#0  0x00005555555c6fb0 in WriteText (x=80, y=26, string=0x5555557694d0 <savegamestrings> "`") at /home/fabian/Source/woof/src/mn_menu.c:3621
#1  0x00005555555c796e in M_DrawSaveLoadBorders () at /home/fabian/Source/woof/src/mn_menu.c:951
#2  0x00005555555c7e6b in M_DrawSave () at /home/fabian/Source/woof/src/mn_menu.c:1274
#3  0x00005555555c6b5e in M_Drawer () at /home/fabian/Source/woof/src/mn_menu.c:3424
#4  0x0000555555587ef7 in D_Display () at /home/fabian/Source/woof/src/d_main.c:406
#5  0x000055555558a62e in D_DoomMain () at /home/fabian/Source/woof/src/d_main.c:2681
#6  0x00005555555a8835 in main (argc=2, argv=0x7fffffffddc8) at /home/fabian/Source/woof/src/i_main.c:70
(gdb) p c
$1 = 63
(gdb) p hu_font[c]
$2 = (patch_t *) 0x0

Mitigated by the following patch:

--- a/src/mn_menu.c
+++ b/src/mn_menu.c
@@ -3612,7 +3612,7 @@ static void WriteText(int x, int y, const char *string)
         }

         c = M_ToUpper(c) - HU_FONTSTART;
-        if (c < 0 || c >= HU_FONTSIZE)
+        if (c < 0 || c >= HU_FONTSIZE || !hu_font[c])
         {
             cx += 4;
             continue;

The same crash happens in MN_StringWidth() again:

Thread 1 "woof" received signal SIGSEGV, Segmentation fault.
0x00005555555cd185 in MN_StringWidth (string=0x5555557694d1 <savegamestrings+1> "") at /home/fabian/Source/woof/src/mn_setup.c:4703
4703            w += SHORT(hu_font[c]->width);
(gdb) bt
#0  0x00005555555cd185 in MN_StringWidth (string=0x5555557694d1 <savegamestrings+1> "") at /home/fabian/Source/woof/src/mn_setup.c:4703
#1  0x00005555555c7eb2 in M_DrawSave () at /home/fabian/Source/woof/src/mn_menu.c:1278
#2  0x00005555555c6b5e in M_Drawer () at /home/fabian/Source/woof/src/mn_menu.c:3424
#3  0x0000555555587ef7 in D_Display () at /home/fabian/Source/woof/src/d_main.c:406
#4  0x000055555558a62e in D_DoomMain () at /home/fabian/Source/woof/src/d_main.c:2681
#5  0x00005555555a8835 in main (argc=2, argv=0x7fffffffddc8) at /home/fabian/Source/woof/src/i_main.c:70
(gdb) p c
$1 = 63
(gdb) p hu_font[c]
$2 = (patch_t *) 0x0

This is, because missing glyph patches are simply stored as NULL pointers in the font arrays here:

https://github.com/fabiangreffrath/woof/blob/e5d2d94c6fb896a0702ed0cdaed38939e7d0bc05/src/st_sbardef.c#L348-L360

This didn't happen in former releases, because missing patches were substituted from the other font, or filled up with the first available character, i.e. '!' here:

https://github.com/fabiangreffrath/woof/blob/1dcef227261f73224d95ea242a1f2db497628c75/src/hu_stuff.c#L440-L455

fabiangreffrath commented 1 hour ago

Apparently there was some back and forth in the SBARDEF PR:

https://github.com/fabiangreffrath/woof/pull/1916/commits/f6f14bca33a2b6f75f1fdbf2e00d95574a47d622

fabiangreffrath commented 1 hour ago

That affected the other font, apparently. Maybe we are just missing the STCFN096 lump that MBF had embedded in its info.c.