Devilutionx: Error loading level 8 (osx)

[dcworldwide]

• Compile from source on osx catalina (pulled from master 5a32d7228fe0d1e4c997a7cb28172a6eb6737a33)
• Loaded save game (level 7)
• Enter level 8, crash

Crash output

build % cmake --build . -j $(sysctl -n hw.ncpu) && ./devilutionx.app/Contents/MacOS/devilutionx
[  2%] Built target PKWare
[  5%] Built target smacker
[ 11%] Built target Radon
[ 16%] Built target StormLib
[ 64%] Built target devilution
[100%] Built target devilutionx
DUMMY: SFileSetBasePath @ SourceX/storm/storm.cpp:784
DUMMY: false_avail : Unhandled SDL event: SDL_AUDIODEVICEADDED 0
DUMMY: false_avail : Unhandled SDL event: SDL_AUDIODEVICEADDED 0
/Users/me/projects/diablo/devilutionX/SourceX/miniwin/misc_msg.cpp:493:32: runtime error: left shift of negative value -22
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/me/projects/diablo/devilutionX/SourceX/miniwin/misc_msg.cpp:493:32 in 
DUMMY: UiProfileGetString @ SourceX/DiabloUI/diabloui.cpp:363
DUMMY: DispatchMessageA @ SourceX/miniwin/misc_msg.cpp:685
DUMMY: CreateFileA : file: /Users/me/Library/Application Support/diasurgical/devilution/single_0.sv (/Users/me/Library/Application Support/diasurgical/devilution/single_0.sv)
DUMMY: CreateFileA : file: /Users/me/Library/Application Support/diasurgical/devilution/single_0.sv (/Users/me/Library/Application Support/diasurgical/devilution/single_0.sv)
/Users/me/projects/diablo/devilutionX/Source/themes.cpp:728:6: runtime error: index -1 out of bounds for type 'dvl::ObjectStruct [127]'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/me/projects/diablo/devilutionX/Source/themes.cpp:728:6 in 
=================================================================
==22668==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000102f08168 at pc 0x00010276118d bp 0x7ffeee08f450 sp 0x7ffeee08f448
WRITE of size 1 at 0x000102f08168 thread T0
    #0 0x10276118c in dvl::Theme_Library(int) themes.cpp:728
    #1 0x1027686b9 in dvl::CreateThemeRooms() themes.cpp:1003
    #2 0x101eb687a in dvl::LoadGameLevel(int, int) diablo.cpp:1539
    #3 0x102071ff7 in dvl::ShowProgress(unsigned int) interfac.cpp:117
    #4 0x101ea2917 in dvl::GM_Game(void*, unsigned int, unsigned long, long) diablo.cpp:626
    #5 0x101beb514 in dvl::DispatchMessageA(dvl::tagMSG const*) misc_msg.cpp:690
    #6 0x101ea2156 in dvl::run_game_loop(unsigned int) diablo.cpp:176
    #7 0x101ea1e0e in dvl::StartGame(int, int) diablo.cpp:113
    #8 0x1021e1121 in dvl::mainmenu_init_menu(int) mainmenu.cpp:145
    #9 0x1021e1012 in dvl::mainmenu_single_player() mainmenu.cpp:133
    #10 0x1021e0e47 in dvl::mainmenu_loop() mainmenu.cpp:102
    #11 0x101ea4f91 in dvl::WinMain(void*, void*, char*, int) diablo.cpp:300
    #12 0x101d34df7 in main main.cpp:28
    #13 0x7fff734672e4 in start (libdyld.dylib:x86_64+0x112e4)

0x000102f08168 is located 56 bytes to the left of global variable 'dvl::object' defined in '/Users/me/projects/diablo/devilutionX/Source/objects.cpp:13:14' (0x102f081a0) of size 16256
0x000102f08168 is located 44 bytes to the right of global variable 'dvl::objectavail' defined in '/Users/me/projects/diablo/devilutionX/Source/objects.cpp:12:5' (0x102f07f40) of size 508
SUMMARY: AddressSanitizer: global-buffer-overflow themes.cpp:728 in dvl::Theme_Library(int)
Shadow bytes around the buggy address:
  0x1000205e0fd0: f9 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
  0x1000205e0fe0: 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x1000205e0ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000205e1000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000205e1010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x1000205e1020: 00 00 00 00 00 00 00 04 f9 f9 f9 f9 f9[f9]f9 f9
  0x1000205e1030: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000205e1040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000205e1050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000205e1060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000205e1070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==22668==ABORTING

single_0.sv.zip

[AJenbo]

Thanks, does it also crash if you make a release build?

[dcworldwide]

With a release build, I didn't get the crash. Only crashing with debug build.

In debug build, commenting out

object[oi]._oSelFlag = 0;
object[oi]._oAnimFrame += 2;

as a hack...loads the level.

p.s. I created a release build as per the following commands, correct me if this is wrong.

cmake -DBINARY_RELEASE=ON ..
cmake --build . -j $(sysctl -n hw.physicalcpu)

[AJenbo]

Thanks, it's probably one of the countless OOB errors in the original game. ASAN is detecting it, but on Mac it's not able to continue afterwards, for releases on x86 it can cause random issues, but is usually fine, other systems might also crash for release builds

[dcworldwide]

Interesting, so the original game suppressed the bug... I couldn't quite wrap my ahead around what the code is trying to do. object does not appear to be defined anywhere.

Will keep logging issues as I play through on mac. At least then they are documented.

[AJenbo]

Back when the game was originally developed they didn't have good tools for detecting oversights like this. The x86 compilers and processor is forgiving in most cases, but it can lead to unstable and unexpected behavior. And at that point it would have been really hard to track down and fix, so lots of these issues where left in the code, we also have to consider that they where relatively inexperienced with writing C/C++.

object is defined here: https://github.com/diasurgical/devilutionX/blob/master/Source/objects.cpp#L13
It is a list of objects in the current level. The problem here is that oi contains a value either smaller then 0 or larger then MAXOBJECTS (127). This is probably because the code assumes that AddObject will always be successful (this isn't the case) and then uses the value from dObject[xp][yp] and then ends up accessing object[-1].

[dcworldwide]

Thanks for the details, it's really helpful

[AJenbo]

This is now fixed in master, thanks for providing a save game for testing this with.