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

[SYSVABI64] Add Guarded Control Stack (GCS) Feature Bit #231

Merged
merged 2 commits into from
Dec 11, 2023

Conversation

smithp35
Copy link
Contributor

Add GNU_PROPERTY_AARCH64_FEATURE_1_GCS to the GNU property GNU_PROPERTY_AARCH64_FEATURE_1_AND.

This permits executables and shared libraries to declare that they are compatible with GCS, this permits a platform to enable GCS for a program when the executable and all of its dependecies are compatible with it.

When GCS is enabled, use of dlopen of a shared library that is not GCS compatible is platform defined. Some platforms may be able to disable GCS, others may decide to refuse to load the library.

Add GNU_PROPERTY_AARCH64_FEATURE_1_GCS to the GNU property
GNU_PROPERTY_AARCH64_FEATURE_1_AND.

This permits executables and shared libraries to declare
that they are compatible with GCS, this permits a platform
to enable GCS for a program when the executable and all of
its dependecies are compatible with it.

When GCS is enabled, use of dlopen of a shared library that
is not GCS compatible is platform defined. Some platforms
may be able to disable GCS, others may decide to refuse to
load the library.
(GCS) mechanism. Minimum requirements for setting this feature bit
include:

* The number of ``procedure return address push operations`` and the

Choose a reason for hiding this comment

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

The terms "procedure return address push operation" and "procedure return address pop operation" are defined only in the GCS specification (as far as I know) and are perhaps a bit misleading here (on first reading I thought it was talking about the STP at function start and LDP at function end, but it's actually talking about BL and RET). Also having a matching number of BL/RET isn't enough, every RET has to match up with the BL that called the function.

I think rewording this to be something like the following would be clearer:

Each function that is called using a BL instruction (or other instruction that is a GCS procedure return address push operation) returns using a RET instruction (or other instruction that is a GSC procedure return address pop operation). This means that RET instructions are used for function returns, and not as an indirect branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, I've used an almost identical wording in a fixup commit that I'll squash prior to merging.

* Each function that is called using a BL instruction (or other
instruction that is a GCS ``procedure return address push
operation``) returns using a RET instruction (or other instruction
that is a ``GCS procedure return address pop operation``). This

Choose a reason for hiding this comment

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

Mismatch on whether GCS is inside the backticks. Though maybe we shouldn't be using backticks here, as it's usually used only for code-like things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies for the mistake. Looking at the spec it should be outside. I want to highlight that procedure return address push operation is a term from a specification, that I expect will eventually be in the Arm ARM as a highlighted term. Without it procedure return address push operation could just be a generic sentence.

Will fix.

This commit will be squashed before merging.
Copy link

@john-brawn-arm john-brawn-arm left a comment

Choose a reason for hiding this comment

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

LGTM.

@smithp35 smithp35 merged commit 24e93a6 into ARM-software:main Dec 11, 2023
1 check passed
john-brawn-arm added a commit to john-brawn-arm/llvm-project that referenced this pull request Dec 11, 2023
john-brawn-arm added a commit to llvm/llvm-project that referenced this pull request Dec 13, 2023
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.

4 participants