This is an archive of the discontinued LLVM Phabricator instance.

[libunwind] Unwind through aarch64/FreeBSD sigreturn frame
Needs ReviewPublic

Authored by dchagin on Jul 12 2023, 4:00 AM.

Details

Reviewers
dim
emaste
Group Reviewers
Restricted Project
Summary

Similar to D90898 (Linux Aarch64).

Diff Detail

Event Timeline

dchagin created this revision.Jul 12 2023, 4:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2023, 4:00 AM
dchagin requested review of this revision.Jul 12 2023, 4:00 AM
dchagin set the repository for this revision to rG LLVM Github Monorepo.Jul 12 2023, 4:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2023, 4:55 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
MaskRay added a subscriber: MaskRay.
MaskRay added inline comments.
libunwind/src/UnwindCursor.hpp
2918

Not indented or are you using tabs? We don't use tabs.

Use clang/tools/clang-format/clang-format-diff.py to format modified lines.

dchagin updated this revision to Diff 539887.Jul 13 2023, 1:04 AM

Done, thanks!

dchagin marked an inline comment as done.Jul 18 2023, 12:55 AM
emaste added inline comments.Jul 21 2023, 5:08 AM
libunwind/include/__libunwind_config.h
38–39

match indentation to existing Linux cases I think

dchagin updated this revision to Diff 542872.Jul 21 2023, 5:31 AM

It looks like clang-format is changing the indents. Done, thanks, Ed

dchagin marked an inline comment as done.Jul 21 2023, 5:31 AM
arichardson added inline comments.
libunwind/src/UnwindCursor.hpp
2906

this function is not aarch64 specific and I assume would be shared by other architectures that support it. I'd guard this block with _LIBUNWIND_CHECK_FREEBSD_SIGRETURN and UnwindCursor<A, R>::stepThroughSigReturn(Registers_arm64 &) with a nested defined(_LIBUNWIND_TARGET_AARCH64) so that other architectures can be added more easily.

2907
2919–2921

If the sysctl fails for some reason we would end up calling it every time. How about setting start and end to zero in that case to avoid calling it again? Also probably makes sense to log an error if the sysctl fails.

2934–2959

Maybe something like this for future extensibility?