View Issue Details

IDProjectCategoryView StatusLast Update
0002004FSSCPHUDpublic2012-04-03 13:15
ReporterFUBAR-BDHR Assigned ToEchelon9  
PrioritylowSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.6.11 
Fixed in Version3.6.14 
Summary0002004: Training message buffer overrun can happen when using non forward view
DescriptionBasically the mission uses a repeating training message to display a distance. Works fine as long as you are looking forward. If you look say up the messages will cash and eventually the buffer (holds 40) will overload and assert.

I believe I ran into a similar error with regular messages being used to display variables. Pretty sure that one was fixed.

Training_message_queue_count is 40. Here's the call stack:

     fs2_open_3_6_11d.exe!debug_int3(char * file=0x00ddbcf4, int line=897) Line 760 C++
     fs2_open_3_6_11d.exe!WinAssert(char * text=0x00e1b7c8, char * filename=0x00e1b5f6, int linenum=819) Line 897 + 0x13 bytes C++
> fs2_open_3_6_11d.exe!message_training_queue(char * text=0x069ef8c0, int timestamp=1, int length=-1) Line 819 + 0x21 bytes C++
     fs2_open_3_6_11d.exe!sexp_send_training_message(int node=414) Line 11862 + 0x23 bytes C++
     fs2_open_3_6_11d.exe!eval_sexp(int cur_node=413, int referenced_node=-1) Line 17063 + 0x9 bytes C++
     fs2_open_3_6_11d.exe!eval_when(int n=412, int use_arguments=0) Line 7003 + 0xb bytes C++
     fs2_open_3_6_11d.exe!eval_sexp(int cur_node=408, int referenced_node=-1) Line 16404 + 0x16 bytes C++
     fs2_open_3_6_11d.exe!mission_process_event(int event=13) Line 913 + 0xb bytes C++
     fs2_open_3_6_11d.exe!mission_eval_goals() Line 1062 + 0x9 bytes C++
     fs2_open_3_6_11d.exe!game_simulation_frame() Line 4269 C++
     fs2_open_3_6_11d.exe!game_frame(int paused=0) Line 4686 C++
     fs2_open_3_6_11d.exe!game_do_frame() Line 5114 + 0x7 bytes C++
     fs2_open_3_6_11d.exe!game_do_state(int state=2) Line 6885 C++
     fs2_open_3_6_11d.exe!gameseq_process_events() Line 405 + 0x14 bytes C++
     fs2_open_3_6_11d.exe!game_main(char * cmdline=0x00152326) Line 7446 + 0x5 bytes C++
     fs2_open_3_6_11d.exe!WinMain(HINSTANCE__ * hInst=0x00400000, HINSTANCE__ * hPrev=0x00000000, char * szCmdLine=0x00152326, int nCmdShow=10) Line 7522 + 0x9 bytes C++
     fs2_open_3_6_11d.exe!__tmainCRTStartup() Line 263 + 0x2c bytes C
     fs2_open_3_6_11d.exe!WinMainCRTStartup() Line 182 C
     kernel32.dll!7c817077()
     [Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]
     fs2_open_3_6_11d.exe!game_show_framerate() Line 2451 + 0xe bytes C++
Additional Information3.6.11 r5618. Should be easy to reproduce in FS2. Just to an ever-time with a training message and switch to a different view.
TagsNo tags attached.

Activities

FUBAR-BDHR

2009-10-13 00:51

developer   ~0011211

5426 was the commit where the standard messages were made dynamic to fix the similar problem with those.

2010-01-02 11:17

 

dynamics_training_message_queue2.patch (6,380 bytes)   
Index: code/mission/missiontraining.cpp
===================================================================
--- code/mission/missiontraining.cpp	(revision 5760)
+++ code/mission/missiontraining.cpp	(working copy)
@@ -47,7 +47,6 @@
 #define TRAINING_OBJ_LINES				50		// number of lines to track in objective list
 #define TRAINING_OBJ_DISPLAY_LINES	5		// only display this many lines on screen max
 #define MAX_TRAINING_MESSAGE_MODS			20
-#define TRAINING_MESSAGE_QUEUE_MAX			40
 
 #define TRAINING_OBJ_STATUS_UNKNOWN		(1 << 28)	// directive status is unknown
 #define TRAINING_OBJ_STATUS_KNOWN		(1 << 29)	// directive status is known (satisfied or failed)
