This is an archive of the discontinued LLVM Phabricator instance.

[libunwind][SystemZ] Use process_vm_readv to avoid potential segfaults
ClosedPublic

Authored by uweigand on Jul 15 2022, 7:42 AM.

Details

Summary

Fix potential crashes during unwind when checking for signal frames and the current PC is invalid.

The same bug was fixed for aarch64 in https://reviews.llvm.org/D126343.

Diff Detail

Event Timeline

uweigand created this revision.Jul 15 2022, 7:42 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 15 2022, 7:42 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
uweigand requested review of this revision.Jul 15 2022, 7:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2022, 7:42 AM
MaskRay accepted this revision.Jul 16 2022, 12:06 AM
This revision is now accepted and ready to land.Jul 16 2022, 12:06 AM

process_vm_readv was introduced in Linux 3.2; is it okay to use it unconditionally for you? (aarch64 support was only added in Linux 3.7, so we were guaranteed to have the syscall available for it.)

libunwind/src/UnwindCursor.hpp
2705

For aarch64, I went through syscall instead of the libc process_vm_readv wrapper because the latter wasn't guaranteed to be present even when the kernel supported the syscall (e.g. on Android 5). If you don't have that constraint, it'd be cleaner to just use the libc wrapper (assuming it's available on SystemZ).

This revision was landed with ongoing or failed builds.Jul 18 2022, 7:57 AM
This revision was automatically updated to reflect the committed changes.

process_vm_readv was introduced in Linux 3.2; is it okay to use it unconditionally for you? (aarch64 support was only added in Linux 3.7, so we were guaranteed to have the syscall available for it.)

Well, while s390x Linux support long precedes Linux 3.2, it's still over 10 years old and all Linux distributions currently in support have a more recent kernel. So I think it's fine to rely on in unconditionally.

libunwind/src/UnwindCursor.hpp
2705

Good point. All supported SystemZ Linux distributions have a recent enough glibc, so I've switched to the glibc wrapper.