On Thu, Oct 16, 2025 at 2:13 PM Jonathan Wakely <[email protected]> wrote:
> On Thu, 16 Oct 2025 at 12:53, Tomasz Kaminski <[email protected]> wrote: > > > > > > > > On Thu, Oct 16, 2025 at 1:24 PM Jonathan Wakely <[email protected]> > wrote: > >> > >> With this change locations that don't have a source file look like this: > >> > >> 18# __libc_start_call_main at [0x7fd6568f6574] > >> 19# __libc_start_main@GLIBC_2.2.5 at [0x7fd6568f6627] > >> 20# _start at [0x4006a4] > >> > >> Instead of: > >> > >> 18# __libc_start_call_main at :0 > >> 19# __libc_start_main@GLIBC_2.2.5 at :0 > >> 20# _start at :0 > >> > >> For a non-empty stacktrace_entry, if the function name returned by > >> description() is an empty string we now print "<unknown>" for the > >> function name. For an empty (e.g. default constructed) stacktrace_entry > >> the entire string representation is now "<unknown>" instead of an empty > >> string. > >> > >> Instead of printing "<unknown>" for the function name, we could set that > >> string in the stacktrace_entry::_Info object, so that description() > >> returns "<unknown>" and then operator<< wouldn't need to handle an empty > >> description() string. However, returning an empty string from that > >> function seems simpler for users to detect, rather than having to parse > >> "<unknown>". > >> > >> We could also choose a different string for an empty stacktrace_entry, > >> maybe "<none>" or "<invalid>", but "<unknown>" seems OK for now. > >> > >> libstdc++-v3/ChangeLog: > >> > >> * include/std/stacktrace > >> (operator<<(ostream&, const stacktrace_entry&)): Improve output > >> when description() or source_file() returns an empty string, > >> or the stacktrace_entry is invalid. > >> (operator<<(ostream&, const basic_stacktrace<A>&)): Use > >> size_type of the correct specialization. > >> --- > >> > >> v2: Restore fmtflags as pointed out by Nathan. Adjust control flow in > >> operator<< as suggested by Tomasz. > >> > >> Tested x86_64-linux. > >> > >> libstdc++-v3/include/std/stacktrace | 29 +++++++++++++++++++++++++---- > >> 1 file changed, 25 insertions(+), 4 deletions(-) > >> > >> diff --git a/libstdc++-v3/include/std/stacktrace > b/libstdc++-v3/include/std/stacktrace > >> index b9e260e19f89..28ffda76328f 100644 > >> --- a/libstdc++-v3/include/std/stacktrace > >> +++ b/libstdc++-v3/include/std/stacktrace > >> @@ -685,11 +685,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > >> { > >> string __desc, __file; > >> int __line; > >> - if (__f._M_get_info(&__desc, &__file, &__line)) > >> + if (__f._M_get_info(&__desc, &__file, &__line)) [[likely]] > >> { > >> - __os.width(4); > >> - __os << __desc << " at " << __file << ':' << __line; > >> + if (__desc.empty()) [[unlikely]] > >> + __os << "<unknown>"; > >> + else > >> + __os << __desc; > >> + __os << string_view(" at "); > >> + if (!__file.empty()) [[likely]] > >> + return __os << __file << ':' << __line; > >> + // else fall through and write native_handle() as the location. > >> } > >> + > >> + if (__f) [[likely]] > >> + { > >> + struct _Flag_guard > >> + { > >> + ios_base& _M_ios; > >> + ios_base::fmtflags _M_f; > >> + ~_Flag_guard() { _M_ios.setf(_M_f); } > >> + }; > >> + _Flag_guard _ = { __os, __os.setf(ios::hex, ios::basefield) }; > >> + __os << "[0x" << __f.native_handle() << ']'; > >> + } > >> + else > >> + __os << "<unknown>"; > >> return __os; > > > > I know that I ask for another change ag, but if (!__f) we will not ever > get info for it, so I would prefer: > > if (!__f) [[unlikelly] > > return __os << "<unknown>"; > > > > string __desc, __file; > > int __line; > > if (__f._M_get_info(&__desc, &__file, &__line)) [[likely]] > > { > > // unchanged > > } > > > > struct _Flag_guard > > { > > ios_base& _M_ios; > > ios_base::fmtflags _M_f; > > ~_Flag_guard() { _M_ios.setf(_M_f); } > > }; > > _Flag_guard _ = { __os, __os.setf(ios::hex, ios::basefield) }; > > __os << "[0x" << __f.native_handle() << ']'; > > return __os; > > > > This way it is clear, that we never produce <unknown> at <unknown> > > > > OK, that makes sense. > > Ignoring the details of the logic in the function for now, the output > currently looks like: > > 15# myfunc(int) at /tmp/bt.cc:48 > 16# myfunc(int) at /tmp/bt.cc:48 > 17# main at /tmp/bt.cc:61 > 18# __libc_start_call_main at [0x7f56578d3574] > 19# __libc_start_main@GLIBC_2.2.5 at [0x7f56578d3627] > 20# _start at [0x400684] > > We could output the hex address even when we have a filename, e.g. > > 15# myfunc(int) at /tmp/bt.cc:48 [0x4008b7] > 16# myfunc(int) at /tmp/bt.cc:48 [0x4008b7] > 17# main at /tmp/bt.cc:61 [0x40091a] > 18# __libc_start_call_main [0x7f0682923574] > 19# __libc_start_main@GLIBC_2.2.5 [0x7f0682923627] > 20# _start [0x400684] > Maybe we could put the hex address at the front, and then pad it 15# [0x4008b7] myfunc(int) at /tmp/bt.cc:48 16# [0x4008b7] myfunc(int) at /tmp/bt.cc:48 17# [0x40091a] main at /tmp/bt.cc:61 18# [0x7f0682923574] __libc_start_call_main 19# [0x7f0682923627] __libc_start_main@GLIBC_2.2.5 20# [0x400684] _start And then maybe pad the address. > > So only write the " at " string when we have a filename, but always > show the hex address. > > This could be useful if the file:line is the location of a macro > definition and several functions have the same file:line, but would > still have a different address. > > So something like: > > if (!__f) [[unlikely]] > return __os << "<unknown>"; > > string __desc, __file; > int __line; > if (__f._M_get_info(&__desc, &__file, &__line)) [[likely]] > { > if (__desc.empty()) [[unlikely]] > __os << "<unknown>"; > else > __os << __desc; > if (!__file.empty()) [[likely]] > __os << " at " << __file << ':' << __line; > } > > struct _Flag_guard > { > ios_base& _M_ios; > ios_base::fmtflags _M_f; > ~_Flag_guard() { _M_ios.setf(_M_f); } > }; > _Flag_guard _ = { __os, __os.setf(ios::hex, ios::basefield) }; > __os << " [0x" << __f.native_handle() << ']'; > > return __os; > >
