diff mbox series

[AARCH64] Add GNU note section with BTI and PAC.

Message ID 4a51a615-697b-43cc-19a8-46e7134da7de@arm.com
State New
Headers show
Series [AARCH64] Add GNU note section with BTI and PAC. | expand

Commit Message

Sudakshina Das March 29, 2019, 2:01 p.m. UTC
Hi

This patch adds the GNU NOTE section to the BTI and/or PAC
enabled objects for linux targets.
The ABI document that we published mentioning GNU NOTE section is below:
https://developer.arm.com/docs/ihi0056/latest/elf-for-the-arm-64-bit-architecture-aarch64-abi-2018q4

The patches needed for these in binutils are already approved and committed.
https://sourceware.org/ml/binutils/2019-03/msg00072.html

Bootstrapped and regression tested with aarch64-none-linux-gnu.
Is this ok for trunk?

Thanks
Sudi

*** gcc/ChangeLog ***

2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>

	* config/aarch64/aarch64-linux.h (TARGET_ASM_FILE_END): Define for
	AArch64.
	(aarch64_file_end_indicate_exec_stack): Add gnu note section.

gcc/testsuite/ChangeLog:

2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>

	* gcc.target/aarch64/bti-1.c: Add scan directive for gnu note section
	for linux targets.
	* gcc.target/aarch64/va_arg_1.c: Don't run for aarch64 linux 		targets 
with --enable-standard-branch-protection.

Comments

Richard Henderson March 29, 2019, 5:51 p.m. UTC | #1
> +#define ASM_LONG "\t.long\t"

Do not replicate targetm.asm_out.aligned_op.si, or integer_asm_op, really.

> +aarch64_file_end_indicate_exec_stack ()
> +{
> +  file_end_indicate_exec_stack ();
> +
> +  if (!aarch64_bti_enabled ()
> +      && aarch64_ra_sign_scope == AARCH64_FUNCTION_NONE)
> +    {
> +      return;
> +    }

This is redundant with...

> +
> +  unsigned feature_1_and = 0;
> +  if (aarch64_bti_enabled ())
> +    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
> +
> +  if (aarch64_ra_sign_scope != AARCH64_FUNCTION_NONE)
> +    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
> +
> +  if (feature_1_and)

... this.  I prefer the second, as it's obvious.

> +      ASM_OUTPUT_ALIGN (asm_out_file, p2align);
> +      /* name length.  */
> +      fprintf (asm_out_file, ASM_LONG " 1f - 0f\n");
> +      /* data length.  */
> +      fprintf (asm_out_file, ASM_LONG " 4f - 1f\n");
> +      /* note type: NT_GNU_PROPERTY_TYPE_0.  */
> +      fprintf (asm_out_file, ASM_LONG " 5\n");
> +      fprintf (asm_out_file, "0:\n");
> +      /* vendor name: "GNU".  */
> +      fprintf (asm_out_file, STRING_ASM_OP " \"GNU\"\n");
> +      fprintf (asm_out_file, "1:\n");
> +      ASM_OUTPUT_ALIGN (asm_out_file, p2align);
> +      /* pr_type: GNU_PROPERTY_AARCH64_FEATURE_1_AND.  */
> +      fprintf (asm_out_file, ASM_LONG " 0x%x\n",
> +	       GNU_PROPERTY_AARCH64_FEATURE_1_AND);
> +      /* pr_datasz.  */\
> +      fprintf (asm_out_file, ASM_LONG " 3f - 2f\n");
> +      fprintf (asm_out_file, "2:\n");
> +      /* GNU_PROPERTY_AARCH64_FEATURE_1_XXX.  */
> +      fprintf (asm_out_file, ASM_LONG " 0x%x\n", feature_1_and);
> +      fprintf (asm_out_file, "3:\n");
> +      ASM_OUTPUT_ALIGN (asm_out_file, p2align);
> +      fprintf (asm_out_file, "4:\n");

This could stand to use a comment, a moment's thinking about the sizes, and to
use the existing asm output functions.

	/* PT_NOTE header: namesz, descsz, type.
	   namesz = 4 ("GNU\0")
	   descsz = 12 (see below)
	   type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
	assemble_align (POINTER_SIZE);
	assemble_integer (GEN_INT (4), 4, 32, 1);
	assemble_integer (GEN_INT (12), 4, 32, 1);
	assemble_integer (GEN_INT (5), 4, 32, 1);

	/* PT_NOTE name */
	assemble_string ("GNU", 4);

	/* PT_NOTE contents for NT_GNU_PROPERTY_TYPE_0:
	   type   = 0xc0000000 (GNU_PROPERTY_AARCH64_FEATURE_1_AND),
           datasz = 4
           data   = feature_1_and
           Note that the current section offset is 16,
           and there has been no padding so far.  */
	assemble_integer (GEN_INT (0xc0000000), 4, 32, 1);
	assemble_integer (GEN_INT (4), 4, 32, 1);
	assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);

	/* Pad the size of the note to the required alignment. */
	assemble_align (POINTER_SIZE);


