This is an archive of the discontinued LLVM Phabricator instance.

[libc++][WIP] Implement P0881R7 (std::stacktrace)
AbandonedPublic

Authored by philnik on Apr 6 2022, 8:43 AM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

The tests are currently incomplete. There are also still some other things not implemented that are required here.

Diff Detail

Event Timeline

philnik created this revision.Apr 6 2022, 8:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 8:43 AM
philnik requested review of this revision.Apr 6 2022, 8:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 8:43 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Nice see this being worked on!
I mainly looked at the code out of curiosity, I didn't do a review.

libcxx/include/__stacktrace/basic_stacktrace.h
24

Since this function is in the dylib it needs an availability macro in <__availability>.

libcxx/src/stacktrace/stacktrace_entry.cpp
41

When line 45 throws the pointer leaks.
I would suggest to do something like

// Unused; the returned pointer is used to test the conversion status.
int __s;
unique_ptr<char, void (*)(void*)> __demangled{abi::__cxa_demangle(__name, nullptr, 0, &__s), std::free};

(I wrote this for a libc++ formatting experiment.)

philnik updated this revision to Diff 421172.Apr 7 2022, 5:47 AM
  • Fix CI
philnik marked 2 inline comments as done.Apr 10 2022, 5:17 AM
philnik added inline comments.
libcxx/src/stacktrace/stacktrace_entry.cpp
41

Thanks, good catch!

philnik updated this revision to Diff 421783.Apr 10 2022, 5:17 AM
philnik marked an inline comment as done.
  • Fix CI
  • Add more tests
philnik updated this revision to Diff 421880.Apr 11 2022, 5:04 AM
  • Remove non-ASCII characters
philnik updated this revision to Diff 422433.Apr 13 2022, 1:45 AM
  • Try to fix CI
philnik updated this revision to Diff 426302.May 1 2022, 11:31 AM
  • Add DWARF 5 32bit (not actually working)
tschuett added inline comments.
libcxx/src/stacktrace/stacktrace_entry.cpp
36

I do not know about the libcxx coding style, but LLVM prefers static over functions in anonymous namespaces. I believe there are quite a few functions that you could make static.

philnik updated this revision to Diff 433528.Jun 1 2022, 1:29 PM
  • DWARF works now for simple programs
Febbe added a subscriber: Febbe.Jun 15 2023, 5:34 AM

What prevents this from a merge? I don't think, that you must implement / support all binary formats to merge this. For now, ELF + DWARF should be enough, right?

What prevents this from a merge? I don't think, that you must implement / support all binary formats to merge this. For now, ELF + DWARF should be enough, right?

If that worked it would be a start, but this doesn't. I think we need some external library (maybe something from LLDB) to support a reasonable set of binaries, so I haven't continued working on this.

Febbe added a comment.Jun 16 2023, 8:12 AM

You could take a look at libbacktrace. The GNU stdlibc++ is using it to generate stacktraces.

philnik abandoned this revision.Oct 1 2023, 7:32 AM

You could take a look at libbacktrace. The GNU stdlibc++ is using it to generate stacktraces.

That doesn't work for licensing reasons.