Printable View
I am writing a game, and so far it consists of 1,474 lines. Aprx every 5 times I run it, it will "generate errors, and need to be closed by windows" at random times.
And there is little to go by, as they happen so random, I havn't a clue what is causing the error.
How do I find what's doing this, in so many lines? (I'm just thinking too, the game is nowhere even close to complete, so I should reach somthing like 7-9,000 lines. What would I do if somthing like this happens then!?!?)
Please help.
Add asserts to check for invalid program states.
Disable complex stuff that can cause random errors, ie threads.
Check common mistakes such as allocating memory before use.
Do logging of some kind.
Comment out code until it starts working properly.
The good part is, I'm almost positive that the problem is in a selected 800 lines, the rest is 'template' code that I made which has been working for me just fine in other programs.
Asserts?
That's all done in my 'template' code, so I don't think thats the problem. I must have more loops than a road map in my 800 lines, I thought it might have been an error with them, but when it is my programs freeze up and toture the cpu, not crash.
Logging like how?
That's why this is so confusing, all of the code works, and is in use while running the game. What's happening is code that was working suddenly stops working... somewhere.
You don't know asserts?
Basically assert(condition). If condition evaluates as false, in debug mode, it will break, so you can check where the heck things went wrong.
With debugging, I mean log all kinds of useful information such as what functions it enters, what values of arguments and information is, etc. Anything that might help you track down the problem.
you can compile you relase version with debug info, so if crash occures you can open debugger and exemine your variables. Of course - if you destroyed the stack with some buffer overrun - you will not see much... But still you will know that this is a problem... And if your just usign some not initialized var - you'll see it immediatly...
Of course, JIT may also help. With Visual Studio, whenever something crashes, you can debug. Then you can probably find out more about why the program crashes.
What does the volume of code have to do with the difficulty of finding bugs?
Absolutely nothing. ;-) It is easier if you catch the bug sooner, so you have an idea of what code is breaking. However, that's not always possible.Quote:
Originally Posted by brewbuck
If your program is segfaulting, a decent debugger should be able to tell you where. It should also give you a backtrace (what was passed to what functions), what the values in variables are, etc. Look for something out of the ordinary, something wrong, a pointer to garbage, etc. Then figure out why that number is wrong, and fix the appropriate code. (Sometimes the hard step.)
When I find my programs crash at random points I make an output file and at strategic positions (like start of functions, end of loops etc), I write something unique to file which will show where the code is at and some variables. When the program crashes the last marker will tell me roughly where the bug is. That's when I didn't use the debugger, mind.
That's a typical logging scheme. It's great when shipping products to end users when it doesn't work for them as it should and the bug isn't obvious.
Huh, I really like the log file idea. I'll try that, and then come back.
Also, how do you operate dev's debugger anyway? I've never used it before.
Random errors are quite annoying. They are often caused by bad management of array limits or using pointers after they have been freed, so checking such things using for example asserts is a good idea. A rarer case is "huge amount of recursion" - again, any recursion function that isn't explicitly bound should make sure it's counting the number of recursion levels and if it exceeds "double the maximum expected", you should cause an error in your application (e.g assert).
Also, if the application crashes, using the built-in debugger should work just fine - it will show you the stack as per where the application was when it crashed.
There is something called "defensive programming", which is based on the approach that "if something can go wrong, it will", and checking that things are right whenever there is a chance that it may be wrong.
Oh, and 1500 lines of code isn't a lot. I work on a project where our group is responsible for 2500+ files [that's just .cpp, .h and .c files, not counting "makefiles" and other files that are also part of the whole project]. One sub-project that I've been involved with consists of some 5-6 .cpp files and is 12000+ lines. Another sub-project has about 10 .cpp files and around 25000 lines each for two possible solutions for the same problem (choosen at build-time which of those two gets used)
.
--
Mats
If you use a log file, make sure you are flushing after each output operation. Since I/O is buffered, you may write something to a file in a function and when your program crashes, the output you do see may not match where the program really was at the time of the crash.
>> Random errors are quite annoying.
You can say that again.
Maybe I don't have alot of code, but for just me and in one cpp & h file it kinda is.
What is the code for that your group is responsible for?
This morning I made the file logging, it went fairly quick, but now when it's activated, my game drops down to 2-1 fps. So now the error doesn't even happen when I run it. (like I said, it only happens some of the time.)
Here is one frame from my log file. (working)
Code:// Do the stuff
ThrustShip(0, TRUE)
ai(1)
aiFindClosest(1, -1, 3)
FireWeapon(1, 2, TRUE, TRUE)
FireWeapon(1, 2, TRUE, FALSE)
FireWeapon(1, 2, FALSE, TRUE)
FireWeapon(1, 2, FALSE, FALSE)
ai(2)
aiFindClosest(2, -1, 3)
ThrustShip(2, TRUE)
FireWeapon(2, 3, TRUE, TRUE)
FireWeapon(2, 3, TRUE, FALSE)
FireWeapon(2, 3, FALSE, TRUE)
FireWeapon(2, 3, FALSE, FALSE)
ai(3)
aiFindClosest(3, -1, 3)
FireWeapon(3, 1, TRUE, TRUE)
FireWeapon(3, 1, TRUE, FALSE)
FireWeapon(3, 1, FALSE, TRUE)
FireWeapon(3, 1, FALSE, FALSE)
ai(4)
aiFindClosest(4, -1, 3)
ThrustShip(4, TRUE)
FireWeapon(4, 6, TRUE, TRUE)
FireWeapon(4, 6, TRUE, FALSE)
FireWeapon(4, 6, FALSE, TRUE)
FireWeapon(4, 6, FALSE, FALSE)
ai(5)
aiFindClosest(5, -1, 3)
ThrustShip(5, TRUE)
FireWeapon(5, 4, TRUE, TRUE)
FireWeapon(5, 4, TRUE, FALSE)
FireWeapon(5, 4, FALSE, TRUE)
FireWeapon(5, 4, FALSE, FALSE)
ai(6)
aiFindClosest(6, -1, 3)
ThrustShip(6, TRUE)
FireWeapon(6, 1, TRUE, TRUE)
FireWeapon(6, 1, TRUE, FALSE)
FireWeapon(6, 1, FALSE, TRUE)
FireWeapon(6, 1, FALSE, FALSE)
ai(7)
aiFindClosest(7, -1, 3)
ThrustShip(7, TRUE)
FireWeapon(7, 4, TRUE, TRUE)
FireWeapon(7, 4, TRUE, FALSE)
FireWeapon(7, 4, FALSE, TRUE)
FireWeapon(7, 4, FALSE, FALSE)
TorpedoImpact(0)
TorpedoImpact(1)
TorpedoImpact(2)
TorpedoImpact(3)
TorpedoImpact(4)
TorpedoImpact(5)
TorpedoImpact(6)
TorpedoImpact(7)
TorpedoImpact(8)
TorpedoImpact(9)
TorpedoImpact(10)
TorpedoImpact(11)
TorpedoImpact(12)
TorpedoImpact(13)
TorpedoImpact(14)
TorpedoImpact(15)
TorpedoImpact(16)
TorpedoImpact(17)
TorpedoImpact(18)
HurtShip(6, 5, 7)
DamageRandomSystem(6)
DestroyTorpedo(18)
TorpedoImpact(18)
TorpedoImpact(19)
TorpedoImpact(20)
TorpedoImpact(21)
// Draw the stuff
DrawGLScene()
Are you running threads at all? If a program slows down a lot, it may yield enough CPU time to sync any threads/unmutexed variables you might have.
Also I see that you are destroying things (ie. torpedo). If your using a delete call, you should make sure that what your passing is a good pointer. Double deletes lead to heap errors, no deletes lead to memory leaks. I prefer using the SAFE_DELETE() macro, its a no-brainer when it comes to deleting things. It sets the pointer to NULL afterwards, which protects against double deletes. You might want to look into that, IMO.
Okay, I know it's not a buffer overrun. I check for that everytime I add to an array.
They look like this
Code:// Declaration
#define MAX_WHATEVERS 100
WHATEVER_STRUCT whatever[MAX_WHATEVERS];
int whatevers = 0;
// When making a whatever
if(whatevers + 2 > MAX_WHATEVERS)
return FALSE;
Yes, that's even got a name: Heisenbug - based on the "Heisenberg uncertainty theory", that says [paraphrazed] "The closer you get to something, the more likely it is to not behave normally", and in code, "The more debugging you do, the more likely it is that the code behaves differently".
Utterly annoying, but true.
Having different levels of debug output, so that you can turn on debugging only on some functions is often key here - if you have some sort of idea of where problem resides that is. There are two ways [which can be combined] of doing such things:
1. A bitmask, where you enable/disable groups of logging. So if you set bit X, then you get logging for component X, if you set bit Y, you get logging for component Y.
2. A level - the more level you give, the more output you get.
Using a standard function to provide ALL of the logging will help tremendously here.
By the way, this looks a bit suspect:
I don't have a clue what your app does, but it sounds like you are destroying stuff before you are finished with it - may be that the function names are a bit wrong here and it does something else...Code:DestroyTorpedo(18)
TorpedoImpact(18)
TorpedoImpact(19)
TorpedoImpact(20)
TorpedoImpact(21)
--
Mats
Mastadex, yes, the SAFE_DELETE sounds good, I do need to look at that. But I use arrays to prevent having to deal with pointers. When I delete somthing, I'm really making everything beyond it's point in the array, fall back on it by one. Then bring down the amount varible by one.
Instead of using a macro, use a function template. Stroustrup gives an example of a destroy() function template that does just what you described for SAFE_DELETE in his answer to Why doesn't delete zero out its operand?Quote:
I prefer using the SAFE_DELETE() macro, its a no-brainer when it comes to deleting things. It sets the pointer to NULL afterwards, which protects against double deletes. You might want to look into that, IMO.
Of course, this is assuming you have to work with raw pointers in the first place instead of working with smart pointers and containers to benefit from RAII.
My gut feeling says the ai is where the program is going haywire, but I have no way I knowing that. I'll have to do that with the logging components/levels.
matsp, Here is my DestroyTorpedo function:
as you can see, DestroyTorpedo makes torpedo 19 become torpedo 18, so that isn't a problem.Code:void DestroyTorpedo(int one)
{
log("DestroyTorpedo(%d)", one); // Added for debugging purposes
for(e = one; e < torps; e++)
torp[e] = torp[e + 1];
torps--;
return;
}
>> Using a standard function to provide ALL of the logging will help tremendously here.
I don't know if I follow. But I'm currently calling log(), and thats the only function I call to record this stuff.
Here is my log function
do you know of any way of increasing it's preformence?Code:// dolog is a boolean that I can activate during runtime by pressing L.
void log(const char *fmt, ...)
{
if(fmt == NULL || !dolog) return;
char text[MAX_PATH];
va_list ap;
va_start(ap, fmt);
vsprintf(text, fmt, ap);
va_end(ap);
FILE *fp = fopen("log.txt", "a+");
fprintf(fp, "%s\r\n", text);
fclose(fp);
return;
}
good advice there, laserlight, thanks.
Ok, so if you add a "level" and/or perhaps a "bitmask" to your "log" function, you can restrict the logging by level and/or bitmask, so you only get overview logging.
Also, using just fflush, rather than opening and closing the file every time will speed the logging up by a fair bit.
--
Mats
How about actually using std::vector + std::vector.at and smart pointers? You can narrow it down a bit further, perhaps even eliminate the problem.
I didn't have time to reply, but I guess you saw the result. Logging and flushing a lot is quite expensive.
Off-by-one error. Condition should be "e < torps-1", or the last iteration will access "torp[torps]", and that's probably an invalid location.Quote:
Code:void DestroyTorpedo(int one)
{
log("DestroyTorpedo(%d)", one); // Added for debugging purposes
for(e = one; e < torps; e++)
torp[e] = torp[e + 1];
torps--;
return;
}
If you use a fixed-size array for torpedoes, that could explain the apparently random crashes: it would only crash when you completely fill the torpedo array.
This loop overruns the torp[] array by a single slot. It iterates until and including e = torps - 1, so the last copy is:Quote:
Originally Posted by Yarin
torp[torps] is off the end of the array. This is a read violation but still plenty capable of causing a crash.Code:torp[torps - 1] = torp[torps]
EDIT: Doh. That's what I get for posting in an old window. CB got it.
Hmm, I never noticed that! Thanks for pointing that out, I didn't realize I've been using bad code in so many programs. (I've been handling game objects in this way for a while now.)
EDIT:
But is that still really a problem when considering this code?
As you can see, the array is never even allowed to get completely full to begin with.Code:BOOL ShootTorpedo(int org, int dest, BOOL bForward)
{
log(CREATE, "ShootTorpedo(%d, %d, %s)", org, dest, bForward ? "TRUE" : "FALSE");
if(torps + 2 > MAX_TORPS)
return FALSE;
torp[torps].x = ship[org].x;
torp[torps].y = ship[org].y;
torp[torps].r = ship[org].r;
if(bForward) torp[torps].r += HALFRAD;
torp[torps].f = 0;
torp[torps].t = dest;
torp[torps].o = org;
torps++;
PlaySound(14);
return TRUE;
}
The only way is to try it!
Is there a reason you don't use vectors and smart pointers?
Don't you still make torps equal to MAX_TORPS and thus cause the off-by-one error?Code:BOOL ShootTorpedo(int org, int dest, BOOL bForward)
{
log(CREATE, "ShootTorpedo(%d, %d, %s)", org, dest, bForward ? "TRUE" : "FALSE");
if(torps + 2 > MAX_TORPS)
return FALSE;
torp[torps].x = ship[org].x;
torp[torps].y = ship[org].y;
torp[torps].r = ship[org].r;
if(bForward) torp[torps].r += HALFRAD;
torp[torps].f = 0;
torp[torps].t = dest;
torp[torps].o = org;
torps++;
PlaySound(14);
return TRUE;
}
Okay, here are the individual types of code logging that can be turned on and off now.
I've still yet to find out what's causing the problem. (I'm thinking there may be more than one now. :()Code:#define DESTROY 0 // When things get destroyed
#define CREATE 1 // When things get created
#define ALTER 2 // When the condition of somting changes
#define THINK 3 // When the AI decides what to do
#define BASIC 4 // When basic root actions happen
The only time the logger was running when it crashed, the function BlowShip(1) was last called.
Here it is:
As you can see, I do check for a possible buffer overrun.Code:BOOL BlowShip(int one)
{
log(CREATE, "BlowShip(%d)", one);
BOOL blowshiptemp = FALSE;
if(booms + 2 < MAX_SHIPS)
{
boom[booms].x = ship[one].x;
boom[booms].y = ship[one].y;
boom[booms].r = -ship[one].r;
boom[booms].s = (shipsizex[ship[one].t] + shipsizey[ship[one].t]) / 120.0;
boom[booms].f = 0;
booms++;
blowshiptemp = TRUE;
}
DestroyShip(one);
PlaySound(3);
return blowshiptemp;
}
Okay, now I've set all my arrays to have 2 more than the max, so an overrun is out of the question, right?
>> Is there a reason you don't use vectors and smart pointers?Code:TORP torp[MAX_TORPS + 2];
What advantage do I gain by using them?
>> What advantage do I gain by using them?
Lots. Especially in terms of array growth and no remembering to delete.
Plus strings.
More lots.
Dealing with memory leaksQuote:
What advantage do I gain by using them?
With smart pointers, you can use a reference count to delete memory only when no longer in use. In other words, you'll be guaranteed that you aren't causing memory bugs by freeing memory to be used later and you guarantee there will be no memory leaks.Quote:
Originally Posted by Yarin
By using std::vector.at, you can make sure the code doesn't do any out-of-bounds since it will throw an exception if you try to access out-of-bounds elements.
Oh. Is the advantage really worth revamping my code though?
OF course it is. No major changes needed. For the most, at least :)
You'll be better protected and you'll see if you can get rid of catch some bugs!
While this may prevent going over bounds with this array, it is still a horrible fix. The reader of the code won't either understand it or realize that the original coder was seriously confused about arrays.Code:TORP torp[MAX_TORPS + 2];
The fix for the algorithm was already suggested. If you want to access this and the next item, don't loop up to the very end.
In addition, vector has the erase method, so that you wouldn't even need to write the DestroyTorpedo function yourself. (And thanks to this and other things, such as a vector knowing its own size rather than relying on a global(?) variable, you're code would contain less bugs and be a bit shorter than 1300 lines. :))
I guess I'll do that. Thank you all for the help. :)
Wrong. You can still access members before 0.Quote:
Originally Posted by Yarin
Yarin, I suggest you use linked lists for your game objects.
It's more efficient for removing (or inserting) things from the middle.
They are also efficient when accessed sequentially, which I
assume is another kind of access you need.
So if you don't need so-called "random access", it's might be best.
Anyone else agree?
Yes, but it will also require a rewrite. It would probably be a good idea if speed is a concern.
Also, look up the discussion I had with Bubba a few months ago about linked lists vs vectors.
I looked it up (again) and while carfully reading it, I run into two questions. Maybe you want to answer them.Quote:
Originally Posted by CornedBee
I tried to find out what problem exist with using expensive copy constructor and vector. I found, that the copy constructor is called twice while adding to the end of a vector, if I dont't call reserve() before insertion. Why does it need to be called twice? And why does an explizit call to reserve can do a similar job (reserving memory) without calling the copy constructor?Quote:
Unless copying of the individual elements is expensive (std::string is one case - but not once we get r-value references in C++09), it takes a lot of elements to overcome the consider- ably greater overhead that, say, a list has.
An could you explain please what exactly will change with the r-value-reference thing?
The other question:
Why does the list need more space?Quote:
When you're dealing with primitives,
it's most noticeable. On a 64-bit system, for example, a list<int> needs 5
times as much space to store a single element as a vector.
Thank you in advance!
Basically, the vector has to reallocate every now and then, and for that it needs to copy its elements around. Furthermore, insertion and deletion in the middle of the vector involves moving all elements after this position around - this, too, involves a lot of copying. That's why, if the copy constructor and copy assignment operator are slow, std::vector takes a performance hit.
If, on the other hand, copying is fast - some vector implementations recognize POD types and use straight memmove() then - the overhead is considerably lower.
With r-value references, the vector will be able to use move semantics. Moving is a destructive copy - the target gets the value of the source, but unlike copying, the value of the source after the operation doesn't matter. For classes like std::string, which allocate memory dynamically and keep a pointer to it, this means that the ownership of the memory can simply be passed to the target object, leaving the source object with an empty value. Obviously this is much faster than allocating a new block and copying the contents over.
The list needs more space because every element needs pointers to the next and previous node. There's two pointers overhead for every single element stored. In a vector, there's zero overhead per element, only some constant overhead for managing the memory.
Nice, thank you. Maybe someone should move this together with the post I quoted from to the FAQ board, because this container releated questions are asked very often (directly or indirectly) and few people seems to have such in depth knowledge about it.
It should also be made clear that although with 64-bit words a list<int> may need 5 times the space, a list<sixety_four_byte_object> will only need 1.25 times the space.
You also must consider how much faster the list may (or may not) be for your most common operations. In particular, lots of insertions and deletions might mean a list is faster overall, particularly if they are mostly in the middle (but your case sounds like insertions may all be at the end). The vector may have faste sequential access, and adding to the end _should_ be efficient (if you reserve lots of space beforehand).