r~
Sudakshina Das April 1, 2019, 1:53 p.m. UTC | #2
Hi Richard

Thanks for the comments and pointing out the much cleaner existing asm 
output functions!

On 29/03/2019 17:51, Richard Henderson wrote:
>> +#define ASM_LONG "\t.long\t"
> 
> Do not replicate targetm.asm_out.aligned_op.si, or integer_asm_op, really.
> 
>> +aarch64_file_end_indicate_exec_stack ()
>> +{
>> +  file_end_indicate_exec_stack ();
>> +
>> +  if (!aarch64_bti_enabled ()
>> +      && aarch64_ra_sign_scope == AARCH64_FUNCTION_NONE)
>> +    {
>> +      return;
>> +    }
> 
> This is redundant with...
> 
>> +
>> +  unsigned feature_1_and = 0;
>> +  if (aarch64_bti_enabled ())
>> +    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
>> +
>> +  if (aarch64_ra_sign_scope != AARCH64_FUNCTION_NONE)
>> +    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
>> +
>> +  if (feature_1_and)
> 
> ... this.  I prefer the second, as it's obvious.
> 
>> +      ASM_OUTPUT_ALIGN (asm_out_file, p2align);
>> +      /* name length.  */
>> +      fprintf (asm_out_file, ASM_LONG " 1f - 0f\n");
>> +      /* data length.  */
>> +      fprintf (asm_out_file, ASM_LONG " 4f - 1f\n");
>> +      /* note type: NT_GNU_PROPERTY_TYPE_0.  */
>> +      fprintf (asm_out_file, ASM_LONG " 5\n");
>> +      fprintf (asm_out_file, "0:\n");
>> +      /* vendor name: "GNU".  */
>> +      fprintf (asm_out_file, STRING_ASM_OP " \"GNU\"\n");
>> +      fprintf (asm_out_file, "1:\n");
>> +      ASM_OUTPUT_ALIGN (asm_out_file, p2align);
>> +      /* pr_type: GNU_PROPERTY_AARCH64_FEATURE_1_AND.  */
>> +      fprintf (asm_out_file, ASM_LONG " 0x%x\n",
>> +	       GNU_PROPERTY_AARCH64_FEATURE_1_AND);
>> +      /* pr_datasz.  */\
>> +      fprintf (asm_out_file, ASM_LONG " 3f - 2f\n");
>> +      fprintf (asm_out_file, "2:\n");
>> +      /* GNU_PROPERTY_AARCH64_FEATURE_1_XXX.  */
>> +      fprintf (asm_out_file, ASM_LONG " 0x%x\n", feature_1_and);
>> +      fprintf (asm_out_file, "3:\n");
>> +      ASM_OUTPUT_ALIGN (asm_out_file, p2align);
>> +      fprintf (asm_out_file, "4:\n");
> 
> This could stand to use a comment, a moment's thinking about the sizes, and to
> use the existing asm output functions.
> 
> 	/* PT_NOTE header: namesz, descsz, type.
> 	   namesz = 4 ("GNU\0")
> 	   descsz = 12 (see below)

I was trying out these changes but the descsz of 12 gets rejected by 
readelf. It hits the following

   unsigned int    size = is_32bit_elf ? 4 : 8;

   printf (_("      Properties: "));

   if (pnote->descsz < 8 || (pnote->descsz % size) != 0)
     {
       printf (_("<corrupt GNU_PROPERTY_TYPE, size = %#lx>\n"), 
pnote->descsz);
       return;
     }

Thanks
Sudi

> 	   type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
> 	assemble_align (POINTER_SIZE);
> 	assemble_integer (GEN_INT (4), 4, 32, 1);
> 	assemble_integer (GEN_INT (12), 4, 32, 1);
> 	assemble_integer (GEN_INT (5), 4, 32, 1);
> 
> 	/* PT_NOTE name */
> 	assemble_string ("GNU", 4);
> 
> 	/* PT_NOTE contents for NT_GNU_PROPERTY_TYPE_0:
> 	   type   = 0xc0000000 (GNU_PROPERTY_AARCH64_FEATURE_1_AND),
>             datasz = 4
>             data   = feature_1_and
>             Note that the current section offset is 16,
>             and there has been no padding so far.  */
> 	assemble_integer (GEN_INT (0xc0000000), 4, 32, 1);
> 	assemble_integer (GEN_INT (4), 4, 32, 1);
> 	assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
> 
> 	/* Pad the size of the note to the required alignment. */
> 	assemble_align (POINTER_SIZE);
> 
> 
> r~
>
Richard Henderson April 2, 2019, 1:56 a.m. UTC | #3
On 4/1/19 8:53 PM, Sudakshina Das wrote:
>> This could stand to use a comment, a moment's thinking about the sizes, and to
>> use the existing asm output functions.
>>
>> 	/* PT_NOTE header: namesz, descsz, type.
>> 	   namesz = 4 ("GNU\0")
>> 	   descsz = 12 (see below)
> I was trying out these changes but the descsz of 12 gets rejected by 
> readelf. It hits the following
> 
>    unsigned int    size = is_32bit_elf ? 4 : 8;
> 
>    printf (_("      Properties: "));
> 
>    if (pnote->descsz < 8 || (pnote->descsz % size) != 0)
>      {
>        printf (_("<corrupt GNU_PROPERTY_TYPE, size = %#lx>\n"), 
> pnote->descsz);
>        return;
>      }

Hmm, interesting.  The docs say that padding is not to be included in descsz
(gabi4.1, page 82).  To my eye this is a bug in binutils, but perhaps we will
have to live with it.

Nick, thoughts?


r~
H.J. Lu April 2, 2019, 2:27 a.m. UTC | #4
On Tue, Apr 2, 2019 at 10:05 AM Richard Henderson <rth@twiddle.net> wrote:
>
> On 4/1/19 8:53 PM, Sudakshina Das wrote:
> >> This could stand to use a comment, a moment's thinking about the sizes, and to
> >> use the existing asm output functions.
> >>
> >>      /* PT_NOTE header: namesz, descsz, type.
> >>         namesz = 4 ("GNU\0")
> >>         descsz = 12 (see below)
> > I was trying out these changes but the descsz of 12 gets rejected by
> > readelf. It hits the following
> >
> >    unsigned int    size = is_32bit_elf ? 4 : 8;
> >
> >    printf (_("      Properties: "));
> >
> >    if (pnote->descsz < 8 || (pnote->descsz % size) != 0)
> >      {
> >        printf (_("<corrupt GNU_PROPERTY_TYPE, size = %#lx>\n"),
> > pnote->descsz);
> >        return;
> >      }
>
> Hmm, interesting.  The docs say that padding is not to be included in descsz
> (gabi4.1, page 82).  To my eye this is a bug in binutils, but perhaps we will
> have to live with it.
>
> Nick, thoughts?

descsz is wrong.  From:

https://github.com/hjl-tools/linux-abi/wiki/Linux-Extensions-to-gABI

n_desc The note descriptor. The first n_descsz bytes in n_desc is the pro-
gram property array.

The program property array
Each array element represents one program property with type, data
size and data.
In 64-bit objects, each element is an array of 8-byte integers in the
format of the
target processor. In 32-bit objects, each element is an array of
4-byte integers in
the format of the target processor.
Sudakshina Das April 2, 2019, 9:25 a.m. UTC | #5
Hi

On 02/04/2019 03:27, H.J. Lu wrote:
> On Tue, Apr 2, 2019 at 10:05 AM Richard Henderson <rth@twiddle.net> wrote:
>>
>> On 4/1/19 8:53 PM, Sudakshina Das wrote:
>>>> This could stand to use a comment, a moment's thinking about the sizes, and to
>>>> use the existing asm output functions.
>>>>
>>>>       /* PT_NOTE header: namesz, descsz, type.
>>>>          namesz = 4 ("GNU\0")
>>>>          descsz = 12 (see below)
>>> I was trying out these changes but the descsz of 12 gets rejected by
>>> readelf. It hits the following
>>>
>>>     unsigned int    size = is_32bit_elf ? 4 : 8;
>>>
>>>     printf (_("      Properties: "));
>>>
>>>     if (pnote->descsz < 8 || (pnote->descsz % size) != 0)
>>>       {
>>>         printf (_("<corrupt GNU_PROPERTY_TYPE, size = %#lx>\n"),
>>> pnote->descsz);
>>>         return;
>>>       }
>>
>> Hmm, interesting.  The docs say that padding is not to be included in descsz
>> (gabi4.1, page 82).  To my eye this is a bug in binutils, but perhaps we will
>> have to live with it.
>>
>> Nick, thoughts?
> 
> descsz is wrong.  From:
> 
> https://github.com/hjl-tools/linux-abi/wiki/Linux-Extensions-to-gABI
> 
> n_desc The note descriptor. The first n_descsz bytes in n_desc is the pro-
> gram property array.
> 
> The program property array
> Each array element represents one program property with type, data
> size and data.
> In 64-bit objects, each element is an array of 8-byte integers in the
> format of the
> target processor. In 32-bit objects, each element is an array of
> 4-byte integers in
> the format of the target processor.

Thanks @HJ for clarifying that. I should have been more careful in 
spotting the difference.

@Richard I will update my patch according to your suggestions but 
keeping in mind decssz should be the size of the entire program property 
array so 16 in this case.

Thanks
Sudi
> 
>
Sudakshina Das April 3, 2019, 10:19 a.m. UTC | #6
Hi Richard

On 02/04/2019 10:25, Sudakshina Das wrote:
> Hi
> 
> On 02/04/2019 03:27, H.J. Lu wrote:
>> On Tue, Apr 2, 2019 at 10:05 AM Richard Henderson <rth@twiddle.net> 
>> wrote:
>>>
>>> On 4/1/19 8:53 PM, Sudakshina Das wrote:
>>>>> This could stand to use a comment, a moment's thinking about the 
>>>>> sizes, and to
>>>>> use the existing asm output functions.
>>>>>
>>>>>       /* PT_NOTE header: namesz, descsz, type.
>>>>>          namesz = 4 ("GNU\0")
>>>>>          descsz = 12 (see below)
>>>> I was trying out these changes but the descsz of 12 gets rejected by
>>>> readelf. It hits the following
>>>>
>>>>     unsigned int    size = is_32bit_elf ? 4 : 8;
>>>>
>>>>     printf (_("      Properties: "));
>>>>
>>>>     if (pnote->descsz < 8 || (pnote->descsz % size) != 0)
>>>>       {
>>>>         printf (_("<corrupt GNU_PROPERTY_TYPE, size = %#lx>\n"),
>>>> pnote->descsz);
>>>>         return;
>>>>       }
>>>
>>> Hmm, interesting.  The docs say that padding is not to be included in 
>>> descsz
>>> (gabi4.1, page 82).  To my eye this is a bug in binutils, but perhaps 
>>> we will
>>> have to live with it.
>>>
>>> Nick, thoughts?
>>
>> descsz is wrong.  From:
>>
>> https://github.com/hjl-tools/linux-abi/wiki/Linux-Extensions-to-gABI
>>
>> n_desc The note descriptor. The first n_descsz bytes in n_desc is the 
>> pro-
>> gram property array.
>>
>> The program property array
>> Each array element represents one program property with type, data
>> size and data.
>> In 64-bit objects, each element is an array of 8-byte integers in the
>> format of the
>> target processor. In 32-bit objects, each element is an array of
>> 4-byte integers in
>> the format of the target processor.
> 
> Thanks @HJ for clarifying that. I should have been more careful in 
> spotting the difference.
> 
> @Richard I will update my patch according to your suggestions but 
> keeping in mind decssz should be the size of the entire program property 
> array so 16 in this case.
> 

I have updated the patch as per your suggestions. The Changelog is still 
valid from my original patch.

Thanks
Sudi

> Thanks
> Sudi
>>
>>
>
diff --git a/gcc/config/aarch64/aarch64-linux.h b/gcc/config/aarch64/aarch64-linux.h
index 9d0292d64f20939ccedd7ab56027aa1282826b23..5e8b34ded03c78493f868e38647bf57c2da5187c 100644
--- a/gcc/config/aarch64/aarch64-linux.h
+++ b/gcc/config/aarch64/aarch64-linux.h
@@ -83,7 +83,7 @@
 
 #define GNU_USER_TARGET_D_CRITSEC_SIZE 48
 
-#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
+#define TARGET_ASM_FILE_END aarch64_file_end_indicate_exec_stack
 
 /* Uninitialized common symbols in non-PIE executables, even with
    strong definitions in dependent shared libraries, will resolve
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b38505b0872688634b2d3f625ab8d313e89cfca0..f25f7da8f0224167db68e61a2ba88f0943316360 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -18744,6 +18744,56 @@ aarch64_stack_protect_guard (void)
   return NULL_TREE;
 }
 
+/* Implement TARGET_ASM_FILE_END for AArch64.  This adds the AArch64 GNU NOTE
+   section at the end if needed.  */
+#define GNU_PROPERTY_AARCH64_FEATURE_1_AND	0xc0000000
+#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI	(1U << 0)
+#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC	(1U << 1)
+void
+aarch64_file_end_indicate_exec_stack ()
+{
+  file_end_indicate_exec_stack ();
+
+  unsigned feature_1_and = 0;
+  if (aarch64_bti_enabled ())
+    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
+
+  if (aarch64_ra_sign_scope != AARCH64_FUNCTION_NONE)
+    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
+
+  if (feature_1_and)
+    {
+      /* Generate .note.gnu.property section.  */
+      switch_to_section (get_section (".note.gnu.property",
+				      SECTION_NOTYPE, NULL));
+
+      /* PT_NOTE header: namesz, descsz, type.
+	 namesz = 4 ("GNU\0")
+	 descsz = 16 (Size of the program property array)
+	 type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
+      assemble_align (POINTER_SIZE);
+      assemble_integer (GEN_INT (4), 4, 32, 1);
+      assemble_integer (GEN_INT (16), 4, 32, 1);
+      assemble_integer (GEN_INT (5), 4, 32, 1);
+
+      /* PT_NOTE name.  */
+      assemble_string ("GNU", 4);
+
+      /* PT_NOTE contents for NT_GNU_PROPERTY_TYPE_0:
+	 type   = GNU_PROPERTY_AARCH64_FEATURE_1_AND
+	 datasz = 4
+	 data   = feature_1_and.  */
+      assemble_integer (GEN_INT (GNU_PROPERTY_AARCH64_FEATURE_1_AND), 4, 32, 1);
+      assemble_integer (GEN_INT (4), 4, 32, 1);
+      assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
+
+      /* Pad the size of the note to the required alignment.  */
+      assemble_align (POINTER_SIZE);
+    }
+}
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_PAC
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_BTI
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_AND
 
 /* Target-specific selftests.  */
 
diff --git a/gcc/testsuite/gcc.target/aarch64/bti-1.c b/gcc/testsuite/gcc.target/aarch64/bti-1.c
index a8c60412e310a4f322372f334ae5314f426d310e..5a556b08ed15679b25676a11fe9c7a64641ee671 100644
--- a/gcc/testsuite/gcc.target/aarch64/bti-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/bti-1.c
@@ -61,3 +61,4 @@ lab2:
 }
 /* { dg-final { scan-assembler-times "hint\t34" 1 } } */
 /* { dg-final { scan-assembler-times "hint\t36" 12 } } */
+/* { dg-final { scan-assembler ".note.gnu.property" { target *-*-linux* } } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
index e8e3cdac51350b545e5c2a644a3e1f4d1c37f88d..1fe92ff08935d4c6f08affcbd77ea91537030640 100644
--- a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
@@ -4,7 +4,9 @@
 int
 f (int a, ...)
 {
-  /* { dg-final { scan-assembler-not "str" } } */
+  /* Fails on aarch64*-*-linux* if configured with
+    --enable-standard-branch-protection because of the GNU NOTE section.  */
+  /* { dg-final { scan-assembler-not "str" { target { ! aarch64*-*-linux* } || { ! default_branch_protection } } } } */
   return a;
 }
Richard Henderson April 3, 2019, 10:28 a.m. UTC | #7
On 4/3/19 5:19 PM, Sudakshina Das wrote:
> +      /* PT_NOTE header: namesz, descsz, type.
> +	 namesz = 4 ("GNU\0")
> +	 descsz = 16 (Size of the program property array)
> +	 type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
> +      assemble_align (POINTER_SIZE);
> +      assemble_integer (GEN_INT (4), 4, 32, 1);
> +      assemble_integer (GEN_INT (16), 4, 32, 1);

So, it's 16 only if POINTER_SIZE == 64.

I think ROUND_UP (12, POINTER_BYTES) is what you want here.


r~
Sudakshina Das April 4, 2019, 4:01 p.m. UTC | #8
Hi Richard

On 03/04/2019 11:28, Richard Henderson wrote:
> On 4/3/19 5:19 PM, Sudakshina Das wrote:
>> +      /* PT_NOTE header: namesz, descsz, type.
>> +	 namesz = 4 ("GNU\0")
>> +	 descsz = 16 (Size of the program property array)
>> +	 type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
>> +      assemble_align (POINTER_SIZE);
>> +      assemble_integer (GEN_INT (4), 4, 32, 1);
>> +      assemble_integer (GEN_INT (16), 4, 32, 1);
> 
> So, it's 16 only if POINTER_SIZE == 64.
> 
> I think ROUND_UP (12, POINTER_BYTES) is what you want here.
>


Ah yes. I have made that change now.

Thanks
Sudi

> 
> r~
>
diff --git a/gcc/config/aarch64/aarch64-linux.h b/gcc/config/aarch64/aarch64-linux.h
index 9d0292d64f20939ccedd7ab56027aa1282826b23..5e8b34ded03c78493f868e38647bf57c2da5187c 100644
--- a/gcc/config/aarch64/aarch64-linux.h
+++ b/gcc/config/aarch64/aarch64-linux.h
@@ -83,7 +83,7 @@
 
 #define GNU_USER_TARGET_D_CRITSEC_SIZE 48
 
-#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
+#define TARGET_ASM_FILE_END aarch64_file_end_indicate_exec_stack
 
 /* Uninitialized common symbols in non-PIE executables, even with
    strong definitions in dependent shared libraries, will resolve
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b38505b0872688634b2d3f625ab8d313e89cfca0..83b8ef84808c19fa1214fa06c32957936f5eb520 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -18744,6 +18744,57 @@ aarch64_stack_protect_guard (void)
   return NULL_TREE;
 }
 
+/* Implement TARGET_ASM_FILE_END for AArch64.  This adds the AArch64 GNU NOTE
+   section at the end if needed.  */
+#define GNU_PROPERTY_AARCH64_FEATURE_1_AND	0xc0000000
+#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI	(1U << 0)
+#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC	(1U << 1)
+void
+aarch64_file_end_indicate_exec_stack ()
+{
+  file_end_indicate_exec_stack ();
+
+  unsigned feature_1_and = 0;
+  if (aarch64_bti_enabled ())
+    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
+
+  if (aarch64_ra_sign_scope != AARCH64_FUNCTION_NONE)
+    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
+
+  if (feature_1_and)
+    {
+      /* Generate .note.gnu.property section.  */
+      switch_to_section (get_section (".note.gnu.property",
+				      SECTION_NOTYPE, NULL));
+
+      /* PT_NOTE header: namesz, descsz, type.
+	 namesz = 4 ("GNU\0")
+	 descsz = 16 (Size of the program property array)
+		  [(12 + padding) * Number of array elements]
+	 type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
+      assemble_align (POINTER_SIZE);
+      assemble_integer (GEN_INT (4), 4, 32, 1);
+      assemble_integer (GEN_INT (ROUND_UP (12, POINTER_BYTES)), 4, 32, 1);
+      assemble_integer (GEN_INT (5), 4, 32, 1);
+
+      /* PT_NOTE name.  */
+      assemble_string ("GNU", 4);
+
+      /* PT_NOTE contents for NT_GNU_PROPERTY_TYPE_0:
+	 type   = GNU_PROPERTY_AARCH64_FEATURE_1_AND
+	 datasz = 4
+	 data   = feature_1_and.  */
+      assemble_integer (GEN_INT (GNU_PROPERTY_AARCH64_FEATURE_1_AND), 4, 32, 1);
+      assemble_integer (GEN_INT (4), 4, 32, 1);
+      assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
+
+      /* Pad the size of the note to the required alignment.  */
+      assemble_align (POINTER_SIZE);
+    }
+}
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_PAC
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_BTI
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_AND
 
 /* Target-specific selftests.  */
 
