Hi Srinath,
Not a full review, just some things that popped out to me.
> On 11 Sep 2024, at 17:50, Srinath Parvathaneni <srinath.parvathan...@arm.com>
> wrote:
>
> External email: Use caution opening links or attachments
>
>
> This patch adds support for aarch64 gcs build attributes. This support
> includes generating two new assembler directives .aeabi_subsection and
> .aeabi_attribute. These directives are generated as per the syntax
> mentioned in spec "Build Attributes for the Arm® 64-bit
> Architecture (AArch64)" available at [1].
>
> To check whether the assembler being used to build the toolchain
> supports these directives, a new gcc configure check is added in
> gcc/configure.ac.
>
> If the assembler support these directives, .aeabi_subsection and
> .aeabi_attribute directives are emitted in the generated assembly,
> when -mbranch-protection=gcs is passed.
>
> If the assembler does not support these directives,
> .note.gnu.property section will emit the relevant gcs information
> in the generated assembly, when -mbranch-protection=gcs is passed.
>
> This patch needs to be applied on top of GCC gcs patch series [2].
>
> Bootstrapped on aarch64-none-linux-gnu and regression tested on
> aarch64-none-elf, no issues.
>
> Ok for master?
>
> Regards,
> Srinath.
>
> [1]: https://github.com/ARM-software/abi-aa/pull/230
> [2]:
> https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/vendors/ARM/heads/gcs
>
> gcc/ChangeLog:
>
> 2024-09-11 Srinath Parvathaneni <srinath.parvathan...@arm.com>
>
> * config.in: Regenerated
> * config/aarch64/aarch64.cc (aarch64_emit_aeabi_attribute): New
> function declaration.
> (aarch64_emit_aeabi_subsection): Likewise.
> (aarch64_start_file): Emit gcs build attributes.
> (aarch64_file_end_indicate_exec_stack): Update gcs bit in
> note.gnu.property section.
> * configure: Regenerated.
> * configure.ac: Add gcc configure check.
>
> gcc/testsuite/ChangeLog:
>
> 2024-09-11 Srinath Parvathaneni <srinath.parvathan...@arm.com>
>
> * gcc.target/aarch64/build-attribute-gcs.c: New test.
> ---
> gcc/config.in | 6 +++
> gcc/config/aarch64/a.out | Bin 0 -> 656 bytes
This binary artifact shouldn’t be in the patch.
> gcc/config/aarch64/aarch64.cc | 43 ++++++++++++++++++
> gcc/configure | 35 ++++++++++++++
> gcc/configure.ac | 7 +++
> .../gcc.target/aarch64/build-attribute-gcs.c | 24 ++++++++++
> 6 files changed, 115 insertions(+)
> create mode 100644 gcc/config/aarch64/a.out
> create mode 100644 gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c
diff --git a/gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c
b/gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c
new file mode 100644
index 00000000000..eb15772757e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/build-attribute-gcs.c
@@ -0,0 +1,24 @@
+/* { dg-do compile { target aarch64*-*-linux* } } */
+
+int main()
+{
+ return 0;
+}
+
+/* { dg-options "-mbranch-protection=gcs" } */
+/* { dg-final { scan-assembler-not "\.aeabi_subsection
\.aeabi-feature-and-bits, 1, 0" } } */
+/* { dg-final { scan-assembler-not "\.aeabi_attribute 3, 1\t\/\/
Tag_Feature_GCS" } } */
+/* { dg-final { scan-assembler ".note.gnu.property" } } */
+
+/* { dg-options "-mbranch-protection=bti" } */
+/* { dg-final { scan-assembler ".note.gnu.property" } } */
+
+
+/* { dg-options "-mbranch-protection=pac-ret" } */
+/* { dg-final { scan-assembler ".note.gnu.property" } } */
+
+
+/* { dg-options "-mbranch-protection=standard" } */
+/* { dg-final { scan-assembler-not "\.aeabi_subsection
\.aeabi-feature-and-bits, 1, 0" } } */
+/* { dg-final { scan-assembler-not "\.aeabi_attribute 3, 1\t\/\/
Tag_Feature_GCS" } } */
+/* { dg-final { scan-assembler ".note.gnu.property" } } */
These scans should be in different tests compiled with different options.
You can’t have multiple dg-options directives in a single test and scanning for
“.note.gnu.property” multiple times in a single test is redundant too.
Thanks,
Kyrill
