Is it evil to redefine assert?

Question

Is it evil to redefine the assert macro?

Some folks recommend using your own macro ASSERT(cond) rather than redefining the existing, standard, assert(cond) macro. But this does not help if you have a lot of legacy code using assert(), that you don't want to make source code changes to, that you want to intercept, regularize, the assertion reporting of.

I have done

 #undef assert
 #define assert(cond)  ... my own assert code ...

in situations such as the above - code already using assert, that I wanted to extend the assert-failing behavior of - when I wanted to do stuff like

1) printing extra error information to make the assertions more useful

2) automatically invoking a debugger or stack track on an assert

... this, 2), can be done without redefining assert, by implementing a SIGABRT signal handler.

3) converting assertion failures into throws.

... this, 3), cannot be done by a signal handler - since you can't throw a C++ exception from a signal handler. (At least not reliably.)

Why might I want to make assert throw? Stacked error handling.

I do this latter usually not because I want the program to continue running after the assertion (although see below), but because I like using exceptions to provide better context on errors. I often do:

int main() {
  try { some_code(); }
  catch(...) { 
     std::string err = "exception caught in command foo";
     std::cerr << err;
     exit(1);;
  }
}

void some_code() { 
  try { some_other_code(); }
  catch(...) { 
     std::string err = "exception caught when trying to set up directories";
     std::cerr << err;
     throw "unhandled exception, throwing to add more context";
  }
}

void some_other_code() { 
  try { some_other2_code(); }
  catch(...) { 
     std::string err = "exception caught when trying to open log file " + logfilename;
     std::cerr << err;
     throw "unhandled exception, throwing to add more context";
  }
}

etc.

I.e. the exception handlers add a bit more error context, and then rethrow.

Sometimes I have the exception handlers print, e.g. to stderr.

