[Scummvm-tracker] [ScummVM :: Bugs] #14614: SIGSEGV in Scumm::ScummEngine_v5::saveLoadWithSerializer(Common::Serializer&)

[ScummVM :: Bugs]

#14614: SIGSEGV in
Scumm::ScummEngine_v5::saveLoadWithSerializer(Common::Serializer&)
--------------------------+----------------------------
Reporter:  lephilousophe  |       Owner:  (none)
    Type:  defect         |      Status:  new
Priority:  normal         |   Component:  Engine: SCUMM
 Version:                 |  Resolution:
Keywords:                 |        Game:
--------------------------+----------------------------
Comment (by PushmePullyu):

 I had a quick look at this:

 Since we arrived at go() via run(), init() must have completed without
 errors (see Scumm::ScummEngine::run() in engines/scumm/scumm.h).

 This means init() called Scumm::ScummEngine::resetScumm() (all the
 overridden resetScumm() methods eventually call this one), which has this
 code:

 {{{
 #ifdef USE_RGB_COLOR
         if (_game.features & GF_16BIT_COLOR
 #ifndef DISABLE_TOWNS_DUAL_LAYER_MODE
                 || (_game.platform == Common::kPlatformFMTowns)
 #endif
                 )
                 _16BitPalette = (uint16 *)calloc(512, sizeof(uint16));
 #endif

 }}}


 _16BitPalette only ever gets free'd in ScummEngine::~ScummEngine().


 According to the report the crash then occurs in
 engines/scumm/saveload.cpp:
 in void ScummEngine_v5::saveLoadWithSerializer(Common::Serializer &s):
 {{{
         // Regenerate 16bit palette after loading.
         // This avoids color issues when loading savegames that have been
 saved with a different ScummVM port
         // that uses a different 16bit color mode than the ScummVM port
 which is currently used.
 #ifdef USE_RGB_COLOR
         if (_game.platform == Common::kPlatformPCEngine && s.isLoading())
 {
                 for (int i = 0; i < 256; ++i)
                         _16BitPalette[i] = get16BitColor(_currentPalette[i
 * 3 + 0], _currentPalette[i * 3 + 1], _currentPalette[i * 3 + 2]);
         }
 #endif
 }}}

 Apart from a genuine calloc() failure or the pointer getting clobbered
 somehow, the only way I can see this crashing is having platform set to
 Common::kPlatformPCEngine but feature GF_16BIT_COLOR unset (which would
 skip the allocation).

 Since the only entry in the detection table that sets platform to
 kPlatformPCEngine is Loom PCE, which has GF_16BIT_COLOR, perhaps platform
 was overridden by the user.

 I tested this by adding SOMI CD/DOS/English and then changing platform to
 PC-Engine in the ScummVM game options. Loading a save from the launcher
 with this leads to a backtrace matching the one reported.

 If this is the cause of the crash, it still exists in current master
 (!e510f0868ea2e2f7fdd724ed3c14104ca7da0851). The following diff fixes it
 (but running a game in such a configuration will of course still cause all
 kinds of problems):


 {{{
 --- a/engines/scumm/saveload.cpp
 +++ b/engines/scumm/saveload.cpp
 @@ -2069,7 +2069,7 @@ void
 ScummEngine_v5::saveLoadWithSerializer(Common::Serializer &s) {
         // This avoids color issues when loading savegames that have been
 saved with a different ScummVM port
         // that uses a different 16bit color mode than the ScummVM port
 which is currently used.
  #ifdef USE_RGB_COLOR
 -       if (_game.platform == Common::kPlatformPCEngine && s.isLoading())
 {
 +       if (_game.platform == Common::kPlatformPCEngine && s.isLoading()
 && _game.features & GF_16BIT_COLOR) {
                 for (int i = 0; i < 256; ++i)
                         _16BitPalette[i] = get16BitColor(_currentPalette[i
 * 3 + 0], _currentPalette[i * 3 + 1], _currentPalette[i * 3 + 2]);
         }
 }}}
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/14614#comment:1>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM