[PING][PATCH][RFC] vterminate use of malloc-less demangler

[Simon Baldwin]

Benjamin Kosnik wrote:

>
>> Testing this work is, unfortunately, rather tricky.  I do have a 
>> simple test case, appended below, that allows me to confirm manually 
>> that __verbose_terminate_handler is operating without heap use.
>>
>> This test, however, is hard to fit into any test framework.  First, 
>> it aborts whether or not it's successful; that's the nature of 
>> changing termination code.  Second, it relies on MALLOC_CHECK_, 
>> making it Linux only and not portable.  And third, it relies on 
>> carnal knowledge of libc's heap organization in order to "lightly" 
>> corrupt the heap without overdoing it.
>>
>> Some of these restrictions might be avoidable.  For example, 
>> interposing malloc, realloc, free, and calloc catches calls to heap 
>> functions in the same way as MALLOC_CHECK_, so could make the test 
>> slightly more portable.  This still isn't fully portable across all 
>> platforms, though, and may require platform-specific link options 
>> even where practical.  Likewise, the abort() can be caught with a 
>> SIGABRT handler, but again is not portable.  I experimented with a 
>> test based on these ideas, but it's probably too complex to be reliable.
>>
>> There seems to be no identifiable current vterminate.cc test I can 
>> extend for this patch.  If you have other ideas for how to add a 
>> suitable test to the libstdc++ test suite, please let me know.
>
>
> I cannot, at the moment, sorry. Also, your test case does not 
> reproduce the issue for me, on FC6.
>
> I get:
>
> %./a.out
> malloc: using debugging hooks
> *** glibc detected *** ./a.out: malloc: top chunk is corrupt: 
> 0x08d40088 ***


Did you link the test program with a libstdc++ that contains my patch?  
If not, then you have successfully reproduced the issue -- this is it.

Without the patch, __cxa_demangle(), called from __verbose_terminate(), 
will attempt to allocate memory to hold the result of demangling.  
Because the test program deliberately corrupts the heap and sets 
MALLOC_CHECK_, demangling will then fail to allocate, and give an error 
like that above.

With the patch, and linking the test program to a patched libstdc++.a, 
you'll see

  %./a.out
  malloc: using debugging hooks
  terminate called after throwing an instance of 'std::bad_alloc'
    what():  St9bad_alloc
  Aborted

indicating that name demangling and other termination handling still 
function even in the presence of a corrupted and unusable heap.

>>> 2) Are you planning on making this new function available to other 
>>> users besides the verbose terminate handler? If so, the declaration 
>>> should go below cxa_demangle in cxxabi.h and the function should be 
>>> exported. If not, the declaration should be removed.
>>
>>
>>
>> The __gcclibcxx_demangle_callback() function is intended as a private 
>> contract between glibc and libstdc++.  There's no plan to use it 
>> elsewhere.  Callback-based demangling for general use is provided by 
>> cplus_demangle_v3_callback(), supplied by libiberty's 
>> include/demangle.h as an extension to the existing 
>> cplus_demangle_v3().  Moving the declaration of 
>> __gcclibcxx_demangle_callback() to cxxabi.h is also slightly 
>> misleading, as the function isn't actually part of the defined ABI.
>>
>> The current declaration can't be removed if not placed in cxxabi.h -- 
>> this would prevent the vterminate.cc from compiling.
>
>
> [snip]
>
>> There don't appear to be any other functions that are private 
>> contracts between glibc and libstdc++, so this name seems 
>> reasonable.  In particular, it's not part of the C++ ABI, so naming 
>> it __cxa_... was deemed unacceptable.  (In practice, it's actually a 
>> variant of __cxa_demangle(), but has to have a different name as the 
>> specification of __cxa_demangle() doesn't permit malloc-less operation).
>
>
> I think you should re-evaluate your approach with this patch. I think 
> this malloc-free demangler addition should be obvious to people using 
> the cxxabi.h interfaces.
>
> In overview, libiberty and libsupc++/libstdc++ share the demangler 
> code. However, libstdc++ has an API-defined interface to the 
> demangler, in __cxa_demangle from cxxabi.h. You're augmenting this, 
> much like other non-ABI extensions (__cxa_cdtor_return_type, 
> __cxa_cdtor_type), and admittedly so from your last parenthetical bit 
> above.
>
> And, I think this is a good thing: clearly, there are issues with 
> __cxa_demangle's memory management facilities. There's even been 
> previous attempts to deal with this in libsupc++, with this. See 
> gcc-3.4.0's struct demangle in libstdc++-v3/ext/demangle.h etc etc.
>
> I'm of the opinion that something like __cxa_demangle_no_alloc, ie 
> something much simpler and direct, without all the callback goo, would 
> be a much cleaner route to get where you want to go. Then, you could 
> use this no-alloc routine in __verbose_terminate_handler. Or, 
> __cxa_demangle_callback if you can convince me that there is ever 
> going to be more than one callback...


__cxa_demangle_callback() was the original name of this demangler 
function, but this was rejected as part of the review of libiberty 
revision 121305, which changed it to _gcclibcxx_demangle_callback().  
The rationale is that since this function is not part of the ia64 ABI, 
it should not be named as though it is.

Please explain how you envision a demangling interface that explicitly 
avoids both allocations and callbacks.  In particular, how should it 
return or otherwise handle the demangled string?

> In addition, I think this addition should be mentioned here:
> http://gcc.gnu.org/onlinedocs/libstdc++/18_support/howto.html#6


This is relevant only for functions that are described by cxxabi.h.  As 
noted above, this addition isn't, since it's not part of the ABI.

Conventional client code access to allocation-less demangling is 
provided by the libiberty functions cplus_demangle_v3_callback(), 
java_demangle_v3_callback(), and cplus_demangle_print_callback(), all 
analogous to the existing libiberty functions that return allocated 
strings -- cplus_demangle_v3(), java_demangle_v3(), and 
cplus_demangle_print().