Cowboy Programming

Game Development and General Hacking by the Old West

February 22nd, 2007

What is invalid parameter noinfo and how do I get rid of it?

_invalid_parameter_noinfo shows up as a problem most often as an “Unresolved external symbol” when you get some mix up between DEBUG and RELEASE modes. But it’s also a performance problem in situations where it’s compiled in unnecessarily.

So what is it? Well, it’s basically the function that gets called when the Microsoft Visual C++ compiled code detects a type of run-time error, such as a vector index out of range, or incrementing an iterator past the end of a container.

There are two functions: _invalid_parameter, which gets called in debug mode, and _invalid_parameter_noinfo, which gets called in non-debug mode. If you have a module compiled without _DEBUG defined, and you link with the debug library, then you will get the “Unresolved external symbol” error. This could be a problem with inconsistent #includes. See this thread on devmaster.net for some practical experiences.

What is _invalid_parameter_noinfo then? Well, first ask what _invalid_parameter is. _invalid_parameter is basically an assert. When an error is detected, _invalid_parameter is called, and is passed debug information, like the file name and line number pointers. It then calls _crt_debugger_hook and _invoke_watson. In non-debug mode (release mode), the debug info is not available, so invalid_parameter_noinfo simply calls invalid_parameter with NULL parameters. It’s actually an optimization, saving having the NULL parameters passed from every bit of your code, instead you code “just” needs to call this one function

You might also be having problems with _invalid_parameter_noinfo if you have code that crashes (on this function) on in release mode. That’s most likely some form of release-only bug, such as uninitialized memory, and the call to _invalid_parameter_noinfo is the end result. DO NOT IGNORE IT, or try to work around it. You need to find out exactly why it is being called.

But suppose your code works fine, and _invalid_parameter_noinfo is never called, you might be peeking through the disassembly, trying to figure out why it is so slow, and you see all these calls to _invalid_parameter_noinfo

Consider this code:

void	CVerletPoint::SatisfyConstraints()
{

	vector::iterator i;
	for (i = mp_constraints.begin(); i != mp_constraints.end(); i++)
	{
		(*i)->Satisfy(this);
	}

}

A nice simple bit of code that just iterates over a vector of CVerletConstraint objects, and calls some function on each one. It compiles to this:

00405C20  push        ebx  
00405C21  push        ebp  
00405C22  push        esi  
00405C23  mov         ebp,ecx 
00405C25  mov         esi,dword ptr [ebp+20h] 
00405C28  cmp         esi,dword ptr [ebp+24h] 
00405C2B  push        edi  
00405C2C  lea         edi,[ebp+1Ch] 
00405C2F  jbe         CVerletPoint::SatisfyConstraints+16h (405C36h) 
00405C31  call        _invalid_parameter_noinfo (407A81h) 
00405C36  mov         ebx,dword ptr [edi+8] 
00405C39  cmp         dword ptr [edi+4],ebx 
00405C3C  jbe         CVerletPoint::SatisfyConstraints+23h (405C43h) 
00405C3E  call        _invalid_parameter_noinfo (407A81h) 
00405C43  test        edi,edi 
00405C45  je          CVerletPoint::SatisfyConstraints+2Bh (405C4Bh) 
00405C47  cmp         edi,edi 
00405C49  je          CVerletPoint::SatisfyConstraints+30h (405C50h) 
00405C4B  call        _invalid_parameter_noinfo (407A81h) 
00405C50  cmp         esi,ebx 
00405C52  je          CVerletPoint::SatisfyConstraints+5Fh (405C7Fh) 
00405C54  test        edi,edi 
00405C56  jne         CVerletPoint::SatisfyConstraints+3Dh (405C5Dh) 
00405C58  call        _invalid_parameter_noinfo (407A81h) 
00405C5D  cmp         esi,dword ptr [edi+8] 
00405C60  jb          CVerletPoint::SatisfyConstraints+47h (405C67h) 
00405C62  call        _invalid_parameter_noinfo (407A81h) 
00405C67  mov         ecx,dword ptr [esi] 
00405C69  mov         eax,dword ptr [ecx] 
00405C6B  mov         edx,dword ptr [eax] 
00405C6D  push        ebp  
00405C6E  call        edx  
00405C70  cmp         esi,dword ptr [edi+8] 
00405C73  jb          CVerletPoint::SatisfyConstraints+5Ah (405C7Ah) 
00405C75  call        _invalid_parameter_noinfo (407A81h) 
00405C7A  add         esi,4 
00405C7D  jmp         CVerletPoint::SatisfyConstraints+16h (405C36h) 
00405C7F  pop         edi  
00405C80  pop         esi  
00405C81  pop         ebp  
00405C82  pop         ebx  
00405C83  ret              

