Opened 3 months ago

Last modified 3 months ago

#13908 new defect

SCUMM: INDY3 (MAC): ASAN crash in Player_V2Base::next_freqs() in Castle Brunwald

Reported by: dwatteau Owned by:
Priority: normal Component: Engine: SCUMM
Version: Keywords: asan, crash, castle brunwald, macintosh
Cc: Game: Indiana Jones 3

Description

Building yesterday's Git HEAD with --enable-optimizations --enable-debug --enable-asan on macOS with clang++ 14.0.0.

This is the Macintosh 16-color version of Indy3.

With ASAN enabled, the game always crashes when arriving at Castle Brunwald:

==12520==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00011029f722 at pc 0x00010f799d18 bp 0x700004034650 sp 0x700004034648
READ of size 1 at 0x00011029f722 thread T6
    #0 0x10f799d17 in Scumm::Player_V2Base::next_freqs(Scumm::Player_V2Base::ChannelInfo*) player_v2base.cpp:607
    #1 0x10f799f39 in Scumm::Player_V2Base::nextTick() player_v2base.cpp:649
    #2 0x10f7747ea in Scumm::Player_V2::readBuffer(short*, int) player_v2.cpp:174
    #3 0x1100c8585 in Audio::CopyRateConverter<true, false>::flow(Audio::AudioStream&, short*, unsigned int, unsigned short, unsigned short) rate.cpp:314
    #4 0x1100be10d in Audio::Channel::mix(short*, unsigned int) mixer.cpp:648
    #5 0x1100bdd7c in Audio::MixerImpl::mixCallback(unsigned char*, unsigned int) mixer.cpp:301
    #6 0x111157c43 in outputCallback+0x1ac (libSDL2-2.0.0.dylib:x86_64+0xe2c43)
    #7 0x7ff80e7b1fe7 in ClientAudioQueue::CallOutputCallback(AudioQueueBuffer*)+0x11d (AudioToolbox:x86_64+0x45fe7)
    #8 0x7ff80e79aa03 in ClientAudioQueue::FetchAndDeliverPendingCallbacks(unsigned int)+0x33b (AudioToolbox:x86_64+0x2ea03)
    #9 0x7ff80e79a64d in _XCallbackNotificationsAvailable+0xa3 (AudioToolbox:x86_64+0x2e64d)
    #10 0x7ff80d6fea8d in mshMIGPerform+0xeb (libAudioToolboxUtility.dylib:x86_64+0xea8d)
    #11 0x7ff800e3a923 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__+0x28 (CoreFoundation:x86_64h+0x80923)
    #12 0x7ff800e3a803 in __CFRunLoopDoSource1+0x26a (CoreFoundation:x86_64h+0x80803)
    #13 0x7ff800e38e6a in __CFRunLoopRun+0x96e (CoreFoundation:x86_64h+0x7ee6a)
    #14 0x7ff800e37e3b in CFRunLoopRunSpecific+0x231 (CoreFoundation:x86_64h+0x7de3b)
    #15 0x11115773c in audioqueue_thread+0x43e (libSDL2-2.0.0.dylib:x86_64+0xe273c)
    #16 0x1110db986 in SDL_RunThread+0x2b (libSDL2-2.0.0.dylib:x86_64+0x66986)
    #17 0x11114a7f2 in RunThread+0x8 (libSDL2-2.0.0.dylib:x86_64+0xd57f2)
    #18 0x7ff800d734e0 in _pthread_start+0x7c (libsystem_pthread.dylib:x86_64+0x64e0)
    #19 0x7ff800d6ef6a in thread_start+0xe (libsystem_pthread.dylib:x86_64+0x1f6a)

Full ASAN log attached.

How to reproduce:

  1. Build with ASAN.
  2. Load the attached savegame, and wait for Indy and Elsa to arrive in front of Castle Brunwald.

Attachments (3)

indy3-ega-mac.s44 (10.0 KB ) - added by dwatteau 3 months ago.
save just before Castle Brunwald in indy3-ega-mac
asan-castle-brunwald-indy3-mac.txt (4.5 KB ) - added by dwatteau 3 months ago.
ASAN log
asan-indy3-vga-ring-bell.txt (9.0 KB ) - added by dwatteau 3 months ago.
Different ASAN issue in Indy3 DOS VGA, after hitting the boxing bell with the mallet

Download all attachments as: .zip

Change History (7)

by dwatteau, 3 months ago

Attachment: indy3-ega-mac.s44 added

save just before Castle Brunwald in indy3-ega-mac

by dwatteau, 3 months ago

ASAN log

comment:1 by eriktorbjorn, 3 months ago

I don't have the time to recompile ScummVM right now, so I hoped I would be able to capture it in Valgrind. Unfortunately not, but it did flag this:

==23698== Conditional jump or move depends on uninitialised value(s)
==23698==    at 0xB54D00: Scumm::ScummEngine::getIntegralTime(double) (scumm.cpp:2353)
==23698==    by 0xB54A7C: Scumm::ScummEngine::waitForTimer(int) (scumm.cpp:2302)
==23698==    by 0xB549D1: Scumm::ScummEngine::go() (scumm.cpp:2286)
==23698==    by 0xA48D45: Scumm::ScummEngine::run() (scumm.h:510)
==23698==    by 0x9FC7D0: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:318)
==23698==    by 0x9FE03E: scummvm_main (main.cpp:619)
==23698==    by 0x9F9AA8: main (posix-main.cpp:44)
==23698== 

Plus a few others, but they were just other ways of calling getIntegralTime().

This isn't directly related, of course.

comment:2 by eriktorbjorn, 3 months ago

Right above, it's trying to limit the value of channel->d.freqmod_offset, but as far as I can see it's still larger than channel->d.freqmod_modulo. That seems bad?

comment:3 by eriktorbjorn, 3 months ago

It's tempting to just change the "if (channel->d.freqmod_offset > channel->d.freqmod_modulo)" to a while. Or simply a modulo operation. Though apparently freqmod_modulo can be 0?

Does anyone know if this issue affects the DOS EGA version as well?

comment:4 by dwatteau, 3 months ago

I'm not able to reproduce this particular crash at Castle Brunwald in my EGA (French) version.

However, I see a different audio ASAN issue in the DOS EGA game, once I'm done using the mallet on the boxing bell. It also happens in the DOS VGA version, so I'm attaching the ASAN log for it, since this release is easier to track down.

by dwatteau, 3 months ago

Different ASAN issue in Indy3 DOS VGA, after hitting the boxing bell with the mallet

Note: See TracTickets for help on using tickets.