This is an archive of the discontinued LLVM Phabricator instance.

[libunwind][SystemZ] Unwind out of signal handlers
ClosedPublic

Authored by uweigand on May 2 2022, 5:46 AM.

Details

Reviewers
MaskRay
ldionne
Group Reviewers
Restricted Project
Commits
rG71672375fe91: [libunwind][SystemZ] Unwind out of signal handlers
Summary

Unwinding out of signal handlers currently does not work since the sigreturn trampoline is not annotated with CFI data.

Fix this by detecting the sigreturn trampoline during unwinding and providing appropriate unwind data manually. This follows closely the approach used by existing code for the AArch64 target.

Diff Detail

Event Timeline

uweigand created this revision.May 2 2022, 5:46 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 2 2022, 5:46 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
uweigand requested review of this revision.May 2 2022, 5:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2022, 5:46 AM

some tests from libcxxabi could be enabled for SystemZ too e.g. libcxxabi/test/forced*

libunwind/src/UnwindCursor.hpp
2673

Please note I recently committed this:
https://reviews.llvm.org/D124522

_info.end_ip need to be set to make forced unwind work.

_info.start_ip = pc;
_info.end_ip = pc;
uweigand updated this revision to Diff 426398.May 2 2022, 6:40 AM

Address review comment.

some tests from libcxxabi could be enabled for SystemZ too e.g. libcxxabi/test/forced*

Unfortunately we don't yet support libcxx or libcxxabi at all on SystemZ. (On Linux there's always libstdc++ available as system library.)

libunwind/src/UnwindCursor.hpp
2673

Thanks! I've updated the patch accordingly.

MaskRay added inline comments.May 2 2022, 1:55 PM
libunwind/src/UnwindCursor.hpp
2667

Does s390 use both __kernel_[rt_]sigreturn? It seems that the Linux kernel source references both. For aarch64 there is only the rt_ version.

Just a note for you to double check the comment:)

2684

Best to leave a comment where 160 comes from. It can be a kernel source filename.

2691
2731

Prefer [a,b) ranges

2736

Prefer [a,b) ranges

2737
libunwind/test/signal_unwind.pass.cpp
11

Use an alphabetical order: aarch64, s390x, x86_64

libunwind/test/unwind_leaffunction.pass.cpp
11

Use an alphabetical order: aarch64, s390x, x86_64

uweigand marked 9 inline comments as done.May 3 2022, 12:36 AM
uweigand added inline comments.
libunwind/src/UnwindCursor.hpp
2667

Yes, we use both - we have two distinct sigreturn vs. rt_sigreturn syscalls (see also the distinction in stepThroughSigreturn below), and there's a VDSO trampoline for each of them.

2684

This is a general property of the s390x ABI (the CFA is always at "incoming SP + 160"). I'll add a comment to that effect.

uweigand updated this revision to Diff 426590.May 3 2022, 12:37 AM
uweigand marked 2 inline comments as done.

Address review comments.

MaskRay accepted this revision.May 3 2022, 4:55 PM
This revision is now accepted and ready to land.May 3 2022, 4:55 PM
This revision was landed with ongoing or failed builds.May 4 2022, 1:45 AM
This revision was automatically updated to reflect the committed changes.