From: "Pop, Sebastian" <spop@amazon.com>
To: Richard Sandiford <richard.sandiford@arm.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, 15 Dec 2022 04:51:28 +0000 [thread overview]
Message-ID: <ff6486e64b9a4219a247a7d282217b0f@amazon.com> (raw)
In-Reply-To: <mpt7cz28hwg.fsf@arm.com>
[-- 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
prev parent reply other threads:[~2022-12-15 4:51 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
2022-12-15 4:51 ` Pop, Sebastian [this message]
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=ff6486e64b9a4219a247a7d282217b0f@amazon.com \
--to=spop@amazon.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=kyrylo.tkachov@arm.com \
--cc=richard.sandiford@arm.com \
--cc=sebpop@gmail.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).