On Thu, 16 Oct 2025 at 13:22, Tomasz Kaminski <[email protected]> wrote: > > > > 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.
That looks OK for 32-bit targets: 15# [0x080487f4] myfunc(int) at /tmp/bt.cc:48 16# [0x080487f4] myfunc(int) at /tmp/bt.cc:48 17# [0x0804885c] main at /tmp/bt.cc:61 18# [0xf7944060] __libc_start_call_main 19# [0xf7944137] __libc_start_main@GLIBC_2.0 20# [0x08048587] _start but the padding makes it look very noisy for 64-bit: 15# [0x00000000004008c7] myfunc(int) at /tmp/bt.cc:48 16# [0x00000000004008c7] myfunc(int) at /tmp/bt.cc:48 17# [0x000000000040092a] main at /tmp/bt.cc:61 18# [0x00007f8dd65e4574] __libc_start_call_main 19# [0x00007f8dd65e4627] __libc_start_main@GLIBC_2.2.5 20# [0x0000000000400694] _start That's assuming we pad with zeros. With spaces (and right-aligned) it looks a bit strange IMHO: 15# [0x400917] myfunc(int) at /tmp/bt.cc:48 16# [0x400917] myfunc(int) at /tmp/bt.cc:48 17# [0x40097a] main at /tmp/bt.cc:61 18# [0x7f900a411574] __libc_start_call_main 19# [0x7f900a411627] __libc_start_main@GLIBC_2.2.5 20# [0x4006e4] _start I think I'll push the version that adds the address at the end, unpadded, and we can revisit it later if we want to.
