Re: [Patch 3/8, Arm, GCC] Add option -mbranch-protection. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
Richard Earnshaw via Gcc-patches writes:
[...]
>> Thanks for the reviews.
>> Add -mbranch-protection option. This option enables the
>> code-generation of
>> pointer signing and authentication instructions in function prologues and
>> epilogues.
>> 2021-10-25 Tejas Belagod
>> gcc/ChangeLog:
>> * config/arm/arm.c (arm_configure_build_target): Parse and
>> validate
>> -mbranch-protection option and initialize appropriate data structures.
>> * config/arm/arm.opt: New option -mbranch-protection.
>
> .../arm.opt (-mbranch-protection) : New option.
>
>> * doc/invoke.texi: Document -mbranch-protection.
>
> .../invoke.texi (Arm Options): Document it.
>
>> Tested the following configurations, OK for trunk?
>> -mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft
>> -marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp
>> mcmodel=small and tiny
>> aarch64-none-linux-gnu native test and bootstrap
>>
>
> +@item
> -mbranch-protection=@var{none}|@var{standard}|@var{pac-ret}[+@var{leaf}][+@var{bti}]|@var{bti}[+@var{pac-ret}[+@var{leaf}]]
> +@opindex mbranch-protection
> +Select the branch protection features to use.
> +@samp{none} is the default and turns off all types of branch protection.
> +@samp{standard} turns on all types of branch protection features. If
> a feature
> +has additional tuning options, then @samp{standard} sets it to its standard
> +level.
> +@samp{pac-ret[+@var{leaf}]} turns on return address signing to its standard
> +level: signing functions that save the return address to memory (non-leaf
> +functions will practically always do this). The optional argument
> @samp{leaf}
> +can be used to extend the signing to include leaf functions.
> +@samp{bti} turns on branch target identification mechanism.
>
> This doesn't really use the right documentation style. Closer would be:
[...]
Hi Richard,
thanks for reviewing, here is the updated patch.
Best Regards
Andrea
>From 3383a1e265b7b695c5d6ce6115ad66eed2a4cb48 Mon Sep 17 00:00:00 2001
From: Andrea Corallo
Date: Mon, 6 Dec 2021 11:39:03 +0100
Subject: [PATCH] Add option -mbranch-protection.
Add -mbranch-protection option. This option enables the
code-generation of pointer signing and authentication instructions in
function prologues and epilogues.
gcc/ChangeLog:
* config/arm/arm.c (arm_configure_build_target): Parse and validate
-mbranch-protection option and initialize appropriate data structures.
* config/arm/arm.opt (-mbranch-protection): New option.
* doc/invoke.texi (Arm Options): Document it.
Co-Authored-By: Tejas Belagod
Co-Authored-By: Richard Earnshaw
---
gcc/config/arm/arm.c | 11 +++
gcc/config/arm/arm.opt | 4
gcc/doc/invoke.texi| 35 +--
3 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 96186b8ad02..ee22acddee5 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3237,6 +3237,17 @@ arm_configure_build_target (struct arm_build_target
*target,
tune_opts = strchr (opts->x_arm_tune_string, '+');
}
+ if (opts->x_arm_branch_protection_string)
+{
+ aarch_validate_mbranch_protection (opts->x_arm_branch_protection_string);
+
+ if (aarch_ra_sign_key != AARCH_KEY_A)
+ {
+ warning (0, "invalid key type for %<-mbranch-protection=%>");
+ aarch_ra_sign_key = AARCH_KEY_A;
+ }
+}
+
if (arm_selected_arch)
{
arm_initialize_isa (target->isa, arm_selected_arch->common.isa_bits);
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 5c5b4f3ae06..4f2754c3e84 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -313,6 +313,10 @@ mbranch-cost=
Target RejectNegative Joined UInteger Var(arm_branch_cost) Init(-1)
Cost to assume for a branch insn.
+mbranch-protection=
+Target RejectNegative Joined Var(arm_branch_protection_string) Save
+Use branch-protection features.
+
mgeneral-regs-only
Target RejectNegative Mask(GENERAL_REGS_ONLY) Save
Generate code which uses the core registers only (r0-r14).
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 3257df5596d..a808c6bb599 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -820,7 +820,9 @@ Objective-C and Objective-C++ Dialects}.
-mpure-code @gol
-mcmse @gol
-mfix-cmse-cve-2021-35465 @gol
--mfdpic}
+-mfdpic @gol
+-mbranch-protection=@var{none}|@var{standard}|@var{pac-ret}[+@var{leaf}]
+[+@var{bti}]|@var{bti}[+@var{pac-ret}[+@var{leaf}]]}
@emph{AVR Options}
@gccoptlist{-mmcu=@var{mcu} -mabsdata -maccumulate-args @gol
@@ -21187,7 +21189,36 @@ The opposite @option{-mno-fdpic} option is useful (and
required) to
build the Linux kernel using the same (@code{arm-*-uclinuxfdpiceabi})
toolchain as the one used to build the userland programs.
-@end table
+@item
+-mbranch-protection=@var{none}|@var{standard}|@var{pac-ret}[+@var{leaf}][+@var{bti}]|@var{bti}[+@var{pac-ret}[+@var{leaf}]]
Re: [Patch 1/8, Arm, AArch64, GCC] Refactor mbranch-protection option parsing and make it common to AArch32 and AArch64 backends. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
Richard Earnshaw writes: > On 30/11/2021 11:11, Andrea Corallo via Gcc-patches wrote: >> Tejas Belagod via Gcc-patches writes: >> >>> Ping for this series. >>> >>> Thanks, >>> Tejas. >> Hi all, >> pinging this series. >> BR >>Andrea >> Hi Richard, thanks for reviewing. > It would be easier to find the 'series' if the messages were properly > threaded together... Feel free to let me know if you want me to ping the other patches as well so they all pops up. Thanks Andrea
Re: [Patch 3/8, Arm, GCC] Add option -mbranch-protection. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
On 28/10/2021 12:42, Tejas Belagod via Gcc-patches wrote:
-Original Message-
From: Richard Earnshaw
Sent: Monday, October 11, 2021 1:58 PM
To: Tejas Belagod ; gcc-patches@gcc.gnu.org
Subject: Re: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.
On 08/10/2021 13:17, Tejas Belagod via Gcc-patches wrote:
Hi,
Add -mbranch-protection option and its associated parsing routines.
This option enables the code-generation of pointer signing and
authentication instructions in function prologues and epilogues.
Tested on arm-none-eabi. OK for trunk?
2021-10-04 Tejas Belagod
gcc/ChangeLog:
* common/config/arm/arm-common.c
(arm_print_hit_for_pacbti_option): New.
(arm_progress_next_token): New.
(arm_parse_pac_ret_clause): New routine for parsing the
pac-ret clause for -mbranch-protection.
(arm_parse_pacbti_option): New routine to parse all the options
to -mbranch-protection.
* config/arm/arm-protos.h (arm_parse_pacbti_option): Export.
* config/arm/arm.c (arm_configure)build_target): Handle option
to -mbranch-protection.
* config/arm/arm.opt (mbranch-protection). New.
(arm_enable_pacbti): New.
You're missing documentation for invoke.texi.
Also, how does this differ from the exising option in aarch64? Can the code
from that be adapted to be made common to both targets rather than doing
a new implementation?
Finally, there are far to many manifest constants in this patch, they need
replacing with enums or #defines as appropriate if we cannot share the
aarch64 code.
Thanks for the reviews.
Add -mbranch-protection option. This option enables the code-generation of
pointer signing and authentication instructions in function prologues and
epilogues.
2021-10-25 Tejas Belagod
gcc/ChangeLog:
* config/arm/arm.c (arm_configure_build_target): Parse and validate
-mbranch-protection option and initialize appropriate data structures.
* config/arm/arm.opt: New option -mbranch-protection.
.../arm.opt (-mbranch-protection) : New option.
* doc/invoke.texi: Document -mbranch-protection.
.../invoke.texi (Arm Options): Document it.
Tested the following configurations, OK for trunk?
-mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft
-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp
mcmodel=small and tiny
aarch64-none-linux-gnu native test and bootstrap
+@item
-mbranch-protection=@var{none}|@var{standard}|@var{pac-ret}[+@var{leaf}][+@var{bti}]|@var{bti}[+@var{pac-ret}[+@var{leaf}]]
+@opindex mbranch-protection
+Select the branch protection features to use.
+@samp{none} is the default and turns off all types of branch protection.
+@samp{standard} turns on all types of branch protection features. If a
feature
+has additional tuning options, then @samp{standard} sets it to its standard
+level.
+@samp{pac-ret[+@var{leaf}]} turns on return address signing to its standard
+level: signing functions that save the return address to memory (non-leaf
+functions will practically always do this). The optional argument
@samp{leaf}
+can be used to extend the signing to include leaf functions.
+@samp{bti} turns on branch target identification mechanism.
This doesn't really use the right documentation style. Closer would be:
===
@item
-mbranch-protection=@var{none}|@var{standard}|@var{pac-ret}[+@var{leaf}][+@var{bti}]|@var{bti}[+@var{pac-ret}[+@var{leaf}]]
@opindex mbranch-protection
Enable branch protection features (armv8.1-m.main only).
@samp{none} generate code without branch protection or return address
signing.
@samp{standard[+@var{leaf}]} generate code with all branch protection
features enabled at their standard level.
@samp{pac-ret[+@var{leaf}]} generate code with return address signing
set to its standard level, which is to sign all functions that save the
return address to memory.
@samp{leaf} When return address signing is enabled, also sign leaf
functions even if they do not write the return address to memory.
+@samp{bti} Add landing-pad instructions at the permitted targets of
indirect branch instructions.
If the @samp{+pacbti} architecture extension is not enabled, then all
branch protection and return address signing operations are constrained
to use only the instructions defined in the architectural-NOP space.
The generated code will remain backwards-compatible with earlier
versions of the architecture, but the additional security can be enabled
at run time on processors that support the @samp{PACBTI} extension.
Branch target enforcement using BTI can only be enabled at runtime if
all code in the application has been compiled with at least
@samp{-mbranch-protection=bti}.
The default is to generate code without branch protection or return
address signing.
R.
Re: [Patch 1/8, Arm, AArch64, GCC] Refactor mbranch-protection option parsing and make it common to AArch32 and AArch64 backends. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
On 30/11/2021 11:11, Andrea Corallo via Gcc-patches wrote: Tejas Belagod via Gcc-patches writes: Ping for this series. Thanks, Tejas. Hi all, pinging this series. BR Andrea It would be easier to find the 'series' if the messages were properly threaded together... R.
Re: [Patch 1/8, Arm, AArch64, GCC] Refactor mbranch-protection option parsing and make it common to AArch32 and AArch64 backends. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
On 10/11/2021 13:55, Andrea Corallo via Gcc-patches wrote:
Tejas Belagod via Gcc-patches writes:
[...]
This change refactors all the mbranch-protection option parsing code and types
to make it common to both AArch32 and AArch64 backends. This change also pulls
in some supporting types from AArch64 to make it common
(aarch_parse_opt_result). The significant changes in this patch are the
movement of all branch protection parsing routines from aarch64.c to
aarch-common.c and supporting data types and static data structures. This
patch also pre-declares variables and types required in the aarch32 back for
moved variables for function sign scope and key to prepare for the impending
series of patches that support parsing the feature mbranch-protection in the
aarch32 back end.
2021-10-25 Tejas Belagod
gcc/ChangeLog:
* common/config/aarch64/aarch64-common.c: Include aarch-common.h.
(all_architectures): Fix comment.
(aarch64_parse_extension): Rename return type, enum value names.
* config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Rename
factored out aarch_ra_sign_scope and aarch_ra_sign_key variables.
Also rename corresponding enum values.
* config/aarch64/aarch64-opts.h (aarch64_function_type): Factor out
aarch64_function_type and move it to common code as aarch_function_type
in aarch-common.h.
* config/aarch64/aarch64-protos.h: Include common types header, move out
types aarch64_parse_opt_result and aarch64_key_type to aarch-common.h
* config/aarch64/aarch64.c: Move mbranch-protection parsing types and
functions out into aarch-common.h and aarch-common.c. Fix up all the
name
changes resulting from the move.
* config/aarch64/aarch64.md: Fix up aarch64_ra_sign_key type name change
and enum value.
* config/aarch64/aarch64.opt: Include aarch-common.h to import type
move.
Fix up name changes from factoring out common code and data.
* config/arm/aarch-common-protos.h: Export factored out routines to both
backends.
* config/arm/aarch-common.c: Include newly factored out types. Move all
mbranch-protection code and data structures from aarch64.c.
* config/arm/aarch-common.h: New header that declares types shared
between
aarch32 and aarch64 backends.
* config/arm/arm-protos.h: Declare types and variables that are made
common
to aarch64 and aarch32 backends - aarch_ra_sign_key,
aarch_ra_sign_scope and
aarch_enable_bti.
Tested the following configurations. OK for trunk?
-mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft
-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp
mcmodel=small and tiny
aarch64-none-linux-gnu native test and bootstrap
Thanks,
Tejas.
Hi Tejas,
going through the code I've spotted a couple of indentation nits that I
guess are coming from the original source that was moved.
diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c
[...]
+ /* Copy the last processed token into the argument to pass it back.
+Used by option and attribute validation to print the offending token. */
+ if (last_str)
+{
+ if (str) strcpy (*last_str, str);
+ else *last_str = NULL;
I think we should have new lines after both if and else here.
Agreed. This doesn't match the GNU style.
+}
There should also be a blank line here, before the next if clause.
+ if (res == AARCH_PARSE_OK)
+{
+ /* If needed, alloc the accepted string then copy in const_str.
+ Used by override_option_after_change_1. */
+ if (!accepted_branch_protection_string)
+ accepted_branch_protection_string = (char *) xmalloc (
+ BRANCH_PROTECT_STR_MAX
+ + 1);
^^
Indentation
It would be best to split this just before the '=', then then rest of
the statement should fit on one line
+ strncpy (accepted_branch_protection_string, const_str,
+ BRANCH_PROTECT_STR_MAX + 1);
^^
Same
+ /* Forcibly null-terminate. */
+ accepted_branch_protection_string[BRANCH_PROTECT_STR_MAX] = '\0';
+}
+ return res;
+}
Thanks
Andrea
+++ b/gcc/config/arm/aarch-common.h
@@ -0,0 +1,73 @@
+/* Types shared between arm and aarch64.
+
+ Copyright (C) 1991-2021 Free Software Foundation, Inc.
+ Contributed by ARM Ltd.
+
I think this should be
/* Types shared between arm and aarch64.
Copyright (C) 2009-2021 Free Software Foundation, Inc.
Contributed by Arm Ltd.
Since all of the code has been derived from the aarch64 port.
-struct aarch64_branch_protect_type
-{
- /* The type's name that the user passes to the branch-protection option
-strin
Re: [Patch 1/8, Arm, AArch64, GCC] Refactor mbranch-protection option parsing and make it common to AArch32 and AArch64 backends. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
Tejas Belagod via Gcc-patches writes: > Ping for this series. > > Thanks, > Tejas. Hi all, pinging this series. BR Andrea
RE: [Patch 1/8, Arm, AArch64, GCC] Refactor mbranch-protection option parsing and make it common to AArch32 and AArch64 backends. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
Ping for this series. Thanks, Tejas. > -Original Message- > From: Gcc-patches bounces+belagod=gcc.gnu@gcc.gnu.org> On Behalf Of Tejas Belagod via > Gcc-patches > Sent: Thursday, October 28, 2021 12:41 PM > To: Richard Earnshaw ; gcc- > patc...@gcc.gnu.org > Subject: [Patch 1/8, Arm, AArch64, GCC] Refactor mbranch-protection option > parsing and make it common to AArch32 and AArch64 backends. [Was RE: > [Patch 2/7, Arm, GCC] Add option -mbranch-protection.] > > > > > -Original Message- > > From: Richard Earnshaw > > Sent: Monday, October 11, 2021 1:58 PM > > To: Tejas Belagod ; gcc-patches@gcc.gnu.org > > Subject: Re: [Patch 2/7, Arm, GCC] Add option -mbranch-protection. > > > > On 08/10/2021 13:17, Tejas Belagod via Gcc-patches wrote: > > > Hi, > > > > > > Add -mbranch-protection option and its associated parsing routines. > > > This option enables the code-generation of pointer signing and > > > authentication instructions in function prologues and epilogues. > > > > > > Tested on arm-none-eabi. OK for trunk? > > > > > > 2021-10-04 Tejas Belagod > > > > > > gcc/ChangeLog: > > > > > > * common/config/arm/arm-common.c > > >(arm_print_hit_for_pacbti_option): New. > > >(arm_progress_next_token): New. > > >(arm_parse_pac_ret_clause): New routine for parsing the > > > pac-ret clause for -mbranch-protection. > > > (arm_parse_pacbti_option): New routine to parse all the options > > > to -mbranch-protection. > > > * config/arm/arm-protos.h (arm_parse_pacbti_option): Export. > > > * config/arm/arm.c (arm_configure)build_target): Handle option > > > to -mbranch-protection. > > > * config/arm/arm.opt (mbranch-protection). New. > > > (arm_enable_pacbti): New. > > > > > > > You're missing documentation for invoke.texi. > > > > Also, how does this differ from the exising option in aarch64? Can > > the code from that be adapted to be made common to both targets rather > > than doing a new implementation? > > > > Finally, there are far to many manifest constants in this patch, they > > need replacing with enums or #defines as appropriate if we cannot > > share the > > aarch64 code. > > > > Thanks for the reviews. > > This change refactors all the mbranch-protection option parsing code and > types to make it common to both AArch32 and AArch64 backends. This > change also pulls in some supporting types from AArch64 to make it common > (aarch_parse_opt_result). The significant changes in this patch are the > movement of all branch protection parsing routines from aarch64.c to aarch- > common.c and supporting data types and static data structures. This patch > also pre-declares variables and types required in the aarch32 back for moved > variables for function sign scope and key to prepare for the impending series > of patches that support parsing the feature mbranch-protection in the > aarch32 back end. > > 2021-10-25 Tejas Belagod > > gcc/ChangeLog: > > * common/config/aarch64/aarch64-common.c: Include aarch- > common.h. > (all_architectures): Fix comment. > (aarch64_parse_extension): Rename return type, enum value > names. > * config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): > Rename > factored out aarch_ra_sign_scope and aarch_ra_sign_key variables. > Also rename corresponding enum values. > * config/aarch64/aarch64-opts.h (aarch64_function_type): Factor out > aarch64_function_type and move it to common code as > aarch_function_type > in aarch-common.h. > * config/aarch64/aarch64-protos.h: Include common types header, > move out > types aarch64_parse_opt_result and aarch64_key_type to aarch- > common.h > * config/aarch64/aarch64.c: Move mbranch-protection parsing types > and > functions out into aarch-common.h and aarch-common.c. Fix up all > the name > changes resulting from the move. > * config/aarch64/aarch64.md: Fix up aarch64_ra_sign_key type name > change > and enum value. > * config/aarch64/aarch64.opt: Include aarch-common.h to import > type move. > Fix up name changes from factoring out common code and data. > * config/arm/aarch-common-protos.h: Export factored out routines > to both > backends. > * config/arm/aarch-common.c: Include newly factored out types. > Move all > mbranch-protection code and data structures from aarch64.c. > * config/arm/aarch-comm
Re: [Patch 1/8, Arm, AArch64, GCC] Refactor mbranch-protection option parsing and make it common to AArch32 and AArch64 backends. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
Tejas Belagod via Gcc-patches writes:
[...]
> This change refactors all the mbranch-protection option parsing code and types
> to make it common to both AArch32 and AArch64 backends. This change also
> pulls
> in some supporting types from AArch64 to make it common
> (aarch_parse_opt_result). The significant changes in this patch are the
> movement of all branch protection parsing routines from aarch64.c to
> aarch-common.c and supporting data types and static data structures. This
> patch also pre-declares variables and types required in the aarch32 back for
> moved variables for function sign scope and key to prepare for the impending
> series of patches that support parsing the feature mbranch-protection in the
> aarch32 back end.
>
> 2021-10-25 Tejas Belagod
>
> gcc/ChangeLog:
>
> * common/config/aarch64/aarch64-common.c: Include aarch-common.h.
> (all_architectures): Fix comment.
> (aarch64_parse_extension): Rename return type, enum value names.
> * config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Rename
> factored out aarch_ra_sign_scope and aarch_ra_sign_key variables.
> Also rename corresponding enum values.
> * config/aarch64/aarch64-opts.h (aarch64_function_type): Factor out
> aarch64_function_type and move it to common code as aarch_function_type
> in aarch-common.h.
> * config/aarch64/aarch64-protos.h: Include common types header, move out
> types aarch64_parse_opt_result and aarch64_key_type to aarch-common.h
> * config/aarch64/aarch64.c: Move mbranch-protection parsing types and
> functions out into aarch-common.h and aarch-common.c. Fix up all the
> name
> changes resulting from the move.
> * config/aarch64/aarch64.md: Fix up aarch64_ra_sign_key type name change
> and enum value.
> * config/aarch64/aarch64.opt: Include aarch-common.h to import type
> move.
> Fix up name changes from factoring out common code and data.
> * config/arm/aarch-common-protos.h: Export factored out routines to both
> backends.
> * config/arm/aarch-common.c: Include newly factored out types. Move all
> mbranch-protection code and data structures from aarch64.c.
> * config/arm/aarch-common.h: New header that declares types shared
> between
> aarch32 and aarch64 backends.
> * config/arm/arm-protos.h: Declare types and variables that are made
> common
> to aarch64 and aarch32 backends - aarch_ra_sign_key,
> aarch_ra_sign_scope and
> aarch_enable_bti.
>
>
> Tested the following configurations. OK for trunk?
>
> -mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft
> -marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp
> mcmodel=small and tiny
> aarch64-none-linux-gnu native test and bootstrap
>
> Thanks,
> Tejas.
Hi Tejas,
going through the code I've spotted a couple of indentation nits that I
guess are coming from the original source that was moved.
> diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c
[...]
> + /* Copy the last processed token into the argument to pass it back.
> +Used by option and attribute validation to print the offending token. */
> + if (last_str)
> +{
> + if (str) strcpy (*last_str, str);
> + else *last_str = NULL;
I think we should have new lines after both if and else here.
> +}
> + if (res == AARCH_PARSE_OK)
> +{
> + /* If needed, alloc the accepted string then copy in const_str.
> + Used by override_option_after_change_1. */
> + if (!accepted_branch_protection_string)
> + accepted_branch_protection_string = (char *) xmalloc (
> + BRANCH_PROTECT_STR_MAX
> + + 1);
^^
Indentation
> + strncpy (accepted_branch_protection_string, const_str,
> + BRANCH_PROTECT_STR_MAX + 1);
^^
Same
> + /* Forcibly null-terminate. */
> + accepted_branch_protection_string[BRANCH_PROTECT_STR_MAX] = '\0';
> +}
> + return res;
> +}
Thanks
Andrea
[Patch 3/8, Arm, GCC] Add option -mbranch-protection. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
> -Original Message-
> From: Richard Earnshaw
> Sent: Monday, October 11, 2021 1:58 PM
> To: Tejas Belagod ; gcc-patches@gcc.gnu.org
> Subject: Re: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.
>
> On 08/10/2021 13:17, Tejas Belagod via Gcc-patches wrote:
> > Hi,
> >
> > Add -mbranch-protection option and its associated parsing routines.
> > This option enables the code-generation of pointer signing and
> > authentication instructions in function prologues and epilogues.
> >
> > Tested on arm-none-eabi. OK for trunk?
> >
> > 2021-10-04 Tejas Belagod
> >
> > gcc/ChangeLog:
> >
> > * common/config/arm/arm-common.c
> > (arm_print_hit_for_pacbti_option): New.
> > (arm_progress_next_token): New.
> > (arm_parse_pac_ret_clause): New routine for parsing the
> > pac-ret clause for -mbranch-protection.
> > (arm_parse_pacbti_option): New routine to parse all the options
> > to -mbranch-protection.
> > * config/arm/arm-protos.h (arm_parse_pacbti_option): Export.
> > * config/arm/arm.c (arm_configure)build_target): Handle option
> > to -mbranch-protection.
> > * config/arm/arm.opt (mbranch-protection). New.
> > (arm_enable_pacbti): New.
> >
>
> You're missing documentation for invoke.texi.
>
> Also, how does this differ from the exising option in aarch64? Can the code
> from that be adapted to be made common to both targets rather than doing
> a new implementation?
>
> Finally, there are far to many manifest constants in this patch, they need
> replacing with enums or #defines as appropriate if we cannot share the
> aarch64 code.
Thanks for the reviews.
Add -mbranch-protection option. This option enables the code-generation of
pointer signing and authentication instructions in function prologues and
epilogues.
2021-10-25 Tejas Belagod
gcc/ChangeLog:
* config/arm/arm.c (arm_configure_build_target): Parse and validate
-mbranch-protection option and initialize appropriate data structures.
* config/arm/arm.opt: New option -mbranch-protection.
* doc/invoke.texi: Document -mbranch-protection.
Tested the following configurations, OK for trunk?
-mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft
-marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp
mcmodel=small and tiny
aarch64-none-linux-gnu native test and bootstrap
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index
a952655db80663f28f5a5d12005f2adb4702894f..946841526ee127105396097d143e755bdfc756f5
100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3216,6 +3216,17 @@ arm_configure_build_target (struct arm_build_target
*target,
tune_opts = strchr (opts->x_arm_tune_string, '+');
}
+ if (opts->x_arm_branch_protection_string)
+{
+ aarch_validate_mbranch_protection (opts->x_arm_branch_protection_string);
+
+ if (aarch_ra_sign_key != AARCH_KEY_A)
+ {
+ warning (0, "invalid key type for %<-mbranch-protection=%>");
+ aarch_ra_sign_key = AARCH_KEY_A;
+ }
+}
+
if (arm_selected_arch)
{
arm_initialize_isa (target->isa, arm_selected_arch->common.isa_bits);
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index
5c5b4f3ae0699a3a9d78df40a5ab65324dcba7b9..4f2754c3e84c436f7058ea0bd1c9f517b3a63ccd
100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -313,6 +313,10 @@ mbranch-cost=
Target RejectNegative Joined UInteger Var(arm_branch_cost) Init(-1)
Cost to assume for a branch insn.
+mbranch-protection=
+Target RejectNegative Joined Var(arm_branch_protection_string) Save
+Use branch-protection features.
+
mgeneral-regs-only
Target RejectNegative Mask(GENERAL_REGS_ONLY) Save
Generate code which uses the core registers only (r0-r14).
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index
27df8cf5bee79c2abac8b81c1ac54f1c3e50c628..7f886db008a39c44819616eb2799c01822d0aae9
100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -810,7 +810,9 @@ Objective-C and Objective-C++ Dialects}.
-mpure-code @gol
-mcmse @gol
-mfix-cmse-cve-2021-35465 @gol
--mfdpic}
+-mfdpic @gol
+-mbranch-protection=@var{none}|@var{standard}|@var{pac-ret}[+@var{leaf}]
+[+@var{bti}]|@var{bti}[+@var{pac-ret}[+@var{leaf}]]}
@emph{AVR Options}
@gccoptlist{-mmcu=@var{mcu} -mabsdata -maccumulate-args @gol
@@ -20969,6 +20971,18 @@ The opposite @option{-mno-fdpic} option is useful (and
required) to
build the Linux kernel using the same (@code{arm-*-uclinuxfdpiceabi})
toolchain as the one used to build the userland programs.
+@item
-mbranch-protection=@var{none}|@var{standard}|@var{pac-ret}[+@var{leaf}][+@var{bti}]|@var{bti}[+@var{pac-ret}[+@var{leaf}]]
+@opindex mbranch-protection
+Select the branch pro
[Patch 1/8, Arm, AArch64, GCC] Refactor mbranch-protection option parsing and make it common to AArch32 and AArch64 backends. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]
> -Original Message- > From: Richard Earnshaw > Sent: Monday, October 11, 2021 1:58 PM > To: Tejas Belagod ; gcc-patches@gcc.gnu.org > Subject: Re: [Patch 2/7, Arm, GCC] Add option -mbranch-protection. > > On 08/10/2021 13:17, Tejas Belagod via Gcc-patches wrote: > > Hi, > > > > Add -mbranch-protection option and its associated parsing routines. > > This option enables the code-generation of pointer signing and > > authentication instructions in function prologues and epilogues. > > > > Tested on arm-none-eabi. OK for trunk? > > > > 2021-10-04 Tejas Belagod > > > > gcc/ChangeLog: > > > > * common/config/arm/arm-common.c > > (arm_print_hit_for_pacbti_option): New. > > (arm_progress_next_token): New. > > (arm_parse_pac_ret_clause): New routine for parsing the > > pac-ret clause for -mbranch-protection. > > (arm_parse_pacbti_option): New routine to parse all the options > > to -mbranch-protection. > > * config/arm/arm-protos.h (arm_parse_pacbti_option): Export. > > * config/arm/arm.c (arm_configure)build_target): Handle option > > to -mbranch-protection. > > * config/arm/arm.opt (mbranch-protection). New. > > (arm_enable_pacbti): New. > > > > You're missing documentation for invoke.texi. > > Also, how does this differ from the exising option in aarch64? Can the code > from that be adapted to be made common to both targets rather than doing > a new implementation? > > Finally, there are far to many manifest constants in this patch, they need > replacing with enums or #defines as appropriate if we cannot share the > aarch64 code. > Thanks for the reviews. This change refactors all the mbranch-protection option parsing code and types to make it common to both AArch32 and AArch64 backends. This change also pulls in some supporting types from AArch64 to make it common (aarch_parse_opt_result). The significant changes in this patch are the movement of all branch protection parsing routines from aarch64.c to aarch-common.c and supporting data types and static data structures. This patch also pre-declares variables and types required in the aarch32 back for moved variables for function sign scope and key to prepare for the impending series of patches that support parsing the feature mbranch-protection in the aarch32 back end. 2021-10-25 Tejas Belagod gcc/ChangeLog: * common/config/aarch64/aarch64-common.c: Include aarch-common.h. (all_architectures): Fix comment. (aarch64_parse_extension): Rename return type, enum value names. * config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Rename factored out aarch_ra_sign_scope and aarch_ra_sign_key variables. Also rename corresponding enum values. * config/aarch64/aarch64-opts.h (aarch64_function_type): Factor out aarch64_function_type and move it to common code as aarch_function_type in aarch-common.h. * config/aarch64/aarch64-protos.h: Include common types header, move out types aarch64_parse_opt_result and aarch64_key_type to aarch-common.h * config/aarch64/aarch64.c: Move mbranch-protection parsing types and functions out into aarch-common.h and aarch-common.c. Fix up all the name changes resulting from the move. * config/aarch64/aarch64.md: Fix up aarch64_ra_sign_key type name change and enum value. * config/aarch64/aarch64.opt: Include aarch-common.h to import type move. Fix up name changes from factoring out common code and data. * config/arm/aarch-common-protos.h: Export factored out routines to both backends. * config/arm/aarch-common.c: Include newly factored out types. Move all mbranch-protection code and data structures from aarch64.c. * config/arm/aarch-common.h: New header that declares types shared between aarch32 and aarch64 backends. * config/arm/arm-protos.h: Declare types and variables that are made common to aarch64 and aarch32 backends - aarch_ra_sign_key, aarch_ra_sign_scope and aarch_enable_bti. Tested the following configurations. OK for trunk? -mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft -marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp mcmodel=small and tiny aarch64-none-linux-gnu native test and bootstrap Thanks, Tejas. > R. diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c index 6d200a186604be2028b19ee9691e7bbf4a7be9c2..92c8f14a17466b9d6c44bdf4ede673a65f1b426f 100644 --- a/gcc/common/config/aarch64/aarch64-common.c +++ b/gcc/common/config/aarch64/aarch64-common.c @@ -30,6 +30,7 @@ #include "opts.h" #include "flags.h" #include "
Re: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.
On 08/10/2021 13:17, Tejas Belagod via Gcc-patches wrote: Hi, Add -mbranch-protection option and its associated parsing routines. This option enables the code-generation of pointer signing and authentication instructions in function prologues and epilogues. Tested on arm-none-eabi. OK for trunk? 2021-10-04 Tejas Belagod gcc/ChangeLog: * common/config/arm/arm-common.c (arm_print_hit_for_pacbti_option): New. (arm_progress_next_token): New. (arm_parse_pac_ret_clause): New routine for parsing the pac-ret clause for -mbranch-protection. (arm_parse_pacbti_option): New routine to parse all the options to -mbranch-protection. * config/arm/arm-protos.h (arm_parse_pacbti_option): Export. * config/arm/arm.c (arm_configure)build_target): Handle option to -mbranch-protection. * config/arm/arm.opt (mbranch-protection). New. (arm_enable_pacbti): New. You're missing documentation for invoke.texi. Also, how does this differ from the exising option in aarch64? Can the code from that be adapted to be made common to both targets rather than doing a new implementation? Finally, there are far to many manifest constants in this patch, they need replacing with enums or #defines as appropriate if we cannot share the aarch64 code. R.
[Patch 2/7, Arm, GCC] Add option -mbranch-protection.
Hi,
Add -mbranch-protection option and its associated parsing routines.
This option enables the code-generation of pointer signing and
authentication instructions in function prologues and epilogues.
Tested on arm-none-eabi. OK for trunk?
2021-10-04 Tejas Belagod
gcc/ChangeLog:
* common/config/arm/arm-common.c
(arm_print_hit_for_pacbti_option): New.
(arm_progress_next_token): New.
(arm_parse_pac_ret_clause): New routine for parsing the
pac-ret clause for -mbranch-protection.
(arm_parse_pacbti_option): New routine to parse all the options
to -mbranch-protection.
* config/arm/arm-protos.h (arm_parse_pacbti_option): Export.
* config/arm/arm.c (arm_configure)build_target): Handle option
to -mbranch-protection.
* config/arm/arm.opt (mbranch-protection). New.
(arm_enable_pacbti): New.
diff --git a/gcc/common/config/arm/arm-common.c
b/gcc/common/config/arm/arm-common.c
index
de898a74165db4d7250aa0097dfab682beb0f99c..188feebb15b52f389d5d0b3ec322be3017efd5a0
100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -475,6 +475,156 @@ arm_parse_arch_option_name (const arch_option *list,
const char *optname,
return NULL;
}
+static void
+arm_print_hint_for_pacbti_option ()
+{
+ const char *s = "pac-ret[+leaf][+b-key][+bti]"
+ " | bti[+pac-ret[+leaf][+b-key]]";
+ inform (input_location, "valid arguments are: %s", s);
+}
+
+/* Progress *E to end of next token delimited by DELIMITER.
+ Cache old *E in *OE. */
+static void
+arm_progress_next_token (const char **oe, const char **e,
+size_t *l, const char delimiter)
+{
+ *oe = *e + 1;
+ *e = strchr (*oe, delimiter);
+ *l = *e ? *e - *oe : strlen (*oe);
+}
+
+/* Parse options to -mbranch-protection. */
+static const char*
+arm_parse_pac_ret_clause (const char *pacret, const char *optname,
+ unsigned int *pacbti)
+{
+ const char *old_end = NULL;
+ const char *end = strchr (pacret, '+');
+ size_t len = end ? end - pacret : strlen (pacret);
+ if (len == 7 && strncmp (pacret, "pac-ret", len) == 0)
+{
+ *pacbti |= 2;
+ if (end != NULL)
+ {
+ /* pac-ret+... */
+ arm_progress_next_token (&old_end, &end, &len, '+');
+ if (len == 4 && strncmp (old_end, "leaf", len) == 0)
+ {
+ *pacbti |= 8;
+ if (end != NULL)
+ {
+ /* pac-ret+leaf+... */
+ arm_progress_next_token (&old_end, &end, &len, '+');
+ if (len == 5 && strncmp (old_end, "b-key", len) == 0)
+ {
+ /* Clear bit for A-key. */
+ *pacbti &= 0xfffd;
+ *pacbti |= 4;
+ /* A non-NULL end indicates its pointing to a '+'.
+Advance it to point to the next option in the string.
*/
+ if (end != NULL)
+ end++;
+ }
+ else
+ /* This could be 'bti', leave it to caller to parse. */
+ end = old_end;
+ }
+ }
+ else if (len == 5 && strncmp (old_end, "b-key", len) == 0)
+ {
+ /* Clear bit for A-key. */
+ *pacbti &= 0xfffd;
+ *pacbti |= 4;
+ if (end != NULL)
+ {
+ /* pac-ret+b-key+... */
+ arm_progress_next_token (&old_end, &end, &len, '+');
+ if (len == 4 && strncmp (old_end, "leaf", len) == 0)
+ {
+ *pacbti |= 8;
+ /* A non-NULL end indicates its pointing to a '+'.
+Advance it to point to the next option in the string.
*/
+ if (end != NULL)
+ end++;
+ }
+ else
+ /* This could be 'bti', leave it to caller to parse. */
+ end = old_end;
+ }
+ }
+ else
+ {
+ /* This could be a 'bti' option, so leave it to the caller to
+parse. Fall through to the return. */
+ end = old_end;
+ }
+ }
+}
+ else
+{
+ error_at (input_location, "unrecognized %s argument: %s", optname,
pacret);
+ arm_print_hint_for_pacbti_option ();
+ return NULL;
+}
+
+ return end;
+}
+
+unsigned int
+arm_parse_pacbti_option (const char *pacbti, const char *optname, bool
complain)
+{
+ unsigned int enable_pacbti = 0;
+ const char *end = strchr (pacbti, '+');
+ size_t len = end ? end - pacbti : strlen (pacbti);
+
+ if (strcmp (pacbti, "none") == 0)
+return 0;
+
+ if (strcmp (pacbti, "standard") == 0)
+return 0x3;
+
+ if (len == 3 && strncmp (pacbti, "bti", len) == 0)
+{
+ /* bti+... */
+