diff --git a/gcc/testsuite/gcc.target/aarch64/bti-1.c b/gcc/testsuite/gcc.target/aarch64/bti-1.c
index a8c60412e310a4f322372f334ae5314f426d310e..5a556b08ed15679b25676a11fe9c7a64641ee671 100644
--- a/gcc/testsuite/gcc.target/aarch64/bti-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/bti-1.c
@@ -61,3 +61,4 @@ lab2:
 }
 /* { dg-final { scan-assembler-times "hint\t34" 1 } } */
 /* { dg-final { scan-assembler-times "hint\t36" 12 } } */
+/* { dg-final { scan-assembler ".note.gnu.property" { target *-*-linux* } } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
index e8e3cdac51350b545e5c2a644a3e1f4d1c37f88d..1fe92ff08935d4c6f08affcbd77ea91537030640 100644
--- a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
@@ -4,7 +4,9 @@
 int
 f (int a, ...)
 {
-  /* { dg-final { scan-assembler-not "str" } } */
+  /* Fails on aarch64*-*-linux* if configured with
+    --enable-standard-branch-protection because of the GNU NOTE section.  */
+  /* { dg-final { scan-assembler-not "str" { target { ! aarch64*-*-linux* } || { ! default_branch_protection } } } } */
   return a;
 }
