On the proper use of assert

Posted 3 Oct 2007 at 15:02 UTC by wlach Share This

Oh humble assert, how much confusion have you caused! This might seem like a banal topic and hardly "master" material, but I continually see people get this wrong, so perhaps an article on this topic is worthwhile.

Please note that I am talking here of user space software (for example, a mail client or server), I realize that some of what I'm talking about probably doesn't apply directly to kernel development. By "assert", I mean a statement that will cause the program to immediately terminate, sometimes with a reason. For example:


void foo(char *bar)
{
   assert(bar);
   ...
}

I continually see people make two mistakes with these statements. First, not using them (or taking them out when they're already there). "I'll just make it print a warning or return instead!" they say. They would rather the function look like this:


void foo(char *bar)
{
   if (!bar)
   {
     printf("Oh no, bar is NULL!\n");
     return;
   }
   ...
}

I have personally had patches passed to me, claiming to have "fixed" crashes in this way. The problem is that they've only fixed the symptom, potentially masking another problem. Why was a NULL pointer passed to this function? What happens if "foo" is accessed by something further down the line which isn't doing this check? What if the function is performing a necessary operation using the input?

What people like this are actually saying: "I don't understand the problem, but I made it go away so everything is fine". No, it is not fine. You should understand how the problem manifested itself in the first place so you can fix it properly. By papering over the problem, all you've done is made it harder to debug in the future, when another crash occurs later in program execution.

The second, less common, mistake is to use assertions for errors that may pop up in the course of normal program execution. For example:


FILE *fp = fopen("beep", "r+");
assert(fp);
char x;
assert(&x, sizeof(char), 1, fp) == 1)
assert(x < 10);

I'm terribly sorry, but you're not allowed to do this either. This goes double when the file to load is specified by the user. Files go missing and/or corrupt themselves all the time. Users make mistakes when passing files as input. For goodness sake, put some proper error handling into your program. If you can't continue if a file isn't as expected, exit gracefully. In the case of a graphical program, try to show a friendly message telling the user approximately what has gone wrong. The same goes for information passed to your program over the network or standard input.

Following this, there is a very good argument not to disable assertions in production builds (as most projects do). So long as you don't make the mistake mentioned above, the only possible way that a program would crash is because a programmer made some kind of error and the program is now in an inconsistent state. Here's a question: do you want your program to continue in this inconsistent state, potentially corrupting your user's data? Or is it better for it to crash, and hopefully send some information back to the developers for analysis and reproduction? If you're worried that your program is going to crash frequently because of hitherto unfound corner cases triggering these asserts, then you seriously need to re-examine your software's quality assurance and release process.

Now, go forward and assert!


not sure about assert that quits, posted 3 Oct 2007 at 16:20 UTC by hub » (Master)

I'm not sure about the assert that quits, because actually bypassing the assert should be useful do help track down problem further.

otherwise I agree with that. And I always end up rewriting or cut&paste assert support somewhere in my projects.

Also, side effects are evil and they kill kittens, posted 3 Oct 2007 at 16:32 UTC by bi » (Journeyer)

Well, I guess this is also a good time to mention that calls to assert() should probably not contain side effects -- in case someone decides to turn on NDEBUG. So

assert(fread(&x, sizeof(char), 1, fp) == 1);

should really be

int res = fread(&x, sizeof(char), 1, fp);
assert(res == 1);

(or just define a function safe_fread() that calls fread() and checks the result...).

hub: I think a stack trace will be nice, except a typical implementation of assert() doesn't provide one.

assertion expense, posted 4 Oct 2007 at 00:13 UTC by ncm » (Master)

To have assertion statements remain in production releases is to discourage coding expensive checks in assertions. Arguably there should be a "cheap_assert()" that remains in production code, where the caller decides whether it's cheap enough. However, abort() is rarely a very helpful event in a production setting. So, the cheap_assert() might better log the event rather than aborting. At that point, though, it's not really an assertion any more. That's probably why we don't see it.

not helpful in prod??, posted 4 Oct 2007 at 09:23 UTC by gobry » (Journeyer)

I don't know about you, but I'd rather have a clean production failure now than trying to understand what's going on down the road once my system started behaving erratically for a month...

Systems do crash. You need to cope with this anyway.

Re: assertion expense, posted 4 Oct 2007 at 12:51 UTC by wlach » (Master)

ncm: I've seen assertion-related code slow down production releases, but in my experience it's actually pretty rare. In practice, it's the sort of thing you should sort out by profiling your code (and/or comparing your product to a previous release).

Assertion expense, posted 4 Oct 2007 at 14:34 UTC by Chicago » (Journeyer)

Sorry if this is wrong - I'm not a very fluent C programmer (spending most my time in higher level languages) but this interests me - "The Complete Reference C" by Herbert Schildt (Fourth Edition Osborne) states

"It is not necessary to remove the assert() statements from the source code once a program is debugged because if the macro NDEBUG is defined (as anything), the assert() macros will be ignored."

