From: Richard Sandiford <richard.sandiford@arm.com>
To: "Pop\, Sebastian" <spop@amazon.com>
Cc: "gcc-patches\@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
"sebpop\@gmail.com" <sebpop@gmail.com>,
Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Subject: Re: AArch64: Add UNSPECV_PATCHABLE_AREA [PR98776]
Date: Thu, 08 Dec 2022 07:38:07 +0000 [thread overview]
Message-ID: <mpt7cz28hwg.fsf@arm.com> (raw)
In-Reply-To: <546a22f86fb046e998e4b10044e028aa@amazon.com> (Sebastian Pop's message of "Wed, 7 Dec 2022 18:46:31 +0000")
"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" } } */
next prev parent reply other threads:[~2022-12-08 7:38 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-01 3:04 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 [this message]
2022-12-15 4:51 ` Pop, Sebastian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=mpt7cz28hwg.fsf@arm.com \
--to=richard.sandiford@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=kyrylo.tkachov@arm.com \
--cc=sebpop@gmail.com \
--cc=spop@amazon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).