Sudakshina Das April 12, 2019, 9:34 a.m. UTC | #9
Ping.

On 04/04/2019 17:01, Sudakshina Das wrote:
> Hi Richard
> 
> On 03/04/2019 11:28, Richard Henderson wrote:
>> On 4/3/19 5:19 PM, Sudakshina Das wrote:
>>> +      /* PT_NOTE header: namesz, descsz, type.
>>> +     namesz = 4 ("GNU\0")
>>> +     descsz = 16 (Size of the program property array)
>>> +     type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
>>> +      assemble_align (POINTER_SIZE);
>>> +      assemble_integer (GEN_INT (4), 4, 32, 1);
>>> +      assemble_integer (GEN_INT (16), 4, 32, 1);
>>
>> So, it's 16 only if POINTER_SIZE == 64.
>>
>> I think ROUND_UP (12, POINTER_BYTES) is what you want here.
>>
> 
> 
> Ah yes. I have made that change now.
> 
> Thanks
> Sudi
> 
>>
>> r~
>>
>
James Greenhalgh April 18, 2019, 8:55 a.m. UTC | #10
On Thu, Apr 04, 2019 at 05:01:06PM +0100, Sudakshina Das wrote:
> Hi Richard
> 
> On 03/04/2019 11:28, Richard Henderson wrote:
> > On 4/3/19 5:19 PM, Sudakshina Das wrote:
> >> +      /* PT_NOTE header: namesz, descsz, type.
> >> +	 namesz = 4 ("GNU\0")
> >> +	 descsz = 16 (Size of the program property array)
> >> +	 type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
> >> +      assemble_align (POINTER_SIZE);
> >> +      assemble_integer (GEN_INT (4), 4, 32, 1);
> >> +      assemble_integer (GEN_INT (16), 4, 32, 1);
> > 
> > So, it's 16 only if POINTER_SIZE == 64.
> > 
> > I think ROUND_UP (12, POINTER_BYTES) is what you want here.
> >
> 
> 
> Ah yes. I have made that change now.

This is OK, but instead of:

> diff --git a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
> index e8e3cdac51350b545e5c2a644a3e1f4d1c37f88d..1fe92ff08935d4c6f08affcbd77ea91537030640 100644
> --- a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
> @@ -4,7 +4,9 @@
>  int
>  f (int a, ...)
>  {
> -  /* { dg-final { scan-assembler-not "str" } } */
> +  /* Fails on aarch64*-*-linux* if configured with
> +    --enable-standard-branch-protection because of the GNU NOTE section.  */
> +  /* { dg-final { scan-assembler-not "str" { target { ! aarch64*-*-linux* } || { ! default_branch_protection } } } } */
>    return a;
>  }

