Opened 4 years ago

Closed 2 years ago

#7018 closed defect (fixed)

SWORD25: use of uninitialised memory when saving a game

Reported by: criezy Owned by: bgK
Priority: blocker Component: Engine: Sword25
Keywords: has-valgrind-logs Cc:
Game: Broken Sword 2.5

Description

I played a bit BS 2.5 (in English) with valgrind on OS X 10.9 with an up to date ScummVM (4b7d49d). This mostly work well but there are a couple of use of uninitialized memory when saving a game (to be exact loading a game from the menu screen to avoid the pain of crawling with valgrind through the intro and then saving one just after the load - the save game is right at the start of the game outside of Nico's appartment):

Here is the first one:
==33893== Syscall param write(buf) points to uninitialised byte(s)
==33893== at 0x35D8E9A: write$NOCANCEL (in /usr/lib/system/libsystem_kernel.dylib)
==33893== by 0x34F8E1A: __sfvwrite (in /usr/lib/system/libsystem_c.dylib)
==33893== by 0x34F9115: fwrite (in /usr/lib/system/libsystem_c.dylib)
==33893== by 0x101B6C6CB: StdioStream::write(void const, unsigned int) (stdiostream.cpp:72)
==33893== by 0x101B6C714: non-virtual thunk to StdioStream::write(void const
, unsigned int) (stdiostream.cpp:73)
==33893== by 0x10145825E: Sword25::PersistenceService::saveGame(unsigned int, Common::String const&) (persistenceservice.cpp:317)
==33893== by 0x101456418: Sword25::saveGame(lua_State) (kernel_script.cpp:441)
==33893== by 0x10147EC8D: luaD_precall(lua_State
, lua_TValue, int) (ldo.cpp:343)
==33893== by 0x101496F3E: luaV_execute(lua_State
, int) (lvm.cpp:587)
==33893== by 0x10147F491: luaD_call(lua_State, lua_TValue, int) (ldo.cpp:401)
==33893== by 0x10147324E: f_call(lua_State, void) (lapi.cpp:803)
==33893== by 0x10147E2C8: luaD_rawrunprotected(lua_State, void ()(lua_State, void), void) (ldo.cpp:140)
==33893== Address 0x10b02bcf3 is 1,280,243 bytes inside a block of size 2,097,152 alloc'd
==33893== at 0x47E1: malloc (vg_replace_malloc.c:300)
==33893== by 0x101186F79: Common::Array::allocCapacity(unsigned int) (array.h:267)
==33893== by 0x1010F2E99: Common::Array::insert_aux(unsigned char
, unsigned char const, unsigned char const) (array.h:306)
==33893== by 0x1010F21A5: Common::Array::push_back(unsigned char const&) (array.h:90)
==33893== by 0x10145725E: Sword25::OutputPersistenceBlock::writeMarker(unsigned char) (outputpersistenceblock.cpp:96)
==33893== by 0x101457292: Sword25::OutputPersistenceBlock::write(unsigned int) (outputpersistenceblock.cpp:58)
==33893== by 0x101464C1F: Sword25::RegionRegistry::persist(Sword25::OutputPersistenceBlock&) (regionregistry.cpp:47)
==33893== by 0x101457FB1: Sword25::PersistenceService::saveGame(unsigned int, Common::String const&) (persistenceservice.cpp:300)
==33893== by 0x101456418: Sword25::saveGame(lua_State) (kernel_script.cpp:441)
==33893== by 0x10147EC8D: luaD_precall(lua_State
, lua_TValue, int) (ldo.cpp:343)
==33893== by 0x101496F3E: luaV_execute(lua_State
, int) (lvm.cpp:587)
==33893== by 0x10147F491: luaD_call(lua_State, lua_TValue, int) (ldo.cpp:401)
==33893== Uninitialised value was created by a heap allocation
==33893== at 0x47E1: malloc (vg_replace_malloc.c:300)
==33893== by 0x56CC: realloc (vg_replace_malloc.c:696)
==33893== by 0x101475C58: l_alloc(void, void, unsigned long, unsigned long) (lauxlib.cpp:641)
==33893== by 0x1014867EC: luaM_realloc_(lua_State, void, unsigned long, unsigned long) (lmem.cpp:77)
==33893== by 0x10148CE82: stack_init(lua_State, lua_State) (lstate.cpp:42)
==33893== by 0x10148CCD5: luaE_newthread(lua_State) (lstate.cpp:121)
==33893== by 0x101470F0B: lua_newthread(lua_State
) (lapi.cpp:147)
==33893== by 0x101475E84: luaB_cocreate(lua_State) (lbaselib.cpp:568)
==33893== by 0x10147EC8D: luaD_precall(lua_State
, lua_TValue, int) (ldo.cpp:343)
==33893== by 0x101496F3E: luaV_execute(lua_State
, int) (lvm.cpp:587)
==33893== by 0x10147F804: resume(lua_State, void) (ldo.cpp:428)
==33893== by 0x10147E2C8: luaD_rawrunprotected(lua_State, void ()(lua_State, void), void*) (ldo.cpp:140)

And the second one:
==33893== Syscall param write(buf) points to uninitialised byte(s)
==33893== at 0x35D8E9A: write$NOCANCEL (in /usr/lib/system/libsystem_kernel.dylib)
==33893== by 0x34F65FC: sflush (in /usr/lib/system/libsystem_c.dylib)
==33893== by 0x34F8DB8:
sfvwrite (in /usr/lib/system/libsystem_c.dylib)
==33893== by 0x34F9115: fwrite (in /usr/lib/system/libsystem_c.dylib)
==33893== by 0x101B6C6CB: StdioStream::write(void const, unsigned int) (stdiostream.cpp:72)
==33893== by 0x101B6C714: non-virtual thunk to StdioStream::write(void const
, unsigned int) (stdiostream.cpp:73)
==33893== by 0x101458377: Sword25::PersistenceService::saveGame(unsigned int, Common::String const&) (persistenceservice.cpp:327)
==33893== by 0x101456418: Sword25::saveGame(lua_State) (kernel_script.cpp:441)
==33893== by 0x10147EC8D: luaD_precall(lua_State
, lua_TValue, int) (ldo.cpp:343)
==33893== by 0x101496F3E: luaV_execute(lua_State
, int) (lvm.cpp:587)
==33893== by 0x10147F491: luaD_call(lua_State, lua_TValue, int) (ldo.cpp:401)
==33893== by 0x10147324E: f_call(lua_State, void) (lapi.cpp:803)
==33893== Address 0x1047b7634 is 356 bytes inside a block of size 4,096 alloc'd
==33893== at 0x47E1: malloc (vg_replace_malloc.c:300)
==33893== by 0x34F9869: smakebuf (in /usr/lib/system/libsystem_c.dylib)
==33893== by 0x350E22B:
swsetup (in /usr/lib/system/libsystem_c.dylib)
==33893== by 0x34F8B28: sfvwrite (in /usr/lib/system/libsystem_c.dylib)
==33893== by 0x34F9115: fwrite (in /usr/lib/system/libsystem_c.dylib)
==33893== by 0x101B6C6CB: StdioStream::write(void const, unsigned int) (stdiostream.cpp:72)
==33893== by 0x101B6C714: non-virtual thunk to StdioStream::write(void const
, unsigned int) (stdiostream.cpp:73)
==33893== by 0x101D1D52B: Common::WriteStream::writeString(Common::String const&) (stream.cpp:32)
==33893== by 0x101457D27: Sword25::PersistenceService::saveGame(unsigned int, Common::String const&) (persistenceservice.cpp:277)
==33893== by 0x101456418: Sword25::saveGame(lua_State) (kernel_script.cpp:441)
==33893== by 0x10147EC8D: luaD_precall(lua_State
, lua_TValue, int) (ldo.cpp:343)
==33893== by 0x101496F3E: luaV_execute(lua_State
, int) (lvm.cpp:587)
==33893== Uninitialised value was created by a heap allocation
==33893== at 0x47E1: malloc (vg_replace_malloc.c:300)
==33893== by 0x60328D: operator new(unsigned long) (in /usr/lib/libc++.1.dylib)
==33893== by 0x1014554C2: Sword25::Kernel::Kernel() (kernel.cpp:87)
==33893== by 0x1014556C4: Sword25::Kernel::Kernel() (kernel.cpp:100)
==33893== by 0x101423506: Sword25::Kernel::getInstance() (kernel.h:136)
==33893== by 0x101422BE7: Sword25::Sword25Engine::appStart() (sword25.cpp:105)
==33893== by 0x1014229FB: Sword25::Sword25Engine::run() (sword25.cpp:82)
==33893== by 0x10000A2C2: runGame(PluginSubclass const*, OSystem&, Common::String const&) (main.cpp:247)
==33893== by 0x100008B39: scummvm_main (main.cpp:492)
==33893== by 0x100006E34: SDL_main (macosx-main.cpp:45)
==33893== by 0x101D58220: -[SDLMain applicationDidFinishLaunching:] (SDLMain.m:300)
==33893== by 0x1A8FE0B:
CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ (in /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation)

Ticket imported from: bugs/7018.

Change History (3)

comment:1 by csnover, 2 years ago

Priority: normalblocker

Raising all identified crasher, hang, and memory violation bugs which I could not fully triage myself to blocker priority for the next release.

comment:2 by csnover, 2 years ago

Keywords: has-valgrind-logs added

comment:3 by bgK, 2 years ago

Owner: set to bgK
Resolution: fixed
Status: newclosed

Fixed in 443211d9, free sound handles containing uninitialized memory were being saved. Harmless.

Note: See TracTickets for help on using tickets.