[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

[Daniel Kiss via Phabricator via llvm-commits]

danielkiss added a comment.

> If any function has the attribute "sign-return-address", then the output note
>  section should have PAC bit set. The return address signing is completely local
>  to the function, and functions with or without return address signing can be
>  freely mixed with each other.

That is true PAC and non-PAC functions can be mixed. 
Does one function makes the "all executable sections" pac-ret enabled?
BTW `GNU_PROPERTY_AARCH64_FEATURE_1_PAC` is not really used for anything.

> Likewise, if any function has the attribute "branch-target-enforcement", then
>  the output note section should have the BTI flag set. Even though code compiled
>  with BTI is not necessarily compatible with non-BTI code:
> 
> - the only way to get BTI code is by explicit use of `-mbranch-protection=...` command-line option, or the corresponding attribute, which we should consider a clear indication about the user's intent to use BTI.
> - the only way to get a mix of present/non-present "branch-target-enforcement" attributes is by the explicit use of the `__attribute__((target("branch-protection=..."))`, in which case we should assume the user knows what they are doing.

`__ARM_FEATURE_PAC_DEFAULT` and `__ARM_FEATURE_BTI_DEFAULT` controlled by the `-mbranch-protection=...` 
https://developer.arm.com/documentation/101028/0011/Feature-test-macros?lang=en

One of the reasons of the introduction of these macros is the management of the function attributes.
For example:

  #ifdef __ARM_FEATURE_PAC_DEFAULT
  #ifdef __ARM_FEATURE_BTI_DEFAULT
  #define NO_PAC_FUNC __attribute__((target("branch-protection=bti")))
  #else
  #define NO_PAC_FUNC __attribute__((target("branch-protection=none")))
  #endif /* __ARM_FEATURE_BTI_DEFAULT */
  ...

In my humble opinion the function attribute is there to alter global setting.
I considered to propagate the function attribute to the module flags but 
that would lead to inconsistent compilation with the macros that I'd avoid.

> What do to if there are no functions in the compile unit?
> 
> Technically, objects produced from such a unit are fully compatible with both PAC and BTI, which
>  means both flags should be set. But looking at the (non-existent) function attributes alone does
>  not allow us to unambiguously derive a user's intent to use PAC/BTI. In this case, I would suggest
>  setting the ELF note flags, according to the LLVM IR module flags.

I think the only clear indication from the user to use PAC/BTI is the explicit use of `-mbranch-protection=...` command-line option.
A few function attributes that would turn PAC/BTI on just on those few functions makes no sense for me in any real world application. 
Valid to turn off PAC/BTI on selected functions while the whole application compiled with them.

We need to turn PAC off on the code path where we change\manage the keys for example.
Exaggerated example for BTI: https://godbolt.org/z/Y9bhe9  Current version of llvm issues a warning and won't emit the note while I think it should.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80791/new/

https://reviews.llvm.org/D80791