Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AAELF64][PAUTHABI64] Assign PAuthABI relocation codes. #227

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

smithp35
Copy link
Contributor

Now that there will be a concrete implementation of PAuthABI in upstream LLVM we need to move the PAuthABI relocation codes out of the space reserved for private experiments to their final codes. These are in the range of codes already reserved for the PAuthABI in AAELF64.

The description of the operations will remain in the PAuthABI64 as this remains an extension.

@@ -698,7 +700,7 @@ Static Data relocations
| | | | |
| | | | |
+=============+==========================+================+=====================================================+
| 0xE100 | R\_AARCH64\_AUTH\_ABS64 | ``PAUTH(S+A)`` | Signing schema encoded in the contents of the place |
| 0x244 | R\_AARCH64\_AUTH\_ABS64 | ``PAUTH(S+A)`` | Signing schema encoded in the contents of the place |
Copy link

@asl asl Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are having two relocations named R_AARCH64_AUTH_ABS64 with different ids (static and dynamic). Should this be named R_AARCH64_AUTH_ABS64_STATIC as otherwise there is an ambiguity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same as R_AARCH64_ABS64 which can be both a static and dynamic relocation. I followed that convention as R_AARCH64_AUTH_ABS64 is its PAuth equivalent.

Changing codes is possible but I thought it better to stick the existing convention.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not the code, it's the name. Essentially we cannot put these two relocations into a single enum – we'd need to rename one entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think it is the code, and I've made a mistake, sorry.

The intention is that the name and code remain the same. In the original both had 0xE100, but in the new one I used the first code in the dynamic range, it needed to retain its original 0x244. I'll update the pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the value. I had it right in aaelf64.rst but not here.

kovdan01 added a commit to kovdan01/llvm-project that referenced this pull request Dec 4, 2023
ARM-software/abi-aa#227 changes IDs of
`R_AARCH64_AUTH_ABS64` and `R_AARCH64_AUTH_RELATIVE` in PAuth ABI
specification from draft ones (`0xe100` and `0xe200`) to final
ones (`0x244` and `0x411`).

This patch changes the values in llvm correspondingly.
kovdan01 added a commit to kovdan01/llvm-project that referenced this pull request Dec 10, 2023
ARM-software/abi-aa#227 changes IDs of
`R_AARCH64_AUTH_ABS64` and `R_AARCH64_AUTH_RELATIVE` in PAuth ABI
specification from draft ones (`0xe100` and `0xe200`) to final
ones (`0x244` and `0x411`).

This patch changes the values in llvm correspondingly.
kovdan01 added a commit to llvm/llvm-project that referenced this pull request Dec 10, 2023
ARM-software/abi-aa#227 changes IDs of
`R_AARCH64_AUTH_ABS64` and `R_AARCH64_AUTH_RELATIVE` in PAuth ABI
specification from draft ones (`0xe100` and `0xe200`) to final ones
(`0x244` and `0x411`).

This patch changes the values in llvm correspondingly.
@asl
Copy link

asl commented Mar 4, 2024

Just to note: the current implementation in LLVM is done according to this spec.

@smithp35
Copy link
Contributor Author

smithp35 commented Mar 5, 2024

I'll set a review deadline for 12/05/2024, I'll merge unless there are any objections.

1 similar comment
@smithp35
Copy link
Contributor Author

smithp35 commented Mar 5, 2024

I'll set a review deadline for 12/05/2024, I'll merge unless there are any objections.

Now that there will be a concrete implementation of PAuthABI in upstream
LLVM we need to move the PAuthABI relocation codes out of the space
reserved for private experiments to their final codes. These are in
the range of codes already reserved for the PAuthABI in AAELF64.

The description of the operations will remain in the PAuthABI64 as
this remains an extension.
@smithp35
Copy link
Contributor Author

Rebased changes prior to merge

@smithp35 smithp35 merged commit 38f7625 into ARM-software:main Mar 18, 2024
1 check passed
Copy link

@sallyarmneale sallyarmneale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants