This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Problems with __cxa_demangle and the verbose terminate handler


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.  

(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
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
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.

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

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.

Benjmain, would you please address the following issues:

a) __cxa_demangle is not available in freestanding environments.

b) the verbose terminate handler writes to a raw file descriptor.

These are both important issues and I would like to see them resolved
before GCC 3.4 is release.

Tested on ia64-hp-hpux11.23, applied on the mainline and on the 3.4
branch.

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

2004-02-21  Mark Mitchell  <mark@codesourcery.com>

	* libsupc++/vterminate.cc
	(__gnu_cxx::__verbose_terminate_handler): Guard against recursive
	calls to terminate.
	* src/demangle.c (__cxa_demangle): Wrap in try-catch block.

Index: libsupc++/vterminate.cc
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/libsupc++/vterminate.cc,v
retrieving revision 1.3
diff -c -5 -p -r1.3 vterminate.cc
*** libsupc++/vterminate.cc	5 Jul 2003 04:05:41 -0000	1.3
--- libsupc++/vterminate.cc	21 Feb 2004 20:55:42 -0000
*************** namespace __gnu_cxx
*** 49,58 ****
--- 49,68 ----
    /* A replacement for the standard terminate_handler which prints
     more information about the terminating exception (if any) on
     stderr.  */
    void __verbose_terminate_handler()
    {
+     static bool terminating;
+ 
+     if (terminating)
+       {
+ 	writestr ("terminate called recursively\n");
+ 	abort ();
+       }
+  
+    terminating = true;
+ 
      // Make sure there was an exception; terminate is also called for an
      // attempt to rethrow when there is no suitable exception.
      type_info *t = __cxa_current_exception_type();
      if (t)
        {
Index: src/demangle.cc
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/src/demangle.cc,v
retrieving revision 1.4
diff -c -5 -p -r1.4 demangle.cc
*** src/demangle.cc	5 Dec 2003 02:40:53 -0000	1.4
--- src/demangle.cc	21 Feb 2004 20:55:43 -0000
*************** namespace __cxxabiv1 
*** 110,167 ****
      typedef demangler::session<std::allocator<char> > session_type;
  
      if (!mangled_name || (buf && !n))
        return failure(invalid_argument, status);
  
!     std::string result;
!     if (mangled_name[0] == '_')		
!     {
!       // External name?
!       if (mangled_name[1] == 'Z')		
        {
! 	// C++ name?
! 	int cnt = session_type::
! 	    decode_encoding(result, mangled_name + 2, INT_MAX);
! 	if (cnt < 0 || mangled_name[cnt + 2] != 0)
! 	  return failure(invalid_mangled_name, status);
! 	return finish(result.data(), result.size(), buf, n, status);
!       }
!       else if (mangled_name[1] == 'G')	
!       {
! 	// Possible _GLOBAL__ extension?
! 	if (!std::strncmp(mangled_name, "_GLOBAL__", 9) 
! 	    && (mangled_name[9] == 'D' || mangled_name[9] == 'I')
! 	    && mangled_name[10] == '_')
  	{
! 	  if (mangled_name[9] == 'D')
! 	    result.assign("global destructors keyed to ", 28);
! 	  else
! 	    result.assign("global constructors keyed to ", 29);
! 	  // Output the disambiguation part as-is.
! 	  result += mangled_name + 11;
  	  return finish(result.data(), result.size(), buf, n, status);
  	}
        }
-     }
  
!     // Ambiguities are possible between extern "C" object names and
!     // internal built-in type names, e.g. "i" may be either an object
!     // named "i" or the built-in "int" type.  Such ambiguities should
!     // be resolved to user names over built-in names.  Builtin types
!     // are any single lower case character.  Any other single
!     // character is not a mangled type so we can treat those the same
!     // here.
!     if (mangled_name[1] == 0)
!       return finish(mangled_name, 1, buf, n, status);
  
!     // Not a built-in type or external name, try to demangle input as
!     // NTBS mangled type name.
!     session_type demangler_session(mangled_name, INT_MAX);
!     if (!demangler_session.decode_type(result) 
! 	|| demangler_session.remaining_input_characters())
!     {
!       // Failure to demangle, assume extern "C" name.
!       result = mangled_name;		
      }
-     return finish(result.data(), result.size(), buf, n, status);
    }
  } // namespace __cxxabiv1
--- 110,171 ----
      typedef demangler::session<std::allocator<char> > session_type;
  
      if (!mangled_name || (buf && !n))
        return failure(invalid_argument, status);
  
!     try {
!       std::string result;
!       if (mangled_name[0] == '_')		
        {
! 	// External name?
! 	if (mangled_name[1] == 'Z')		
  	{
! 	  // C++ name?
! 	  int cnt = session_type::
! 	      decode_encoding(result, mangled_name + 2, INT_MAX);
! 	  if (cnt < 0 || mangled_name[cnt + 2] != 0)
! 	    return failure(invalid_mangled_name, status);
  	  return finish(result.data(), result.size(), buf, n, status);
  	}
+ 	else if (mangled_name[1] == 'G')	
+ 	{
+ 	  // Possible _GLOBAL__ extension?
+ 	  if (!std::strncmp(mangled_name, "_GLOBAL__", 9) 
+ 	      && (mangled_name[9] == 'D' || mangled_name[9] == 'I')
+ 	      && mangled_name[10] == '_')
+ 	  {
+ 	    if (mangled_name[9] == 'D')
+ 	      result.assign("global destructors keyed to ", 28);
+ 	    else
+ 	      result.assign("global constructors keyed to ", 29);
+ 	    // Output the disambiguation part as-is.
+ 	    result += mangled_name + 11;
+ 	    return finish(result.data(), result.size(), buf, n, status);
+ 	  }
+ 	}
        }
  
!       // Ambiguities are possible between extern "C" object names and
!       // internal built-in type names, e.g. "i" may be either an object
!       // named "i" or the built-in "int" type.  Such ambiguities should
!       // be resolved to user names over built-in names.  Builtin types
!       // are any single lower case character.  Any other single
!       // character is not a mangled type so we can treat those the same
!       // here.
!       if (mangled_name[1] == 0)
! 	return finish(mangled_name, 1, buf, n, status);
  
!       // Not a built-in type or external name, try to demangle input as
!       // NTBS mangled type name.
!       session_type demangler_session(mangled_name, INT_MAX);
!       if (!demangler_session.decode_type(result) 
! 	  || demangler_session.remaining_input_characters())
!       {
! 	// Failure to demangle, assume extern "C" name.
! 	result = mangled_name;		
!       }
!       return finish(result.data(), result.size(), buf, n, status);
!     } catch (const std::bad_alloc&) {
!       return failure(memory_allocation_failure, status);
      }
    }
  } // namespace __cxxabiv1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]