Can you just change the regex to check for str followed by a tab, or
something that looks else which looks like the instruction and doesn't
match against 'string'.

Thanks,
James


> 
> Thanks
> Sudi
> 
> > 
> > r~
> > 
>
Sudakshina Das April 23, 2019, 3:55 p.m. UTC | #11
Hi James

-----Original Message-----
From: James Greenhalgh <james.greenhalgh@arm.com> 
Sent: 18 April 2019 09:56
To: Sudakshina Das <Sudi.Das@arm.com>
Cc: Richard Henderson <richard.henderson@linaro.org>; H.J. Lu <hjl.tools@gmail.com>; Richard Henderson <rth@twiddle.net>; gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft <Marcus.Shawcroft@arm.com>; nickc@redhat.com
Subject: Re: [PATCH, GCC, AARCH64] Add GNU note section with BTI and PAC.

On Thu, Apr 04, 2019 at 05:01:06PM +0100, Sudakshina Das wrote:
> Hi Richard
> 
> On 03/04/2019 11:28, Richard Henderson wrote:
> > On 4/3/19 5:19 PM, Sudakshina Das wrote:
> >> +      /* PT_NOTE header: namesz, descsz, type.
> >> +	 namesz = 4 ("GNU\0")
> >> +	 descsz = 16 (Size of the program property array)
> >> +	 type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
> >> +      assemble_align (POINTER_SIZE);
> >> +      assemble_integer (GEN_INT (4), 4, 32, 1);
> >> +      assemble_integer (GEN_INT (16), 4, 32, 1);
> > 
> > So, it's 16 only if POINTER_SIZE == 64.
> > 
> > I think ROUND_UP (12, POINTER_BYTES) is what you want here.
> >
> 
> 
> Ah yes. I have made that change now.

This is OK, but instead of:

> diff --git a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c 
> b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
> index 
> e8e3cdac51350b545e5c2a644a3e1f4d1c37f88d..1fe92ff08935d4c6f08affcbd77e
> a91537030640 100644
> --- a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
> @@ -4,7 +4,9 @@
>  int
>  f (int a, ...)
>  {
> -  /* { dg-final { scan-assembler-not "str" } } */
> +  /* Fails on aarch64*-*-linux* if configured with
> +    --enable-standard-branch-protection because of the GNU NOTE 
> + section.  */
> +  /* { dg-final { scan-assembler-not "str" { target { ! 
> + aarch64*-*-linux* } || { ! default_branch_protection } } } } */
>    return a;
>  }

