Problems with __cxa_demangle and the verbose terminate handler

[Gabriel Dos Reis]

Mark Mitchell <mark@codesourcery.com> writes:

| Originally, I objected strongly to the inclusion of the C++ demangler
| implementation in the V3 library on the grounds that it was a needless
| duplication of code, since C-only applications would never be able to
| use it.  
| 
| Ian's new C demangler is a very good demangler implementation in C.
| 
| Code duplication is not the only problem with the C++ implementation.
| 
| People have now noticed that libsupc++ and libstdc++ are no longer
| separable, since __cxa_demangle uses the new demangler, which itself
| uses std::string and std::vector.  That is a problem for embedded
| systems and other similar environments -- it makes G++ incompatible
| with the C++ ABI in those contexts in that the freestanding library no
| longer contains __cxa_demangle.
| 
| Furthermore, the new demangler allocates dynamic memory with "new",
| rather than with "malloc".  It calls "new" directly, and it also makes
| use of "std::string" and "std::vector", which themselves call "new".
| That means that memory allocation failures cause exceptions to be
| thrown.  

When called from "C" land, those containers should probably be
instantiated with an allocator that either calls malloc() or use the
nothrow form of new.  Useing "new" in  itself is not the problem, it
not is calling the nothrow-form of new that is problematic.

| (The implementation of "__cxa_demangle" in demangle.cc is buggy in
| that it does not try to catch "std::bad_alloc"; therefore, an
| exception thrown by one of the calls to "new" results in an exception
| escaping "__cxa_demangle", which is a bug, since it is clearly
| documented to return an error code if insufficient memory is
| available.  I've fixed that with the attached patch.)
| 
| It is a bad idea for "__cxa_demangle" to throw exceptions, even if
| they are caught.  The reason is that "__cxa_demangle" is often called

If the allocator does not use the throw-form, then the "bad idea" is gone.

| during some kind of severe error situation (such as the V3 verbose
| terminate handler).  One of the cases in which "std::terminate" is
| called is when the exception-handling machinery (such as
| "_Unwind_RaiseException") has failed.  The terminate handler should
| never call a function which might raise another exception as the
| internal unwinder state is not guaranteed to be consistent at this

That is true.

| point.  Trying to raise another exception may just result in a
| recursive call to "std::terminate".  (I have a program which does
| exactly that; it eventually gets killed by the OS due to stack
| overflow via recursive calls to "std::terminate".)
| 
| That argues, again, for using the C demangler.

No.  It argues only for not using the throw-form.

In French, there is a saying: "si on veut noyer un chien, on prétend
qu'il a la gâle".

| In this patch, I've made the verbose terminate handler defensively
| notice recursive calls to "std::terminate".

Thanks.

| I did not change this code in vterminate.cc, though it is similarly
| dangerous:
| 
| 	// If the exception is derived from std::exception, we can give more
| 	// information.
| 	try { __throw_exception_again; }
| 
| If we really want to do get at the exception object, the right thing
| to do is to create an API function that lets us do that.  Actually
| throwing an exception from within the terminate handler is a bad idea
| because it may lead to recursive calls to "std::terminate".
| 
| Perhaps most dangerous of all is this code in vterminate.cc:
| 
|   # define writestr(str)  write(2, str, __builtin_strlen(str))
| 
| This bit of cleverness is designed to let the terminate handler write
| to the standard error stream.  However, there is no way to know that
| file descriptor 2 *is* the standard error stream.  If the process
| closes stderr, and then opens another file (say, "/dev/launch_missle",
| or "/dev/database") that new file descriptor may very well be
| descriptor 2.  Using "fputs" to write to "stderr" does not have this
| problem.

I suppose fputs() does not have the drawback for fprintf().  I would
lean toward your suggestion.

-- Gaby