Opened 2 years ago

Closed 2 years ago

#13601 closed defect (fixed)

AGS: Time Gentlemen, Please - random crashes to desktop in the first scene of the game

Reported by: antoniou79 Owned by: criezy
Priority: normal Component: Engine: AGS
Version: Keywords:
Cc: Game:

Description (last modified by antoniou79)

This is on Windows 10 x64. Tested the GOG version with the current daily stable for upcoming 2.6.0 and with a local build of 2.7.0git from master HEAD.

Steps to reproduce:

  • Start a new game
  • Skip intro cutscenes
  • Once you resume control in the first scene, start interacting with objects and the protagonists. Usually (but not always) this happens if you grab Dan and use him on Ben and then try to do something else.

I always got a crash (even after loading into the game to the first scene, but it seems that in that case more actions are needed to cause the crash), but not always after specific interaction so I don't have a deterministic sequence of actions. There's no error message on the console window. The game just exits.

ScummVM log does not offer any help either on this.

Edit:
Trying to quickly debug this with Visual Studio, I got two different unhandled thrown exceptions.

  • One was in graphics\surface.cpp, Surface::free() at the first line:
    ::free(pixels);
    
  • The other (in a separate play session) was in the destructor of VorbisStream (in audio\decoders\vorbis.cpp):
    VorbisStream::~VorbisStream() {
    	ov_clear(&_ovFile);
    }
    

Change History (5)

comment:1 by antoniou79, 2 years ago

Description: modified (diff)

comment:2 by criezy, 2 years ago

I can reproduce on macOS. Address sanitizer reports the following issue:

==63735==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x000130cb00d8 at pc 0x00010312fbf0 bp 0x00016ce35ab0 sp 0x00016ce35aa8
WRITE of size 4 at 0x000130cb00d8 thread T0
    #0 in AGS3::putpixel(AGS3::BITMAP*, int, int, int) gfx.cpp:247
    #1 in AGS3::alfont_textout_ex(AGS3::BITMAP*, AGS3::ALFONT_FONT*, char const*, int, int, int, int) alfont.cpp:2391
    #2 in AGS3::alfont_textout(AGS3::BITMAP*, AGS3::ALFONT_FONT*, char const*, int, int, int) alfont.cpp:1823
    #3 in AGS3::TTFFontRenderer::RenderText(char const*, int, AGS3::BITMAP*, int, int, int) ttf_font_renderer.cpp:66
    #4 in AGS3::wouttextxy(AGS3::AGS::Shared::Bitmap*, int, int, unsigned long, int, char const*) fonts.cpp:377
    #5 in AGS3::wouttextxy_AutoOutline(AGS3::AGS::Shared::Bitmap*, unsigned long, int, char const*, int&, int&) display.cpp:479
    #6 in AGS3::wouttext_outline(AGS3::AGS::Shared::Bitmap*, int, int, int, int, char const*) display.cpp:532
    #7 in AGS3::wouttext_aligned(AGS3::AGS::Shared::Bitmap*, int, int, int, int, int, char const*, AGS3::HorAlignment) display.cpp:547
    #8 in AGS3::_display_main(int, int, int, char const*, int, int, int, int, int, bool, bool) display.cpp:231
    #9 in AGS3::_display_at(int, int, int, char const*, int, int, int, int, bool) display.cpp:371
    #10 in AGS3::_displayspeech(char const*, int, int, int, int, int) character.cpp:2704
    #11 in AGS3::DisplaySpeech(char const*, int) character.cpp:2750
    #12 in AGS3::_DisplaySpeechCore(int, char const*) character.cpp:2259
    #13 in AGS3::Character_Say(AGS3::CharacterInfo*, char const*) character.cpp:702
    #14 in AGS3::Sc_Character_Say(void*, AGS3::RuntimeScriptValue const*, int) character.cpp:3035
    #15 in AGS3::ccInstance::Run(int) cc_instance.cpp:1034
    #16 in AGS3::ccInstance::CallScriptFunction(char const*, int, AGS3::RuntimeScriptValue const*) cc_instance.cpp:367
    #17 in AGS3::RunScriptFunction(AGS3::ccInstance*, char const*, unsigned long, AGS3::RuntimeScriptValue const*) script.cpp:362
    #18 in AGS3::RunScriptFunctionAuto(AGS3::ScriptInstType, char const*, unsigned long, AGS3::RuntimeScriptValue const*) script.cpp:451
    #19 in AGS3::post_script_cleanup() script.cpp:555
    #20 in AGS3::RunScriptFunction(AGS3::ccInstance*, char const*, unsigned long, AGS3::RuntimeScriptValue const*) script.cpp:374
    #21 in AGS3::RunUnclaimableEvent(char const*) script.cpp:419
    #22 in AGS3::RunScriptFunctionAuto(AGS3::ScriptInstType, char const*, unsigned long, AGS3::RuntimeScriptValue const*) script.cpp:439
    #23 in AGS3::QueueScriptFunction(AGS3::ScriptInstType, char const*, unsigned long, AGS3::RuntimeScriptValue const*) script.cpp:282
    #24 in AGS3::process_event(AGS3::EventHappened const*) event.cpp:143
    #25 in AGS3::processallevents() event.cpp:364
    #26 in AGS3::game_loop_update_events() game_run.cpp:648
    #27 in AGS3::UpdateGameOnce(bool, AGS3::AGS::Engine::IDriverDependantBitmap*, int, int) game_run.cpp:780
    #28 in AGS3::GameTick() game_run.cpp:929
    #29 in AGS3::RunGameUntilAborted() game_run.cpp:1023
    #30 in AGS3::initialize_start_and_play_game(int, int) game_start.cpp:122
    #31 in AGS3::initialize_engine(AGS3::std::map<AGS3::AGS::Shared::String, AGS3::std::map<AGS3::AGS::Shared::String, AGS3::AGS::Shared::String, Common::Less<AGS3::AGS::Shared::String> >, Common::Less<AGS3::AGS::Shared::String> > const&) engine.cpp:1194
    #32 in AGS::AGSEngine::run() ags.cpp:195
    #33 in runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) main.cpp:318
    #34 in scummvm_main main.cpp:619
    #35 in main macosx-main.cpp:44
    #36 in start+0x0 (libdyld.dylib:arm64e+0x1842c)

