Menu
▾
▴
#106 Fix MMapFile::release(void *pView, int size)
I recently stumpled upon a crash on mac os x 10.4 (G4 PPC) where the g_output (defaults to stdout) FILE structure has its internal buffer unmapped. The crash shows up like this in gdb:
#0 0xffff8818 in ___memcpy () at /System/Library/Frameworks/System.framework/PrivateHeaders/ppc/cpu_capabilities.h:189
#1 0x9000ffcc in __sfvwrite ()
#2 0x9000f928 in __vfprintf$LDBL128 ()
#3 0x90068f8c in vfprintf$LDBL128 ()
#4 0x000071f0 in CEXEBuild::SCRIPT_MSG (this=0xbfffd75c, s=0x75248 " bytes\n") at Source/build.cpp:3075
#5 0x00032608 in CEXEBuild::add_file (this=0xbfffd75c, dir=@0xbfff4190, file=@0x60fc00, attrib=0, name_override=0x0, generatecode=1, data_handle=0x0) at Source/script.cpp:6085
#6 0x00033748 in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x60d8ec "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04/lib/zi/America/", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff42a4, dir_created=true) at Source/script.cpp:5908
#7 0x00033b8c in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x610cdc "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04/lib/zi/", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff43a4, dir_created=true) at Source/script.cpp:5951
#8 0x00033b8c in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x610c0c "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04/lib/", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff44a4, dir_created=true) at Source/script.cpp:5951
#9 0x00033b8c in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x61081c "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04/", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff45a4, dir_created=true) at Source/script.cpp:5951
#10 0x00033b8c in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x604510 "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff4a44, dir_created=true) at Source/script.cpp:5951
#11 0x0004331c in CEXEBuild::doCommand (this=0xbfffd75c, which_token=135, line=@0xbfff9120) at Source/script.cpp:4344
#12 0x0004b3b4 in CEXEBuild::doParse (this=0xbfffd75c, str=0x505002 "File /r /x \"*.svn\" \"/Users/elias/svn/games/tt/installer\\windows\\jre1.5.0_04\"") at Source/script.cpp:517
#13 0x0004b88c in CEXEBuild::parseScript (this=0xbfffd75c) at Source/script.cpp:703
#14 0x0004bcac in CEXEBuild::process_script (this=0xbfffd75c, filepointer=0xa000db8c, filename=0xbfffd35c "template.nsi") at Source/script.cpp:219
#15 0x00026f94 in main (argc=3, argv=0xbffff93c) at Source/makenssi.cpp:481
resulting in an odd crash in a vfprintf. This unmapping was caused by this call:
#0 0x90049558 in munmap ()
#1 0x00028198 in MMapFile::release (this=0xbffff6a8, pView=0x3dc9d7, size=2600) at Source/mmap.cpp:337
#2 0x000292cc in MMapBuf::release (this=0xbffff688, pView=0x3dc9d7, size=2600) at Source/mmap.cpp:503
#3 0x00003910 in CEXEBuild::datablock_optimize (this=0xbfffd75c, start_offset=47542194, first_int=77) at Source/build.cpp:623
#4 0x00006d28 in CEXEBuild::add_db_data (this=0xbfffd75c, mmap=0xbfff3bf8) at Source/build.cpp:784
#5 0x00032558 in CEXEBuild::add_file (this=0xbfffd75c, dir=@0xbfff4190, file=@0x60fc00, attrib=0, name_override=0x0, generatecode=1, data_handle=0x0) at Source/script.cpp:6067
#6 0x00033748 in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x60d8ec "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04/lib/zi/America/", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff42a4, dir_created=true) at Source/script.cpp:5908
#7 0x00033b8c in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x610cdc "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04/lib/zi/", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff43a4, dir_created=true) at Source/script.cpp:5951
#8 0x00033b8c in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x610c0c "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04/lib/", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff44a4, dir_created=true) at Source/script.cpp:5951
#9 0x00033b8c in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x61081c "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04/", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff45a4, dir_created=true) at Source/script.cpp:5951
#10 0x00033b8c in CEXEBuild::do_add_file (this=0xbfffd75c, lgss=0x604510 "/Users/elias/svn/games/tt/installer/windows/jre1.5.0_04", attrib=0, recurse=1, total_files=0xbfff4c38, name_override=0x0, generatecode=1, data_handle=0x0, excluded=@0xbfff4c1c, basedir=@0xbfff4a44, dir_created=true) at Source/script.cpp:5951
#11 0x0004331c in CEXEBuild::doCommand (this=0xbfffd75c, which_token=135, line=@0xbfff9120) at Source/script.cpp:4344
#12 0x0004b3b4 in CEXEBuild::doParse (this=0xbfffd75c, str=0x505002 "File /r /x \"*.svn\" \"/Users/elias/svn/games/tt/installer\\windows\\jre1.5.0_04\"") at Source/script.cpp:517
#13 0x0004b88c in CEXEBuild::parseScript (this=0xbfffd75c) at Source/script.cpp:703
#14 0x0004bcac in CEXEBuild::process_script (this=0xbfffd75c, filepointer=0xa000db8c, filename=0xbfffd35c "template.nsi") at Source/script.cpp:219
#15 0x00026f94 in main (argc=3, argv=0xbffff93c) at Source/makenssi.cpp:481
looking at the datablock_optimize function and MMapFile::getmore() and MMapFile::release(void *, int), it seems that the problem is that the void *oldstuff address result from getmore() is not adjusted for page alignment, while the adjusted la size is. This causes a problem in release(void *, int) which simply takes the address and size and munmaps that. Note that this is not a problem on windows, since the size parameter is not used in the windows version of munmap.
To fix this, I noted that getmore() and release(void *, int) was only used in datablock_optimize, so I changed the getmore() parameters so that the adjusted size was not returned, and changed release(void *, int) to adjust both size and the address. This seems to have fixed the crash I experienced.
Note that the patch uses ptrdiff_t and intptr_t instead of ints to be more 64 bit safe. This can be changed if those types don't work on windows.
- elias
patch
Logged In: YES
user_id=584402
Originator: NO
Thanks! Patch applied.
I have used unsigned instead of ptrdiff_t and intptr_t, because they are indeed not supported by VC. It would spit out a warning, but it can be ignored, at least for regular page sizes like 4096, since the part of the number affecting the modulo of 4096 is all in the lower 32-bit. It's just the lowest 12 bits.
A better overall solution that came to me while running over the code again, would be to get MMapFile a copy constructor, or a clone method that'd allow simply using get() and release() with a new instance. Or better yet, a method that returns an object that has just get() and release() and does nothing to the mmap itself, so there won't be any need for a reference count. Let me know if you're up to it. I prefer this solution to the current one, because the current one assumes the mapped address is page aligned while MSDN doesn't directly state that. It'd should also be a lot simpler code-wise.