I found two disturbing issues in the magnetsElectric.c sample in chipmunk 5.3.3:
1) minor: There is an obvious memory leak there which does not matter much until one tries to heavily benchmark the thing with a zillion of successive runs without restarting the whole application. All the allocations for the Singularities are not freed ever.
2) MAJOR: I dont know why the heck this works at all. This code heavily uses uninitialized float values for the angle arrays of the Singularity struct.
see line 228 where the property angle is used. These are not initilized correctly in both the methods "make_charged"(line 331) and "make_mix"(line 372). This was so hard to track down cause it led to very strange and hardly reproducable errors (endless loops in the hashFunctions cause the bounding boxes suddenly span the whole float value range). Anyway the really strange thing is that i just encountered that in the D version of the code which is an exact port of the original C version. Can anybody explain why the C version doesn't care about this ?
Memory corruption in Magnets Sample
-
- Posts: 5
- Joined: Sun Nov 28, 2010 12:47 pm
- Contact:
- slembcke
- Site Admin
- Posts: 4166
- Joined: Tue Aug 14, 2007 7:13 pm
- Contact:
Re: Memory corruption in Magnets Sample
I actually didn't write this demo. It was contributed by somebody else and I thought it was cool enough to include it. I've never actually noticed any memory leaks in it though.
Can't sleep... Chipmunks will eat me...
Check out our latest projects! -> http://howlingmoonsoftware.com/wordpress/
Check out our latest projects! -> http://howlingmoonsoftware.com/wordpress/
-
- Posts: 5
- Joined: Sun Nov 28, 2010 12:47 pm
- Contact:
Re: Memory corruption in Magnets Sample
cpmallocs that are never freed:slembcke wrote:I've never actually noticed any memory leaks in it though.
-
- Posts: 5
- Joined: Sun Nov 28, 2010 12:47 pm
- Contact:
Re: Memory corruption in Magnets Sample
I investigated this a bit further:Extrawurst wrote: 2) MAJOR: I dont know why the heck this works at all. This code heavily uses uninitialized float values for the angle arrays of the Singularity struct.
see line 228 where the property angle is used. These are not initilized correctly in both the methods "make_charged"(line 331) and "make_mix"(line 372). This was so hard to track down cause it led to very strange and hardly reproducable errors (endless loops in the hashFunctions cause the bounding boxes suddenly span the whole float value range). Anyway the really strange thing is that i just encountered that in the D version of the code which is an exact port of the original C version. Can anybody explain why the C version doesn't care about this ?
As it seems C does not care about the FPU flags and that way it can easily use uninitilized floats like it is done in this sample. The values of them are simply crap until they are used in a sin/cos function. These seem to be able to return a sane result for every freakin input value. The point is that this is all good for C but colossally poops itself when it is used done in languages or compilers where FPU exception flags do matter. For the Magnets demo itself i must say i don't care at all but my question is: Does chipmunk relies on this behaviour in the lib itself at some place ?
- slembcke
- Site Admin
- Posts: 4166
- Joined: Tue Aug 14, 2007 7:13 pm
- Contact:
Re: Memory corruption in Magnets Sample
Personally I rarely ever declare a variable without an initializer. It's just to easy to introduce bugs that way and I burned myself many times as a novice C programmer by trying to be clever about initializations. 95% of the time the bugs aren't apparent because you meant to initialize a value to something near 0, and there is 50/50 chance that you will get a very small garbage value from the stack.
Again, I've never actually looked closely at how that demo works. I just included it because it was neat. I guess I should actually clean it up some if I'm going to keep distributing it though.
Again, I've never actually looked closely at how that demo works. I just included it because it was neat. I guess I should actually clean it up some if I'm going to keep distributing it though.
Can't sleep... Chipmunks will eat me...
Check out our latest projects! -> http://howlingmoonsoftware.com/wordpress/
Check out our latest projects! -> http://howlingmoonsoftware.com/wordpress/
Who is online
Users browsing this forum: No registered users and 8 guests