> Can you just change the regex to check for str followed by a tab, or something that looks else which looks like the instruction and doesn't match against 'string'.

>Thanks,
>James

Ah yes, I have reduced the diff in this test to only update the scan directive to look for 'str\t' instead.
Committed as r270515.

Thanks
Sudi

> 
> Thanks
> Sudi
> 
> > 
> > r~
> > 
>
diff mbox series

Patch

diff --git a/gcc/config/aarch64/aarch64-linux.h b/gcc/config/aarch64/aarch64-linux.h
index 9d0292d64f20939ccedd7ab56027aa1282826b23..5e8b34ded03c78493f868e38647bf57c2da5187c 100644
--- a/gcc/config/aarch64/aarch64-linux.h
+++ b/gcc/config/aarch64/aarch64-linux.h
@@ -83,7 +83,7 @@ 
 
 #define GNU_USER_TARGET_D_CRITSEC_SIZE 48
 
-#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
+#define TARGET_ASM_FILE_END aarch64_file_end_indicate_exec_stack
 
 /* Uninitialized common symbols in non-PIE executables, even with
    strong definitions in dependent shared libraries, will resolve
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b38505b0872688634b2d3f625ab8d313e89cfca0..d616c8360b396ebe3ab2ac0fb799b30830df2b3e 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -18744,6 +18744,67 @@  aarch64_stack_protect_guard (void)
   return NULL_TREE;
 }
 
+/* Implement TARGET_ASM_FILE_END for AArch64.  This adds the AArch64 GNU NOTE
+   section at the end if needed.  */
+#define ASM_LONG "\t.long\t"
+#define GNU_PROPERTY_AARCH64_FEATURE_1_AND	0xc0000000
+#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI	(1U << 0)
+#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC	(1U << 1)
+void
+aarch64_file_end_indicate_exec_stack ()
+{
+  file_end_indicate_exec_stack ();
+
+  if (!aarch64_bti_enabled ()
+      && aarch64_ra_sign_scope == AARCH64_FUNCTION_NONE)
+    {
+      return;
+    }
+
+  unsigned feature_1_and = 0;
+  if (aarch64_bti_enabled ())
+    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
+
+  if (aarch64_ra_sign_scope != AARCH64_FUNCTION_NONE)
+    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
+
+  if (feature_1_and)
+    {
+      int p2align = ptr_mode == SImode ? 2 : 3;
+
+      /* Generate GNU_PROPERTY_AARCH64_FEATURE_1_XXX.  */
+      switch_to_section (get_section (".note.gnu.property",
+				      SECTION_NOTYPE, NULL));
+
+      ASM_OUTPUT_ALIGN (asm_out_file, p2align);
+      /* name length.  */
+      fprintf (asm_out_file, ASM_LONG " 1f - 0f\n");
+      /* data length.  */
+      fprintf (asm_out_file, ASM_LONG " 4f - 1f\n");
+      /* note type: NT_GNU_PROPERTY_TYPE_0.  */
+      fprintf (asm_out_file, ASM_LONG " 5\n");
+      fprintf (asm_out_file, "0:\n");
+      /* vendor name: "GNU".  */
+      fprintf (asm_out_file, STRING_ASM_OP " \"GNU\"\n");
+      fprintf (asm_out_file, "1:\n");
+      ASM_OUTPUT_ALIGN (asm_out_file, p2align);
+      /* pr_type: GNU_PROPERTY_AARCH64_FEATURE_1_AND.  */
+      fprintf (asm_out_file, ASM_LONG " 0x%x\n",
+	       GNU_PROPERTY_AARCH64_FEATURE_1_AND);
+      /* pr_datasz.  */\
+      fprintf (asm_out_file, ASM_LONG " 3f - 2f\n");
+      fprintf (asm_out_file, "2:\n");
+      /* GNU_PROPERTY_AARCH64_FEATURE_1_XXX.  */
+      fprintf (asm_out_file, ASM_LONG " 0x%x\n", feature_1_and);
+      fprintf (asm_out_file, "3:\n");
+      ASM_OUTPUT_ALIGN (asm_out_file, p2align);
+      fprintf (asm_out_file, "4:\n");
+    }
+}
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_PAC
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_BTI
+#undef GNU_PROPERTY_AARCH64_FEATURE_1_AND
+#undef ASM_LONG
 
 /* Target-specific selftests.  */
 
