Page MenuHomePhabricator

[libunwind] Add support for PC reg column in arm64
ClosedPublic

Authored by charco on Feb 17 2021, 2:24 PM.

Details

Reviewers
mcgrathr
phosek
Group Reviewers
Restricted Project
Commits
rG78eabcaa48df: [libunwind] Add support for PC reg column in arm64
Summary

This change adds support for the dwarf PC register column in arm64, allowing
CFI directives to make use of it.

As of the last revision of the DWARF for ARM 64-bit architecture[0], the pc
register has been added as a valir register, with number 32.

This allows libunwinder to restore both pc and lr, which is useful
for stack switches and signal contexts.

[0]:
https://github.com/ARM-software/abi-aa/blob/f52e1ad3f81254497a83578dc102f6aac89e52d0/aadwarf64/aadwarf64.rst

Diff Detail

Event Timeline

charco created this revision.Feb 17 2021, 2:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2021, 2:24 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
charco requested review of this revision.Feb 17 2021, 2:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2021, 2:24 PM

Could you add [libunwind] to the beginning of title?
but otherwise LGTM as the author of the corresponding dwarf spec change.

Thanks for this patch.

phosek accepted this revision.Feb 17 2021, 5:13 PM

LGTM

This revision is now accepted and ready to land.Feb 17 2021, 5:13 PM
charco retitled this revision from Add support for PC reg column in arm64 to [libunwind] Add support for PC reg column in arm64.Feb 17 2021, 5:20 PM
charco updated this revision to Diff 324475.Feb 17 2021, 5:23 PM

Add [libunwind] to the commit message.

This revision was automatically updated to reflect the committed changes.

Could you add [libunwind] to the beginning of title?
but otherwise LGTM as the author of the corresponding dwarf spec change.

Thanks for this patch.

Thanks for the review! Maybe with this change the sigreturn code for linux can be simplified? (see https://reviews.llvm.org/D90898 for context)

joerg added a subscriber: joerg.Feb 17 2021, 5:52 PM

I don't get this change. The restore code is literally returning by jumping to x30, so how is splitting PC and LR supposed to work?

I don't get this change. The restore code is literally returning by jumping to x30, so how is splitting PC and LR supposed to work?

My understanding is that it would be used with .cfi_return_column 32, so x30 is no longer used as the return address. I tested it locally in Fuchsia and it worked.

joerg added a comment.Feb 17 2021, 6:13 PM

Let me rephrase then. The restore code explicitly ends in "ret x30". It cannot set "pc" separate from "x30" in the current form. I'm not saying that the DWARF register shouldn't be supported, but this change doesn't do that.

Let me rephrase then. The restore code explicitly ends in "ret x30". It cannot set "pc" separate from "x30" in the current form. I'm not saying that the DWARF register shouldn't be supported, but this change doesn't do that.

I am not really familiar with libunwind. What restore code are you talking about? If there's anything else that needs to be changed, I would be happy to do so.

joerg added a comment.Feb 17 2021, 6:35 PM

Registers_arm64::jumpto in UnwindRegistersRestore.S.

Registers_arm64::jumpto in UnwindRegistersRestore.S.

Isn't that code filling x30 with the __pc register?

Line 587 says: ldr x30, [x0, #0x100] // restore pc into lr

And offset #0x100 is __pc

Registers_arm64::jumpto in UnwindRegistersRestore.S.

Isn't that code filling x30 with the __pc register?

Line 587 says: ldr x30, [x0, #0x100] // restore pc into lr

And offset #0x100 is __pc

Ah, but we are not restoring the x30 register.