View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002908 | FSSCP | Pilot data | public | 2013-08-11 08:42 | 2013-08-13 09:21 |
Reporter | niffiwan | Assigned To | niffiwan | ||
Priority | urgent | Severity | block | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.7.0 RC2 | ||||
Target Version | 3.7.0 | ||||
Summary | 0002908: fix error handling in pilot conversions | ||||
Description | Some exceptions thrown in the pilot conversion are not working correctly. See attachment for a log & a test pilot. Error introduced by r9732 & r9739. | ||||
Additional Information | ERROR: AddressSanitizer: heap-use-after-free on address 0x6070001187a8 at pc 0x1055ce4a3 bp 0x7fff5fbfbc30 sp 0x7fff5fbfbc08 READ of size 41 at 0x6070001187a8 thread T0 #0 0x1055ce4a2 in wrap_strlen (in libclang_rt.asan_osx_dynamic.dylib) + 274 0000001 0x101323588 in std::char_traits<char>::length char_traits.h:258 0000002 0x1007cf203 in std::basic_string<char, std::char_traits<char>, SCP_vm_allocator<char> >::append basic_string.h:837 0000003 0x1007ba120 in std::basic_string<char, std::char_traits<char>, SCP_vm_allocator<char> >::operator+= basic_string.h:782 0000004 0x101dd4d3d in vsprintf parselo.cpp:3700 0000005 0x101b27e2c in outwnd_printf2 outwnd_unix.cpp:167 0000006 0x100069e74 in pilotfile_convert::csg_convert csg_convert.cpp:1162 0000007 0x1000dfa83 in convert_pilot_files pilotfile_convert.cpp:143 0000008 0x1002d0d5d in game_init freespace.cpp:2006 0000009 0x10030f899 in game_main freespace.cpp:6975 0000010 0x100311474 in SDL_main freespace.cpp:7166 #11 0x100003543 in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300 0000012 0x7fff95444ed9 in _CFXNotificationPost (in CoreFoundation) + 2553 0000013 0x7fff8f5c67b5 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63 0000014 0x7fff98e5c52c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291 0000015 0x7fff98e5c265 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215 0000016 0x7fff98e59451 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565 0000017 0x7fff98e5904b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350 0000018 0x7fff8f5e007a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307 0000019 0x7fff8f5dfedc in _NSAppleEventManagerGenericHandler (in Foundation) + 105 0000020 0x7fff94e91077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306 0000021 0x7fff94e90ed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36 0000022 0x7fff94e90d98 in aeProcessAppleEvent (in AE) + 317 0000023 0x7fff93c24708 in AEProcessAppleEvent (in HIToolbox) + 99 0000024 0x7fff98e55835 in _DPSNextEvent (in AppKit) + 1455 0000025 0x7fff98e54df1 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127 0000026 0x7fff98e4c1a2 in -[NSApplication run] (in AppKit) + 516 0000027 0x1000056b2 in CustomApplicationMain SDLMain.m:227 0000028 0x10000511f in main SDLMain.m:377 0000029 0x100002263 in start (in FS2_Open (debug)) + 51 0000030 0x0 in 0x0 0x6070001187a8 is located 24 bytes inside of 65-byte region [0x607000118790,0x6070001187d1) freed by thread T0 here: #0 0x1055d2313 in wrap_free (in libclang_rt.asan_osx_dynamic.dylib) + 99 0000001 0x1025f9db0 in _vm_free stubs.cpp:689 0000002 0x1000074de in operator delete fsmemory.cpp:19 0000003 0x10004ae82 in pilotfile_convert::csg_import_ships_weapons csg_convert.cpp:223 0000004 0x100059a22 in pilotfile_convert::csg_import csg_convert.cpp:649 0000005 0x100069c86 in pilotfile_convert::csg_convert csg_convert.cpp:1160 0000006 0x1000dfa83 in convert_pilot_files pilotfile_convert.cpp:143 0000007 0x1002d0d5d in game_init freespace.cpp:2006 0000008 0x10030f899 in game_main freespace.cpp:6975 0000009 0x100311474 in SDL_main freespace.cpp:7166 0000010 0x100003543 in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300 #11 0x7fff95444ed9 in _CFXNotificationPost (in CoreFoundation) + 2553 0000012 0x7fff8f5c67b5 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63 0000013 0x7fff98e5c52c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291 0000014 0x7fff98e5c265 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215 0000015 0x7fff98e59451 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565 0000016 0x7fff98e5904b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350 0000017 0x7fff8f5e007a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307 0000018 0x7fff8f5dfedc in _NSAppleEventManagerGenericHandler (in Foundation) + 105 0000019 0x7fff94e91077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306 0000020 0x7fff94e90ed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36 0000021 0x7fff94e90d98 in aeProcessAppleEvent (in AE) + 317 0000022 0x7fff93c24708 in AEProcessAppleEvent (in HIToolbox) + 99 0000023 0x7fff98e55835 in _DPSNextEvent (in AppKit) + 1455 0000024 0x7fff98e54df1 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127 0000025 0x7fff98e4c1a2 in -[NSApplication run] (in AppKit) + 516 0000026 0x1000056b2 in CustomApplicationMain SDLMain.m:227 0000027 0x10000511f in main SDLMain.m:377 0000028 0x100002263 in start (in FS2_Open (debug)) + 51 0000029 0x0 in 0x0 previously allocated by thread T0 here: #0 0x1055d2260 in wrap_malloc (in libclang_rt.asan_osx_dynamic.dylib) + 96 0000001 0x1025f7e58 in _vm_malloc stubs.cpp:571 0000002 0x10000708a in operator new fsmemory.cpp:8 0000003 0x7fff8f91543d in std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in libstdc++..dylib) + 103 0000004 0x7fff8f8eb8e5 in char* std::string::_S_construct<char*>(char*, char*, std::allocator<char> const&, std::forward_iterator_tag) (in libstdc++..dylib) + 57 0000005 0x7fff8f91094d in std::basic_stringbuf<char, std::char_traits<char>, std::allocator<char> >::str() const (in libstdc++..dylib) + 69 0000006 0x7fff8f911db6 in std::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >::str() const (in libstdc++..dylib) + 20 0000007 0x10004aa6e in pilotfile_convert::csg_import_ships_weapons csg_convert.cpp:166 0000008 0x100059a22 in pilotfile_convert::csg_import csg_convert.cpp:649 0000009 0x100069c86 in pilotfile_convert::csg_convert csg_convert.cpp:1160 0000010 0x1000dfa83 in convert_pilot_files pilotfile_convert.cpp:143 #11 0x1002d0d5d in game_init freespace.cpp:2006 0000012 0x10030f899 in game_main freespace.cpp:6975 0000013 0x100311474 in SDL_main freespace.cpp:7166 0000014 0x100003543 in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300 0000015 0x7fff95444ed9 in _CFXNotificationPost (in CoreFoundation) + 2553 0000016 0x7fff8f5c67b5 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63 0000017 0x7fff98e5c52c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291 0000018 0x7fff98e5c265 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215 0000019 0x7fff98e59451 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565 0000020 0x7fff98e5904b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350 0000021 0x7fff8f5e007a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307 0000022 0x7fff8f5dfedc in _NSAppleEventManagerGenericHandler (in Foundation) + 105 0000023 0x7fff94e91077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306 0000024 0x7fff94e90ed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36 0000025 0x7fff94e90d98 in aeProcessAppleEvent (in AE) + 317 0000026 0x7fff93c24708 in AEProcessAppleEvent (in HIToolbox) + 99 0000027 0x7fff98e55835 in _DPSNextEvent (in AppKit) + 1455 0000028 0x7fff98e54df1 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127 0000029 0x7fff98e4c1a2 in -[NSApplication run] (in AppKit) + 516 Shadow bytes around the buggy address: 0x1c0e000230a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x1c0e000230b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x1c0e000230c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x1c0e000230d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x1c0e000230e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x1c0e000230f0: fa fa fd fd fd[fd]fd fd fd fd fd fa fa fa fa fa 0x1c0e00023100: fd fd fd fd fd fd fd fd fd fd fa fa fa fa fd fd 0x1c0e00023110: fd fd fd fd fd fd fd fd fa fa fa fa 00 00 00 00 0x1c0e00023120: 00 00 00 00 00 00 fa fa fa fa fd fd fd fd fd fd 0x1c0e00023130: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fd 0x1c0e00023140: fd fa fa fa fa fa fd fd fd fd fd fd fd fd fd fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 ASan internal: fe ==45138==ABORTING | ||||
Tags | No tags attached. | ||||
|
|
|
mantis2908-svn.patch (4,700 bytes)
Index: code/pilotfile/csg_convert.cpp =================================================================== --- code/pilotfile/csg_convert.cpp (revision 9746) +++ code/pilotfile/csg_convert.cpp (working copy) @@ -162,8 +162,8 @@ if (ilist.index < 0) { std::ostringstream error_msg; - error_msg << "Data mismatch (ship lookup)! '" << ilist.name << "'"; - throw error_msg.str().c_str(); + error_msg << "Data mismatch (ship lookup: " << ilist.name << ")!"; + throw std::runtime_error(error_msg.str().c_str()); } csg->ship_list.push_back( ilist ); @@ -180,8 +180,8 @@ if (ilist.index < 0) { std::ostringstream error_msg; - error_msg << "Data mismatch (weapon lookup)! '" << ilist.name << "'"; - throw error_msg.str().c_str(); + error_msg << "Data mismatch (weapon lookup: " << ilist.name << ")!"; + throw std::runtime_error(error_msg.str().c_str()); } csg->weapon_list.push_back( ilist ); @@ -197,8 +197,8 @@ if (csg->last_ship_flown_index < 0) { std::ostringstream error_msg; - error_msg << "Data mismatch (player ship)! '" << csg->last_ship_flown_index << "'"; - throw error_msg.str().c_str(); + error_msg << "Data mismatch (player ship: " << csg->last_ship_flown_index << ")!"; + throw std::runtime_error(error_msg.str().c_str()); } // create list of medals (since it's missing from the old files) @@ -415,7 +415,7 @@ ras.ship_class = cfread_int(cfp); if (ras.ship_class >= ship_list_size) { - throw "Data failure (RedAlert-ship)!"; + throw std::runtime_error("Data failure (RedAlert-ship)!"); } // system status @@ -440,7 +440,7 @@ i = cfread_int(cfp); if (i >= weapon_list_size || i < 0) { - throw "Data check failure (RedAlert-weapon)!"; + throw std::runtime_error("Data check failure (RedAlert-weapon)!"); } weapons.index = csg->weapon_list[i].index; @@ -457,7 +457,7 @@ i = cfread_int(cfp); if (i >= weapon_list_size) { - throw "Data check failure (RedAlert-weapon)!"; + throw std::runtime_error("Data check failure (RedAlert-weapon)!"); } weapons.index = csg->weapon_list[i].index; @@ -493,7 +493,7 @@ in = cfread_ubyte(cfp); if (in > 1) { - throw "Data check failure (techroom-ship)!"; + throw std::runtime_error("Data check failure (techroom-ship)!"); } visible = (in == 1) ? true : false; @@ -508,7 +508,7 @@ in = cfread_ubyte(cfp); if (in > 1) { - throw "Data check failure (techroom-weapon)!"; + throw std::runtime_error("Data check failure (techroom-weapon)!"); } visible = (in == 1) ? true : false; @@ -523,7 +523,7 @@ in = cfread_ubyte(cfp); if (in > 1) { - throw "Data check failure (techroom-intel)!"; + throw std::runtime_error("Data check failure (techroom-intel)!"); } visible = (in == 1) ? true : false; @@ -597,7 +597,7 @@ // NOTE: could be less, but never greater than if ( list_size > (int)csg->stats.ship_kills.size() ) { - throw "Data check failure (kills size)!"; + throw std::runtime_error("Data check failure (kills size)!"); } for (idx = 0; idx < list_size; idx++) { @@ -626,13 +626,13 @@ unsigned int csg_id = cfread_uint(cfp); if (csg_id != 0xbeefcafe) { - throw "Invalid file signature!"; + throw std::runtime_error("Invalid file signature!"); } fver = cfread_uint(cfp); if (fver != 15) { - throw "Unsupported file version!"; + throw std::runtime_error("Unsupported file version!"); } // campaign type (single/multi) @@ -664,15 +664,15 @@ // final data checks if ( csg->ship_list.size() != csg->ships_allowed.size() ) { - throw "Data check failure (ship size)!"; + throw std::runtime_error("Data check failure (ship size)!"); } else if ( csg->ship_list.size() != csg->ships_techroom.size() ) { - throw "Data check failure (ship size)!"; + throw std::runtime_error("Data check failure (ship size)!"); } else if ( csg->weapon_list.size() != csg->weapons_allowed.size() ) { - throw "Data check failure (weapon size)!"; + throw std::runtime_error("Data check failure (weapon size)!"); } else if ( csg->weapon_list.size() != csg->weapons_techroom.size() ) { - throw "Data check failure (weapon size)!"; + throw std::runtime_error("Data check failure (weapon size)!"); } else if ( csg->intel_list.size() != csg->intel_techroom.size() ) { - throw "Data check failure (intel size)!"; + throw std::runtime_error("Data check failure (intel size)!"); } @@ -1158,8 +1158,8 @@ try { csg_import(inferno); - } catch (const char *err) { - mprintf((" CS2 => Import ERROR: %s\n", err)); + } catch (const std::exception& err) { + mprintf((" CS2 => Import ERROR: %s\n", err.what())); rval = false; } |
|
This *should be* sorted now! (It was far more trouble than it should have been). Work for 2 busted pilots that I've tested with, more testing would be super. |
|
Have added the AddressSanitizer error report I received when starting up FS2Open before the player selection screen appears when: - Using revision 9746, and - Having the Default player file in the attached zip included in my paths. Will test the proposed patch shortly. |
|
Patch as attached does indeed resolve the AddressSanitizer-report heap use after free. |
|
Fix committed to trunk@9747. |
fs2open: trunk r9747 2013-08-13 06:32 Ported: N/A Details Diff |
Fix mantis 2908: finally get "richer" pilot conversion exceptions data working correctly THANK YOU Tony D @stackoverflow! http://stackoverflow.com/questions/6248404/c-exceptions-is-throwing-c-string-as-an-exception-bad |
Affected Issues 0002908 |
|
mod - /trunk/fs2_open/code/pilotfile/csg_convert.cpp | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2013-08-11 08:42 | niffiwan | New Issue | |
2013-08-11 08:42 | niffiwan | Status | new => assigned |
2013-08-11 08:42 | niffiwan | Assigned To | => niffiwan |
2013-08-11 08:43 | niffiwan | File Added: 2901d.zip | |
2013-08-11 10:09 | niffiwan | File Added: mantis2908-svn.patch | |
2013-08-11 10:11 | niffiwan | Note Added: 0015226 | |
2013-08-11 10:11 | niffiwan | Status | assigned => code review |
2013-08-11 10:54 | niffiwan | Description Updated | |
2013-08-12 09:46 | Echelon9 | Priority | normal => urgent |
2013-08-12 09:46 | Echelon9 | Severity | minor => block |
2013-08-12 09:46 | Echelon9 | Target Version | => 3.7.0 |
2013-08-12 09:46 | Echelon9 | Additional Information Updated | |
2013-08-12 09:48 | Echelon9 | Note Added: 0015230 | |
2013-08-12 09:55 | Echelon9 | Note Added: 0015232 | |
2013-08-13 09:21 | niffiwan | Changeset attached | => fs2open trunk r9747 |
2013-08-13 09:21 | niffiwan | Note Added: 0015233 | |
2013-08-13 09:21 | niffiwan | Status | code review => resolved |
2013-08-13 09:21 | niffiwan | Resolution | open => fixed |