@@ -69,7 +68,7 @@
 	int timestamp;
 	int length;
 	char *special_message;
-} training_message_queue;
+} Training_Message;
 
 char Training_buf[TRAINING_MESSAGE_LENGTH];
 char *Training_lines[MAX_TRAINING_MESSAGE_LINES];  // Training message split into lines
@@ -83,10 +82,9 @@
 int Training_voice_handle;
 int Training_flag = 0;
 int Training_failure = 0;
-int Training_message_queue_count = 0;
 int Training_bind_warning = -1;  // Missiontime at which we last gave warning
 int Training_message_visible;
-training_message_queue Training_message_queue[TRAINING_MESSAGE_QUEUE_MAX];
+SCP_vector<Training_Message> Training_Message_Queue;
 
 // coordinates for training messages
 int Training_message_window_coords[GR_NUM_RESOLUTIONS][2] = {
@@ -300,14 +298,10 @@
 
 	Assert(!Training_num_lines);
 	Training_obj_num_lines = 0;
-	Training_message_queue_count = 0;
 	Training_failure = 0;
 	for (i=0; i<TRAINING_OBJ_LINES; i++)
 		Training_obj_lines[i] = -1;
 
-	// Goober5000
-	for (i = 0; i < TRAINING_MESSAGE_QUEUE_MAX; i++)
-		Training_message_queue[i].special_message = NULL;
 
 	if ( !Directive_frames_loaded ) {
 		for ( i = 0; i < NUM_DIRECTIVE_GAUGES; i++ ) {
@@ -552,8 +546,6 @@
 // called to do cleanup when leaving a mission
 void training_mission_shutdown()
 {
-	int i;
-
 	if (Training_voice >= 0) {
 		if (Training_voice_type) {
 			audiostream_close_file(Training_voice_handle, 0);
@@ -563,15 +555,7 @@
 		}
 	}
 
-	// Goober5000
-	for (i = 0; i < TRAINING_MESSAGE_QUEUE_MAX; i++)
-	{
-		if (Training_message_queue[i].special_message != NULL)
-		{
-			vm_free(Training_message_queue[i].special_message);
-			Training_message_queue[i].special_message = NULL;
-		}
-	}
+	Training_Message_Queue.clear();
 
 	Training_voice = -1;
 	Training_num_lines = Training_obj_num_lines = 0;
@@ -762,7 +746,7 @@
 	return Training_voice;
 }
 
-// one time initializations done when we want to display a new training mission.  This does
+// one time initializations done when we want to display a new training message.  This does
 // all the processing and setup required to actually display it, including starting the
 // voice file playing
 void message_training_setup(int m, int length, char *special_message)
@@ -813,60 +797,38 @@
 // add a message to the queue to be sent later.
 void message_training_queue(char *text, int timestamp, int length)
 {
+	Training_Message training_message;
 	int m;
 	char temp_buf[TRAINING_MESSAGE_LENGTH];
 
-	Assert(Training_message_queue_count < TRAINING_MESSAGE_QUEUE_MAX);
-	if (Training_message_queue_count < TRAINING_MESSAGE_QUEUE_MAX) {
-		if (!stricmp(text, NOX("none"))) {
-			m = -1;
-		} else {
-			for (m=0; m<Num_messages; m++)
-				if (!stricmp(text, Messages[m].name))
-					break;
+	if (!stricmp(text, NOX("none"))) {
+		m = -1;
+	} else {
+		for (m=0; m<Num_messages; m++)
+			if (!stricmp(text, Messages[m].name))
+				break;
 
-			Assert(m < Num_messages);
-			if (m >= Num_messages)
-				return;
-		}
+		Assert(m < Num_messages);
+		if (m >= Num_messages)
+			return;
+	}
 
-		Training_message_queue[Training_message_queue_count].num = m;
-		Training_message_queue[Training_message_queue_count].timestamp = timestamp;
-		Training_message_queue[Training_message_queue_count].length = length;
+	training_message.num = m;
+	training_message.timestamp = timestamp;
+	training_message.length = length;
 
-		// Goober5000 - this shouldn't happen, but let's be safe
-		if (Training_message_queue[Training_message_queue_count].special_message != NULL)
-		{
-			Int3();
-			vm_free(Training_message_queue[Training_message_queue_count].special_message);
-			Training_message_queue[Training_message_queue_count].special_message = NULL;
-		}
+	// Goober5000 - replace variables if necessary
+	strcpy_s(temp_buf, Messages[m].message);
+	if (sexp_replace_variable_names_with_values(temp_buf, MESSAGE_LENGTH))
+		training_message.special_message = vm_strdup(temp_buf);
 
-		// Goober5000 - replace variables if necessary
-		strcpy_s(temp_buf, Messages[m].message);
-		if (sexp_replace_variable_names_with_values(temp_buf, MESSAGE_LENGTH))
-			Training_message_queue[Training_message_queue_count].special_message = vm_strdup(temp_buf);
-
-		Training_message_queue_count++;
-	}
+	Training_Message_Queue.push_back(training_message); 
 }
 
-// Goober5000 - removes current message from the queue
+// Echelon9 - removes current message from the queue
 void message_training_remove_from_queue(int idx)
 {	
-	Training_message_queue[idx].length = -1;
-	Training_message_queue[idx].num = -1;
-	Training_message_queue[idx].timestamp = -1;
-
-	if (Training_message_queue[idx].special_message != NULL)
-	{
-		vm_free(Training_message_queue[idx].special_message);
-		Training_message_queue[idx].special_message = NULL;
-	}
-
-	for (int j=idx+1; j<Training_message_queue_count; j++)
-		Training_message_queue[j - 1] = Training_message_queue[j];
-	Training_message_queue_count--;
+	Training_Message_Queue.erase(Training_Message_Queue.begin()+idx);
 }
 
 // check the training message queue to see if we should play a new message yet or not.
@@ -888,9 +850,9 @@
 	if (Training_failure)
 		return;
 
-	for (i=0; i<Training_message_queue_count; i++) {
-		if (timestamp_elapsed(Training_message_queue[i].timestamp)) {
-			message_training_setup(Training_message_queue[i].num, Training_message_queue[i].length, Training_message_queue[i].special_message);
+	for (i = 0; i < (int)Training_Message_Queue.size(); i++) {
+		if (timestamp_elapsed(Training_Message_Queue[i].timestamp)) {
+			message_training_setup(Training_Message_Queue[i].num, Training_Message_Queue[i].length, Training_Message_Queue[i].special_message);
 
 			// remove this message from the queue now.
 			message_training_remove_from_queue(i);

Echelon9

2010-01-02 11:17

developer   ~0011481

Patch provided; peer code review in progress before I commit this to SVN.

Echelon9

2010-01-02 13:33

developer   ~0011482

Committed in revision 5761.

Echelon9

2010-01-17 04:12

developer   ~0011529

Fix will require further testing & modification to be stable enough to commit to trunk

The_E

2012-01-22 13:42

administrator   ~0013111

Is this still current? I thought it was fixed already?

Echelon9

2012-01-22 19:45

developer   ~0013112

The original fix was backed out of trunk due to related problems.
But then recently I've been unable to trigger the bug anymore, which is odd.
I'm leaving open for an abundance of caution

Issue History

Date Modified Username Field Change
2009-10-13 00:26 FUBAR-BDHR New Issue
2009-10-13 00:51 FUBAR-BDHR Note Added: 0011211
2010-01-02 10:45 Echelon9 Status new => assigned
2010-01-02 10:45 Echelon9 Assigned To => Echelon9
2010-01-02 11:17 Echelon9 File Added: dynamics_training_message_queue2.patch
2010-01-02 11:17 Echelon9 Note Added: 0011481
2010-01-02 11:17 Echelon9 Status assigned => feedback
2010-01-02 13:33 Echelon9 Note Added: 0011482
2010-01-02 13:33 Echelon9 Status feedback => resolved
2010-01-02 13:33 Echelon9 Fixed in Version => 3.6.11
2010-01-02 13:33 Echelon9 Resolution open => fixed
2010-01-17 04:12 Echelon9 Note Added: 0011529
2010-01-17 04:12 Echelon9 Status resolved => feedback
2010-01-17 04:12 Echelon9 Resolution fixed => reopened
2010-12-11 04:31 The_E Category gameplay => HUD
2012-01-22 12:21 Echelon9 Priority normal => low
2012-01-22 12:21 Echelon9 Fixed in Version 3.6.11 => 3.6.14
2012-01-22 13:42 The_E Note Added: 0013111
2012-01-22 19:45 Echelon9 Note Added: 0013112
2012-04-03 13:15 Echelon9 Status feedback => resolved
2012-04-03 13:15 Echelon9 Resolution reopened => fixed