0x000130cb00d8 is located 312 bytes to the right of 64416-byte region [0x000130ca0400,0x000130caffa0)
allocated by thread T0 here:
    #0 in wrap_calloc+0x9c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3d320)
    #1 in Graphics::Surface::create(short, short, Graphics::PixelFormat const&) surface.cpp:75
    #2 in Graphics::ManagedSurface::create(short, short, Graphics::PixelFormat const&) managed_surface.cpp:152
    #3 in Graphics::ManagedSurface::ManagedSurface(int, int, Graphics::PixelFormat const&) managed_surface.cpp:59
    #4 in AGS3::Surface::Surface(int, int, Graphics::PixelFormat const&) surface.h:288
    #5 in AGS3::Surface::Surface(int, int, Graphics::PixelFormat const&) surface.h:288
    #6 in AGS3::create_bitmap_ex(int, int, int) surface.cpp:455
    #7 in AGS3::AGS::Shared::Bitmap::Create(int, int, int) allegro_bitmap.cpp:70
    #8 in AGS3::alloc_font_outline_buffers(unsigned long, AGS3::AGS::Shared::Bitmap**, AGS3::AGS::Shared::Bitmap**, int, int, int) fonts.cpp:448
    #9 in AGS3::wouttextxy_AutoOutline(AGS3::AGS::Shared::Bitmap*, unsigned long, int, char const*, int&, int&) display.cpp:474
    #10 in AGS3::wouttext_outline(AGS3::AGS::Shared::Bitmap*, int, int, int, int, char const*) display.cpp:532
    #11 in AGS3::wouttext_aligned(AGS3::AGS::Shared::Bitmap*, int, int, int, int, int, char const*, AGS3::HorAlignment) display.cpp:547
    #12 in AGS3::_display_main(int, int, int, char const*, int, int, int, int, int, bool, bool) display.cpp:231
    #13 in AGS3::_display_at(int, int, int, char const*, int, int, int, int, bool) display.cpp:371
    #14 in AGS3::_displayspeech(char const*, int, int, int, int, int) character.cpp:2704
    #15 in AGS3::DisplaySpeech(char const*, int) character.cpp:2750
    #16 in AGS3::_DisplaySpeechCore(int, char const*) character.cpp:2259
    #17 in AGS3::Character_Say(AGS3::CharacterInfo*, char const*) character.cpp:702
    #18 in AGS3::Sc_Character_Say(void*, AGS3::RuntimeScriptValue const*, int) character.cpp:3035
    #19 in AGS3::ccInstance::Run(int) cc_instance.cpp:1034
    #20 in AGS3::ccInstance::CallScriptFunction(char const*, int, AGS3::RuntimeScriptValue const*) cc_instance.cpp:367
    #21 in AGS3::RunScriptFunction(AGS3::ccInstance*, char const*, unsigned long, AGS3::RuntimeScriptValue const*) script.cpp:362
    #22 in AGS3::RunScriptFunctionInRoom(char const*, unsigned long, AGS3::RuntimeScriptValue const*) script.cpp:399
    #23 in AGS3::RunScriptFunctionAuto(AGS3::ScriptInstType, char const*, unsigned long, AGS3::RuntimeScriptValue const*) script.cpp:435
    #24 in AGS3::QueueScriptFunction(AGS3::ScriptInstType, char const*, unsigned long, AGS3::RuntimeScriptValue const*) script.cpp:282
    #25 in AGS3::run_interaction_script(AGS3::AGS::Shared::InteractionScripts*, int, int) script.cpp:190
    #26 in AGS3::process_event(AGS3::EventHappened const*) event.cpp:178
    #27 in AGS3::processallevents() event.cpp:364
    #28 in AGS3::game_loop_update_events() game_run.cpp:648
    #29 in AGS3::UpdateGameOnce(bool, AGS3::AGS::Engine::IDriverDependantBitmap*, int, int) game_run.cpp:780