Yikes! What exactly is going on there? Lots of run time error checking, that’s what. Why is it doing this? Well, it’s to make your code “secure”, it makes it so that if something goes out of bounds, then the program will halt, preventing it from doing any harm (or being exploited by a hacker), and allowing you to debug it.

Is this a good thing? It depend on what you want. If you are writing code with lots of convoluted data structures and container interactions, then maybe this is something you want. But for code that operates on a data structure that does not change from frame to frame, then this code is just getting in the way. If it works the first frame, it will work forever. In release mode I do not expect this kind of error checking, and it certainly look like it would hurt performance. It is tests that always return true, skipping over a function that is never called.

So, you can turn it off with:

#ifndef _DEBUG
#define _SECURE_SCL 0
#endif

You will need that either defined in every file, or have _SECURE_SCL defined as 0 in the release build process.

Effect: Our code from above now shrinks to:

00405AF0  push        esi  
00405AF1  push        edi  
00405AF2  mov         edi,ecx 
00405AF4  mov         esi,dword ptr [edi+20h] 
00405AF7  cmp         esi,dword ptr [edi+24h] 
00405AFA  je          CVerletPoint::SatisfyConstraints+21h (405B11h) 
00405AFC  lea         esp,[esp] 
00405B00  mov         ecx,dword ptr [esi] 
00405B02  mov         eax,dword ptr [ecx] 
00405B04  mov         edx,dword ptr [eax] 
00405B06  push        edi  
00405B07  call        edx  
00405B09  add         esi,4 
00405B0C  cmp         esi,dword ptr [edi+24h] 
00405B0F  jne         CVerletPoint::SatisfyConstraints+10h (405B00h) 
00405B11  pop         edi  
00405B12  pop         esi  
00405B13  ret

Much better. six tests have been eliminated. saving at least 12 lines of assembly code. And the big news, the framerate of my blob program goes from 150fps to 170fps.

Check here for an investigation of different ways of iterating over STL vectors:
http://einaros.blogspot.com/2007/02/iteration-techniques-put-on-benchmark.html

So, is turning off _SECURE_SCL a bad cowboy practice? Well for games I think it’s quite reasonable to switch it off for a “FINAL” build (i.e. the one you are going to release to the consumer). Leaving it on might be a useful debugging tool. Just be aware of the potential for significant performance degradation in instances like the one above. That kind of case might be ripe for some kind of refactoring with error checking that is only performed when the container changes.

February 14th, 2007

Comments vs. Self Documenting Code

I was reading a thread over on GameDev about the use of comments. There were a variety of opinions, ranging from the extremes of “comment every line of code” to “code should be self documenting and have no comments”.

Personally I use quite a few comments in my code. Game code tends to involve the interaction of a large number of complex systems. Self documenting code is really only a reality if you are insanely familiar with the context of that code. That’s plausible for a period of time, on smaller projects. But for large game projects, especially console games, that often use code that has evolved over several years, and interface with several third-party middleware libraries, it’s really not practical.

What do real code gurus say about commenting code? Steve McConnell has a whole chapter on “Self Documenting Code” in Code Complete 2, which covers the cases for and against comments, concluding (page 817:

The question of whether to comment is a legitimate one. Done poorly, commenting is a waste of time and sometimes harmful. Done well, commenting is worthwhile

In The Pragmatic Programmer Hunt and Thomas say (page 249)

In general, comments should discuss why something is done, its purpose and its goal. The code already shows how it is done, so commenting on this is redundant.

In The C++ Programming language, 2nd ed., Bjarne Stroustrup (the creator of C++) says (page 139)

A well-chosen and well-written set of comments is an essential part of a good program. Writing good comments can be as difficult as writing the program itself

Jef Raskin, Creator of the Apple Mac project, says (here)

Do not believe any programmer, manager, or salesperson who claims that code can be self-documenting or automatically documented. It ain't so. Good documentation includes background and decision information that cannot be derived from the code.

The question of “Comments vs. Self Documenting Code” is a false dichotomy. You want both. If your code is really so messed up that it needs lots of comments, then it’s smelly code, and may be ripe for refactoring. Obviously it’s a good thing to have meaningful identifiers in code, and to avoid stating the obvious in comments. But if you can clarify something with a comment, then you should.

Unfortunately dogma is something you find often in programmers, even in good programmers. “Goto considered harmful”, “premature optimization is evil”, “comments indicate sloppy code”, “pointers are evil”, “singletons are evil”, “multiple check-outs are evil”.

All these things: goto, early optimization, comments, pointers, etc., are useful tools. They can also be dangerous tools. They can be over-used, and mis-used by bad programmers. Hence simple minded programmers (and managers) rail against them, and impressionable programmers regurgitate the dogma without really understanding the reasons behind it.

Read Code Complete 2, it’s all you need to know (which is quite a lot).

|