* AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
@ 2022-12-01 3:04 Pop, Sebastian
2022-12-05 11:34 ` Richard Sandiford
0 siblings, 1 reply; 7+ messages in thread
From: Pop, Sebastian @ 2022-12-01 3:04 UTC (permalink / raw)
To: gcc-patches; +Cc: sebpop, Kyrylo Tkachov, Richard Sandiford
[-- Attachment #1.1: Type: text/plain, Size: 991 bytes --]
Hi,
Currently patchable area is at the wrong place on AArch64. It is placed
immediately after function label, before .cfi_startproc. This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
modifies aarch64_print_patchable_function_entry to avoid placing
patchable area before .cfi_startproc.
The patch passed bootstrap and regression test on aarch64-linux.
Ok to commit to trunk and backport to active release branches?
Thanks,
Sebastian
gcc/
PR target/93492
* config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
Declared.
* config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
(aarch64_output_patchable_area): New.
* config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
(patchable_area): Define.
gcc/testsuite/
PR target/93492
* gcc.target/aarch64/pr98776.c: New.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch --]
[-- Type: text/x-patch; name="0001-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch", Size: 4289 bytes --]
From b9cf87bcdf65f515b38f1851eb95c18aaa180253 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop@amazon.com>
Date: Wed, 30 Nov 2022 19:45:24 +0000
Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
Currently patchable area is at the wrong place on AArch64. It is placed
immediately after function label, before .cfi_startproc. This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
modifies aarch64_print_patchable_function_entry to avoid placing
patchable area before .cfi_startproc.
gcc/
PR target/93492
* config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
Declared.
* config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
(aarch64_output_patchable_area): New.
* config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
(patchable_area): Define.
gcc/testsuite/
PR target/93492
* gcc.target/aarch64/pr98776.c: New.
---
gcc/config/aarch64/aarch64-protos.h | 2 ++
gcc/config/aarch64/aarch64.cc | 24 +++++++++++++++++++++-
gcc/config/aarch64/aarch64.md | 14 +++++++++++++
gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++++++++
4 files changed, 50 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 4be93c93c26..2fba24d947d 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
extern bool aarch64_harden_sls_retbr_p (void);
extern bool aarch64_harden_sls_blr_p (void);
+extern void aarch64_output_patchable_area (unsigned int, bool);
+
#endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index e97f3b32f7c..e84b33b958c 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -22684,7 +22684,29 @@ aarch64_print_patchable_function_entry (FILE *file,
asm_fprintf (file, "\thint\t34 // bti c\n");
}
- default_print_patchable_function_entry (file, patch_area_size, record_p);
+ if (cfun->machine->label_is_assembled)
+ {
+ rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
+ GEN_INT (record_p));
+ basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+ rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
+ INSN_ADDRESSES_NEW (insn, -1);
+ }
+ else
+ {
+ default_print_patchable_function_entry (file, patch_area_size,
+ record_p);
+ }
+}
+
+/* Output patchable area. */
+
+void
+aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
+{
+ default_print_patchable_function_entry (asm_out_file,
+ patch_area_size,
+ record_p);
}
/* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 76b6898ca04..6501503eb25 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -303,6 +303,7 @@
UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
UNSPEC_LD1RO
UNSPEC_SALT_ADDR
+ UNSPECV_PATCHABLE_AREA
])
(define_c_enum "unspecv" [
@@ -7821,6 +7822,19 @@
[(set_attr "type" "ls64")]
)
+(define_insn "patchable_area"
+ [(unspec_volatile [(match_operand 0 "const_int_operand")
+ (match_operand 1 "const_int_operand")]
+ UNSPECV_PATCHABLE_AREA)]
+ ""
+{
+ aarch64_output_patchable_area (INTVAL (operands[0]),
+ INTVAL (operands[1]) != 0);
+ return "";
+}
+ [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
+)
+
;; AdvSIMD Stuff
(include "aarch64-simd.md")
diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
new file mode 100644
index 00000000000..b075b8f75ef
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
@@ -0,0 +1,11 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE0 label. */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
--
2.37.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
2022-12-01 3:04 AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776] Pop, Sebastian
@ 2022-12-05 11:34 ` Richard Sandiford
2022-12-07 0:51 ` Pop, Sebastian
0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2022-12-05 11:34 UTC (permalink / raw)
To: Pop, Sebastian; +Cc: gcc-patches, sebpop, Kyrylo Tkachov
"Pop, Sebastian" <spop@amazon.com> writes:
> Hi,
>
> Currently patchable area is at the wrong place on AArch64. It is placed
> immediately after function label, before .cfi_startproc. This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> modifies aarch64_print_patchable_function_entry to avoid placing
> patchable area before .cfi_startproc.
>
> The patch passed bootstrap and regression test on aarch64-linux.
> Ok to commit to trunk and backport to active release branches?
Looks good, but doesn't the problem described in the PR then still
apply to the BTI emitted by:
if (cfun->machine->label_is_assembled
&& aarch64_bti_enabled ()
&& !cgraph_node::get (cfun->decl)->only_called_directly_p ())
{
/* Remove the BTI that follows the patch area and insert a new BTI
before the patch area right after the function label. */
rtx_insn *insn = next_real_nondebug_insn (get_insns ());
if (insn
&& INSN_P (insn)
&& GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
&& XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
delete_insn (insn);
asm_fprintf (file, "\thint\t34 // bti c\n");
}
? It seems like the BTI will be before the cfi_startproc and the
patchable entry afterwards.
I guess we should keep the BTI instruction as-is (rather than printing
a .hint) and emit the new UNSPECV_PATCHABLE_AREA after the BTI rather
than before it.
Thanks,
Richard
> Thanks,
> Sebastian
>
> gcc/
> PR target/93492
> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
> Declared.
> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
> (aarch64_output_patchable_area): New.
> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
> (patchable_area): Define.
>
> gcc/testsuite/
> PR target/93492
> * gcc.target/aarch64/pr98776.c: New.
>
>
> From b9cf87bcdf65f515b38f1851eb95c18aaa180253 Mon Sep 17 00:00:00 2001
> From: Sebastian Pop <spop@amazon.com>
> Date: Wed, 30 Nov 2022 19:45:24 +0000
> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> Currently patchable area is at the wrong place on AArch64. It is placed
> immediately after function label, before .cfi_startproc. This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> modifies aarch64_print_patchable_function_entry to avoid placing
> patchable area before .cfi_startproc.
>
> gcc/
> PR target/93492
> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
> Declared.
> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
> (aarch64_output_patchable_area): New.
> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
> (patchable_area): Define.
>
> gcc/testsuite/
> PR target/93492
> * gcc.target/aarch64/pr98776.c: New.
> ---
> gcc/config/aarch64/aarch64-protos.h | 2 ++
> gcc/config/aarch64/aarch64.cc | 24 +++++++++++++++++++++-
> gcc/config/aarch64/aarch64.md | 14 +++++++++++++
> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++++++++
> 4 files changed, 50 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 4be93c93c26..2fba24d947d 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
> extern bool aarch64_harden_sls_retbr_p (void);
> extern bool aarch64_harden_sls_blr_p (void);
>
> +extern void aarch64_output_patchable_area (unsigned int, bool);
> +
> #endif /* GCC_AARCH64_PROTOS_H */
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index e97f3b32f7c..e84b33b958c 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -22684,7 +22684,29 @@ aarch64_print_patchable_function_entry (FILE *file,
> asm_fprintf (file, "\thint\t34 // bti c\n");
> }
>
> - default_print_patchable_function_entry (file, patch_area_size, record_p);
> + if (cfun->machine->label_is_assembled)
> + {
> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
> + GEN_INT (record_p));
> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
> + INSN_ADDRESSES_NEW (insn, -1);
> + }
> + else
> + {
> + default_print_patchable_function_entry (file, patch_area_size,
> + record_p);
> + }
> +}
> +
> +/* Output patchable area. */
> +
> +void
> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
> +{
> + default_print_patchable_function_entry (asm_out_file,
> + patch_area_size,
> + record_p);
> }
>
> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 76b6898ca04..6501503eb25 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -303,6 +303,7 @@
> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
> UNSPEC_LD1RO
> UNSPEC_SALT_ADDR
> + UNSPECV_PATCHABLE_AREA
> ])
>
> (define_c_enum "unspecv" [
> @@ -7821,6 +7822,19 @@
> [(set_attr "type" "ls64")]
> )
>
> +(define_insn "patchable_area"
> + [(unspec_volatile [(match_operand 0 "const_int_operand")
> + (match_operand 1 "const_int_operand")]
> + UNSPECV_PATCHABLE_AREA)]
> + ""
> +{
> + aarch64_output_patchable_area (INTVAL (operands[0]),
> + INTVAL (operands[1]) != 0);
> + return "";
> +}
> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
> +)
> +
> ;; AdvSIMD Stuff
> (include "aarch64-simd.md")
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> new file mode 100644
> index 00000000000..b075b8f75ef
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> @@ -0,0 +1,11 @@
> +/* { dg-do "compile" } */
> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
> +
> +/* Test the placement of the .LPFE0 label. */
> +
> +void
> +foo (void)
> +{
> +}
> +
> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
2022-12-05 11:34 ` Richard Sandiford
@ 2022-12-07 0:51 ` Pop, Sebastian
2022-12-07 9:12 ` Richard Sandiford
0 siblings, 1 reply; 7+ messages in thread
From: Pop, Sebastian @ 2022-12-07 0:51 UTC (permalink / raw)
To: Richard Sandiford; +Cc: gcc-patches, sebpop, Kyrylo Tkachov
[-- Attachment #1.1: Type: text/plain, Size: 7733 bytes --]
Thanks Richard for your review and for pointing out the issue with BTI.
The current patch removes the existing BTI instruction,
and then adds the BTI hint when expanding the patchable_area pseudo.
The attached patch passed bootstrap and regression test on arm64-linux.
Ok to commit to gcc trunk?
Thank you,
Sebastian
________________________________
From: Richard Sandiford <richard.sandiford@arm.com>
Sent: Monday, December 5, 2022 5:34:40 AM
To: Pop, Sebastian
Cc: gcc-patches@gcc.gnu.org; sebpop@gmail.com; Kyrylo Tkachov
Subject: RE: [EXTERNAL]AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
"Pop, Sebastian" <spop@amazon.com> writes:
> Hi,
>
> Currently patchable area is at the wrong place on AArch64. It is placed
> immediately after function label, before .cfi_startproc. This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> modifies aarch64_print_patchable_function_entry to avoid placing
> patchable area before .cfi_startproc.
>
> The patch passed bootstrap and regression test on aarch64-linux.
> Ok to commit to trunk and backport to active release branches?
Looks good, but doesn't the problem described in the PR then still
apply to the BTI emitted by:
if (cfun->machine->label_is_assembled
&& aarch64_bti_enabled ()
&& !cgraph_node::get (cfun->decl)->only_called_directly_p ())
{
/* Remove the BTI that follows the patch area and insert a new BTI
before the patch area right after the function label. */
rtx_insn *insn = next_real_nondebug_insn (get_insns ());
if (insn
&& INSN_P (insn)
&& GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
&& XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
delete_insn (insn);
asm_fprintf (file, "\thint\t34 // bti c\n");
}
? It seems like the BTI will be before the cfi_startproc and the
patchable entry afterwards.
I guess we should keep the BTI instruction as-is (rather than printing
a .hint) and emit the new UNSPECV_PATCHABLE_AREA after the BTI rather
than before it.
Thanks,
Richard
> Thanks,
> Sebastian
>
> gcc/
> PR target/93492
> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
> Declared.
> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
> (aarch64_output_patchable_area): New.
> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
> (patchable_area): Define.
>
> gcc/testsuite/
> PR target/93492
> * gcc.target/aarch64/pr98776.c: New.
>
>
> From b9cf87bcdf65f515b38f1851eb95c18aaa180253 Mon Sep 17 00:00:00 2001
> From: Sebastian Pop <spop@amazon.com>
> Date: Wed, 30 Nov 2022 19:45:24 +0000
> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> Currently patchable area is at the wrong place on AArch64. It is placed
> immediately after function label, before .cfi_startproc. This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> modifies aarch64_print_patchable_function_entry to avoid placing
> patchable area before .cfi_startproc.
>
> gcc/
> PR target/93492
> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
> Declared.
> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
> (aarch64_output_patchable_area): New.
> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
> (patchable_area): Define.
>
> gcc/testsuite/
> PR target/93492
> * gcc.target/aarch64/pr98776.c: New.
> ---
> gcc/config/aarch64/aarch64-protos.h | 2 ++
> gcc/config/aarch64/aarch64.cc | 24 +++++++++++++++++++++-
> gcc/config/aarch64/aarch64.md | 14 +++++++++++++
> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++++++++
> 4 files changed, 50 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 4be93c93c26..2fba24d947d 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
> extern bool aarch64_harden_sls_retbr_p (void);
> extern bool aarch64_harden_sls_blr_p (void);
>
> +extern void aarch64_output_patchable_area (unsigned int, bool);
> +
> #endif /* GCC_AARCH64_PROTOS_H */
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index e97f3b32f7c..e84b33b958c 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -22684,7 +22684,29 @@ aarch64_print_patchable_function_entry (FILE *file,
> asm_fprintf (file, "\thint\t34 // bti c\n");
> }
>
> - default_print_patchable_function_entry (file, patch_area_size, record_p);
> + if (cfun->machine->label_is_assembled)
> + {
> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
> + GEN_INT (record_p));
> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
> + INSN_ADDRESSES_NEW (insn, -1);
> + }
> + else
> + {
> + default_print_patchable_function_entry (file, patch_area_size,
> + record_p);
> + }
> +}
> +
> +/* Output patchable area. */
> +
> +void
> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
> +{
> + default_print_patchable_function_entry (asm_out_file,
> + patch_area_size,
> + record_p);
> }
>
> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 76b6898ca04..6501503eb25 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -303,6 +303,7 @@
> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
> UNSPEC_LD1RO
> UNSPEC_SALT_ADDR
> + UNSPECV_PATCHABLE_AREA
> ])
>
> (define_c_enum "unspecv" [
> @@ -7821,6 +7822,19 @@
> [(set_attr "type" "ls64")]
> )
>
> +(define_insn "patchable_area"
> + [(unspec_volatile [(match_operand 0 "const_int_operand")
> + (match_operand 1 "const_int_operand")]
> + UNSPECV_PATCHABLE_AREA)]
> + ""
> +{
> + aarch64_output_patchable_area (INTVAL (operands[0]),
> + INTVAL (operands[1]) != 0);
> + return "";
> +}
> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
> +)
> +
> ;; AdvSIMD Stuff
> (include "aarch64-simd.md")
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> new file mode 100644
> index 00000000000..b075b8f75ef
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> @@ -0,0 +1,11 @@
> +/* { dg-do "compile" } */
> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
> +
> +/* Test the placement of the .LPFE0 label. */
> +
> +void
> +foo (void)
> +{
> +}
> +
> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch --]
[-- Type: text/x-patch; name="0001-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch", Size: 7319 bytes --]
From a26a38ef91d3cb8bd776c66be735a2ec1f46d8fb Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop@amazon.com>
Date: Wed, 30 Nov 2022 19:45:24 +0000
Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
Currently patchable area is at the wrong place on AArch64. It is placed
immediately after function label, before .cfi_startproc. This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
modifies aarch64_print_patchable_function_entry to avoid placing
patchable area before .cfi_startproc.
gcc/
PR target/93492
* config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
Declared.
* config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
(aarch64_output_patchable_area): New.
* config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
(patchable_area): Define.
gcc/testsuite/
PR target/93492
* gcc.target/aarch64/pr98776.c: New.
* gcc.target/aarch64/pr92424-2.c: Adjust pattern.
* gcc.target/aarch64/pr92424-3.c: Adjust pattern.
---
gcc/config/aarch64/aarch64-protos.h | 2 +
gcc/config/aarch64/aarch64.cc | 56 +++++++++++++++-----
gcc/config/aarch64/aarch64.md | 14 +++++
gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
6 files changed, 71 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 4be93c93c26..2fba24d947d 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
extern bool aarch64_harden_sls_retbr_p (void);
extern bool aarch64_harden_sls_blr_p (void);
+extern void aarch64_output_patchable_area (unsigned int, bool);
+
#endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index e97f3b32f7c..4d25f492cfa 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -22661,30 +22661,58 @@ aarch64_declare_function_name (FILE *stream, const char* name,
cfun->machine->label_is_assembled = true;
}
-/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
- the function label and emit a BTI if necessary. */
+/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
void
aarch64_print_patchable_function_entry (FILE *file,
unsigned HOST_WIDE_INT patch_area_size,
bool record_p)
{
- if (cfun->machine->label_is_assembled
- && aarch64_bti_enabled ()
+ if (!cfun->machine->label_is_assembled)
+ {
+ /* Emit the patching area before the entry label, if any. */
+ default_print_patchable_function_entry (file, patch_area_size,
+ record_p);
+ }
+ else
+ {
+ if (aarch64_bti_enabled ()
+ && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+ {
+ /* Remove the BTI that follows the patch area and insert a new BTI
+ before the patch area right after the function label. */
+ rtx_insn *insn = next_real_nondebug_insn (get_insns ());
+ if (insn
+ && INSN_P (insn)
+ && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
+ && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
+ delete_insn (insn);
+ }
+
+ /* Emit a pseudo PATCHABLE_AREA instruction. The pseudo gets expanded
+ with aarch64_output_patchable_area. */
+ rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
+ GEN_INT (record_p));
+ basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+ rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
+ INSN_ADDRESSES_NEW (insn, -1);
+ }
+}
+
+/* Output patchable area. */
+
+void
+aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
+{
+ /* Emit a BTI if necessary. */
+ if (aarch64_bti_enabled ()
&& !cgraph_node::get (cfun->decl)->only_called_directly_p ())
{
- /* Remove the BTI that follows the patch area and insert a new BTI
- before the patch area right after the function label. */
- rtx_insn *insn = next_real_nondebug_insn (get_insns ());
- if (insn
- && INSN_P (insn)
- && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
- && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
- delete_insn (insn);
- asm_fprintf (file, "\thint\t34 // bti c\n");
+ asm_fprintf (asm_out_file, "\thint\t34 // bti c\n");
}
- default_print_patchable_function_entry (file, patch_area_size, record_p);
+ default_print_patchable_function_entry (asm_out_file, patch_area_size,
+ record_p);
}
/* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 76b6898ca04..6501503eb25 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -303,6 +303,7 @@
UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
UNSPEC_LD1RO
UNSPEC_SALT_ADDR
+ UNSPECV_PATCHABLE_AREA
])
(define_c_enum "unspecv" [
@@ -7821,6 +7822,19 @@
[(set_attr "type" "ls64")]
)
+(define_insn "patchable_area"
+ [(unspec_volatile [(match_operand 0 "const_int_operand")
+ (match_operand 1 "const_int_operand")]
+ UNSPECV_PATCHABLE_AREA)]
+ ""
+{
+ aarch64_output_patchable_area (INTVAL (operands[0]),
+ INTVAL (operands[1]) != 0);
+ return "";
+}
+ [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
+)
+
;; AdvSIMD Stuff
(include "aarch64-simd.md")
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
index 12465213aef..0a79901108c 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
f10_bti ()
{
}
-/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
index 2c6a73789f0..854bb7f9fec 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
f10_pac ()
{
}
-/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
new file mode 100644
index 00000000000..b075b8f75ef
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
@@ -0,0 +1,11 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE0 label. */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
--
2.38.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
2022-12-07 0:51 ` Pop, Sebastian
@ 2022-12-07 9:12 ` Richard Sandiford
2022-12-07 18:46 ` Pop, Sebastian
0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2022-12-07 9:12 UTC (permalink / raw)
To: Pop, Sebastian; +Cc: gcc-patches, sebpop, Kyrylo Tkachov
"Pop, Sebastian" <spop@amazon.com> writes:
> Thanks Richard for your review and for pointing out the issue with BTI.
>
>
> The current patch removes the existing BTI instruction,
>
> and then adds the BTI hint when expanding the patchable_area pseudo.
Thanks. I still think...
> The attached patch passed bootstrap and regression test on arm64-linux.
>
> Ok to commit to gcc trunk?
>
>
> Thank you,
> Sebastian
>
> ________________________________
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: Monday, December 5, 2022 5:34:40 AM
> To: Pop, Sebastian
> Cc: gcc-patches@gcc.gnu.org; sebpop@gmail.com; Kyrylo Tkachov
> Subject: RE: [EXTERNAL]AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> "Pop, Sebastian" <spop@amazon.com> writes:
>> Hi,
>>
>> Currently patchable area is at the wrong place on AArch64. It is placed
>> immediately after function label, before .cfi_startproc. This patch
>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>> modifies aarch64_print_patchable_function_entry to avoid placing
>> patchable area before .cfi_startproc.
>>
>> The patch passed bootstrap and regression test on aarch64-linux.
>> Ok to commit to trunk and backport to active release branches?
>
> Looks good, but doesn't the problem described in the PR then still
> apply to the BTI emitted by:
>
> if (cfun->machine->label_is_assembled
> && aarch64_bti_enabled ()
> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
> {
> /* Remove the BTI that follows the patch area and insert a new BTI
> before the patch area right after the function label. */
> rtx_insn *insn = next_real_nondebug_insn (get_insns ());
> if (insn
> && INSN_P (insn)
> && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
> && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
> delete_insn (insn);
> asm_fprintf (file, "\thint\t34 // bti c\n");
> }
>
> ? It seems like the BTI will be before the cfi_startproc and the
> patchable entry afterwards.
>
> I guess we should keep the BTI instruction as-is (rather than printing
> a .hint) and emit the new UNSPECV_PATCHABLE_AREA after the BTI rather
> than before it.
...this approach would be slightly cleaner though. The .hint asm string
we're emitting here is exactly the same as the one emiitted by the
original bti_c instruction. The only reason for deleting the
instruction and emitting text was because we were emitting the
patchable entry directly as text, and the BTI text had to come
before the patchable entry text.
Now that we're emitting the patchable entry via a normal instruction
(a good thing!) we can keep the preceding bti_c as a normal instruction
too. That is, I think we should use emit_insn_after to emit the entry
after the bti_c insn (if it exists) instead of before BB_HEAD.
Thanks,
Richard
>> gcc/
>> PR target/93492
>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>> Declared.
>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>> (aarch64_output_patchable_area): New.
>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>> (patchable_area): Define.
>>
>> gcc/testsuite/
>> PR target/93492
>> * gcc.target/aarch64/pr98776.c: New.
>>
>>
>> From b9cf87bcdf65f515b38f1851eb95c18aaa180253 Mon Sep 17 00:00:00 2001
>> From: Sebastian Pop <spop@amazon.com>
>> Date: Wed, 30 Nov 2022 19:45:24 +0000
>> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>>
>> Currently patchable area is at the wrong place on AArch64. It is placed
>> immediately after function label, before .cfi_startproc. This patch
>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>> modifies aarch64_print_patchable_function_entry to avoid placing
>> patchable area before .cfi_startproc.
>>
>> gcc/
>> PR target/93492
>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>> Declared.
>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>> (aarch64_output_patchable_area): New.
>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>> (patchable_area): Define.
>>
>> gcc/testsuite/
>> PR target/93492
>> * gcc.target/aarch64/pr98776.c: New.
>> ---
>> gcc/config/aarch64/aarch64-protos.h | 2 ++
>> gcc/config/aarch64/aarch64.cc | 24 +++++++++++++++++++++-
>> gcc/config/aarch64/aarch64.md | 14 +++++++++++++
>> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++++++++
>> 4 files changed, 50 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>>
>> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
>> index 4be93c93c26..2fba24d947d 100644
>> --- a/gcc/config/aarch64/aarch64-protos.h
>> +++ b/gcc/config/aarch64/aarch64-protos.h
>> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
>> extern bool aarch64_harden_sls_retbr_p (void);
>> extern bool aarch64_harden_sls_blr_p (void);
>>
>> +extern void aarch64_output_patchable_area (unsigned int, bool);
>> +
>> #endif /* GCC_AARCH64_PROTOS_H */
>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> index e97f3b32f7c..e84b33b958c 100644
>> --- a/gcc/config/aarch64/aarch64.cc
>> +++ b/gcc/config/aarch64/aarch64.cc
>> @@ -22684,7 +22684,29 @@ aarch64_print_patchable_function_entry (FILE *file,
>> asm_fprintf (file, "\thint\t34 // bti c\n");
>> }
>>
>> - default_print_patchable_function_entry (file, patch_area_size, record_p);
>> + if (cfun->machine->label_is_assembled)
>> + {
>> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
>> + GEN_INT (record_p));
>> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
>> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
>> + INSN_ADDRESSES_NEW (insn, -1);
>> + }
>> + else
>> + {
>> + default_print_patchable_function_entry (file, patch_area_size,
>> + record_p);
>> + }
>> +}
>> +
>> +/* Output patchable area. */
>> +
>> +void
>> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
>> +{
>> + default_print_patchable_function_entry (asm_out_file,
>> + patch_area_size,
>> + record_p);
>> }
>>
>> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index 76b6898ca04..6501503eb25 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -303,6 +303,7 @@
>> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
>> UNSPEC_LD1RO
>> UNSPEC_SALT_ADDR
>> + UNSPECV_PATCHABLE_AREA
>> ])
>>
>> (define_c_enum "unspecv" [
>> @@ -7821,6 +7822,19 @@
>> [(set_attr "type" "ls64")]
>> )
>>
>> +(define_insn "patchable_area"
>> + [(unspec_volatile [(match_operand 0 "const_int_operand")
>> + (match_operand 1 "const_int_operand")]
>> + UNSPECV_PATCHABLE_AREA)]
>> + ""
>> +{
>> + aarch64_output_patchable_area (INTVAL (operands[0]),
>> + INTVAL (operands[1]) != 0);
>> + return "";
>> +}
>> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
>> +)
>> +
>> ;; AdvSIMD Stuff
>> (include "aarch64-simd.md")
>>
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>> new file mode 100644
>> index 00000000000..b075b8f75ef
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>> @@ -0,0 +1,11 @@
>> +/* { dg-do "compile" } */
>> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
>> +
>> +/* Test the placement of the .LPFE0 label. */
>> +
>> +void
>> +foo (void)
>> +{
>> +}
>> +
>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
> From a26a38ef91d3cb8bd776c66be735a2ec1f46d8fb Mon Sep 17 00:00:00 2001
> From: Sebastian Pop <spop@amazon.com>
> Date: Wed, 30 Nov 2022 19:45:24 +0000
> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> Currently patchable area is at the wrong place on AArch64. It is placed
> immediately after function label, before .cfi_startproc. This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> modifies aarch64_print_patchable_function_entry to avoid placing
> patchable area before .cfi_startproc.
>
> gcc/
> PR target/93492
> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
> Declared.
> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
> (aarch64_output_patchable_area): New.
> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
> (patchable_area): Define.
>
> gcc/testsuite/
> PR target/93492
> * gcc.target/aarch64/pr98776.c: New.
> * gcc.target/aarch64/pr92424-2.c: Adjust pattern.
> * gcc.target/aarch64/pr92424-3.c: Adjust pattern.
> ---
> gcc/config/aarch64/aarch64-protos.h | 2 +
> gcc/config/aarch64/aarch64.cc | 56 +++++++++++++++-----
> gcc/config/aarch64/aarch64.md | 14 +++++
> gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
> gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
> 6 files changed, 71 insertions(+), 16 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 4be93c93c26..2fba24d947d 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
> extern bool aarch64_harden_sls_retbr_p (void);
> extern bool aarch64_harden_sls_blr_p (void);
>
> +extern void aarch64_output_patchable_area (unsigned int, bool);
> +
> #endif /* GCC_AARCH64_PROTOS_H */
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index e97f3b32f7c..4d25f492cfa 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -22661,30 +22661,58 @@ aarch64_declare_function_name (FILE *stream, const char* name,
> cfun->machine->label_is_assembled = true;
> }
>
> -/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
> - the function label and emit a BTI if necessary. */
> +/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
>
> void
> aarch64_print_patchable_function_entry (FILE *file,
> unsigned HOST_WIDE_INT patch_area_size,
> bool record_p)
> {
> - if (cfun->machine->label_is_assembled
> - && aarch64_bti_enabled ()
> + if (!cfun->machine->label_is_assembled)
> + {
> + /* Emit the patching area before the entry label, if any. */
> + default_print_patchable_function_entry (file, patch_area_size,
> + record_p);
> + }
> + else
> + {
> + if (aarch64_bti_enabled ()
> + && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
> + {
> + /* Remove the BTI that follows the patch area and insert a new BTI
> + before the patch area right after the function label. */
> + rtx_insn *insn = next_real_nondebug_insn (get_insns ());
> + if (insn
> + && INSN_P (insn)
> + && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
> + && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
> + delete_insn (insn);
> + }
> +
> + /* Emit a pseudo PATCHABLE_AREA instruction. The pseudo gets expanded
> + with aarch64_output_patchable_area. */
> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
> + GEN_INT (record_p));
> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
> + INSN_ADDRESSES_NEW (insn, -1);
> + }
> +}
> +
> +/* Output patchable area. */
> +
> +void
> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
> +{
> + /* Emit a BTI if necessary. */
> + if (aarch64_bti_enabled ()
> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
> {
> - /* Remove the BTI that follows the patch area and insert a new BTI
> - before the patch area right after the function label. */
> - rtx_insn *insn = next_real_nondebug_insn (get_insns ());
> - if (insn
> - && INSN_P (insn)
> - && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
> - && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
> - delete_insn (insn);
> - asm_fprintf (file, "\thint\t34 // bti c\n");
> + asm_fprintf (asm_out_file, "\thint\t34 // bti c\n");
> }
>
> - default_print_patchable_function_entry (file, patch_area_size, record_p);
> + default_print_patchable_function_entry (asm_out_file, patch_area_size,
> + record_p);
> }
>
> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 76b6898ca04..6501503eb25 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -303,6 +303,7 @@
> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
> UNSPEC_LD1RO
> UNSPEC_SALT_ADDR
> + UNSPECV_PATCHABLE_AREA
> ])
>
> (define_c_enum "unspecv" [
> @@ -7821,6 +7822,19 @@
> [(set_attr "type" "ls64")]
> )
>
> +(define_insn "patchable_area"
> + [(unspec_volatile [(match_operand 0 "const_int_operand")
> + (match_operand 1 "const_int_operand")]
> + UNSPECV_PATCHABLE_AREA)]
> + ""
> +{
> + aarch64_output_patchable_area (INTVAL (operands[0]),
> + INTVAL (operands[1]) != 0);
> + return "";
> +}
> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
> +)
> +
> ;; AdvSIMD Stuff
> (include "aarch64-simd.md")
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> index 12465213aef..0a79901108c 100644
> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
> f10_bti ()
> {
> }
> -/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> index 2c6a73789f0..854bb7f9fec 100644
> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
> f10_pac ()
> {
> }
> -/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> new file mode 100644
> index 00000000000..b075b8f75ef
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> @@ -0,0 +1,11 @@
> +/* { dg-do "compile" } */
> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
> +
> +/* Test the placement of the .LPFE0 label. */
> +
> +void
> +foo (void)
> +{
> +}
> +
> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
2022-12-07 9:12 ` Richard Sandiford
@ 2022-12-07 18:46 ` Pop, Sebastian
2022-12-08 7:38 ` Richard Sandiford
0 siblings, 1 reply; 7+ messages in thread
From: Pop, Sebastian @ 2022-12-07 18:46 UTC (permalink / raw)
To: Richard Sandiford; +Cc: gcc-patches, sebpop, Kyrylo Tkachov
[-- Attachment #1.1: Type: text/plain, Size: 17596 bytes --]
Hi Richard,
Please find attached a patch that follows your recommendations to generate the BTI_C instructions.
Please let me know if the patch can be further improved.
The patch passed bootstrap and regressions tests on arm64-linux.
Thanks,
Sebastian
________________________________
From: Richard Sandiford <richard.sandiford@arm.com>
Sent: Wednesday, December 7, 2022 3:12:08 AM
To: Pop, Sebastian
Cc: gcc-patches@gcc.gnu.org; sebpop@gmail.com; Kyrylo Tkachov
Subject: RE: [EXTERNAL]AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
"Pop, Sebastian" <spop@amazon.com> writes:
> Thanks Richard for your review and for pointing out the issue with BTI.
>
>
> The current patch removes the existing BTI instruction,
>
> and then adds the BTI hint when expanding the patchable_area pseudo.
Thanks. I still think...
> The attached patch passed bootstrap and regression test on arm64-linux.
>
> Ok to commit to gcc trunk?
>
>
> Thank you,
> Sebastian
>
> ________________________________
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: Monday, December 5, 2022 5:34:40 AM
> To: Pop, Sebastian
> Cc: gcc-patches@gcc.gnu.org; sebpop@gmail.com; Kyrylo Tkachov
> Subject: RE: [EXTERNAL]AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> "Pop, Sebastian" <spop@amazon.com> writes:
>> Hi,
>>
>> Currently patchable area is at the wrong place on AArch64. It is placed
>> immediately after function label, before .cfi_startproc. This patch
>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>> modifies aarch64_print_patchable_function_entry to avoid placing
>> patchable area before .cfi_startproc.
>>
>> The patch passed bootstrap and regression test on aarch64-linux.
>> Ok to commit to trunk and backport to active release branches?
>
> Looks good, but doesn't the problem described in the PR then still
> apply to the BTI emitted by:
>
> if (cfun->machine->label_is_assembled
> && aarch64_bti_enabled ()
> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
> {
> /* Remove the BTI that follows the patch area and insert a new BTI
> before the patch area right after the function label. */
> rtx_insn *insn = next_real_nondebug_insn (get_insns ());
> if (insn
> && INSN_P (insn)
> && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
> && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
> delete_insn (insn);
> asm_fprintf (file, "\thint\t34 // bti c\n");
> }
>
> ? It seems like the BTI will be before the cfi_startproc and the
> patchable entry afterwards.
>
> I guess we should keep the BTI instruction as-is (rather than printing
> a .hint) and emit the new UNSPECV_PATCHABLE_AREA after the BTI rather
> than before it.
...this approach would be slightly cleaner though. The .hint asm string
we're emitting here is exactly the same as the one emiitted by the
original bti_c instruction. The only reason for deleting the
instruction and emitting text was because we were emitting the
patchable entry directly as text, and the BTI text had to come
before the patchable entry text.
Now that we're emitting the patchable entry via a normal instruction
(a good thing!) we can keep the preceding bti_c as a normal instruction
too. That is, I think we should use emit_insn_after to emit the entry
after the bti_c insn (if it exists) instead of before BB_HEAD.
Thanks,
Richard
>> gcc/
>> PR target/93492
>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>> Declared.
>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>> (aarch64_output_patchable_area): New.
>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>> (patchable_area): Define.
>>
>> gcc/testsuite/
>> PR target/93492
>> * gcc.target/aarch64/pr98776.c: New.
>>
>>
>> From b9cf87bcdf65f515b38f1851eb95c18aaa180253 Mon Sep 17 00:00:00 2001
>> From: Sebastian Pop <spop@amazon.com>
>> Date: Wed, 30 Nov 2022 19:45:24 +0000
>> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>>
>> Currently patchable area is at the wrong place on AArch64. It is placed
>> immediately after function label, before .cfi_startproc. This patch
>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>> modifies aarch64_print_patchable_function_entry to avoid placing
>> patchable area before .cfi_startproc.
>>
>> gcc/
>> PR target/93492
>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>> Declared.
>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>> (aarch64_output_patchable_area): New.
>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>> (patchable_area): Define.
>>
>> gcc/testsuite/
>> PR target/93492
>> * gcc.target/aarch64/pr98776.c: New.
>> ---
>> gcc/config/aarch64/aarch64-protos.h | 2 ++
>> gcc/config/aarch64/aarch64.cc | 24 +++++++++++++++++++++-
>> gcc/config/aarch64/aarch64.md | 14 +++++++++++++
>> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++++++++
>> 4 files changed, 50 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>>
>> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
>> index 4be93c93c26..2fba24d947d 100644
>> --- a/gcc/config/aarch64/aarch64-protos.h
>> +++ b/gcc/config/aarch64/aarch64-protos.h
>> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
>> extern bool aarch64_harden_sls_retbr_p (void);
>> extern bool aarch64_harden_sls_blr_p (void);
>>
>> +extern void aarch64_output_patchable_area (unsigned int, bool);
>> +
>> #endif /* GCC_AARCH64_PROTOS_H */
>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> index e97f3b32f7c..e84b33b958c 100644
>> --- a/gcc/config/aarch64/aarch64.cc
>> +++ b/gcc/config/aarch64/aarch64.cc
>> @@ -22684,7 +22684,29 @@ aarch64_print_patchable_function_entry (FILE *file,
>> asm_fprintf (file, "\thint\t34 // bti c\n");
>> }
>>
>> - default_print_patchable_function_entry (file, patch_area_size, record_p);
>> + if (cfun->machine->label_is_assembled)
>> + {
>> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
>> + GEN_INT (record_p));
>> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
>> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
>> + INSN_ADDRESSES_NEW (insn, -1);
>> + }
>> + else
>> + {
>> + default_print_patchable_function_entry (file, patch_area_size,
>> + record_p);
>> + }
>> +}
>> +
>> +/* Output patchable area. */
>> +
>> +void
>> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
>> +{
>> + default_print_patchable_function_entry (asm_out_file,
>> + patch_area_size,
>> + record_p);
>> }
>>
>> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index 76b6898ca04..6501503eb25 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -303,6 +303,7 @@
>> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
>> UNSPEC_LD1RO
>> UNSPEC_SALT_ADDR
>> + UNSPECV_PATCHABLE_AREA
>> ])
>>
>> (define_c_enum "unspecv" [
>> @@ -7821,6 +7822,19 @@
>> [(set_attr "type" "ls64")]
>> )
>>
>> +(define_insn "patchable_area"
>> + [(unspec_volatile [(match_operand 0 "const_int_operand")
>> + (match_operand 1 "const_int_operand")]
>> + UNSPECV_PATCHABLE_AREA)]
>> + ""
>> +{
>> + aarch64_output_patchable_area (INTVAL (operands[0]),
>> + INTVAL (operands[1]) != 0);
>> + return "";
>> +}
>> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
>> +)
>> +
>> ;; AdvSIMD Stuff
>> (include "aarch64-simd.md")
>>
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>> new file mode 100644
>> index 00000000000..b075b8f75ef
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>> @@ -0,0 +1,11 @@
>> +/* { dg-do "compile" } */
>> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
>> +
>> +/* Test the placement of the .LPFE0 label. */
>> +
>> +void
>> +foo (void)
>> +{
>> +}
>> +
>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
> From a26a38ef91d3cb8bd776c66be735a2ec1f46d8fb Mon Sep 17 00:00:00 2001
> From: Sebastian Pop <spop@amazon.com>
> Date: Wed, 30 Nov 2022 19:45:24 +0000
> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> Currently patchable area is at the wrong place on AArch64. It is placed
> immediately after function label, before .cfi_startproc. This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> modifies aarch64_print_patchable_function_entry to avoid placing
> patchable area before .cfi_startproc.
>
> gcc/
> PR target/93492
> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
> Declared.
> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
> (aarch64_output_patchable_area): New.
> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
> (patchable_area): Define.
>
> gcc/testsuite/
> PR target/93492
> * gcc.target/aarch64/pr98776.c: New.
> * gcc.target/aarch64/pr92424-2.c: Adjust pattern.
> * gcc.target/aarch64/pr92424-3.c: Adjust pattern.
> ---
> gcc/config/aarch64/aarch64-protos.h | 2 +
> gcc/config/aarch64/aarch64.cc | 56 +++++++++++++++-----
> gcc/config/aarch64/aarch64.md | 14 +++++
> gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
> gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
> 6 files changed, 71 insertions(+), 16 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 4be93c93c26..2fba24d947d 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
> extern bool aarch64_harden_sls_retbr_p (void);
> extern bool aarch64_harden_sls_blr_p (void);
>
> +extern void aarch64_output_patchable_area (unsigned int, bool);
> +
> #endif /* GCC_AARCH64_PROTOS_H */
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index e97f3b32f7c..4d25f492cfa 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -22661,30 +22661,58 @@ aarch64_declare_function_name (FILE *stream, const char* name,
> cfun->machine->label_is_assembled = true;
> }
>
> -/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
> - the function label and emit a BTI if necessary. */
> +/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
>
> void
> aarch64_print_patchable_function_entry (FILE *file,
> unsigned HOST_WIDE_INT patch_area_size,
> bool record_p)
> {
> - if (cfun->machine->label_is_assembled
> - && aarch64_bti_enabled ()
> + if (!cfun->machine->label_is_assembled)
> + {
> + /* Emit the patching area before the entry label, if any. */
> + default_print_patchable_function_entry (file, patch_area_size,
> + record_p);
> + }
> + else
> + {
> + if (aarch64_bti_enabled ()
> + && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
> + {
> + /* Remove the BTI that follows the patch area and insert a new BTI
> + before the patch area right after the function label. */
> + rtx_insn *insn = next_real_nondebug_insn (get_insns ());
> + if (insn
> + && INSN_P (insn)
> + && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
> + && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
> + delete_insn (insn);
> + }
> +
> + /* Emit a pseudo PATCHABLE_AREA instruction. The pseudo gets expanded
> + with aarch64_output_patchable_area. */
> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
> + GEN_INT (record_p));
> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
> + INSN_ADDRESSES_NEW (insn, -1);
> + }
> +}
> +
> +/* Output patchable area. */
> +
> +void
> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
> +{
> + /* Emit a BTI if necessary. */
> + if (aarch64_bti_enabled ()
> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
> {
> - /* Remove the BTI that follows the patch area and insert a new BTI
> - before the patch area right after the function label. */
> - rtx_insn *insn = next_real_nondebug_insn (get_insns ());
> - if (insn
> - && INSN_P (insn)
> - && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
> - && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
> - delete_insn (insn);
> - asm_fprintf (file, "\thint\t34 // bti c\n");
> + asm_fprintf (asm_out_file, "\thint\t34 // bti c\n");
> }
>
> - default_print_patchable_function_entry (file, patch_area_size, record_p);
> + default_print_patchable_function_entry (asm_out_file, patch_area_size,
> + record_p);
> }
>
> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 76b6898ca04..6501503eb25 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -303,6 +303,7 @@
> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
> UNSPEC_LD1RO
> UNSPEC_SALT_ADDR
> + UNSPECV_PATCHABLE_AREA
> ])
>
> (define_c_enum "unspecv" [
> @@ -7821,6 +7822,19 @@
> [(set_attr "type" "ls64")]
> )
>
> +(define_insn "patchable_area"
> + [(unspec_volatile [(match_operand 0 "const_int_operand")
> + (match_operand 1 "const_int_operand")]
> + UNSPECV_PATCHABLE_AREA)]
> + ""
> +{
> + aarch64_output_patchable_area (INTVAL (operands[0]),
> + INTVAL (operands[1]) != 0);
> + return "";
> +}
> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
> +)
> +
> ;; AdvSIMD Stuff
> (include "aarch64-simd.md")
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> index 12465213aef..0a79901108c 100644
> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
> f10_bti ()
> {
> }
> -/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> index 2c6a73789f0..854bb7f9fec 100644
> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
> f10_pac ()
> {
> }
> -/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> new file mode 100644
> index 00000000000..b075b8f75ef
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> @@ -0,0 +1,11 @@
> +/* { dg-do "compile" } */
> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
> +
> +/* Test the placement of the .LPFE0 label. */
> +
> +void
> +foo (void)
> +{
> +}
> +
> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch --]
[-- Type: text/x-patch; name="0001-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch", Size: 7174 bytes --]
From 656b6906847c781caa8ee38639e0e5bbd679771b Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop@amazon.com>
Date: Wed, 30 Nov 2022 19:45:24 +0000
Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
Currently patchable area is at the wrong place on AArch64. It is placed
immediately after function label, before .cfi_startproc. This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
modifies aarch64_print_patchable_function_entry to avoid placing
patchable area before .cfi_startproc.
gcc/
PR target/93492
* config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
Declared.
* config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
(aarch64_output_patchable_area): New.
* config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
(patchable_area): Define.
gcc/testsuite/
PR target/93492
* gcc.target/aarch64/pr98776.c: New.
* gcc.target/aarch64/pr92424-2.c: Adjust pattern.
* gcc.target/aarch64/pr92424-3.c: Adjust pattern.
---
gcc/config/aarch64/aarch64-protos.h | 2 +
gcc/config/aarch64/aarch64.cc | 54 +++++++++++++++-----
gcc/config/aarch64/aarch64.md | 14 +++++
gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
6 files changed, 70 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 4be93c93c26..2fba24d947d 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
extern bool aarch64_harden_sls_retbr_p (void);
extern bool aarch64_harden_sls_blr_p (void);
+extern void aarch64_output_patchable_area (unsigned int, bool);
+
#endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index e97f3b32f7c..3951aa9d1a1 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -22661,30 +22661,58 @@ aarch64_declare_function_name (FILE *stream, const char* name,
cfun->machine->label_is_assembled = true;
}
-/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
- the function label and emit a BTI if necessary. */
+/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
void
aarch64_print_patchable_function_entry (FILE *file,
unsigned HOST_WIDE_INT patch_area_size,
bool record_p)
{
- if (cfun->machine->label_is_assembled
- && aarch64_bti_enabled ()
+ if (!cfun->machine->label_is_assembled)
+ {
+ /* Emit the patching area before the entry label, if any. */
+ default_print_patchable_function_entry (file, patch_area_size,
+ record_p);
+ return;
+ }
+
+ /* Emit a pseudo PATCHABLE_AREA instruction. The pseudo gets expanded
+ with aarch64_output_patchable_area. */
+ rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
+ GEN_INT (record_p));
+ basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+
+ if (aarch64_bti_enabled ()
&& !cgraph_node::get (cfun->decl)->only_called_directly_p ())
{
- /* Remove the BTI that follows the patch area and insert a new BTI
- before the patch area right after the function label. */
rtx_insn *insn = next_real_nondebug_insn (get_insns ());
- if (insn
- && INSN_P (insn)
- && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
- && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
- delete_insn (insn);
- asm_fprintf (file, "\thint\t34 // bti c\n");
+ if (!insn
+ || !INSN_P (insn)
+ || GET_CODE (PATTERN (insn)) != UNSPEC_VOLATILE
+ || XINT (PATTERN (insn), 1) != UNSPECV_BTI_C)
+ {
+ /* Emit a BTI_C. */
+ insn = emit_insn_before (gen_bti_c (), BB_HEAD (bb));
+ }
+
+ /* Emit the patchable_area after BTI_C. */
+ insn = emit_insn_after (pa, insn);
+ INSN_ADDRESSES_NEW (insn, -1);
+ return;
}
- default_print_patchable_function_entry (file, patch_area_size, record_p);
+ /* Emit the patchable_area at the beginning of the function. */
+ rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
+ INSN_ADDRESSES_NEW (insn, -1);
+}
+
+/* Output patchable area. */
+
+void
+aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
+{
+ default_print_patchable_function_entry (asm_out_file, patch_area_size,
+ record_p);
}
/* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 76b6898ca04..6501503eb25 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -303,6 +303,7 @@
UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
UNSPEC_LD1RO
UNSPEC_SALT_ADDR
+ UNSPECV_PATCHABLE_AREA
])
(define_c_enum "unspecv" [
@@ -7821,6 +7822,19 @@
[(set_attr "type" "ls64")]
)
+(define_insn "patchable_area"
+ [(unspec_volatile [(match_operand 0 "const_int_operand")
+ (match_operand 1 "const_int_operand")]
+ UNSPECV_PATCHABLE_AREA)]
+ ""
+{
+ aarch64_output_patchable_area (INTVAL (operands[0]),
+ INTVAL (operands[1]) != 0);
+ return "";
+}
+ [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
+)
+
;; AdvSIMD Stuff
(include "aarch64-simd.md")
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
index 12465213aef..0a79901108c 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
f10_bti ()
{
}
-/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
index 2c6a73789f0..854bb7f9fec 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
f10_pac ()
{
}
-/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
new file mode 100644
index 00000000000..b075b8f75ef
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
@@ -0,0 +1,11 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE0 label. */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
--
2.38.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
2022-12-07 18:46 ` Pop, Sebastian
@ 2022-12-08 7:38 ` Richard Sandiford
2022-12-15 4:51 ` Pop, Sebastian
0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2022-12-08 7:38 UTC (permalink / raw)
To: Pop, Sebastian; +Cc: gcc-patches, sebpop, Kyrylo Tkachov
"Pop, Sebastian" <spop@amazon.com> writes:
> Hi Richard,
>
>
> Please find attached a patch that follows your recommendations to generate the BTI_C instructions.
>
> Please let me know if the patch can be further improved.
>
> The patch passed bootstrap and regressions tests on arm64-linux.
LGTM. OK for trunk, thanks, and for release branches after a grace period.
Richard
> Thanks,
>
> Sebastian
>
> ________________________________
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: Wednesday, December 7, 2022 3:12:08 AM
> To: Pop, Sebastian
> Cc: gcc-patches@gcc.gnu.org; sebpop@gmail.com; Kyrylo Tkachov
> Subject: RE: [EXTERNAL]AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> "Pop, Sebastian" <spop@amazon.com> writes:
>> Thanks Richard for your review and for pointing out the issue with BTI.
>>
>>
>> The current patch removes the existing BTI instruction,
>>
>> and then adds the BTI hint when expanding the patchable_area pseudo.
>
> Thanks. I still think...
>
>> The attached patch passed bootstrap and regression test on arm64-linux.
>>
>> Ok to commit to gcc trunk?
>>
>>
>> Thank you,
>> Sebastian
>>
>> ________________________________
>> From: Richard Sandiford <richard.sandiford@arm.com>
>> Sent: Monday, December 5, 2022 5:34:40 AM
>> To: Pop, Sebastian
>> Cc: gcc-patches@gcc.gnu.org; sebpop@gmail.com; Kyrylo Tkachov
>> Subject: RE: [EXTERNAL]AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>>
>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>>
>>
>>
>> "Pop, Sebastian" <spop@amazon.com> writes:
>>> Hi,
>>>
>>> Currently patchable area is at the wrong place on AArch64. It is placed
>>> immediately after function label, before .cfi_startproc. This patch
>>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>>> modifies aarch64_print_patchable_function_entry to avoid placing
>>> patchable area before .cfi_startproc.
>>>
>>> The patch passed bootstrap and regression test on aarch64-linux.
>>> Ok to commit to trunk and backport to active release branches?
>>
>> Looks good, but doesn't the problem described in the PR then still
>> apply to the BTI emitted by:
>>
>> if (cfun->machine->label_is_assembled
>> && aarch64_bti_enabled ()
>> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
>> {
>> /* Remove the BTI that follows the patch area and insert a new BTI
>> before the patch area right after the function label. */
>> rtx_insn *insn = next_real_nondebug_insn (get_insns ());
>> if (insn
>> && INSN_P (insn)
>> && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
>> && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
>> delete_insn (insn);
>> asm_fprintf (file, "\thint\t34 // bti c\n");
>> }
>>
>> ? It seems like the BTI will be before the cfi_startproc and the
>> patchable entry afterwards.
>>
>> I guess we should keep the BTI instruction as-is (rather than printing
>> a .hint) and emit the new UNSPECV_PATCHABLE_AREA after the BTI rather
>> than before it.
>
> ...this approach would be slightly cleaner though. The .hint asm string
> we're emitting here is exactly the same as the one emiitted by the
> original bti_c instruction. The only reason for deleting the
> instruction and emitting text was because we were emitting the
> patchable entry directly as text, and the BTI text had to come
> before the patchable entry text.
>
> Now that we're emitting the patchable entry via a normal instruction
> (a good thing!) we can keep the preceding bti_c as a normal instruction
> too. That is, I think we should use emit_insn_after to emit the entry
> after the bti_c insn (if it exists) instead of before BB_HEAD.
>
> Thanks,
> Richard
>
>>> gcc/
>>> PR target/93492
>>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>>> Declared.
>>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>>> (aarch64_output_patchable_area): New.
>>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>>> (patchable_area): Define.
>>>
>>> gcc/testsuite/
>>> PR target/93492
>>> * gcc.target/aarch64/pr98776.c: New.
>>>
>>>
>>> From b9cf87bcdf65f515b38f1851eb95c18aaa180253 Mon Sep 17 00:00:00 2001
>>> From: Sebastian Pop <spop@amazon.com>
>>> Date: Wed, 30 Nov 2022 19:45:24 +0000
>>> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>>>
>>> Currently patchable area is at the wrong place on AArch64. It is placed
>>> immediately after function label, before .cfi_startproc. This patch
>>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>>> modifies aarch64_print_patchable_function_entry to avoid placing
>>> patchable area before .cfi_startproc.
>>>
>>> gcc/
>>> PR target/93492
>>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>>> Declared.
>>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>>> (aarch64_output_patchable_area): New.
>>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>>> (patchable_area): Define.
>>>
>>> gcc/testsuite/
>>> PR target/93492
>>> * gcc.target/aarch64/pr98776.c: New.
>>> ---
>>> gcc/config/aarch64/aarch64-protos.h | 2 ++
>>> gcc/config/aarch64/aarch64.cc | 24 +++++++++++++++++++++-
>>> gcc/config/aarch64/aarch64.md | 14 +++++++++++++
>>> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++++++++
>>> 4 files changed, 50 insertions(+), 1 deletion(-)
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>>>
>>> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
>>> index 4be93c93c26..2fba24d947d 100644
>>> --- a/gcc/config/aarch64/aarch64-protos.h
>>> +++ b/gcc/config/aarch64/aarch64-protos.h
>>> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
>>> extern bool aarch64_harden_sls_retbr_p (void);
>>> extern bool aarch64_harden_sls_blr_p (void);
>>>
>>> +extern void aarch64_output_patchable_area (unsigned int, bool);
>>> +
>>> #endif /* GCC_AARCH64_PROTOS_H */
>>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>>> index e97f3b32f7c..e84b33b958c 100644
>>> --- a/gcc/config/aarch64/aarch64.cc
>>> +++ b/gcc/config/aarch64/aarch64.cc
>>> @@ -22684,7 +22684,29 @@ aarch64_print_patchable_function_entry (FILE *file,
>>> asm_fprintf (file, "\thint\t34 // bti c\n");
>>> }
>>>
>>> - default_print_patchable_function_entry (file, patch_area_size, record_p);
>>> + if (cfun->machine->label_is_assembled)
>>> + {
>>> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
>>> + GEN_INT (record_p));
>>> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
>>> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
>>> + INSN_ADDRESSES_NEW (insn, -1);
>>> + }
>>> + else
>>> + {
>>> + default_print_patchable_function_entry (file, patch_area_size,
>>> + record_p);
>>> + }
>>> +}
>>> +
>>> +/* Output patchable area. */
>>> +
>>> +void
>>> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
>>> +{
>>> + default_print_patchable_function_entry (asm_out_file,
>>> + patch_area_size,
>>> + record_p);
>>> }
>>>
>>> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
>>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>>> index 76b6898ca04..6501503eb25 100644
>>> --- a/gcc/config/aarch64/aarch64.md
>>> +++ b/gcc/config/aarch64/aarch64.md
>>> @@ -303,6 +303,7 @@
>>> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
>>> UNSPEC_LD1RO
>>> UNSPEC_SALT_ADDR
>>> + UNSPECV_PATCHABLE_AREA
>>> ])
>>>
>>> (define_c_enum "unspecv" [
>>> @@ -7821,6 +7822,19 @@
>>> [(set_attr "type" "ls64")]
>>> )
>>>
>>> +(define_insn "patchable_area"
>>> + [(unspec_volatile [(match_operand 0 "const_int_operand")
>>> + (match_operand 1 "const_int_operand")]
>>> + UNSPECV_PATCHABLE_AREA)]
>>> + ""
>>> +{
>>> + aarch64_output_patchable_area (INTVAL (operands[0]),
>>> + INTVAL (operands[1]) != 0);
>>> + return "";
>>> +}
>>> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
>>> +)
>>> +
>>> ;; AdvSIMD Stuff
>>> (include "aarch64-simd.md")
>>>
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>>> new file mode 100644
>>> index 00000000000..b075b8f75ef
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>>> @@ -0,0 +1,11 @@
>>> +/* { dg-do "compile" } */
>>> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
>>> +
>>> +/* Test the placement of the .LPFE0 label. */
>>> +
>>> +void
>>> +foo (void)
>>> +{
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
>> From a26a38ef91d3cb8bd776c66be735a2ec1f46d8fb Mon Sep 17 00:00:00 2001
>> From: Sebastian Pop <spop@amazon.com>
>> Date: Wed, 30 Nov 2022 19:45:24 +0000
>> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>>
>> Currently patchable area is at the wrong place on AArch64. It is placed
>> immediately after function label, before .cfi_startproc. This patch
>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>> modifies aarch64_print_patchable_function_entry to avoid placing
>> patchable area before .cfi_startproc.
>>
>> gcc/
>> PR target/93492
>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>> Declared.
>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>> (aarch64_output_patchable_area): New.
>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>> (patchable_area): Define.
>>
>> gcc/testsuite/
>> PR target/93492
>> * gcc.target/aarch64/pr98776.c: New.
>> * gcc.target/aarch64/pr92424-2.c: Adjust pattern.
>> * gcc.target/aarch64/pr92424-3.c: Adjust pattern.
>> ---
>> gcc/config/aarch64/aarch64-protos.h | 2 +
>> gcc/config/aarch64/aarch64.cc | 56 +++++++++++++++-----
>> gcc/config/aarch64/aarch64.md | 14 +++++
>> gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
>> gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
>> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
>> 6 files changed, 71 insertions(+), 16 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>>
>> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
>> index 4be93c93c26..2fba24d947d 100644
>> --- a/gcc/config/aarch64/aarch64-protos.h
>> +++ b/gcc/config/aarch64/aarch64-protos.h
>> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
>> extern bool aarch64_harden_sls_retbr_p (void);
>> extern bool aarch64_harden_sls_blr_p (void);
>>
>> +extern void aarch64_output_patchable_area (unsigned int, bool);
>> +
>> #endif /* GCC_AARCH64_PROTOS_H */
>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> index e97f3b32f7c..4d25f492cfa 100644
>> --- a/gcc/config/aarch64/aarch64.cc
>> +++ b/gcc/config/aarch64/aarch64.cc
>> @@ -22661,30 +22661,58 @@ aarch64_declare_function_name (FILE *stream, const char* name,
>> cfun->machine->label_is_assembled = true;
>> }
>>
>> -/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
>> - the function label and emit a BTI if necessary. */
>> +/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
>>
>> void
>> aarch64_print_patchable_function_entry (FILE *file,
>> unsigned HOST_WIDE_INT patch_area_size,
>> bool record_p)
>> {
>> - if (cfun->machine->label_is_assembled
>> - && aarch64_bti_enabled ()
>> + if (!cfun->machine->label_is_assembled)
>> + {
>> + /* Emit the patching area before the entry label, if any. */
>> + default_print_patchable_function_entry (file, patch_area_size,
>> + record_p);
>> + }
>> + else
>> + {
>> + if (aarch64_bti_enabled ()
>> + && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
>> + {
>> + /* Remove the BTI that follows the patch area and insert a new BTI
>> + before the patch area right after the function label. */
>> + rtx_insn *insn = next_real_nondebug_insn (get_insns ());
>> + if (insn
>> + && INSN_P (insn)
>> + && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
>> + && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
>> + delete_insn (insn);
>> + }
>> +
>> + /* Emit a pseudo PATCHABLE_AREA instruction. The pseudo gets expanded
>> + with aarch64_output_patchable_area. */
>> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
>> + GEN_INT (record_p));
>> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
>> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
>> + INSN_ADDRESSES_NEW (insn, -1);
>> + }
>> +}
>> +
>> +/* Output patchable area. */
>> +
>> +void
>> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
>> +{
>> + /* Emit a BTI if necessary. */
>> + if (aarch64_bti_enabled ()
>> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
>> {
>> - /* Remove the BTI that follows the patch area and insert a new BTI
>> - before the patch area right after the function label. */
>> - rtx_insn *insn = next_real_nondebug_insn (get_insns ());
>> - if (insn
>> - && INSN_P (insn)
>> - && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
>> - && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
>> - delete_insn (insn);
>> - asm_fprintf (file, "\thint\t34 // bti c\n");
>> + asm_fprintf (asm_out_file, "\thint\t34 // bti c\n");
>> }
>>
>> - default_print_patchable_function_entry (file, patch_area_size, record_p);
>> + default_print_patchable_function_entry (asm_out_file, patch_area_size,
>> + record_p);
>> }
>>
>> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index 76b6898ca04..6501503eb25 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -303,6 +303,7 @@
>> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
>> UNSPEC_LD1RO
>> UNSPEC_SALT_ADDR
>> + UNSPECV_PATCHABLE_AREA
>> ])
>>
>> (define_c_enum "unspecv" [
>> @@ -7821,6 +7822,19 @@
>> [(set_attr "type" "ls64")]
>> )
>>
>> +(define_insn "patchable_area"
>> + [(unspec_volatile [(match_operand 0 "const_int_operand")
>> + (match_operand 1 "const_int_operand")]
>> + UNSPECV_PATCHABLE_AREA)]
>> + ""
>> +{
>> + aarch64_output_patchable_area (INTVAL (operands[0]),
>> + INTVAL (operands[1]) != 0);
>> + return "";
>> +}
>> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
>> +)
>> +
>> ;; AdvSIMD Stuff
>> (include "aarch64-simd.md")
>>
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
>> index 12465213aef..0a79901108c 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
>> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
>> f10_bti ()
>> {
>> }
>> -/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
>> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
>> index 2c6a73789f0..854bb7f9fec 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
>> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
>> f10_pac ()
>> {
>> }
>> -/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
>> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>> new file mode 100644
>> index 00000000000..b075b8f75ef
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>> @@ -0,0 +1,11 @@
>> +/* { dg-do "compile" } */
>> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
>> +
>> +/* Test the placement of the .LPFE0 label. */
>> +
>> +void
>> +foo (void)
>> +{
>> +}
>> +
>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
> From 656b6906847c781caa8ee38639e0e5bbd679771b Mon Sep 17 00:00:00 2001
> From: Sebastian Pop <spop@amazon.com>
> Date: Wed, 30 Nov 2022 19:45:24 +0000
> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> Currently patchable area is at the wrong place on AArch64. It is placed
> immediately after function label, before .cfi_startproc. This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> modifies aarch64_print_patchable_function_entry to avoid placing
> patchable area before .cfi_startproc.
>
> gcc/
> PR target/93492
> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
> Declared.
> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
> (aarch64_output_patchable_area): New.
> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
> (patchable_area): Define.
>
> gcc/testsuite/
> PR target/93492
> * gcc.target/aarch64/pr98776.c: New.
> * gcc.target/aarch64/pr92424-2.c: Adjust pattern.
> * gcc.target/aarch64/pr92424-3.c: Adjust pattern.
> ---
> gcc/config/aarch64/aarch64-protos.h | 2 +
> gcc/config/aarch64/aarch64.cc | 54 +++++++++++++++-----
> gcc/config/aarch64/aarch64.md | 14 +++++
> gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
> gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
> 6 files changed, 70 insertions(+), 15 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 4be93c93c26..2fba24d947d 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
> extern bool aarch64_harden_sls_retbr_p (void);
> extern bool aarch64_harden_sls_blr_p (void);
>
> +extern void aarch64_output_patchable_area (unsigned int, bool);
> +
> #endif /* GCC_AARCH64_PROTOS_H */
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index e97f3b32f7c..3951aa9d1a1 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -22661,30 +22661,58 @@ aarch64_declare_function_name (FILE *stream, const char* name,
> cfun->machine->label_is_assembled = true;
> }
>
> -/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
> - the function label and emit a BTI if necessary. */
> +/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
>
> void
> aarch64_print_patchable_function_entry (FILE *file,
> unsigned HOST_WIDE_INT patch_area_size,
> bool record_p)
> {
> - if (cfun->machine->label_is_assembled
> - && aarch64_bti_enabled ()
> + if (!cfun->machine->label_is_assembled)
> + {
> + /* Emit the patching area before the entry label, if any. */
> + default_print_patchable_function_entry (file, patch_area_size,
> + record_p);
> + return;
> + }
> +
> + /* Emit a pseudo PATCHABLE_AREA instruction. The pseudo gets expanded
> + with aarch64_output_patchable_area. */
> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
> + GEN_INT (record_p));
> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
> +
> + if (aarch64_bti_enabled ()
> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
> {
> - /* Remove the BTI that follows the patch area and insert a new BTI
> - before the patch area right after the function label. */
> rtx_insn *insn = next_real_nondebug_insn (get_insns ());
> - if (insn
> - && INSN_P (insn)
> - && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
> - && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
> - delete_insn (insn);
> - asm_fprintf (file, "\thint\t34 // bti c\n");
> + if (!insn
> + || !INSN_P (insn)
> + || GET_CODE (PATTERN (insn)) != UNSPEC_VOLATILE
> + || XINT (PATTERN (insn), 1) != UNSPECV_BTI_C)
> + {
> + /* Emit a BTI_C. */
> + insn = emit_insn_before (gen_bti_c (), BB_HEAD (bb));
> + }
> +
> + /* Emit the patchable_area after BTI_C. */
> + insn = emit_insn_after (pa, insn);
> + INSN_ADDRESSES_NEW (insn, -1);
> + return;
> }
>
> - default_print_patchable_function_entry (file, patch_area_size, record_p);
> + /* Emit the patchable_area at the beginning of the function. */
> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
> + INSN_ADDRESSES_NEW (insn, -1);
> +}
> +
> +/* Output patchable area. */
> +
> +void
> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
> +{
> + default_print_patchable_function_entry (asm_out_file, patch_area_size,
> + record_p);
> }
>
> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 76b6898ca04..6501503eb25 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -303,6 +303,7 @@
> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
> UNSPEC_LD1RO
> UNSPEC_SALT_ADDR
> + UNSPECV_PATCHABLE_AREA
> ])
>
> (define_c_enum "unspecv" [
> @@ -7821,6 +7822,19 @@
> [(set_attr "type" "ls64")]
> )
>
> +(define_insn "patchable_area"
> + [(unspec_volatile [(match_operand 0 "const_int_operand")
> + (match_operand 1 "const_int_operand")]
> + UNSPECV_PATCHABLE_AREA)]
> + ""
> +{
> + aarch64_output_patchable_area (INTVAL (operands[0]),
> + INTVAL (operands[1]) != 0);
> + return "";
> +}
> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
> +)
> +
> ;; AdvSIMD Stuff
> (include "aarch64-simd.md")
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> index 12465213aef..0a79901108c 100644
> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
> f10_bti ()
> {
> }
> -/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> index 2c6a73789f0..854bb7f9fec 100644
> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
> f10_pac ()
> {
> }
> -/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> new file mode 100644
> index 00000000000..b075b8f75ef
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> @@ -0,0 +1,11 @@
> +/* { dg-do "compile" } */
> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
> +
> +/* Test the placement of the .LPFE0 label. */
> +
> +void
> +foo (void)
> +{
> +}
> +
> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
2022-12-08 7:38 ` Richard Sandiford
@ 2022-12-15 4:51 ` Pop, Sebastian
0 siblings, 0 replies; 7+ messages in thread
From: Pop, Sebastian @ 2022-12-15 4:51 UTC (permalink / raw)
To: Richard Sandiford; +Cc: gcc-patches, sebpop, Kyrylo Tkachov
[-- Attachment #1.1: Type: text/plain, Size: 26865 bytes --]
Hi Richard,
I will commit tomorrow the attached patches to the active branches gcc-10, 11, and 12.
The patches passed bootstrap and regression test on arm64-linux.
Sebastian
________________________________
From: Richard Sandiford <richard.sandiford@arm.com>
Sent: Thursday, December 8, 2022 1:38:07 AM
To: Pop, Sebastian
Cc: gcc-patches@gcc.gnu.org; sebpop@gmail.com; Kyrylo Tkachov
Subject: RE: [EXTERNAL]AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
"Pop, Sebastian" <spop@amazon.com> writes:
> Hi Richard,
>
>
> Please find attached a patch that follows your recommendations to generate the BTI_C instructions.
>
> Please let me know if the patch can be further improved.
>
> The patch passed bootstrap and regressions tests on arm64-linux.
LGTM. OK for trunk, thanks, and for release branches after a grace period.
Richard
> Thanks,
>
> Sebastian
>
> ________________________________
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: Wednesday, December 7, 2022 3:12:08 AM
> To: Pop, Sebastian
> Cc: gcc-patches@gcc.gnu.org; sebpop@gmail.com; Kyrylo Tkachov
> Subject: RE: [EXTERNAL]AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> "Pop, Sebastian" <spop@amazon.com> writes:
>> Thanks Richard for your review and for pointing out the issue with BTI.
>>
>>
>> The current patch removes the existing BTI instruction,
>>
>> and then adds the BTI hint when expanding the patchable_area pseudo.
>
> Thanks. I still think...
>
>> The attached patch passed bootstrap and regression test on arm64-linux.
>>
>> Ok to commit to gcc trunk?
>>
>>
>> Thank you,
>> Sebastian
>>
>> ________________________________
>> From: Richard Sandiford <richard.sandiford@arm.com>
>> Sent: Monday, December 5, 2022 5:34:40 AM
>> To: Pop, Sebastian
>> Cc: gcc-patches@gcc.gnu.org; sebpop@gmail.com; Kyrylo Tkachov
>> Subject: RE: [EXTERNAL]AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>>
>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>>
>>
>>
>> "Pop, Sebastian" <spop@amazon.com> writes:
>>> Hi,
>>>
>>> Currently patchable area is at the wrong place on AArch64. It is placed
>>> immediately after function label, before .cfi_startproc. This patch
>>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>>> modifies aarch64_print_patchable_function_entry to avoid placing
>>> patchable area before .cfi_startproc.
>>>
>>> The patch passed bootstrap and regression test on aarch64-linux.
>>> Ok to commit to trunk and backport to active release branches?
>>
>> Looks good, but doesn't the problem described in the PR then still
>> apply to the BTI emitted by:
>>
>> if (cfun->machine->label_is_assembled
>> && aarch64_bti_enabled ()
>> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
>> {
>> /* Remove the BTI that follows the patch area and insert a new BTI
>> before the patch area right after the function label. */
>> rtx_insn *insn = next_real_nondebug_insn (get_insns ());
>> if (insn
>> && INSN_P (insn)
>> && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
>> && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
>> delete_insn (insn);
>> asm_fprintf (file, "\thint\t34 // bti c\n");
>> }
>>
>> ? It seems like the BTI will be before the cfi_startproc and the
>> patchable entry afterwards.
>>
>> I guess we should keep the BTI instruction as-is (rather than printing
>> a .hint) and emit the new UNSPECV_PATCHABLE_AREA after the BTI rather
>> than before it.
>
> ...this approach would be slightly cleaner though. The .hint asm string
> we're emitting here is exactly the same as the one emiitted by the
> original bti_c instruction. The only reason for deleting the
> instruction and emitting text was because we were emitting the
> patchable entry directly as text, and the BTI text had to come
> before the patchable entry text.
>
> Now that we're emitting the patchable entry via a normal instruction
> (a good thing!) we can keep the preceding bti_c as a normal instruction
> too. That is, I think we should use emit_insn_after to emit the entry
> after the bti_c insn (if it exists) instead of before BB_HEAD.
>
> Thanks,
> Richard
>
>>> gcc/
>>> PR target/93492
>>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>>> Declared.
>>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>>> (aarch64_output_patchable_area): New.
>>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>>> (patchable_area): Define.
>>>
>>> gcc/testsuite/
>>> PR target/93492
>>> * gcc.target/aarch64/pr98776.c: New.
>>>
>>>
>>> From b9cf87bcdf65f515b38f1851eb95c18aaa180253 Mon Sep 17 00:00:00 2001
>>> From: Sebastian Pop <spop@amazon.com>
>>> Date: Wed, 30 Nov 2022 19:45:24 +0000
>>> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>>>
>>> Currently patchable area is at the wrong place on AArch64. It is placed
>>> immediately after function label, before .cfi_startproc. This patch
>>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>>> modifies aarch64_print_patchable_function_entry to avoid placing
>>> patchable area before .cfi_startproc.
>>>
>>> gcc/
>>> PR target/93492
>>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>>> Declared.
>>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>>> (aarch64_output_patchable_area): New.
>>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>>> (patchable_area): Define.
>>>
>>> gcc/testsuite/
>>> PR target/93492
>>> * gcc.target/aarch64/pr98776.c: New.
>>> ---
>>> gcc/config/aarch64/aarch64-protos.h | 2 ++
>>> gcc/config/aarch64/aarch64.cc | 24 +++++++++++++++++++++-
>>> gcc/config/aarch64/aarch64.md | 14 +++++++++++++
>>> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++++++++
>>> 4 files changed, 50 insertions(+), 1 deletion(-)
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>>>
>>> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
>>> index 4be93c93c26..2fba24d947d 100644
>>> --- a/gcc/config/aarch64/aarch64-protos.h
>>> +++ b/gcc/config/aarch64/aarch64-protos.h
>>> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
>>> extern bool aarch64_harden_sls_retbr_p (void);
>>> extern bool aarch64_harden_sls_blr_p (void);
>>>
>>> +extern void aarch64_output_patchable_area (unsigned int, bool);
>>> +
>>> #endif /* GCC_AARCH64_PROTOS_H */
>>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>>> index e97f3b32f7c..e84b33b958c 100644
>>> --- a/gcc/config/aarch64/aarch64.cc
>>> +++ b/gcc/config/aarch64/aarch64.cc
>>> @@ -22684,7 +22684,29 @@ aarch64_print_patchable_function_entry (FILE *file,
>>> asm_fprintf (file, "\thint\t34 // bti c\n");
>>> }
>>>
>>> - default_print_patchable_function_entry (file, patch_area_size, record_p);
>>> + if (cfun->machine->label_is_assembled)
>>> + {
>>> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
>>> + GEN_INT (record_p));
>>> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
>>> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
>>> + INSN_ADDRESSES_NEW (insn, -1);
>>> + }
>>> + else
>>> + {
>>> + default_print_patchable_function_entry (file, patch_area_size,
>>> + record_p);
>>> + }
>>> +}
>>> +
>>> +/* Output patchable area. */
>>> +
>>> +void
>>> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
>>> +{
>>> + default_print_patchable_function_entry (asm_out_file,
>>> + patch_area_size,
>>> + record_p);
>>> }
>>>
>>> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
>>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>>> index 76b6898ca04..6501503eb25 100644
>>> --- a/gcc/config/aarch64/aarch64.md
>>> +++ b/gcc/config/aarch64/aarch64.md
>>> @@ -303,6 +303,7 @@
>>> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
>>> UNSPEC_LD1RO
>>> UNSPEC_SALT_ADDR
>>> + UNSPECV_PATCHABLE_AREA
>>> ])
>>>
>>> (define_c_enum "unspecv" [
>>> @@ -7821,6 +7822,19 @@
>>> [(set_attr "type" "ls64")]
>>> )
>>>
>>> +(define_insn "patchable_area"
>>> + [(unspec_volatile [(match_operand 0 "const_int_operand")
>>> + (match_operand 1 "const_int_operand")]
>>> + UNSPECV_PATCHABLE_AREA)]
>>> + ""
>>> +{
>>> + aarch64_output_patchable_area (INTVAL (operands[0]),
>>> + INTVAL (operands[1]) != 0);
>>> + return "";
>>> +}
>>> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
>>> +)
>>> +
>>> ;; AdvSIMD Stuff
>>> (include "aarch64-simd.md")
>>>
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>>> new file mode 100644
>>> index 00000000000..b075b8f75ef
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>>> @@ -0,0 +1,11 @@
>>> +/* { dg-do "compile" } */
>>> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
>>> +
>>> +/* Test the placement of the .LPFE0 label. */
>>> +
>>> +void
>>> +foo (void)
>>> +{
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
>> From a26a38ef91d3cb8bd776c66be735a2ec1f46d8fb Mon Sep 17 00:00:00 2001
>> From: Sebastian Pop <spop@amazon.com>
>> Date: Wed, 30 Nov 2022 19:45:24 +0000
>> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>>
>> Currently patchable area is at the wrong place on AArch64. It is placed
>> immediately after function label, before .cfi_startproc. This patch
>> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
>> modifies aarch64_print_patchable_function_entry to avoid placing
>> patchable area before .cfi_startproc.
>>
>> gcc/
>> PR target/93492
>> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
>> Declared.
>> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
>> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
>> (aarch64_output_patchable_area): New.
>> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
>> (patchable_area): Define.
>>
>> gcc/testsuite/
>> PR target/93492
>> * gcc.target/aarch64/pr98776.c: New.
>> * gcc.target/aarch64/pr92424-2.c: Adjust pattern.
>> * gcc.target/aarch64/pr92424-3.c: Adjust pattern.
>> ---
>> gcc/config/aarch64/aarch64-protos.h | 2 +
>> gcc/config/aarch64/aarch64.cc | 56 +++++++++++++++-----
>> gcc/config/aarch64/aarch64.md | 14 +++++
>> gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
>> gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
>> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
>> 6 files changed, 71 insertions(+), 16 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>>
>> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
>> index 4be93c93c26..2fba24d947d 100644
>> --- a/gcc/config/aarch64/aarch64-protos.h
>> +++ b/gcc/config/aarch64/aarch64-protos.h
>> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
>> extern bool aarch64_harden_sls_retbr_p (void);
>> extern bool aarch64_harden_sls_blr_p (void);
>>
>> +extern void aarch64_output_patchable_area (unsigned int, bool);
>> +
>> #endif /* GCC_AARCH64_PROTOS_H */
>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> index e97f3b32f7c..4d25f492cfa 100644
>> --- a/gcc/config/aarch64/aarch64.cc
>> +++ b/gcc/config/aarch64/aarch64.cc
>> @@ -22661,30 +22661,58 @@ aarch64_declare_function_name (FILE *stream, const char* name,
>> cfun->machine->label_is_assembled = true;
>> }
>>
>> -/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
>> - the function label and emit a BTI if necessary. */
>> +/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
>>
>> void
>> aarch64_print_patchable_function_entry (FILE *file,
>> unsigned HOST_WIDE_INT patch_area_size,
>> bool record_p)
>> {
>> - if (cfun->machine->label_is_assembled
>> - && aarch64_bti_enabled ()
>> + if (!cfun->machine->label_is_assembled)
>> + {
>> + /* Emit the patching area before the entry label, if any. */
>> + default_print_patchable_function_entry (file, patch_area_size,
>> + record_p);
>> + }
>> + else
>> + {
>> + if (aarch64_bti_enabled ()
>> + && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
>> + {
>> + /* Remove the BTI that follows the patch area and insert a new BTI
>> + before the patch area right after the function label. */
>> + rtx_insn *insn = next_real_nondebug_insn (get_insns ());
>> + if (insn
>> + && INSN_P (insn)
>> + && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
>> + && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
>> + delete_insn (insn);
>> + }
>> +
>> + /* Emit a pseudo PATCHABLE_AREA instruction. The pseudo gets expanded
>> + with aarch64_output_patchable_area. */
>> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
>> + GEN_INT (record_p));
>> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
>> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
>> + INSN_ADDRESSES_NEW (insn, -1);
>> + }
>> +}
>> +
>> +/* Output patchable area. */
>> +
>> +void
>> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
>> +{
>> + /* Emit a BTI if necessary. */
>> + if (aarch64_bti_enabled ()
>> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
>> {
>> - /* Remove the BTI that follows the patch area and insert a new BTI
>> - before the patch area right after the function label. */
>> - rtx_insn *insn = next_real_nondebug_insn (get_insns ());
>> - if (insn
>> - && INSN_P (insn)
>> - && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
>> - && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
>> - delete_insn (insn);
>> - asm_fprintf (file, "\thint\t34 // bti c\n");
>> + asm_fprintf (asm_out_file, "\thint\t34 // bti c\n");
>> }
>>
>> - default_print_patchable_function_entry (file, patch_area_size, record_p);
>> + default_print_patchable_function_entry (asm_out_file, patch_area_size,
>> + record_p);
>> }
>>
>> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index 76b6898ca04..6501503eb25 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -303,6 +303,7 @@
>> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
>> UNSPEC_LD1RO
>> UNSPEC_SALT_ADDR
>> + UNSPECV_PATCHABLE_AREA
>> ])
>>
>> (define_c_enum "unspecv" [
>> @@ -7821,6 +7822,19 @@
>> [(set_attr "type" "ls64")]
>> )
>>
>> +(define_insn "patchable_area"
>> + [(unspec_volatile [(match_operand 0 "const_int_operand")
>> + (match_operand 1 "const_int_operand")]
>> + UNSPECV_PATCHABLE_AREA)]
>> + ""
>> +{
>> + aarch64_output_patchable_area (INTVAL (operands[0]),
>> + INTVAL (operands[1]) != 0);
>> + return "";
>> +}
>> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
>> +)
>> +
>> ;; AdvSIMD Stuff
>> (include "aarch64-simd.md")
>>
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
>> index 12465213aef..0a79901108c 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
>> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
>> f10_bti ()
>> {
>> }
>> -/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
>> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
>> index 2c6a73789f0..854bb7f9fec 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
>> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
>> f10_pac ()
>> {
>> }
>> -/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
>> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>> new file mode 100644
>> index 00000000000..b075b8f75ef
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
>> @@ -0,0 +1,11 @@
>> +/* { dg-do "compile" } */
>> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
>> +
>> +/* Test the placement of the .LPFE0 label. */
>> +
>> +void
>> +foo (void)
>> +{
>> +}
>> +
>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
> From 656b6906847c781caa8ee38639e0e5bbd679771b Mon Sep 17 00:00:00 2001
> From: Sebastian Pop <spop@amazon.com>
> Date: Wed, 30 Nov 2022 19:45:24 +0000
> Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
>
> Currently patchable area is at the wrong place on AArch64. It is placed
> immediately after function label, before .cfi_startproc. This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> modifies aarch64_print_patchable_function_entry to avoid placing
> patchable area before .cfi_startproc.
>
> gcc/
> PR target/93492
> * config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
> Declared.
> * config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
> Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
> (aarch64_output_patchable_area): New.
> * config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
> (patchable_area): Define.
>
> gcc/testsuite/
> PR target/93492
> * gcc.target/aarch64/pr98776.c: New.
> * gcc.target/aarch64/pr92424-2.c: Adjust pattern.
> * gcc.target/aarch64/pr92424-3.c: Adjust pattern.
> ---
> gcc/config/aarch64/aarch64-protos.h | 2 +
> gcc/config/aarch64/aarch64.cc | 54 +++++++++++++++-----
> gcc/config/aarch64/aarch64.md | 14 +++++
> gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
> gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
> gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
> 6 files changed, 70 insertions(+), 15 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 4be93c93c26..2fba24d947d 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -1074,4 +1074,6 @@ const char *aarch64_indirect_call_asm (rtx);
> extern bool aarch64_harden_sls_retbr_p (void);
> extern bool aarch64_harden_sls_blr_p (void);
>
> +extern void aarch64_output_patchable_area (unsigned int, bool);
> +
> #endif /* GCC_AARCH64_PROTOS_H */
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index e97f3b32f7c..3951aa9d1a1 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -22661,30 +22661,58 @@ aarch64_declare_function_name (FILE *stream, const char* name,
> cfun->machine->label_is_assembled = true;
> }
>
> -/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
> - the function label and emit a BTI if necessary. */
> +/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
>
> void
> aarch64_print_patchable_function_entry (FILE *file,
> unsigned HOST_WIDE_INT patch_area_size,
> bool record_p)
> {
> - if (cfun->machine->label_is_assembled
> - && aarch64_bti_enabled ()
> + if (!cfun->machine->label_is_assembled)
> + {
> + /* Emit the patching area before the entry label, if any. */
> + default_print_patchable_function_entry (file, patch_area_size,
> + record_p);
> + return;
> + }
> +
> + /* Emit a pseudo PATCHABLE_AREA instruction. The pseudo gets expanded
> + with aarch64_output_patchable_area. */
> + rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
> + GEN_INT (record_p));
> + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
> +
> + if (aarch64_bti_enabled ()
> && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
> {
> - /* Remove the BTI that follows the patch area and insert a new BTI
> - before the patch area right after the function label. */
> rtx_insn *insn = next_real_nondebug_insn (get_insns ());
> - if (insn
> - && INSN_P (insn)
> - && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
> - && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
> - delete_insn (insn);
> - asm_fprintf (file, "\thint\t34 // bti c\n");
> + if (!insn
> + || !INSN_P (insn)
> + || GET_CODE (PATTERN (insn)) != UNSPEC_VOLATILE
> + || XINT (PATTERN (insn), 1) != UNSPECV_BTI_C)
> + {
> + /* Emit a BTI_C. */
> + insn = emit_insn_before (gen_bti_c (), BB_HEAD (bb));
> + }
> +
> + /* Emit the patchable_area after BTI_C. */
> + insn = emit_insn_after (pa, insn);
> + INSN_ADDRESSES_NEW (insn, -1);
> + return;
> }
>
> - default_print_patchable_function_entry (file, patch_area_size, record_p);
> + /* Emit the patchable_area at the beginning of the function. */
> + rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
> + INSN_ADDRESSES_NEW (insn, -1);
> +}
> +
> +/* Output patchable area. */
> +
> +void
> +aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
> +{
> + default_print_patchable_function_entry (asm_out_file, patch_area_size,
> + record_p);
> }
>
> /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 76b6898ca04..6501503eb25 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -303,6 +303,7 @@
> UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
> UNSPEC_LD1RO
> UNSPEC_SALT_ADDR
> + UNSPECV_PATCHABLE_AREA
> ])
>
> (define_c_enum "unspecv" [
> @@ -7821,6 +7822,19 @@
> [(set_attr "type" "ls64")]
> )
>
> +(define_insn "patchable_area"
> + [(unspec_volatile [(match_operand 0 "const_int_operand")
> + (match_operand 1 "const_int_operand")]
> + UNSPECV_PATCHABLE_AREA)]
> + ""
> +{
> + aarch64_output_patchable_area (INTVAL (operands[0]),
> + INTVAL (operands[1]) != 0);
> + return "";
> +}
> + [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
> +)
> +
> ;; AdvSIMD Stuff
> (include "aarch64-simd.md")
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> index 12465213aef..0a79901108c 100644
> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
> f10_bti ()
> {
> }
> -/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> index 2c6a73789f0..854bb7f9fec 100644
> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
> f10_pac ()
> {
> }
> -/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
> +/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> new file mode 100644
> index 00000000000..b075b8f75ef
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
> @@ -0,0 +1,11 @@
> +/* { dg-do "compile" } */
> +/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
> +
> +/* Test the placement of the .LPFE0 label. */
> +
> +void
> +foo (void)
> +{
> +}
> +
> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gcc12-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch --]
[-- Type: text/x-patch; name="gcc12-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch", Size: 7200 bytes --]
From 576bb0049459371c6b619f425583eb58b3e5a1cd Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop@amazon.com>
Date: Wed, 30 Nov 2022 19:45:24 +0000
Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
Currently patchable area is at the wrong place on AArch64. It is placed
immediately after function label, before .cfi_startproc. This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
modifies aarch64_print_patchable_function_entry to avoid placing
patchable area before .cfi_startproc.
gcc/
PR target/98776
* config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
Declared.
* config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
(aarch64_output_patchable_area): New.
* config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
(patchable_area): Define.
gcc/testsuite/
PR target/98776
* gcc.target/aarch64/pr98776.c: New.
* gcc.target/aarch64/pr92424-2.c: Adjust pattern.
* gcc.target/aarch64/pr92424-3.c: Adjust pattern.
---
gcc/config/aarch64/aarch64-protos.h | 2 +
gcc/config/aarch64/aarch64.cc | 56 ++++++++++++++------
gcc/config/aarch64/aarch64.md | 14 +++++
gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
6 files changed, 70 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 031c6d74775..82c8896c7fe 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -1079,4 +1079,6 @@ const char *aarch64_indirect_call_asm (rtx);
extern bool aarch64_harden_sls_retbr_p (void);
extern bool aarch64_harden_sls_blr_p (void);
+extern void aarch64_output_patchable_area (unsigned int, bool);
+
#endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 4a2d58bb343..263bd138d27 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -22593,30 +22593,56 @@ aarch64_declare_function_name (FILE *stream, const char* name,
cfun->machine->label_is_assembled = true;
}
-/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
- the function label and emit a BTI if necessary. */
+/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
void
aarch64_print_patchable_function_entry (FILE *file,
unsigned HOST_WIDE_INT patch_area_size,
bool record_p)
{
- if (cfun->machine->label_is_assembled
- && aarch64_bti_enabled ()
- && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+ if (!cfun->machine->label_is_assembled)
{
- /* Remove the BTI that follows the patch area and insert a new BTI
- before the patch area right after the function label. */
- rtx_insn *insn = next_real_nondebug_insn (get_insns ());
- if (insn
- && INSN_P (insn)
- && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
- && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
- delete_insn (insn);
- asm_fprintf (file, "\thint\t34 // bti c\n");
+ /* Emit the patching area before the entry label, if any. */
+ default_print_patchable_function_entry (file, patch_area_size,
+ record_p);
+ return;
+ }
+
+ rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
+ GEN_INT (record_p));
+ basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+
+ if (!aarch64_bti_enabled ()
+ || cgraph_node::get (cfun->decl)->only_called_directly_p ())
+ {
+ /* Emit the patchable_area at the beginning of the function. */
+ rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
+ INSN_ADDRESSES_NEW (insn, -1);
+ return;
+ }
+
+ rtx_insn *insn = next_real_nondebug_insn (get_insns ());
+ if (!insn
+ || !INSN_P (insn)
+ || GET_CODE (PATTERN (insn)) != UNSPEC_VOLATILE
+ || XINT (PATTERN (insn), 1) != UNSPECV_BTI_C)
+ {
+ /* Emit a BTI_C. */
+ insn = emit_insn_before (gen_bti_c (), BB_HEAD (bb));
}
- default_print_patchable_function_entry (file, patch_area_size, record_p);
+ /* Emit the patchable_area after BTI_C. */
+ insn = emit_insn_after (pa, insn);
+ INSN_ADDRESSES_NEW (insn, -1);
+}
+
+/* Output patchable area. */
+
+void
+aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
+{
+ default_print_patchable_function_entry (asm_out_file, patch_area_size,
+ record_p);
}
/* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 7a04f45cacc..d24c8afcfa6 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -303,6 +303,7 @@
UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
UNSPEC_LD1RO
UNSPEC_SALT_ADDR
+ UNSPECV_PATCHABLE_AREA
])
(define_c_enum "unspecv" [
@@ -7710,6 +7711,19 @@
[(set_attr "type" "ls64")]
)
+(define_insn "patchable_area"
+ [(unspec_volatile [(match_operand 0 "const_int_operand")
+ (match_operand 1 "const_int_operand")]
+ UNSPECV_PATCHABLE_AREA)]
+ ""
+{
+ aarch64_output_patchable_area (INTVAL (operands[0]),
+ INTVAL (operands[1]) != 0);
+ return "";
+}
+ [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
+)
+
;; AdvSIMD Stuff
(include "aarch64-simd.md")
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
index 0e75657a153..86bb92e6f32 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
f10_bti ()
{
}
-/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
index 0a1f74d4096..daac2faf18e 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
f10_pac ()
{
}
-/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
new file mode 100644
index 00000000000..d193df8e66d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
@@ -0,0 +1,11 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label. */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
--
2.38.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: gcc11-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch --]
[-- Type: text/x-patch; name="gcc11-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch", Size: 7198 bytes --]
From 7c2ff339b5a6d17169fa3e346579a73df4a03d13 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop@amazon.com>
Date: Wed, 30 Nov 2022 19:45:24 +0000
Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
Currently patchable area is at the wrong place on AArch64. It is placed
immediately after function label, before .cfi_startproc. This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
modifies aarch64_print_patchable_function_entry to avoid placing
patchable area before .cfi_startproc.
gcc/
PR target/98776
* config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
Declared.
* config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
(aarch64_output_patchable_area): New.
* config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
(patchable_area): Define.
gcc/testsuite/
PR target/98776
* gcc.target/aarch64/pr98776.c: New.
* gcc.target/aarch64/pr92424-2.c: Adjust pattern.
* gcc.target/aarch64/pr92424-3.c: Adjust pattern.
---
gcc/config/aarch64/aarch64-protos.h | 2 +
gcc/config/aarch64/aarch64.c | 56 ++++++++++++++------
gcc/config/aarch64/aarch64.md | 14 +++++
gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
6 files changed, 70 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index ad62da81d31..fdc79b1dced 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -1052,4 +1052,6 @@ const char *aarch64_indirect_call_asm (rtx);
extern bool aarch64_harden_sls_retbr_p (void);
extern bool aarch64_harden_sls_blr_p (void);
+extern void aarch64_output_patchable_area (unsigned int, bool);
+
#endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b3a96e14e7c..7f45b582918 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -21539,30 +21539,56 @@ aarch64_declare_function_name (FILE *stream, const char* name,
cfun->machine->label_is_assembled = true;
}
-/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
- the function label and emit a BTI if necessary. */
+/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
void
aarch64_print_patchable_function_entry (FILE *file,
unsigned HOST_WIDE_INT patch_area_size,
bool record_p)
{
- if (cfun->machine->label_is_assembled
- && aarch64_bti_enabled ()
- && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+ if (!cfun->machine->label_is_assembled)
{
- /* Remove the BTI that follows the patch area and insert a new BTI
- before the patch area right after the function label. */
- rtx_insn *insn = next_real_nondebug_insn (get_insns ());
- if (insn
- && INSN_P (insn)
- && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
- && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
- delete_insn (insn);
- asm_fprintf (file, "\thint\t34 // bti c\n");
+ /* Emit the patching area before the entry label, if any. */
+ default_print_patchable_function_entry (file, patch_area_size,
+ record_p);
+ return;
+ }
+
+ rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
+ GEN_INT (record_p));
+ basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+
+ if (!aarch64_bti_enabled ()
+ || cgraph_node::get (cfun->decl)->only_called_directly_p ())
+ {
+ /* Emit the patchable_area at the beginning of the function. */
+ rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
+ INSN_ADDRESSES_NEW (insn, -1);
+ return;
+ }
+
+ rtx_insn *insn = next_real_nondebug_insn (get_insns ());
+ if (!insn
+ || !INSN_P (insn)
+ || GET_CODE (PATTERN (insn)) != UNSPEC_VOLATILE
+ || XINT (PATTERN (insn), 1) != UNSPECV_BTI_C)
+ {
+ /* Emit a BTI_C. */
+ insn = emit_insn_before (gen_bti_c (), BB_HEAD (bb));
}
- default_print_patchable_function_entry (file, patch_area_size, record_p);
+ /* Emit the patchable_area after BTI_C. */
+ insn = emit_insn_after (pa, insn);
+ INSN_ADDRESSES_NEW (insn, -1);
+}
+
+/* Output patchable area. */
+
+void
+aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
+{
+ default_print_patchable_function_entry (asm_out_file, patch_area_size,
+ record_p);
}
/* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 06616c66a80..1cca0f5c79f 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -294,6 +294,7 @@
UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
UNSPEC_LD1RO
UNSPEC_SALT_ADDR
+ UNSPECV_PATCHABLE_AREA
])
(define_c_enum "unspecv" [
@@ -7522,6 +7523,19 @@
[(set_attr "type" "memtag")]
)
+(define_insn "patchable_area"
+ [(unspec_volatile [(match_operand 0 "const_int_operand")
+ (match_operand 1 "const_int_operand")]
+ UNSPECV_PATCHABLE_AREA)]
+ ""
+{
+ aarch64_output_patchable_area (INTVAL (operands[0]),
+ INTVAL (operands[1]) != 0);
+ return "";
+}
+ [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
+)
+
;; AdvSIMD Stuff
(include "aarch64-simd.md")
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
index 0e75657a153..86bb92e6f32 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
f10_bti ()
{
}
-/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
index 0a1f74d4096..daac2faf18e 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
f10_pac ()
{
}
-/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
new file mode 100644
index 00000000000..d193df8e66d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
@@ -0,0 +1,11 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label. */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
--
2.38.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: gcc10-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch --]
[-- Type: text/x-patch; name="gcc10-AArch64-Add-UNSPECV_PATCHABLE_AREA-PR98776.patch", Size: 7196 bytes --]
From cca9241030f14b96d8edcc31e520300ed071148d Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop@amazon.com>
Date: Wed, 30 Nov 2022 19:45:24 +0000
Subject: [PATCH] AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
Currently patchable area is at the wrong place on AArch64. It is placed
immediately after function label, before .cfi_startproc. This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
modifies aarch64_print_patchable_function_entry to avoid placing
patchable area before .cfi_startproc.
gcc/
PR target/98776
* config/aarch64/aarch64-protos.h (aarch64_output_patchable_area):
Declared.
* config/aarch64/aarch64.cc (aarch64_print_patchable_function_entry):
Emit an UNSPECV_PATCHABLE_AREA pseudo instruction.
(aarch64_output_patchable_area): New.
* config/aarch64/aarch64.md (UNSPECV_PATCHABLE_AREA): New.
(patchable_area): Define.
gcc/testsuite/
PR target/98776
* gcc.target/aarch64/pr98776.c: New.
* gcc.target/aarch64/pr92424-2.c: Adjust pattern.
* gcc.target/aarch64/pr92424-3.c: Adjust pattern.
---
gcc/config/aarch64/aarch64-protos.h | 2 +
gcc/config/aarch64/aarch64.c | 56 ++++++++++++++------
gcc/config/aarch64/aarch64.md | 14 +++++
gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 2 +-
gcc/testsuite/gcc.target/aarch64/pr98776.c | 11 ++++
6 files changed, 70 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr98776.c
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 61e2ad54f88..ef24c48fc14 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -805,4 +805,6 @@ const char *aarch64_indirect_call_asm (rtx);
extern bool aarch64_harden_sls_retbr_p (void);
extern bool aarch64_harden_sls_blr_p (void);
+extern void aarch64_output_patchable_area (unsigned int, bool);
+
#endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 67c2f1123b4..dd9a3832b5a 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -19498,30 +19498,56 @@ aarch64_declare_function_name (FILE *stream, const char* name,
cfun->machine->label_is_assembled = true;
}
-/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after
- the function label and emit a BTI if necessary. */
+/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. */
void
aarch64_print_patchable_function_entry (FILE *file,
unsigned HOST_WIDE_INT patch_area_size,
bool record_p)
{
- if (cfun->machine->label_is_assembled
- && aarch64_bti_enabled ()
- && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+ if (!cfun->machine->label_is_assembled)
{
- /* Remove the BTI that follows the patch area and insert a new BTI
- before the patch area right after the function label. */
- rtx_insn *insn = next_real_nondebug_insn (get_insns ());
- if (insn
- && INSN_P (insn)
- && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
- && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C)
- delete_insn (insn);
- asm_fprintf (file, "\thint\t34 // bti c\n");
+ /* Emit the patching area before the entry label, if any. */
+ default_print_patchable_function_entry (file, patch_area_size,
+ record_p);
+ return;
+ }
+
+ rtx pa = gen_patchable_area (GEN_INT (patch_area_size),
+ GEN_INT (record_p));
+ basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+
+ if (!aarch64_bti_enabled ()
+ || cgraph_node::get (cfun->decl)->only_called_directly_p ())
+ {
+ /* Emit the patchable_area at the beginning of the function. */
+ rtx_insn *insn = emit_insn_before (pa, BB_HEAD (bb));
+ INSN_ADDRESSES_NEW (insn, -1);
+ return;
+ }
+
+ rtx_insn *insn = next_real_nondebug_insn (get_insns ());
+ if (!insn
+ || !INSN_P (insn)
+ || GET_CODE (PATTERN (insn)) != UNSPEC_VOLATILE
+ || XINT (PATTERN (insn), 1) != UNSPECV_BTI_C)
+ {
+ /* Emit a BTI_C. */
+ insn = emit_insn_before (gen_bti_c (), BB_HEAD (bb));
}
- default_print_patchable_function_entry (file, patch_area_size, record_p);
+ /* Emit the patchable_area after BTI_C. */
+ insn = emit_insn_after (pa, insn);
+ INSN_ADDRESSES_NEW (insn, -1);
+}
+
+/* Output patchable area. */
+
+void
+aarch64_output_patchable_area (unsigned int patch_area_size, bool record_p)
+{
+ default_print_patchable_function_entry (asm_out_file, patch_area_size,
+ record_p);
}
/* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 552aed3ce89..96e72bc6c06 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -282,6 +282,7 @@
UNSPEC_TAG_SPACE ; Translate address to MTE tag address space.
UNSPEC_LD1RO
UNSPEC_SALT_ADDR
+ UNSPECV_PATCHABLE_AREA
])
(define_c_enum "unspecv" [
@@ -7606,6 +7607,19 @@
[(set_attr "type" "memtag")]
)
+(define_insn "patchable_area"
+ [(unspec_volatile [(match_operand 0 "const_int_operand")
+ (match_operand 1 "const_int_operand")]
+ UNSPECV_PATCHABLE_AREA)]
+ ""
+{
+ aarch64_output_patchable_area (INTVAL (operands[0]),
+ INTVAL (operands[1]) != 0);
+ return "";
+}
+ [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))]
+)
+
;; AdvSIMD Stuff
(include "aarch64-simd.md")
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
index 0e75657a153..86bb92e6f32 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
f10_bti ()
{
}
-/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
index 0a1f74d4096..daac2faf18e 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
@@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
f10_pac ()
{
}
-/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
+/* { dg-final { scan-assembler "hint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr98776.c b/gcc/testsuite/gcc.target/aarch64/pr98776.c
new file mode 100644
index 00000000000..d193df8e66d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr98776.c
@@ -0,0 +1,11 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label. */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
--
2.38.1
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-15 4:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 3:04 AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776] Pop, Sebastian
2022-12-05 11:34 ` Richard Sandiford
2022-12-07 0:51 ` Pop, Sebastian
2022-12-07 9:12 ` Richard Sandiford
2022-12-07 18:46 ` Pop, Sebastian
2022-12-08 7:38 ` Richard Sandiford
2022-12-15 4:51 ` Pop, Sebastian
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).