Opened 8 years ago

Closed 6 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
Version: 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, 6 years ago

Priority: normal → blocker

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, 6 years ago

Keywords: has-valgrind-logs added

comment:3 by bgK, 6 years ago

Owner: set to bgK
Resolution: → fixed
Status: new → closed

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

Note: See TracTickets for help on using tickets.