However ncm, gobry and wlach all seem to think that its in on their "preduction" code? Is the NDEBUG flag not used very often (a flag I believe to stand for No Debug) or have I missed something from this?

Schildt, posted 4 Oct 2007 at 15:41 UTC by redi » (Master)

Chicago, it's common to leave NDEBUG undefined in production code, meaning the assertions are still present.

Schildt is not the most reliable source, he gets his very own entry in the alt.comp.lang.learn.c-c++ FAQ

Ahh, posted 5 Oct 2007 at 08:43 UTC by Chicago » (Journeyer)

Ahh but you didn't see the price I got it for ;) (as stated, I work in higher level languages, and only used when I have to look at my colleagues code and go "WHAT THE HELL???" on random function names that are actually part of stdio.

Interestingly, C# seems to have an assert as well (yea!). C#'s assert however gives the option to abort, go to code or continue (ignore that assert line).

Assert Demo shows what it might look like when using Visual Studio.

If you're using Visual Studio, compiling as "release" instead of "build" does automatically add the debug flag, and I'm pretty sure I've got my eclipse & mono settings set to doing the same.

Although thinking about it if I looked at it it might be the other way around, i.e. DEBUG is defined on the debugging builds, and nothing is defined on the release ones (other then my other defined vars).

More like proper abuse of asserts, posted 6 Oct 2007 at 18:46 UTC by braden » (Journeyer)

Writing your own macro that can do checks you want in production builds is not hard. Do it. Don't lazily abuse assert.

Regularly compiling production code without NDEBUG defined? Shame on you.

Profiling code is for finding real bottlenecks—not for finding costly asserts that can be removed.

NDEBUG, posted 26 Oct 2007 at 14:15 UTC by abraham » (Master)

NDEBUG is the flag you set to celebrate that your have finally mathematically proved your program to be correct. Then "debugging" is no longer relevant.

Until then, you don't set it in any program for which a runtime error is preferable to a wrong answer.

An assert done properly is not costly, posted 29 Oct 2007 at 15:36 UTC by Omnifarious » (Journeyer)

If your assert is costly it's likely your doing the assert in the wrong place or asserting the wrong thing. An assert check should, in general, be something that compiles to a few instructions and not be in an inner loop.

Asserts are so enormously helpful in finding problems that I would never think of compiling a program without them. It is possible I would consider releasing a library who's non-debug version compiled out the asserts, but since a lot of what I code now is Open Source I think maybe leaving them in might help whoever is using it.

It's not just the cost of the assert..., posted 29 Oct 2007 at 17:59 UTC by wlach » (Master)

... but also the debugging code that may surround it. For example, WvStreams has a feature called "last will" that lets you specify a string that can be dumped to a crash file in the event that an assert is triggered. This feature proved invaluable for debugging a few things in an application I was working on around a year ago.

However, there was one instance where we used this feature to print out a complicated, formatted string on every iteration through our mainloop. This resulted in a performance regression of around 10%, which is obviously not acceptable in a release version. So, after it had served its primary purpose (helping shake out a major change to WvStreams), we removed it. Not a big deal.

It all depends..., posted 29 Oct 2007 at 19:45 UTC by Ankh » (Master)

If you ship a library that uses assert() and calls exit() directly, programs using your library can end up crashing. If you write a database that can exit and leave your database in a corrupt state, or a text editor that quits without letting the user save the data, you've done a disservice to people using your software.

The answer is not turning off assert() for production code. Robust code needs to check pretty much all the same things at all times. Procudures cannot blindly trust their arguments, nor can they blindly trust the return values of other procedures that they call.

I generally have distinguished in code I've written between tests purely for debugging, and things that must always be tested. In addition, when the second sort of test fails, an error may or may not be reported depending on what sort of recovery is possible.

I got into the habit years ago of a printf-like error function whose first argument is or'd flags, Error(E_WARN|E_LOWMEMORY, "could not allocate memory for new copy of wikipedia; bytes wanted: %ld", badsize); but watch out for internationalisation issues there.

The trick here is that you can have Error() bring up a dialogue box for example, or close a database; you could do this with your own assert() too.

Of course, an "out of memory" dialogue box might not work -- on some windowing systems you need to create it at startup and show it only if needed, because if there's not enough memory you might not be able to create it.

In other words, yes, using assert() is a step forward for many people, but it's better to think error-handling through a little more and come up with something that will continue to work in production, and will help your program be robust by recovering from internal errors. The assert() macro can still be part of that strategy, but it's not the whole story.

Liam

New Advogato Features

New HTML Parser: The long-awaited libxml2 based HTML parser code is live. It needs further work but already handles most markup better than the original parser.

Keep up with the latest Advogato features by reading the Advogato status blog.

If you're a C programmer with some spare time, take a look at the mod_virgule project page and help us with one of the tasks on the ToDo list!

X
Share this page