RE: [PATCH 6/6] aarch64: Fix BTI support in libitm

2020-07-08 Thread Kyrylo Tkachov



> -Original Message-
> From: Szabolcs Nagy 
> Sent: 08 July 2020 17:28
> To: gcc-patches@gcc.gnu.org
> Cc: Richard Earnshaw ; Kyrylo Tkachov
> 
> Subject: [PATCH 6/6] aarch64: Fix BTI support in libitm
> 
> sjlj.S did not have the GNU property note markup and the BTI c
> instructions that are necessary when it is built with branch
> protection.
> 
> The notes are only added when libitm is built with branch
> protection, because old linkers mishandle the note (merge
> them incorrectly or emit warnings), the BTI instructions
> are added unconditionally.
> 
> libitm/ChangeLog:
> 
> 2020-07-08  Szabolcs Nagy  
> 
>   * config/aarch64/sjlj.S: Add BTI marking and related definitions,
>   and add BTI c to function entries.
> 
> ---
> Note: there is some redundancy: the libgcc fixup patch needed
> the same macro definitions, but i did not find a convenient
> place from where both libgcc and libitm can include them. Since
> this is a common problem i expect a change in the assembler
> that will be able to add the note without doing this manually,
> until then i think we can live with the code duplication.

Ok.
Thanks,
Kyrill

> ---
>  libitm/config/aarch64/sjlj.S | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/libitm/config/aarch64/sjlj.S b/libitm/config/aarch64/sjlj.S
> index 5b97b973e27..e2093ca1a97 100644
> --- a/libitm/config/aarch64/sjlj.S
> +++ b/libitm/config/aarch64/sjlj.S
> @@ -24,6 +24,8 @@
> 
>  #include "asmcfi.h"
> 
> +#define BTI_Chint34
> +
>   .text
>   .align  2
>   .global _ITM_beginTransaction
> @@ -31,6 +33,7 @@
> 
>  _ITM_beginTransaction:
>   cfi_startproc
> + BTI_C
>   mov x1, sp
>   stp x29, x30, [sp, -11*16]!
>   cfi_adjust_cfa_offset(11*16)
> @@ -70,6 +73,7 @@ GTM_longjmp:
>   /* The first parameter becomes the return value (x0).
>  The third parameter is ignored for now.  */
>   cfi_startproc
> + BTI_C
>   ldp x19, x20, [x1, 1*16]
>   ldp x21, x22, [x1, 2*16]
>   ldp x23, x24, [x1, 3*16]
> @@ -87,6 +91,29 @@ GTM_longjmp:
>   cfi_endproc
>   .size   GTM_longjmp, . - GTM_longjmp
> 
> +/* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code.  */
> +#define FEATURE_1_AND 0xc000
> +#define FEATURE_1_BTI 1
> +#define FEATURE_1_PAC 2
> +
> +/* Add a NT_GNU_PROPERTY_TYPE_0 note.  */
> +#define GNU_PROPERTY(type, value)\
> +  .section .note.gnu.property, "a";  \
> +  .p2align 3;\
> +  .word 4;   \
> +  .word 16;  \
> +  .word 5;   \
> +  .asciz "GNU";  \
> +  .word type;\
> +  .word 4;   \
> +  .word value;   \
> +  .word 0;
> +
>  #if defined(__linux__) || defined(__FreeBSD__)
>  .section .note.GNU-stack, "", %progbits
> +
> +/* Add GNU property note if built with branch protection.  */
> +# ifdef __ARM_FEATURE_BTI_DEFAULT
> +GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
> +# endif
>  #endif
> --
> 2.17.1



[PATCH 6/6] aarch64: Fix BTI support in libitm

2020-07-08 Thread Szabolcs Nagy
sjlj.S did not have the GNU property note markup and the BTI c
instructions that are necessary when it is built with branch
protection.

The notes are only added when libitm is built with branch
protection, because old linkers mishandle the note (merge
them incorrectly or emit warnings), the BTI instructions
are added unconditionally.

libitm/ChangeLog:

2020-07-08  Szabolcs Nagy  

* config/aarch64/sjlj.S: Add BTI marking and related definitions,
and add BTI c to function entries.

---
Note: there is some redundancy: the libgcc fixup patch needed
the same macro definitions, but i did not find a convenient
place from where both libgcc and libitm can include them. Since
this is a common problem i expect a change in the assembler
that will be able to add the note without doing this manually,
until then i think we can live with the code duplication.
---
 libitm/config/aarch64/sjlj.S | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/libitm/config/aarch64/sjlj.S b/libitm/config/aarch64/sjlj.S
index 5b97b973e27..e2093ca1a97 100644
--- a/libitm/config/aarch64/sjlj.S
+++ b/libitm/config/aarch64/sjlj.S
@@ -24,6 +24,8 @@
 
 #include "asmcfi.h"
 
+#define BTI_C  hint34
+
.text
.align  2
.global _ITM_beginTransaction
@@ -31,6 +33,7 @@
 
 _ITM_beginTransaction:
cfi_startproc
+   BTI_C
mov x1, sp
stp x29, x30, [sp, -11*16]!
cfi_adjust_cfa_offset(11*16)
@@ -70,6 +73,7 @@ GTM_longjmp:
/* The first parameter becomes the return value (x0).
   The third parameter is ignored for now.  */
cfi_startproc
+   BTI_C
ldp x19, x20, [x1, 1*16]
ldp x21, x22, [x1, 2*16]
ldp x23, x24, [x1, 3*16]
@@ -87,6 +91,29 @@ GTM_longjmp:
cfi_endproc
.size   GTM_longjmp, . - GTM_longjmp
 
+/* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code.  */
+#define FEATURE_1_AND 0xc000
+#define FEATURE_1_BTI 1
+#define FEATURE_1_PAC 2
+
+/* Add a NT_GNU_PROPERTY_TYPE_0 note.  */
+#define GNU_PROPERTY(type, value)  \
+  .section .note.gnu.property, "a";\
+  .p2align 3;  \
+  .word 4; \
+  .word 16;\
+  .word 5; \
+  .asciz "GNU";\
+  .word type;  \
+  .word 4; \
+  .word value; \
+  .word 0;
+
 #if defined(__linux__) || defined(__FreeBSD__)
 .section .note.GNU-stack, "", %progbits
+
+/* Add GNU property note if built with branch protection.  */
+# ifdef __ARM_FEATURE_BTI_DEFAULT
+GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
+# endif
 #endif
-- 
2.17.1