This is an archive of the discontinued LLVM Phabricator instance.

Fix sanitizer stack traces on aarch64.
ClosedPublic

Authored by rsundahl on Apr 20 2022, 6:41 PM.

Details

Summary

The bp (base pointer) variable was being loaded from register LR and
not FP on aarch64 (except for this narrow case):
defined(IPHONE_8_0) && IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_8_0

Without a valid bp from the FP register, it is not possible to traverse
previous frames for a complete stack trace. The rationale for fetching
the LR as the bp for all cases except above is not clear but since the
FP register is the canonical register for use as the frame pointer, this
commit removes the restriction above for unconditional use all aarch64.

Diff Detail

Event Timeline

rsundahl created this revision.Apr 20 2022, 6:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2022, 6:41 PM
rsundahl requested review of this revision.Apr 20 2022, 6:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2022, 6:41 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
rsundahl updated this revision to Diff 424076.Apr 20 2022, 6:58 PM

Fixed markup issue (double underlines) in summary.

yln added a comment.Apr 21 2022, 1:46 PM

arm64 and iOS support were introduced at the same time, but without mention why a special case is necessary:
https://reviews.llvm.org/D10510

One possible explanation for not using FP could be -fomit-frame-pointer. Is this option enabled on higher optimization levels? Do we always implicitly do -fno-omit-frame-pointer even on higher optimization levels to increase debuggability on iOS/Darwin?

Please also add a test or specify which existing test this fixes (on macOS/arm64).

yln accepted this revision.Apr 21 2022, 1:49 PM
yln added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
910

Note to self: this is already a Darwin-specific file, so we are not changing behavior for any other platforms other than "non-iOS Darwin".

This revision is now accepted and ready to land.Apr 21 2022, 1:49 PM
This revision was landed with ongoing or failed builds.Apr 21 2022, 2:28 PM
This revision was automatically updated to reflect the committed changes.