SUMMARY: AddressSanitizer: heap-buffer-overflow gfx.cpp:247 in AGS3::putpixel(AGS3::BITMAP*, int, int, int)

comment:3 by criezy, 2 years ago

This seems to be a regression. If I go back to the code from March 14 (before Dreammaster started to backport a bunch of changes from upstream) , I cannot reproduce the crash after 5 minutes trying.

It would be interesting to try with the current code from upstream AGS to see if the crash is present there as well. However that can only be done on Windows because of the Nickenstien plugin (and I am not even sure the plugin works with current AGS upstream code, so it may not be possible on Windows either).

comment:4 by criezy, 2 years ago

Bisection gives the following result:

7132742e04805b31a5226edacfac45ac92ce011b is the first bad commit
commit 7132742e04805b31a5226edacfac45ac92ce011b
Author: Paul Gilbert <dreammaster@scummvm.org>
Date:   Thu Mar 24 21:10:24 2022 -0700

    AGS: Reintroduce upstream's full alfont implementation

comment:5 by criezy, 2 years ago

Owner: set to criezy
Resolution: fixed
Status: newclosed

Our alfont code is almost identical to the code from upstream AGS and I could not detect a difference that would explain the crash.

It does seem to have an inconsistency between alfont_get_font_real_height() and the max_bmp_y computed in alfont_textout_ex(), causing the allocated surface to be smaller than expected for the draw by a few pixels. There are a number of hack on the computation of the real height, so possible one of those is the culprit. But I don't want to start changing those hacks, so instead I have added a sanity check in putpixel() to make sure we do not attempt to write outside of the surface.

Note: See TracTickets for help on using tickets.