Sometimes I have the exception handlers push onto a stack of error messages. (Obviously that won't work when the problem is running out of memory.)

** These assert exceptions still exit ... **

Somebody who commented on this post, @IanGoldby, said "The idea of an assert that doesn't exit doesn't make any sense to me."

Lest I was not clear: I usually have such exceptions exit. But eventually, perhaps not immediately.

E.g. instead of

#include <iostream>
#include <assert.h>

#define OS_CYGWIN 1

void baz(int n)
{
#if OS_CYGWIN
  assert( n == 1 && "I don't know how to do baz(1) on Cygwin). Should not call baz(1) on Cygwin." );
#else
  std::cout << "I know how to do baz(n) most places, and baz(n), n!=1 on Cygwin, but not baz(1) on Cygwin.\n";
#endif
}
void bar(int n)
{
  baz(n);
}
void foo(int n)
{
  bar(n);
}

int main(int argc, char** argv)
{
  foo( argv[0] == std::string("1") );
}

producing only

% ./assert-exceptions
assertion "n == 1 && "I don't know how to do baz(1) on Cygwin). Should not call baz(1) on Cygwin."" failed: file "assert-exceptions.cpp", line 9, function: void baz(int)
/bin/sh: line 1: 22180 Aborted                 (core dumped) ./assert-exceptions/
%

you might do

#include <iostream>
//#include <assert.h>
#define assert_error_report_helper(cond) "assertion failed: " #cond
#define assert(cond)  {if(!(cond)) { std::cerr << assert_error_report_helper(cond) "\n"; throw assert_error_report_helper(cond); } }
     //^ TBD: yes, I know assert needs more stuff to match the definition: void, etc.

#define OS_CYGWIN 1

void baz(int n)
{
#if OS_CYGWIN
  assert( n == 1 && "I don't know how to do baz(1) on Cygwin). Should not call baz(1) on Cygwin." );
#else
  std::cout << "I know how to do baz(n) most places, and baz(n), n!=1 on Cygwin, but not baz(1) on Cygwin.\n";
#endif
}
void bar(int n)
{
  try {
baz(n);
  }
  catch(...) {
std::cerr << "trying to accomplish bar by baz\n";
    throw "bar";
  }
}
void foo(int n)
{
  bar(n);
}

int secondary_main(int argc, char** argv)
{
     foo( argv[0] == std::string("1") );
}
int main(int argc, char** argv)
{
  try {
return secondary_main(argc,argv);
  }
  catch(...) {
std::cerr << "main exiting because of unknown exception ...\n";
  }
}

and get the slightly more meaningful error messages

assertion failed: n == 1 && "I don't know how to do baz(1) on Cygwin). Should not call baz(1) on Cygwin."
trying to accomplish bar by baz
main exiting because of unknown exception ...

I should not have to explain why these context sensitive error messages can be more meaningful. E.g. the user may not have the slightest idea why baz(1) is being called. It may well ne a pogram error - on cygwin, you may have to call cygwin_alternative_to_baz(1).

But the user may understand what "bar" is.

Yes: this is not guaranteed to work. But, for that matter, asserts are not guaranteed to work, if they do anything more complicated than calling in the abort handler.

write(2,"error baz(1) has occurred",64);

and even that is not guaranteed to work (there's a secure bug in this invocation.)

E.g. if malloc or sbrk has failed.

Why might I want to make assert throw? Testing

The other big reason that I have occasionally redefined assert has been to write unit tests for legacy code, code that uses assert to signal errors, which I am not allowed to rewrite.

If this code is library code, then it is convenient to wrap calls via try/catch. See if the error is detected, and go on.

Oh, heck, I might as well admit it: sometimes I wrote this legacy code. And I deliberately used assert() to signal errors. Because I could not rely on the user doing try/catch/throw - in fact, oftentimes the same code must be used in a C/C++ environment. I did not want to use my own ASSERT macro - because, believe it or not, ASSERT often conflicts. I find code that is littered with FOOBAR_ASSERT() and A_SPECIAL_ASSERT() ugly. No... simply using assert() by itself is elegant, works basically. And can be extended.... if it is okay to override assert().

Anyway, whether the code that uses assert() is mine or from someone else: sometimes you want code to fail, by calling SIGABRT or exit(1) - and sometimes you want it to throw.

I know how to test code that fails by exit(a) or SIGABRT - something like

for all  tests do
   fork
      ... run test in child
   wait
   check exit status

but this code is slow. Not always portable. And often runs several thousand times slower

for all  tests do
   try {
      ... run test in child
   } catch (... ) {
      ...
   }

This is a riskier than just stacking error message context since you may continue operating. But you can always choose types of exceptions to cactch.

Meta-Observation

I am with Andrei Alexandresciu in thinking that exceptions are the best known method to report errors in code that wants to be secure. (Because the programmer cannot forget to check an error return code.)

If this is right ... if there is a phase change in error reporting, from exit(1)/signals/ to exceptions ... one still has the question of how to live with the legacy code.

And, overall - there are several error reporting schemes. If different libraries use different schemes, how make them live together.

Answer 1

Redefining a Standard macro is an ugly idea, and you can be sure the behaviour's technically undefined, but in the end macros are just source code substitutions and it's hard to see how it could cause problems, as long as the assertion causes your program to exit.

That said, your intended substitution may not be reliably used if any code in the translation unit after your definition itself redefines assert, which suggests a need for a specific order of includes etc. - damned fragile.

If your assert substitutes code that doesn't exit, you open up new problems. There are pathological edge cases where your ideas about throwing instead could fail, such as:

int f(int n)
{
    try
    {
        assert(n != 0);
        call_some_library_that_might_throw(n);
    }
    catch (...)
    {
        // ignore errors...
    }
    return 12 / n;
}

Above, a value of 0 for n starts crashing the application instead of stopping it with a sane error message: any explanation in the thrown message won't be seen.

I am with Andrei Alexandresciu in thinking that exceptions are the best known method to report errors in code that wants to be secure. (Because the programmer cannot forget to check an error return code.)

I don't recall Andrei saying quite that - do you have a quote? He's certainly thought very carefully about how to create objects that encourage reliable exception handling, but I've never heard/seen him suggest that a stop-the-program assert is inappropriate in certain cases. Assertions are a normal way of enforcing invariants - there's definitely a line to be drawn concerning which potential assertions can be continued from and which can't, but on one side of that line assertions continue to be useful.

The choice between returning an error value and using exceptions is the traditional ground for the kind of argument/preference you mention, as they're more legitimately alternatives.

If this is right ... if there is a phase change in error reporting, from exit(1)/signals/ to exceptions ... one still has the question of how to live with the legacy code.

As above, you shouldn't try to migrate all existing exit() / assert etc. to exceptions. In many cases, there will be no way to meaningfully continue processing, and throwing an exception just creates doubt about whether the issue will be recorded properly and lead to the intended termination.

And, overall - there are several error reporting schemes. If different libraries use different schemes, how make them live together.

Where that becomes a real issue, you'd generally select one approach and wrap the non-conforming libraries with a layer that provides the error handling you like.