View Issue Details

IDProjectCategoryView StatusLast Update
0002908FSSCPPilot datapublic2013-08-13 09:21
Reporterniffiwan Assigned Toniffiwan  
PriorityurgentSeverityblockReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.7.0 RC2 
Target Version3.7.0 
Summary0002908: fix error handling in pilot conversions
DescriptionSome exceptions thrown in the pilot conversion are not working correctly. See attachment for a log & a test pilot.

Error introduced by r9732 & r9739.
Additional InformationERROR: 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
TagsNo tags attached.

Activities

niffiwan

2013-08-11 08:43

developer  

2901d.zip (8,797 bytes)

niffiwan

2013-08-11 10:09

developer  

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;
 	}
 
mantis2908-svn.patch (4,700 bytes)   

niffiwan

2013-08-11 10:11

developer   ~0015226

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.

Echelon9

2013-08-12 09:48

developer   ~0015230

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.

Echelon9

2013-08-12 09:55

developer   ~0015232

Patch as attached does indeed resolve the AddressSanitizer-report heap use after free.

niffiwan

2013-08-13 09:21

developer   ~0015233

Fix committed to trunk@9747.

Related Changesets

fs2open: trunk r9747

2013-08-13 06:32

niffiwan


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

Issue History

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