Re: [PATCH 2/3][GCC][AARCH64] Add new -mbranch-protection option to combine pointer signing and BTI

[Sam Tebbs <Sam dot Tebbs at arm dot com>]

On 11/22/18 4:54 PM, Sam Tebbs wrote:
> On 11/12/18 6:24 PM, Sudakshina Das wrote:
>> Hi Sam
>>
>> On 02/11/18 17:31, Sam Tebbs wrote:
>>> Hi all,
>>>
>>> The -mbranch-protection option combines the functionality of
>>> -msign-return-address and the BTI features new in Armv8.5 to better reflect
>>> their relationship. This new option therefore supersedes and deprecates the
>>> existing -msign-return-address option.
>>>
>>> -mbranch-protection=[none|standard|<types>] - Turns on different types of branch
>>> protection available where:
>>>
>>>         * "none": Turn of all types of branch protection
>>>         * "standard" : Turns on all the types of protection to their respective
>>>           standard levels.
>>>         * <types> can be "+" separated protection types:
>>>
>>>         	* "bti" : Branch Target Identification Mechanism.
>>> 	* "pac-ret{+leaf+b-key}": Return Address Signing. The default return
>>> 	  address signing is enabled by signing functions that save the return
>>> 	  address to memory (non-leaf functions will practically always do this)
>>> 	  using the a-key. The optional tuning arguments allow the user to
>>> 	  extend the scope of return address signing to include leaf functions
>>> 	  and to change the key to b-key. The tuning arguments must proceed the
>>> 	  protection type "pac-ret".
>>>
>>> Thus -mbranch-protection=standard -> -mbranch-protection=bti+pac-ret.
>>>
>>> Its mapping to -msign-return-address is as follows:
>>>
>>>         * -mbranch-protection=none -> -msign-return-address=none
>>>         * -mbranch-protection=standard -> -msign-return-address=leaf
>>>         * -mbranch-protection=pac-ret -> -msign-return-address=non-leaf
>>>         * -mbranch-protection=pac-ret+leaf -> -msign-return-address=all
>>>
>>> This patch implements the option's skeleton and the "none", "standard" and
>>> "pac-ret" types (along with its "leaf" subtype).
>>>
>>> The previous patch in this series is here:
>>> https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00103.html
>>>
>>> Bootstrapped successfully and tested on aarch64-none-elf with no regressions.
>>>
>>> OK for trunk?
>>>
>> Thank for doing this. I am not a maintainer so you will need a
>> maintainer's approval. Only nit, that I would add is that it would
>> be good to have more test coverage, specially for the new parsing
>> functions that have been added and the errors that are added.
>>
>> Example checking a few valid and invalid combinations of the options
>> like:
>> -mbranch-protection=pac-ret -mbranch-protection=none //disables
>> everything
>> -mbranch-protection=leaf  //errors out
>> -mbranch-protection=none+pac-ret //errors out
>> ... etc
>> Also instead of removing all the old deprecated options, you can keep
>> one (or a copy of one) to check for the deprecated warning.
> Hi Sudi,
>
> Thanks for the feedback, I've incorporated your suggestions into the
> attached patch and the updated changelog.
>
> gcc/ChangeLog:
>
> 2018-11-22  Sam Tebbs<sam.tebbs@arm.com>
>
> 	* config/aarch64/aarch64.c (BRANCH_PROTEC_STR_MAX,
> 	aarch64_parse_branch_protection,
> 	struct aarch64_branch_protec_type,
> 	aarch64_handle_no_branch_protection,
> 	aarch64_handle_standard_branch_protection,
> 	aarch64_validate_mbranch_protection,
> 	aarch64_handle_pac_ret_protection,
> 	aarch64_handle_attr_branch_protection,
> 	accepted_branch_protection_string,
> 	aarch64_pac_ret_subtypes,
> 	aarch64_branch_protec_types,
> 	aarch64_handle_pac_ret_leaf): Define.
> 	(aarch64_override_options_after_change_1): Add check for
> 	accepted_branch_protection_string.
> 	(aarch64_override_options): Add check for
> 	accepted_branch_protection_string.
> 	(aarch64_option_save): Save accepted_branch_protection_string.
> 	(aarch64_option_restore): Save
> 	accepted_branch_protection_string.
> 	* config/aarch64/aarch64.c (aarch64_attributes): Add branch-protection.
> 	* config/aarch64/aarch64.opt: Add mbranch-protection. Deprecate
> 	msign-return-address.
> 	* doc/invoke.texi: Add mbranch-protection.
>
> gcc/testsuite/ChangeLog:
>
> 2018-11-22  Sam Tebbs<sam.tebbs@arm.com>
>
> 	* (gcc.target/aarch64/return_address_sign_1.c,
> 	gcc.target/aarch64/return_address_sign_2.c,
> 	gcc.target/aarch64/return_address_sign_3.c (__attribute__)): Change
> 	option to -mbranch-protection.
> 	* gcc.target/aarch64/(branch-protection-option.c,
> 	branch-protection-option-2.c, branch-protection-attr.c,
> 	branch-protection-attr-2.c): New file.

Hi all,

Attached is an updated patch with branch_protec_type renamed to 
branch_protect_type, some unneeded ATTRIBUTE_USED removed and an added 
use of ARRAY_SIZE.

Below is the updated changelog.

OK for trunk? I have committed the preceding patch in the series.

gcc/ChangeLog:

2018-12-20  Sam Tebbs<sam.tebbs@arm.com>

	* config/aarch64/aarch64.c (BRANCH_PROTECT_STR_MAX,
	aarch64_parse_branch_protection,
	struct aarch64_branch_protect_type,
	aarch64_handle_no_branch_protection,
	aarch64_handle_standard_branch_protection,
	aarch64_validate_mbranch_protection,
	aarch64_handle_pac_ret_protection,
	aarch64_handle_attr_branch_protection,
	accepted_branch_protection_string,
	aarch64_pac_ret_subtypes,
	aarch64_branch_protect_types,
	aarch64_handle_pac_ret_leaf): Define.
	(aarch64_override_options_after_change_1): Add check for
	accepted_branch_protection_string.
	(aarch64_override_options): Add check for
	accepted_branch_protection_string.
	(aarch64_option_save): Save accepted_branch_protection_string.
	(aarch64_option_restore): Save
	accepted_branch_protection_string.
	* config/aarch64/aarch64.c (aarch64_attributes): Add branch-protection.
	* config/aarch64/aarch64.opt: Add mbranch-protection. Deprecate
	msign-return-address.
	* doc/invoke.texi: Add mbranch-protection.

gcc/testsuite/ChangeLog:

2018-12-20  Sam Tebbs<sam.tebbs@arm.com>

	* (gcc.target/aarch64/return_address_sign_1.c,
	gcc.target/aarch64/return_address_sign_2.c,
	gcc.target/aarch64/return_address_sign_3.c (__attribute__)): Change
	option to -mbranch-protection.
	* gcc.target/aarch64/(branch-protection-option.c,
	branch-protection-option-2.c, branch-protection-attr.c,
	branch-protection-attr-2.c): New file.

Attachment: diff
Description: diff