chill added a comment.

In D80791#2164543 <https://reviews.llvm.org/D80791#2164543>, @danielkiss wrote:

>> 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?

Yes, the presence of even a single function is a clear indication of what the 
user whats - enable PAC/BTI.
The default is not having PAC/BTI code gen, therefore its presence is a result 
of a deliberate action by the user, 
therefore unambiguously conveys the user's intent.

> BTW `GNU_PROPERTY_AARCH64_FEATURE_1_PAC` is not really used for anything.

I may not be used today in GNU/Linux, but still, it has to have sensible 
semantics.

> 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 */
>   ...

I don't see how this example is relevant to the discussion of what notes to 
emit.
Our starting point is we have some default state (in module flags or whatever), 
some
individual function state and we have to decide what notes to emit, 
//regardless of the specific way
we came up with those function attributes.//

> 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.

The compilation of a single given function does not necessarily need to be
consistent with the value of these macros. Quite the opposite really, the 
macros themselves are
suffixed by `_DEFAULT` in order to explicitly acknowledge that possibility.

>> 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.

Using the attribute is no less clear and even carries more weight, as it 
overrides the 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.

Turning on/off PAC/BTI is completely symmetrical - one can achieve exactly the 
same effect with:

- command-line options enabling PAC/BTI and individual attributes disabling BTI
- command-line options disabling PAC/BIT (e.g. not having a command-line option 
at all) and individual attributes enabling it

We shouldn't be guessing and prescribing how applications should use the 
mechanisms we make available and certainly
shouldn't be judging what is a real-world application and what is not.

> 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.

Just as valid is to turn on PAC/BTI on selected functions, while the while 
compilation unit (*not* application) is compiled without them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80791

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to