| Message ID | 5547D73C.6070602@gmail.com |
|---|---|
| State | New |
| Headers | show |
Hello
Is it ok ?
François
On 04/05/2015 22:31, François Dumont wrote:
> Hi
>
> Here is the patch to demangle symbols in debug messages. I have
> also simplify code in formatter.h.
>
> Here is an example of assertion message:
>
> /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/functions.h:213:
>
> error: function requires a valid iterator range [__first, __last).
>
> Objects involved in the operation:
> iterator "__first" @ 0x0x7fff165d68b0 {
> type =
> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*,
> std::__cxx1998::vector<int, std::allocator<int> > >,
> std::__debug::vector<int, std::allocator<int> > > (mutable iterator);
> state = dereferenceable;
> references sequence with type `std::__debug::vector<int,
> std::allocator<int> >' @ 0x0x7fff165d69d0
> }
> iterator "__last" @ 0x0x7fff165d68e0 {
> type =
> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*,
> std::__cxx1998::vector<int, std::allocator<int> > >,
> std::__debug::vector<int, std::allocator<int> > > (mutable iterator);
> state = dereferenceable;
> references sequence with type `std::__debug::vector<int,
> std::allocator<int> >' @ 0x0x7fff165d69d0
> }
>
>
> * include/debug/formatter.h (_GLIBCXX_TYPEID): New macro to simplify
> usage of typeid.
> (_Error_formatter::_M_print_type): New.
> * src/c++11/debug.cc
> (_Error_formatter::_Parameter::_M_print_field): Use latter.
> (_Error_formatter::_M_print_type): Implement latter using
> __cxaabiv1::__cxa_demangle to print demangled type name.
>
> I just hope that __cxa_demangle is portable.
>
> Ok to commit ?
>
> François
>
On 04/05/15 22:31 +0200, François Dumont wrote: >Hi > > Here is the patch to demangle symbols in debug messages. I have >also simplify code in formatter.h. > > Here is an example of assertion message: > >/home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/functions.h:213: > error: function requires a valid iterator range [__first, __last). > >Objects involved in the operation: >iterator "__first" @ 0x0x7fff165d68b0 { > type = >__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, >std::__cxx1998::vector<int, std::allocator<int> > >, >std::__debug::vector<int, std::allocator<int> > > (mutable iterator); > state = dereferenceable; > references sequence with type `std::__debug::vector<int, >std::allocator<int> >' @ 0x0x7fff165d69d0 >} >iterator "__last" @ 0x0x7fff165d68e0 { > type = >__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, >std::__cxx1998::vector<int, std::allocator<int> > >, >std::__debug::vector<int, std::allocator<int> > > (mutable iterator); > state = dereferenceable; > references sequence with type `std::__debug::vector<int, >std::allocator<int> >' @ 0x0x7fff165d69d0 >} > > > * include/debug/formatter.h (_GLIBCXX_TYPEID): New macro to simplify > usage of typeid. > (_Error_formatter::_M_print_type): New. > * src/c++11/debug.cc > (_Error_formatter::_Parameter::_M_print_field): Use latter. > (_Error_formatter::_M_print_type): Implement latter using > __cxaabiv1::__cxa_demangle to print demangled type name. > >I just hope that __cxa_demangle is portable. It's provided by GCC itself so is always available in the runtime. (It is also portable, because it's defined by the Itanium C++ ABI). >Ok to commit ? Yes, this is great, thanks!
On 20/05/15 11:17 +0100, Jonathan Wakely wrote: >On 04/05/15 22:31 +0200, François Dumont wrote: >>Hi >> >> Here is the patch to demangle symbols in debug messages. I have >>also simplify code in formatter.h. >> >> Here is an example of assertion message: >> >>/home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/functions.h:213: >> error: function requires a valid iterator range [__first, __last). >> >>Objects involved in the operation: >>iterator "__first" @ 0x0x7fff165d68b0 { >> type = >>__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, >>std::__cxx1998::vector<int, std::allocator<int> > >, >>std::__debug::vector<int, std::allocator<int> > > (mutable >>iterator); >> state = dereferenceable; >> references sequence with type `std::__debug::vector<int, >>std::allocator<int> >' @ 0x0x7fff165d69d0 >>} >>iterator "__last" @ 0x0x7fff165d68e0 { >> type = >>__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, >>std::__cxx1998::vector<int, std::allocator<int> > >, >>std::__debug::vector<int, std::allocator<int> > > (mutable >>iterator); >> state = dereferenceable; >> references sequence with type `std::__debug::vector<int, >>std::allocator<int> >' @ 0x0x7fff165d69d0 >>} >> >> >> * include/debug/formatter.h (_GLIBCXX_TYPEID): New macro to simplify >> usage of typeid. >> (_Error_formatter::_M_print_type): New. >> * src/c++11/debug.cc >> (_Error_formatter::_Parameter::_M_print_field): Use latter. >> (_Error_formatter::_M_print_type): Implement latter using >> __cxaabiv1::__cxa_demangle to print demangled type name. >> >>I just hope that __cxa_demangle is portable. > >It's provided by GCC itself so is always available in the runtime. >(It is also portable, because it's defined by the Itanium C++ ABI). > > >>Ok to commit ? > >Yes, this is great, thanks! Does this fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65392 ?
On 20/05/2015 12:19, Jonathan Wakely wrote: > On 20/05/15 11:17 +0100, Jonathan Wakely wrote: >> On 04/05/15 22:31 +0200, François Dumont wrote: >>> Hi >>> >>> Here is the patch to demangle symbols in debug messages. I have >>> also simplify code in formatter.h. >>> >>> Here is an example of assertion message: >>> >>> /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/functions.h:213: >>> >>> error: function requires a valid iterator range [__first, __last). >>> >>> Objects involved in the operation: >>> iterator "__first" @ 0x0x7fff165d68b0 { >>> type = >>> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, >>> std::__cxx1998::vector<int, std::allocator<int> > >, >>> std::__debug::vector<int, std::allocator<int> > > (mutable iterator); >>> state = dereferenceable; >>> references sequence with type `std::__debug::vector<int, >>> std::allocator<int> >' @ 0x0x7fff165d69d0 >>> } >>> iterator "__last" @ 0x0x7fff165d68e0 { >>> type = >>> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, >>> std::__cxx1998::vector<int, std::allocator<int> > >, >>> std::__debug::vector<int, std::allocator<int> > > (mutable iterator); >>> state = dereferenceable; >>> references sequence with type `std::__debug::vector<int, >>> std::allocator<int> >' @ 0x0x7fff165d69d0 >>> } >>> >>> >>> * include/debug/formatter.h (_GLIBCXX_TYPEID): New macro to simplify >>> usage of typeid. >>> (_Error_formatter::_M_print_type): New. >>> * src/c++11/debug.cc >>> (_Error_formatter::_Parameter::_M_print_field): Use latter. >>> (_Error_formatter::_M_print_type): Implement latter using >>> __cxaabiv1::__cxa_demangle to print demangled type name. >>> >>> I just hope that __cxa_demangle is portable. >> >> It's provided by GCC itself so is always available in the runtime. >> (It is also portable, because it's defined by the Itanium C++ ABI). >> >> >>> Ok to commit ? >> >> Yes, this is great, thanks! > > Does this fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65392 ? > With the patch this code of the bug report generates the following debug message: /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/safe_iterator.h:395: error: attempt to retreat a past-the-end iterator 2 steps, which falls outside its valid range. Objects involved in the operation: iterator @ 0x0x7fff32365c50 { type = __gnu_debug::_Safe_iterator<std::__cxx1998::_Deque_iterator<int, int&, int*>, std::__debug::deque<int, std::allocator<int> > > (mutable iterator); state = past-the-end; references sequence with type `std::__debug::deque<int, std::allocator<int> >' @ 0x0x7fff32365cd0 } which looks nice. However I wouldn't say that bug is fixed because debug mode do not generate mangle name, it simply rely on typeid to get it. Shouldn't bug report be saying so ? Whatever, symbol generated by typeid can be demangle by __cxa_demangle so it mustn't be that bad. François
On 20/05/15 21:45 +0200, François Dumont wrote: >On 20/05/2015 12:19, Jonathan Wakely wrote: >Does this fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65392 ? >> >With the patch this code of the bug report generates the following >debug message: > >/home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/safe_iterator.h:395: > error: attempt to retreat a past-the-end iterator 2 steps, which falls > outside its valid range. > >Objects involved in the operation: >iterator @ 0x0x7fff32365c50 { > type = >__gnu_debug::_Safe_iterator<std::__cxx1998::_Deque_iterator<int, int&, >int*>, std::__debug::deque<int, std::allocator<int> > > (mutable >iterator); > state = past-the-end; > references sequence with type `std::__debug::deque<int, >std::allocator<int> >' @ 0x0x7fff32365cd0 >} > >which looks nice. > >However I wouldn't say that bug is fixed because debug mode do not >generate mangle name, it simply rely on typeid to get it. Shouldn't >bug report be saying so ? Whatever, symbol generated by typeid can be >demangle by __cxa_demangle so it mustn't be that bad. I was trying to demangle the names with c++filt, which failed. Users should not have to write a C++ program using __cxa_demangle to read the output. If they are automatically demangled now then the bug is fixed.
diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h index 6767cd9..32dcf92 100644 --- a/libstdc++-v3/include/debug/formatter.h +++ b/libstdc++-v3/include/debug/formatter.h @@ -31,7 +31,17 @@ #include <bits/c++config.h> #include <bits/cpp_type_traits.h> -#include <typeinfo> + +#if __cpp_rtti +# include <typeinfo> +# define _GLIBCXX_TYPEID(_Type) &typeid(_Type) +#else +namespace std +{ + class type_info; +} +# define _GLIBCXX_TYPEID(_Type) 0 +#endif namespace __gnu_debug { @@ -218,21 +228,13 @@ namespace __gnu_debug { _M_variant._M_iterator._M_name = __name; _M_variant._M_iterator._M_address = &__it; -#if __cpp_rtti - _M_variant._M_iterator._M_type = &typeid(__it); -#else - _M_variant._M_iterator._M_type = 0; -#endif + _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it); _M_variant._M_iterator._M_constness = std::__are_same<_Safe_iterator<_Iterator, _Sequence>, typename _Sequence::iterator>:: __value ? __mutable_iterator : __const_iterator; _M_variant._M_iterator._M_sequence = __it._M_get_sequence(); -#if __cpp_rtti - _M_variant._M_iterator._M_seq_type = &typeid(_Sequence); -#else - _M_variant._M_iterator._M_seq_type = 0; -#endif + _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence); if (__it._M_singular()) _M_variant._M_iterator._M_state = __singular; @@ -256,21 +258,13 @@ namespace __gnu_debug { _M_variant._M_iterator._M_name = __name; _M_variant._M_iterator._M_address = &__it; -#if __cpp_rtti - _M_variant._M_iterator._M_type = &typeid(__it); -#else - _M_variant._M_iterator._M_type = 0; -#endif + _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it); _M_variant._M_iterator._M_constness = std::__are_same<_Safe_local_iterator<_Iterator, _Sequence>, typename _Sequence::local_iterator>:: __value ? __mutable_iterator : __const_iterator; _M_variant._M_iterator._M_sequence = __it._M_get_sequence(); -#if __cpp_rtti - _M_variant._M_iterator._M_seq_type = &typeid(_Sequence); -#else - _M_variant._M_iterator._M_seq_type = 0; -#endif + _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence); if (__it._M_singular()) _M_variant._M_iterator._M_state = __singular; @@ -291,11 +285,7 @@ namespace __gnu_debug { _M_variant._M_iterator._M_name = __name; _M_variant._M_iterator._M_address = &__it; -#if __cpp_rtti - _M_variant._M_iterator._M_type = &typeid(__it); -#else - _M_variant._M_iterator._M_type = 0; -#endif + _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it); _M_variant._M_iterator._M_constness = __mutable_iterator; _M_variant._M_iterator._M_state = __it? __unknown_state : __singular; _M_variant._M_iterator._M_sequence = 0; @@ -308,11 +298,7 @@ namespace __gnu_debug { _M_variant._M_iterator._M_name = __name; _M_variant._M_iterator._M_address = &__it; -#if __cpp_rtti - _M_variant._M_iterator._M_type = &typeid(__it); -#else - _M_variant._M_iterator._M_type = 0; -#endif + _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it); _M_variant._M_iterator._M_constness = __const_iterator; _M_variant._M_iterator._M_state = __it? __unknown_state : __singular; _M_variant._M_iterator._M_sequence = 0; @@ -325,11 +311,7 @@ namespace __gnu_debug { _M_variant._M_iterator._M_name = __name; _M_variant._M_iterator._M_address = &__it; -#if __cpp_rtti - _M_variant._M_iterator._M_type = &typeid(__it); -#else - _M_variant._M_iterator._M_type = 0; -#endif + _M_variant._M_iterator._M_type = _GLIBCXX_TYPEID(__it); _M_variant._M_iterator._M_constness = __unknown_constness; _M_variant._M_iterator._M_state = __gnu_debug::__check_singular(__it)? __singular : __unknown_state; @@ -345,11 +327,7 @@ namespace __gnu_debug _M_variant._M_sequence._M_name = __name; _M_variant._M_sequence._M_address = static_cast<const _Sequence*>(&__seq); -#if __cpp_rtti - _M_variant._M_sequence._M_type = &typeid(_Sequence); -#else - _M_variant._M_sequence._M_type = 0; -#endif + _M_variant._M_sequence._M_type = _GLIBCXX_TYPEID(_Sequence); } template<typename _Sequence> @@ -358,11 +336,7 @@ namespace __gnu_debug { _M_variant._M_sequence._M_name = __name; _M_variant._M_sequence._M_address = &__seq; -#if __cpp_rtti - _M_variant._M_sequence._M_type = &typeid(_Sequence); -#else - _M_variant._M_sequence._M_type = 0; -#endif + _M_variant._M_sequence._M_type = _GLIBCXX_TYPEID(_Sequence); } void @@ -439,6 +413,10 @@ namespace __gnu_debug _M_print_string(const char* __string) const; void + _M_print_type(const type_info* __info, + const char* __unknown_name) const; + + void _M_get_max_length() const throw (); enum { __max_parameters = 9 }; @@ -461,4 +439,6 @@ namespace __gnu_debug }; } // namespace __gnu_debug +#undef _GLIBCXX_TYPEID + #endif diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc index 5041c92..f51435a 100644 --- a/libstdc++-v3/src/c++11/debug.cc +++ b/libstdc++-v3/src/c++11/debug.cc @@ -35,6 +35,8 @@ #include <cstdlib> #include <functional> +#include <cxxabi.h> // for __cxa_demangle + using namespace std; namespace @@ -182,7 +184,8 @@ namespace __gnu_debug " container only holds %3; buckets", "load factor shall be positive", "allocators must be equal", - "attempt to insert with an iterator range [%1.name;, %2.name;) from this container" + "attempt to insert with an iterator range [%1.name;, %2.name;) from this" + " container" }; void @@ -525,8 +528,9 @@ namespace __gnu_debug const int __bufsize = 64; char __buf[__bufsize]; - if (_M_kind == __iterator) + switch (_M_kind) { + case __iterator: if (strcmp(__name, "name") == 0) { assert(_M_variant._M_iterator._M_name); @@ -539,14 +543,8 @@ namespace __gnu_debug __formatter->_M_print_word(__buf); } else if (strcmp(__name, "type") == 0) - { - if (!_M_variant._M_iterator._M_type) - __formatter->_M_print_word("<unknown type>"); - else - // TBD: demangle! - __formatter->_M_print_word(_M_variant._M_iterator. - _M_type->name()); - } + __formatter->_M_print_type(_M_variant._M_iterator._M_type, + "<unknown type>"); else if (strcmp(__name, "constness") == 0) { static const char* __constness_names[__last_constness] = @@ -581,19 +579,12 @@ namespace __gnu_debug __formatter->_M_print_word(__buf); } else if (strcmp(__name, "seq_type") == 0) - { - if (!_M_variant._M_iterator._M_seq_type) - __formatter->_M_print_word("<unknown seq_type>"); - else - // TBD: demangle! - __formatter->_M_print_word(_M_variant._M_iterator. - _M_seq_type->name()); - } + __formatter->_M_print_type(_M_variant._M_iterator._M_seq_type, + "<unknown seq_type>"); else assert(false); - } - else if (_M_kind == __sequence) - { + break; + case __sequence: if (strcmp(__name, "name") == 0) { assert(_M_variant._M_sequence._M_name); @@ -607,19 +598,12 @@ namespace __gnu_debug __formatter->_M_print_word(__buf); } else if (strcmp(__name, "type") == 0) - { - if (!_M_variant._M_sequence._M_type) - __formatter->_M_print_word("<unknown type>"); - else - // TBD: demangle! - __formatter->_M_print_word(_M_variant._M_sequence. - _M_type->name()); - } + __formatter->_M_print_type(_M_variant._M_sequence._M_type, + "<unknown type>"); else assert(false); - } - else if (_M_kind == __integer) - { + break; + case __integer: if (strcmp(__name, "name") == 0) { assert(_M_variant._M_integer._M_name); @@ -627,9 +611,8 @@ namespace __gnu_debug } else assert(false); - } - else if (_M_kind == __string) - { + break; + case __string: if (strcmp(__name, "name") == 0) { assert(_M_variant._M_string._M_name); @@ -637,10 +620,10 @@ namespace __gnu_debug } else assert(false); - } - else - { + break; + default: assert(false); + break; } } @@ -651,8 +634,9 @@ namespace __gnu_debug const int __bufsize = 128; char __buf[__bufsize]; - if (_M_kind == __iterator) + switch (_M_kind) { + case __iterator: __formatter->_M_print_word("iterator "); if (_M_variant._M_iterator._M_name) { @@ -700,9 +684,8 @@ namespace __gnu_debug __formatter->_M_print_word(__buf); } __formatter->_M_print_word("}\n"); - } - else if (_M_kind == __sequence) - { + break; + case __sequence: __formatter->_M_print_word("sequence "); if (_M_variant._M_sequence._M_name) { @@ -722,6 +705,9 @@ namespace __gnu_debug __formatter->_M_print_word(";\n"); } __formatter->_M_print_word("}\n"); + break; + default: + break; } } @@ -766,9 +752,10 @@ namespace __gnu_debug bool __has_noninteger_parameters = false; for (unsigned int __i = 0; __i < _M_num_parameters; ++__i) { - if (_M_parameters[__i]._M_kind == _Parameter::__iterator - || _M_parameters[__i]._M_kind == _Parameter::__sequence) + switch (_M_parameters[__i]._M_kind) { + case _Parameter::__iterator: + case _Parameter::__sequence: if (!__has_noninteger_parameters) { _M_first_line = true; @@ -776,6 +763,9 @@ namespace __gnu_debug __has_noninteger_parameters = true; } _M_parameters[__i]._M_print_description(this); + break; + default: + break; } } @@ -795,7 +785,6 @@ namespace __gnu_debug #endif } - void _Error_formatter::_M_print_word(const char* __word) const { @@ -932,6 +921,22 @@ namespace __gnu_debug } void + _Error_formatter::_M_print_type(const type_info* __info, + const char* __unknown_name) const + { + if (!__info) + _M_print_word(__unknown_name); + else + { + int __status; + char* __demangled_name = + __cxxabiv1::__cxa_demangle(__info->name(), NULL, NULL, &__status); + _M_print_word(__status == 0 ? __demangled_name : __info->name()); + free(__demangled_name); + } + } + + void _Error_formatter::_M_get_max_length() const throw () { const char* __nptr = std::getenv("GLIBCXX_DEBUG_MESSAGE_LENGTH");
Hi Here is the patch to demangle symbols in debug messages. I have also simplify code in formatter.h. Here is an example of assertion message: /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/functions.h:213: error: function requires a valid iterator range [__first, __last). Objects involved in the operation: iterator "__first" @ 0x0x7fff165d68b0 { type = __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> > > (mutable iterator); state = dereferenceable; references sequence with type `std::__debug::vector<int, std::allocator<int> >' @ 0x0x7fff165d69d0 } iterator "__last" @ 0x0x7fff165d68e0 { type = __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> > > (mutable iterator); state = dereferenceable; references sequence with type `std::__debug::vector<int, std::allocator<int> >' @ 0x0x7fff165d69d0 } * include/debug/formatter.h (_GLIBCXX_TYPEID): New macro to simplify usage of typeid. (_Error_formatter::_M_print_type): New. * src/c++11/debug.cc (_Error_formatter::_Parameter::_M_print_field): Use latter. (_Error_formatter::_M_print_type): Implement latter using __cxaabiv1::__cxa_demangle to print demangled type name. I just hope that __cxa_demangle is portable. Ok to commit ? François