diff --git a/gcc/testsuite/gcc.target/aarch64/bti-1.c b/gcc/testsuite/gcc.target/aarch64/bti-1.c
index a8c60412e310a4f322372f334ae5314f426d310e..5a556b08ed15679b25676a11fe9c7a64641ee671 100644
--- a/gcc/testsuite/gcc.target/aarch64/bti-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/bti-1.c
@@ -61,3 +61,4 @@  lab2:
 }
 /* { dg-final { scan-assembler-times "hint\t34" 1 } } */
 /* { dg-final { scan-assembler-times "hint\t36" 12 } } */
+/* { dg-final { scan-assembler ".note.gnu.property" { target *-*-linux* } } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
index e8e3cdac51350b545e5c2a644a3e1f4d1c37f88d..1fe92ff08935d4c6f08affcbd77ea91537030640 100644
--- a/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/va_arg_1.c
@@ -4,7 +4,9 @@ 
 int
 f (int a, ...)
 {
-  /* { dg-final { scan-assembler-not "str" } } */
+  /* Fails on aarch64*-*-linux* if configured with
+    --enable-standard-branch-protection because of the GNU NOTE section.  */
+  /* { dg-final { scan-assembler-not "str" { target { ! aarch64*-*-linux* } || { ! default_branch_protection } } } } */
   return a;
 }