[Scummvm-tracker] [ScummVM :: Bugs] #15885: SCUMM: (IMS/SAMNMAX): syncWithSerializer division by zero loading an older save

[ScummVM :: Bugs]

#15885: SCUMM: (IMS/SAMNMAX): syncWithSerializer division by zero loading an older
save
------------------------+---------------------------
Reporter:  dwatteau     |      Owner:  (none)
    Type:  defect       |     Status:  new
Priority:  normal       |  Component:  Engine: SCUMM
 Version:               |   Keywords:
    Game:  Sam and Max  |
------------------------+---------------------------
 Current Git HEAD (before and after commit
 5f382329f1927e97d75443635728a5436d5d5965).

 How to reproduce:

 * Build with `--enable-asan --enable-ubsan`
 * Have a French release of Sam&Max (sorry, it's the only one where I have
 saves, but I believe it's on GOG/Steam)
 * Load the attached save (made sometime in 2022, FWIW), either from the
 GMM or in-game

 I'm then getting this division by zero:

 {{{
 engines/scumm/imuse/imuse_player.cpp:1123:19: runtime error: division by
 zero
 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
 engines/scumm/imuse/imuse_player.cpp:1123:19 in
 AddressSanitizer:DEADLYSIGNAL
 =================================================================
 ==10518==ERROR: AddressSanitizer: FPE on unknown address 0x000108118e2d
 (pc 0x000108118e2d bp 0x7ff7b85a7b10 sp 0x7ff7b85a7820 T0)
     #0 0x108118e2d in Scumm::syncWithSerializer(Common::Serializer&,
 Scumm::ParameterFader&) imuse_player.cpp:1123
     #1 0x108118293 in void
 Common::Serializer::syncArray<Scumm::ParameterFader>(Scumm::ParameterFader*,
 unsigned long, void (*)(Common::Serializer&, Scumm::ParameterFader&),
 unsigned int, unsigned int) serializer.h:297
     #2 0x108116e5a in
 Scumm::Player::saveLoadWithSerializer(Common::Serializer&)
 imuse_player.cpp:1193
     #3 0x10809eda6 in
 Scumm::IMuseInternal::saveLoadIMuse(Common::Serializer&,
 Scumm::ScummEngine*, bool) imuse.cpp:413
     #4 0x10885ad34 in
 Scumm::ScummEngine::saveLoadWithSerializer(Common::Serializer&)
 saveload.cpp:2058
     #5 0x108825bd2 in Scumm::ScummEngine::loadState(int, bool,
 Common::String&) saveload.cpp:772
     #6 0x108821928 in Scumm::ScummEngine::loadState(int, bool)
 saveload.cpp:629
     #7 0x108b58421 in Scumm::ScummEngine::go() scumm.cpp:2570
     #8 0x107d13e1f in Scumm::ScummEngine::run() scumm.h:572
     #9 0x1079dbf08 in runGame(Plugin const*, OSystem&, DetectedGame
 const&, void const*) main.cpp:311
     #10 0x1079ce2bf in scummvm_main main.cpp:796
     #11 0x1079b280f in main macosx-main.cpp:44
     #12 0x7ff804057417 in start+0x767 (dyld:x86_64+0xfffffffffff6e417)

 ==10518==Register values:
 rax = 0x000000000000007f  rbx = 0x00007ff7b85a7880  rcx =
 0x0000000000000000  rdx = 0x0000000000000000
 rdi = 0x00007ff7b85a6b60  rsi = 0x0000000000000000  rbp =
 0x00007ff7b85a7b10  rsp = 0x00007ff7b85a7820
  r8 = 0x000000010d2ae2c0   r9 = 0x0000000000000000  r10 =
 0x00007ff80437624e  r11 = 0x0000000000200206
 r12 = 0x00007ff7b85b3918  r13 = 0x00007ff7b85b3990  r14 =
 0x00007ff7b85b3950  r15 = 0x00007ff7b85b37e0
 AddressSanitizer can not provide additional info.
 SUMMARY: AddressSanitizer: FPE imuse_player.cpp:1123 in
 Scumm::syncWithSerializer(Common::Serializer&, Scumm::ParameterFader&)
 ==10518==ABORTING
 Abort trap: 6
 }}}

 Impacted line is
 <https://github.com/scummvm/scummvm/blob/a3bff044fa5b8efe0fe8111831d4b1bc3ff40172/engines/scumm/imuse/imuse_player.cpp#L1123>

 With commit 5f382329f1927e97d75443635728a5436d5d5965 I still have the
 division by zero error, but then UBSan adds a warning about
 Common::Serializer usage (which is maybe to what's been said on Discord
 about this change). I'll add it below for reference, but it's probably
 unrelated.
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/15885>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM