View Issue Details

IDProjectCategoryView StatusLast Update
0000415FSSCPgameplaypublic2005-05-18 12:58
ReporterGoober5000 Assigned Totaylor  
PriorityurgentSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Summary0000415: CTD upon exit
DescriptionIn the most recent CVS, the game CTDs whenever it exits. I think Phreak said this had something to do with shoddy memory management. Needs fixing ASAP.
TagsNo tags attached.

Activities

phreak

2005-05-12 13:16

developer   ~0002367

it has to do with the dynamically allocated medals stuff WMC implemented. passing the buck.

taylor

2005-05-12 15:52

administrator   ~0002370

Last edited: 2005-05-12 22:28

Specifically this is a problem with the promotion_text getting free'd. Happens every time for me with the "Double Ace" text specifically. Only the Triple Ace promotion text is actually correct, the other two are either trash or get overwritten by something else.

EDIT: Here is a backtrace from the Linux version...

[New Thread 1084229984 (LWP 19213)]
[New Thread 1094719840 (LWP 19214)]
[Thread 1084229984 (zombie) exited]
[Thread 1094719840 (LWP 19214) exited]
*** glibc detected *** double free or corruption (out): 0x0000000003116650 ***

Program received signal SIGABRT, Aborted.
[Switching to Thread 46912498041280 (LWP 19212)]
0x0000003e5522e37d in *__GI_raise (sig=19212) at ../nptl/sysdeps/unix/sysv/linux/raise.c:67
67 int res = INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
(gdb) bt
#0 0x0000003e5522e37d in *__GI_raise (sig=19212) at ../nptl/sysdeps/unix/sysv/linux/raise.c:67
0000001 0x0000003e5522faae in *__GI_abort () at ../sysdeps/generic/abort.c:88
0000002 0x0000003e55262a01 in __libc_message (do_abort=2, fmt=0x3e55310830 "*** glibc detected *** %s: 0x%s ***\n")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:145
0000003 0x0000003e552685ce in _int_free (av=0x3e5542f620, mem=0x4b0c) at malloc.c:5523
0000004 0x0000003e55268916 in __libc_free (mem=0x6) at malloc.c:3404
0000005 0x00000000006d8459 in _vm_free (ptr=0x3116650, filename=0x76cd63 "./stats/medals.h", line=110)
    at windows_stub/stubs.cpp:589
0000006 0x00000000006951aa in ~medal_stuff (this=0x311f418) at medals.h:110
0000007 0x0000000000695c33 in std::_Destroy<medal_stuff> (__pointer=0x311f418) at stl_construct.h:107
0000008 0x0000000000695fb5 in std::__destroy_aux<medal_stuff*> (__first=0x311f418, __last=0x311f580)
    at stl_construct.h:120
0000009 0x0000000000695f8f in std::_Destroy<medal_stuff*> (__first=0x311ed10, __last=0x311f580)
    at stl_construct.h:152
0000010 0x0000000000695e99 in ~vector (this=0x2db7090) at stl_vector.h:256
#11 0x0000000000695164 in __tcf_0 () at stats/medals.cpp:218
0000012 0x0000003e55230cd5 in *__GI_exit (status=0) at exit.c:60
0000013 0x0000003e5521c4c2 in __libc_start_main (main=0x413eaa <main>, argc=4, ubp_av=0x7ffffffff808,
    init=0x710640 <__libc_csu_init>, fini=0, rtld_fini=0x4b0c, stack_end=0x7ffffffff7f8)
    at ../sysdeps/generic/libc-start.c:240
0000014 0x000000000040801a in _start ()
0000015 0x00007ffffffff7f8 in ?? ()
0000016 0x000000000000001c in ?? ()
0000017 0x0000000000000004 in ?? ()
0000018 0x00007ffffffffa50 in ?? ()
0000019 0x00007ffffffffa77 in ?? ()
0000020 0x00007ffffffffa7c in ?? ()
0000021 0x00007ffffffffa80 in ?? ()
0000022 0x0000000000000000 in ?? ()

edited on: 05-12-05 18:28

WMCoolmon

2005-05-13 06:17

developer   ~0002375

Last edited: 2005-05-13 06:22

Yeah I was going to guess it was something like that.

Probably one of the pointers isn't getting set to NULL.

Edit: Er, ISN'T

edited on: 05-13-05 02:22

phreak

2005-05-13 19:44

developer   ~0002387

Last edited: 2005-05-13 19:47

here's the data:

- this 0x0325c868
+ name 0x0325c868 "Ace"
+ bitmap 0x0325c888 "Medal14.pcx"
    num_versions 1
    kills_needed 60
    i 0
+ voice_base 0x0325c8b4 "badge_a.wav"
+ promotion_text 0x084ed760 "îþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþ"


This may have something to do with the vector resizing itself. If i declare the vector with an inital size of 20, then the crash doesn't occur.

edited on: 05-13-05 15:47

2005-05-14 12:08

 

fs2-medals.diff (2,091 bytes)   
--- code/stats/medals.orig	Sat May 14 12:41:06 2005
+++ code/stats/medals.cpp	Sat May 14 14:57:33 2005
@@ -375,13 +375,11 @@
 	reset_parse();
 
 	// parse in all the rank names
-	medal_stuff temp_medal;
 	Num_medals = 0;
 	required_string("#Medals");
 	while ( required_string_either("#End", "$Name:") )
 	{
-		//Clear this
-		temp_medal = medal_stuff();
+		medal_stuff temp_medal;
 
 		required_string("$Name:");
 		stuff_string( temp_medal.name, F_NAME, NULL );
@@ -819,3 +817,24 @@
 	gr_bitmap(Medal_coords[gr_screen.res][RANK_MEDAL_REGION][0], Medal_coords[gr_screen.res][RANK_MEDAL_REGION][1]);
 }
 
+void medal_stuff::clone(const medal_stuff &m) {
+	memcpy(name, m.name, NAME_LENGTH);
+	memcpy(bitmap, m.bitmap, NAME_LENGTH);
+        num_versions = m.num_versions;
+	kills_needed = m.kills_needed;
+	memcpy(voice_base, m.voice_base, MAX_FILENAME_LEN);
+	if (m.promotion_text)
+		promotion_text = vm_strdup(m.promotion_text);
+	else
+		promotion_text = NULL;
+}
+
+// assignment operator
+const medal_stuff &medal_stuff::operator=(const medal_stuff &m) {
+	if (this != &m) {
+		if (promotion_text)
+			vm_free(promotion_text);
+		clone(m);
+	}
+	return *this;
+}
--- code/stats/medals.horig	Sat May 14 14:05:49 2005
+++ code/stats/medals.h	Sat May 14 14:57:36 2005
@@ -100,14 +100,18 @@
 	char	bitmap[NAME_LENGTH];
 	int	num_versions;
 	int	kills_needed;
-	int i;
 
 	//If this is a badge (kills_needed > 1)
 	char voice_base[MAX_FILENAME_LEN];
 	char *promotion_text;
 
-	medal_stuff(){name[0]='\0';bitmap[0]='\0';num_versions=1;kills_needed=0;i=0;voice_base[0]='\0';promotion_text=NULL;}
-	~medal_stuff(){if(promotion_text != NULL){mprintf(("Freeing promo: %s\n",promotion_text));vm_free(promotion_text);}}
+	medal_stuff(){name[0]='\0';bitmap[0]='\0';num_versions=1;kills_needed=0;voice_base[0]='\0';promotion_text=NULL;}
+	medal_stuff(const medal_stuff &m) {clone(m);}
+	~medal_stuff(){if(promotion_text)vm_free(promotion_text);}
+	const medal_stuff &operator=(const medal_stuff &m);
+
+private:
+	void clone(const medal_stuff &m);
 } medal_stuff;
 /*
 typedef struct badge_stuff {
fs2-medals.diff (2,091 bytes)   

fizz

2005-05-14 12:09

reporter   ~0002396

Here's a fix.

Basically, you're using the destructor in a way that's incompatible with vector unless you also implement a copy constructor and an assignment operator.

phreak

2005-05-14 15:09

developer   ~0002398

thats the problem, fixed

taylor

2005-05-15 06:06

administrator   ~0002408

It's crashing on startup now for me. Same basic problem though. Fizz's patch works fine for me but what got into CVS does not.

fizz

2005-05-15 10:11

reporter   ~0002412

The code that got into CVS is broken in several ways. The copy constructor implementation must at least set promotion_text to NULL before assignment. The assignment operator is broken as well (though only for the a = a case), and the "badfood" stuff really doesn't belong in production code...

Also, IMHO, the scoping of temp_medal in my patch is much cleaner than the original clean/copy, clean/copy, clean/copy. I'm probably biased, though... And what's that "i" member for? It's not used anywhere, and poorly named anyway.

phreak

2005-05-15 15:02

developer   ~0002414

i think the 0xbaadf00d thing is what MSVC sets everything to when a vector expands. The program will want to free it and crash

- this 0x0325d1a8
+ name 0x0325d1a8 "Epsilon Pegasi Liberation"
+ bitmap 0x0325d1c8 "Medal00.pcx"
    num_versions 4
    kills_needed 0
+ voice_base 0x0325d1f0 ""
+ promotion_text 0xbaadf00d ""

fizz

2005-05-15 15:16

reporter   ~0002415

That's very certainly because you don't NULL the promotion_text in the second constructor.

phreak

2005-05-16 02:15

developer   ~0002418

makes sense. that damn java class ruined my C++ class knowledge.

taylor

2005-05-18 01:02

administrator   ~0002422

The "i" was some debugging stuff from me that slipped in with my vm_* commit. Doesn't serve any purpose.

So, are you going to fix it phreak or should I just commit fizz's patch instead? I've been waiting since I thought you might do it but it sucks to leave it broken. I'll get the working patch in later tonight if you don't get there first.

phreak

2005-05-18 03:48

developer   ~0002426

oh sorry forgot about it. commit what you have

taylor

2005-05-18 12:58

administrator   ~0002429

Ok, that should be it for this thing then. I can't get it to crash on start or exit.

Fixered.

Issue History

Date Modified Username Field Change
2005-05-12 09:14 Goober5000 New Issue
2005-05-12 13:16 phreak Note Added: 0002367
2005-05-12 13:16 phreak Assigned To phreak => WMCoolmon
2005-05-12 15:52 taylor Note Added: 0002370
2005-05-12 22:28 taylor Note Edited: 0002370
2005-05-13 06:17 WMCoolmon Note Added: 0002375
2005-05-13 06:22 WMCoolmon Note Edited: 0002375
2005-05-13 19:44 phreak Note Added: 0002387
2005-05-13 19:47 phreak Note Edited: 0002387
2005-05-14 12:08 fizz File Added: fs2-medals.diff
2005-05-14 12:09 fizz Note Added: 0002396
2005-05-14 15:09 phreak Note Added: 0002398
2005-05-14 15:09 phreak Status assigned => resolved
2005-05-14 15:09 phreak Resolution open => fixed
2005-05-14 15:09 phreak Assigned To WMCoolmon => phreak
2005-05-15 06:06 taylor Status resolved => feedback
2005-05-15 06:06 taylor Resolution fixed => reopened
2005-05-15 06:06 taylor Note Added: 0002408
2005-05-15 10:11 fizz Note Added: 0002412
2005-05-15 15:02 phreak Note Added: 0002414
2005-05-15 15:16 fizz Note Added: 0002415
2005-05-16 02:15 phreak Note Added: 0002418
2005-05-18 01:02 taylor Note Added: 0002422
2005-05-18 03:36 taylor Status feedback => assigned
2005-05-18 03:36 taylor Assigned To phreak => taylor
2005-05-18 03:48 phreak Note Added: 0002426
2005-05-18 12:58 taylor Status assigned => resolved
2005-05-18 12:58 taylor Resolution reopened => fixed
2005-05-18 12:58 taylor Note Added: 0002429