Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Richard Earnshaw via Gcc-patches]

On 18/08/2022 01:00, Andrew Pinski via Gcc-patches wrote:

On Fri, Nov 2, 2018 at 11:39 AM Sudakshina Das  wrote:


Hi

This patch is part of a series that enables ARMv8.5-A in GCC and
adds Branch Target Identification Mechanism.
(https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)

This patch adds a new pass called "bti" which is triggered by the
command line argument -mbranch-protection whenever "bti" is turned on.

The pass iterates through the instructions and adds appropriated BTI
instructions based on the following:
 * Add a new "BTI C" at the beginning of a function, unless its already
   protected by a "PACIASP/PACIBSP". We exempt the functions that are
   only called directly.


Coming back to this because the check only_called_directly_p does not
work if the linker will insert a veneer as the compiler does not know
about that.
This is recorded as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106671 .


I think the linker has to make sure to insert a veneer that ends with a 
branch in that case.


R.



Thanks,
Andrew Pinski



 * Add a new "BTI J" for every target of an indirect jump, jump table
   targets, non-local goto targets or labels that might be referenced
   by variables, constant pools, etc (NOTE_INSN_DELETED_LABEL)

Since we have already changed the use of indirect tail calls to only x16
and x17, we do not have to use "BTI JC".
(check patch 3/6).

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

Thanks
Sudi

*** gcc/ChangeLog ***

2018-xx-xx  Sudakshina Das  
 Ramana Radhakrishnan  

 * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
 * gcc/config/aarch64/aarch64.h: Update comment for
 TRAMPOLINE_SIZE.
 * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
 Update if bti is enabled.
 * config/aarch64/aarch64-bti-insert.c: New file.
 * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
 bti pass.
 * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
 Declare the new bti pass.
 * config/aarch64/aarch64.md (bti_nop): Define.
 * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.

*** gcc/testsuite/ChangeLog ***

2018-xx-xx  Sudakshina Das  

 * gcc.target/aarch64/bti-1.c: New test.
 * gcc.target/aarch64/bti-2.c: New test.
 * lib/target-supports.exp
 (check_effective_target_aarch64_bti_hw): Add new check for
 BTI hw.

Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Andrew Pinski via Gcc-patches]

On Fri, Nov 2, 2018 at 11:39 AM Sudakshina Das  wrote:
>
> Hi
>
> This patch is part of a series that enables ARMv8.5-A in GCC and
> adds Branch Target Identification Mechanism.
> (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)
>
> This patch adds a new pass called "bti" which is triggered by the
> command line argument -mbranch-protection whenever "bti" is turned on.
>
> The pass iterates through the instructions and adds appropriated BTI
> instructions based on the following:
> * Add a new "BTI C" at the beginning of a function, unless its already
>   protected by a "PACIASP/PACIBSP". We exempt the functions that are
>   only called directly.

Coming back to this because the check only_called_directly_p does not
work if the linker will insert a veneer as the compiler does not know
about that.
This is recorded as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106671 .

Thanks,
Andrew Pinski


> * Add a new "BTI J" for every target of an indirect jump, jump table
>   targets, non-local goto targets or labels that might be referenced
>   by variables, constant pools, etc (NOTE_INSN_DELETED_LABEL)
>
> Since we have already changed the use of indirect tail calls to only x16
> and x17, we do not have to use "BTI JC".
> (check patch 3/6).
>
> Bootstrapped and regression tested with aarch64-none-linux-gnu. Added
> new tests.
> Is this ok for trunk?
>
> Thanks
> Sudi
>
> *** gcc/ChangeLog ***
>
> 2018-xx-xx  Sudakshina Das  
> Ramana Radhakrishnan  
>
> * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
> * gcc/config/aarch64/aarch64.h: Update comment for
> TRAMPOLINE_SIZE.
> * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
> Update if bti is enabled.
> * config/aarch64/aarch64-bti-insert.c: New file.
> * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
> bti pass.
> * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
> Declare the new bti pass.
> * config/aarch64/aarch64.md (bti_nop): Define.
> * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2018-xx-xx  Sudakshina Das  
>
> * gcc.target/aarch64/bti-1.c: New test.
> * gcc.target/aarch64/bti-2.c: New test.
> * lib/target-supports.exp
> (check_effective_target_aarch64_bti_hw): Add new check for
> BTI hw.
>

Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Sudakshina Das]

Hi Christophe

On 10/01/19 15:46, Christophe Lyon wrote:
> On Wed, 9 Jan 2019 at 15:42, Sudakshina Das  wrote:
>>
>> Hi
>>
>> On 20/12/18 16:40, Sudakshina Das wrote:
>>> Hi James
>>>
>>> On 19/12/18 3:40 PM, James Greenhalgh wrote:
 On Fri, Dec 14, 2018 at 10:09:03AM -0600, Sudakshina Das wrote:

 

> I have updated the patch according to our discussions offline.
> The md pattern is now split into 4 patterns and i have added a new
> test for the setjmp case along with some comments where missing.

 This is OK for trunk.

>>>
>>> Thanks for the approvals. With this my series is ready to go in trunk. I
>>> will wait for Sam's options patch to go in trunk before I commit mine.
>>>
>>
>> Series is committed with a rebase without Sam Tebbs's 3rd patch for
>> B-Key addition as r267765 to r267770.
>>
>> Thanks
>> Sudi
>>
> 
> Hi Sudi,
> 
> I think the new bti-1.c test lacks
> /* { dg-require-effective-target lp64 } */
> as I see it failing when using -mabi=ilp32:
> cc1: sorry, unimplemented: return address signing is only supported
> for -mabi=lp64
> 

Ah yes, I actually added code for exempting ilp32 in my configure patch 
but missed it while adding test cases. Testing a patch right now.

Sudi

> Christophe
> 
>>> Thanks
>>> Sudi
>>>
 Thanks,
 James

> *** gcc/ChangeLog ***
>
> 2018-xx-xx  Sudakshina Das  
>  Ramana Radhakrishnan  
>
>  * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
>  * gcc/config/aarch64/aarch64.h: Update comment for
>  TRAMPOLINE_SIZE.
>  * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
>  Update if bti is enabled.
>  * config/aarch64/aarch64-bti-insert.c: New file.
>  * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
>  bti pass.
>  * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
>  Declare the new bti pass.
>  * config/aarch64/aarch64.md (unspecv): Add UNSPECV_BTI_NOARG,
>  UNSPECV_BTI_C, UNSPECV_BTI_J and UNSPECV_BTI_JC.
>  (bti_noarg, bti_j, bti_c, bti_jc): New define_insns.
>  * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2018-xx-xx  Sudakshina Das  
>
>  * gcc.target/aarch64/bti-1.c: New test.
>  * gcc.target/aarch64/bti-2.c: New test.
>  * gcc.target/aarch64/bti-3.c: New test.
>  * lib/target-supports.exp
>  (check_effective_target_aarch64_bti_hw): Add new check for
>  BTI hw.
>
> Thanks
> Sudi
>>

Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Christophe Lyon]

On Wed, 9 Jan 2019 at 15:42, Sudakshina Das  wrote:
>
> Hi
>
> On 20/12/18 16:40, Sudakshina Das wrote:
> > Hi James
> >
> > On 19/12/18 3:40 PM, James Greenhalgh wrote:
> >> On Fri, Dec 14, 2018 at 10:09:03AM -0600, Sudakshina Das wrote:
> >>
> >> 
> >>
> >>> I have updated the patch according to our discussions offline.
> >>> The md pattern is now split into 4 patterns and i have added a new
> >>> test for the setjmp case along with some comments where missing.
> >>
> >> This is OK for trunk.
> >>
> >
> > Thanks for the approvals. With this my series is ready to go in trunk. I
> > will wait for Sam's options patch to go in trunk before I commit mine.
> >
>
> Series is committed with a rebase without Sam Tebbs's 3rd patch for
> B-Key addition as r267765 to r267770.
>
> Thanks
> Sudi
>

Hi Sudi,

I think the new bti-1.c test lacks
/* { dg-require-effective-target lp64 } */
as I see it failing when using -mabi=ilp32:
cc1: sorry, unimplemented: return address signing is only supported
for -mabi=lp64

Christophe

> > Thanks
> > Sudi
> >
> >> Thanks,
> >> James
> >>
> >>> *** gcc/ChangeLog ***
> >>>
> >>> 2018-xx-xx  Sudakshina Das  
> >>> Ramana Radhakrishnan  
> >>>
> >>> * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
> >>> * gcc/config/aarch64/aarch64.h: Update comment for
> >>> TRAMPOLINE_SIZE.
> >>> * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
> >>> Update if bti is enabled.
> >>> * config/aarch64/aarch64-bti-insert.c: New file.
> >>> * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
> >>> bti pass.
> >>> * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
> >>> Declare the new bti pass.
> >>> * config/aarch64/aarch64.md (unspecv): Add UNSPECV_BTI_NOARG,
> >>> UNSPECV_BTI_C, UNSPECV_BTI_J and UNSPECV_BTI_JC.
> >>> (bti_noarg, bti_j, bti_c, bti_jc): New define_insns.
> >>> * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.
> >>>
> >>> *** gcc/testsuite/ChangeLog ***
> >>>
> >>> 2018-xx-xx  Sudakshina Das  
> >>>
> >>> * gcc.target/aarch64/bti-1.c: New test.
> >>> * gcc.target/aarch64/bti-2.c: New test.
> >>> * gcc.target/aarch64/bti-3.c: New test.
> >>> * lib/target-supports.exp
> >>> (check_effective_target_aarch64_bti_hw): Add new check for
> >>> BTI hw.
> >>>
> >>> Thanks
> >>> Sudi
>

Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Sudakshina Das]

Hi

On 20/12/18 16:40, Sudakshina Das wrote:
> Hi James
> 
> On 19/12/18 3:40 PM, James Greenhalgh wrote:
>> On Fri, Dec 14, 2018 at 10:09:03AM -0600, Sudakshina Das wrote:
>>
>> 
>>
>>> I have updated the patch according to our discussions offline.
>>> The md pattern is now split into 4 patterns and i have added a new
>>> test for the setjmp case along with some comments where missing.
>>
>> This is OK for trunk.
>>
> 
> Thanks for the approvals. With this my series is ready to go in trunk. I 
> will wait for Sam's options patch to go in trunk before I commit mine.
> 

Series is committed with a rebase without Sam Tebbs's 3rd patch for 
B-Key addition as r267765 to r267770.

Thanks
Sudi

> Thanks
> Sudi
> 
>> Thanks,
>> James
>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2018-xx-xx  Sudakshina Das  
>>>     Ramana Radhakrishnan  
>>>
>>> * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
>>> * gcc/config/aarch64/aarch64.h: Update comment for
>>> TRAMPOLINE_SIZE.
>>> * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
>>> Update if bti is enabled.
>>> * config/aarch64/aarch64-bti-insert.c: New file.
>>> * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
>>> bti pass.
>>> * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
>>> Declare the new bti pass.
>>> * config/aarch64/aarch64.md (unspecv): Add UNSPECV_BTI_NOARG,
>>> UNSPECV_BTI_C, UNSPECV_BTI_J and UNSPECV_BTI_JC.
>>> (bti_noarg, bti_j, bti_c, bti_jc): New define_insns.
>>> * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2018-xx-xx  Sudakshina Das  
>>>
>>> * gcc.target/aarch64/bti-1.c: New test.
>>> * gcc.target/aarch64/bti-2.c: New test.
>>> * gcc.target/aarch64/bti-3.c: New test.
>>> * lib/target-supports.exp
>>> (check_effective_target_aarch64_bti_hw): Add new check for
>>> BTI hw.
>>>
>>> Thanks
>>> Sudi

Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Sudakshina Das]

Hi James

On 19/12/18 3:40 PM, James Greenhalgh wrote:
> On Fri, Dec 14, 2018 at 10:09:03AM -0600, Sudakshina Das wrote:
> 
> 
> 
>> I have updated the patch according to our discussions offline.
>> The md pattern is now split into 4 patterns and i have added a new
>> test for the setjmp case along with some comments where missing.
> 
> This is OK for trunk.
> 

Thanks for the approvals. With this my series is ready to go in trunk. I 
will wait for Sam's options patch to go in trunk before I commit mine.

Thanks
Sudi

> Thanks,
> James
> 
>> *** gcc/ChangeLog ***
>>
>> 2018-xx-xx  Sudakshina Das  
>>  Ramana Radhakrishnan  
>>
>>  * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
>>  * gcc/config/aarch64/aarch64.h: Update comment for
>>  TRAMPOLINE_SIZE.
>>  * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
>>  Update if bti is enabled.
>>  * config/aarch64/aarch64-bti-insert.c: New file.
>>  * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
>>  bti pass.
>>  * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
>>  Declare the new bti pass.
>>  * config/aarch64/aarch64.md (unspecv): Add UNSPECV_BTI_NOARG,
>>  UNSPECV_BTI_C, UNSPECV_BTI_J and UNSPECV_BTI_JC.
>>  (bti_noarg, bti_j, bti_c, bti_jc): New define_insns.
>>  * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2018-xx-xx  Sudakshina Das  
>>
>>  * gcc.target/aarch64/bti-1.c: New test.
>>  * gcc.target/aarch64/bti-2.c: New test.
>>  * gcc.target/aarch64/bti-3.c: New test.
>>  * lib/target-supports.exp
>>  (check_effective_target_aarch64_bti_hw): Add new check for
>>  BTI hw.
>>
>> Thanks
>> Sudi

Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[James Greenhalgh]

On Fri, Dec 14, 2018 at 10:09:03AM -0600, Sudakshina Das wrote:



> I have updated the patch according to our discussions offline.
> The md pattern is now split into 4 patterns and i have added a new
> test for the setjmp case along with some comments where missing.

This is OK for trunk.

Thanks,
James

> *** gcc/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  
>   Ramana Radhakrishnan  
> 
>   * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
>   * gcc/config/aarch64/aarch64.h: Update comment for
>   TRAMPOLINE_SIZE.
>   * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
>   Update if bti is enabled.
>   * config/aarch64/aarch64-bti-insert.c: New file.
>   * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
>   bti pass.
>   * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
>   Declare the new bti pass.
>   * config/aarch64/aarch64.md (unspecv): Add UNSPECV_BTI_NOARG,
>   UNSPECV_BTI_C, UNSPECV_BTI_J and UNSPECV_BTI_JC.
>   (bti_noarg, bti_j, bti_c, bti_jc): New define_insns.
>   * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.
> 
> *** gcc/testsuite/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  
> 
>   * gcc.target/aarch64/bti-1.c: New test.
>   * gcc.target/aarch64/bti-2.c: New test.
>   * gcc.target/aarch64/bti-3.c: New test.
>   * lib/target-supports.exp
>   (check_effective_target_aarch64_bti_hw): Add new check for
>   BTI hw.
> 
> Thanks
> Sudi

Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Sudakshina Das]

Hi James

On 29/11/18 16:47, Sudakshina Das wrote:
> Hi
> 
> On 13/11/18 14:47, Sudakshina Das wrote:
>> Hi
>>
>> On 02/11/18 18:38, Sudakshina Das wrote:
>>> Hi
>>>
>>> This patch is part of a series that enables ARMv8.5-A in GCC and
>>> adds Branch Target Identification Mechanism.
>>> (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)
>>>
>>> This patch adds a new pass called "bti" which is triggered by the
>>> command line argument -mbranch-protection whenever "bti" is turned on.
>>>
>>> The pass iterates through the instructions and adds appropriated BTI
>>> instructions based on the following:
>>>* Add a new "BTI C" at the beginning of a function, unless its 
>>> already
>>>  protected by a "PACIASP/PACIBSP". We exempt the functions that are
>>>  only called directly.
>>>* Add a new "BTI J" for every target of an indirect jump, jump table
>>>  targets, non-local goto targets or labels that might be referenced
>>>  by variables, constant pools, etc (NOTE_INSN_DELETED_LABEL)
>>>
>>> Since we have already changed the use of indirect tail calls to only x16
>>> and x17, we do not have to use "BTI JC".
>>> (check patch 3/6).
>>>
>>
>> I missed out on the explanation for the changes to the trampoline code.
>> The patch also updates the trampoline code in case BTI is enabled. Since
>> the trampoline code is a target of an indirect branch, we need to add an
>> appropriate BTI instruction at the beginning of it to avoid a branch
>> target exception.
>>
>>> Bootstrapped and regression tested with aarch64-none-linux-gnu. Added
>>> new tests.
>>> Is this ok for trunk?
>>>
>>> Thanks
>>> Sudi
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2018-xx-xx  Sudakshina Das  
>>> Ramana Radhakrishnan  
>>>
>>> * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
>>> * gcc/config/aarch64/aarch64.h: Update comment for
>>> TRAMPOLINE_SIZE.
>>> * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
>>> Update if bti is enabled.
>>> * config/aarch64/aarch64-bti-insert.c: New file.
>>> * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
>>> bti pass.
>>> * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
>>> Declare the new bti pass.
>>> * config/aarch64/aarch64.md (bti_nop): Define.
>>> * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2018-xx-xx  Sudakshina Das  
>>>
>>> * gcc.target/aarch64/bti-1.c: New test.
>>> * gcc.target/aarch64/bti-2.c: New test.
>>> * lib/target-supports.exp
>>> (check_effective_target_aarch64_bti_hw): Add new check for
>>> BTI hw.
>>>
>>
>> Updated patch attached with more comments and a bit of simplification
>> in aarch64-bti-insert.c. ChangeLog still applies.
>>
>> Thanks
>> Sudi
>>
> 
> I found a missed case in the bti pass and edited the patch to include
> it. This made me realize that the only 2 regressions I saw with the
> BTI enabled model can now be avoided. (as quoted below from my 6/6
> patch)
> "Bootstrapped and regression tested with aarch64-none-linux-gnu with
> and without the configure option turned on.
> Also tested on aarch64-none-elf with and without configure option with a
> BTI enabled aem. Only 2 regressions and these were because newlib
> requires patches to protect hand coded libraries with BTI."
> 
> The ChangeLog still applies.
> 
> Sudi
> 
I have updated the patch according to our discussions offline.
The md pattern is now split into 4 patterns and i have added a new
test for the setjmp case along with some comments where missing.

*** gcc/ChangeLog ***

2018-xx-xx  Sudakshina Das  
Ramana Radhakrishnan  

* config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
* gcc/config/aarch64/aarch64.h: Update comment for
TRAMPOLINE_SIZE.
* config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
Update if bti is enabled.
* config/aarch64/aarch64-bti-insert.c: New file.
* config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
bti pass.
* config/aarch64/aarch64-protos.h (make_pass_insert_bti):
Declare the new bti pass.
* config/aarch64/aarch64.md (unspecv): Add UNSPECV_BTI_NOARG,
UNSPECV_BTI_C, UNSPECV_BTI_J and UNSPECV_BTI_JC.
(bti_noarg, bti_j, bti_c, bti_jc): New define_insns.
* config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.

*** gcc/testsuite/ChangeLog ***

2018-xx-xx  Sudakshina Das  

* gcc.target/aarch64/bti-1.c: New test.
* gcc.target/aarch64/bti-2.c: New test.
* gcc.target/aarch64/bti-3.c: New test.
* lib/target-supports.exp
(check_effective_target_aarch64_bti_hw): Add new check for
BTI hw.

Thanks
Sudi
diff --git a/gcc/config.gcc b/gcc/config.gcc
index cbabd21b33723a65790e2eafe8aa4979051cae48..f3dd3feceb3375374a29ca58e4ad87f949cea44d 100644
-

Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Sudakshina Das]

Hi

On 13/11/18 14:47, Sudakshina Das wrote:
> Hi
> 
> On 02/11/18 18:38, Sudakshina Das wrote:
>> Hi
>>
>> This patch is part of a series that enables ARMv8.5-A in GCC and
>> adds Branch Target Identification Mechanism.
>> (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)
>>
>> This patch adds a new pass called "bti" which is triggered by the
>> command line argument -mbranch-protection whenever "bti" is turned on.
>>
>> The pass iterates through the instructions and adds appropriated BTI
>> instructions based on the following:
>>   * Add a new "BTI C" at the beginning of a function, unless its already
>> protected by a "PACIASP/PACIBSP". We exempt the functions that are
>> only called directly.
>>   * Add a new "BTI J" for every target of an indirect jump, jump table
>> targets, non-local goto targets or labels that might be referenced
>> by variables, constant pools, etc (NOTE_INSN_DELETED_LABEL)
>>
>> Since we have already changed the use of indirect tail calls to only x16
>> and x17, we do not have to use "BTI JC".
>> (check patch 3/6).
>>
> 
> I missed out on the explanation for the changes to the trampoline code.
> The patch also updates the trampoline code in case BTI is enabled. Since
> the trampoline code is a target of an indirect branch, we need to add an
> appropriate BTI instruction at the beginning of it to avoid a branch
> target exception.
> 
>> Bootstrapped and regression tested with aarch64-none-linux-gnu. Added
>> new tests.
>> Is this ok for trunk?
>>
>> Thanks
>> Sudi
>>
>> *** gcc/ChangeLog ***
>>
>> 2018-xx-xx  Sudakshina Das  
>>  Ramana Radhakrishnan  
>>
>>  * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
>>  * gcc/config/aarch64/aarch64.h: Update comment for
>>  TRAMPOLINE_SIZE.
>>  * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
>>  Update if bti is enabled.
>>  * config/aarch64/aarch64-bti-insert.c: New file.
>>  * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
>>  bti pass.
>>  * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
>>  Declare the new bti pass.
>>  * config/aarch64/aarch64.md (bti_nop): Define.
>>  * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2018-xx-xx  Sudakshina Das  
>>
>>  * gcc.target/aarch64/bti-1.c: New test.
>>  * gcc.target/aarch64/bti-2.c: New test.
>>  * lib/target-supports.exp
>>  (check_effective_target_aarch64_bti_hw): Add new check for
>>  BTI hw.
>>
> 
> Updated patch attached with more comments and a bit of simplification
> in aarch64-bti-insert.c. ChangeLog still applies.
> 
> Thanks
> Sudi
> 

I found a missed case in the bti pass and edited the patch to include
it. This made me realize that the only 2 regressions I saw with the
BTI enabled model can now be avoided. (as quoted below from my 6/6
patch)
"Bootstrapped and regression tested with aarch64-none-linux-gnu with
and without the configure option turned on.
Also tested on aarch64-none-elf with and without configure option with a
BTI enabled aem. Only 2 regressions and these were because newlib
requires patches to protect hand coded libraries with BTI."

The ChangeLog still applies.

Sudi
diff --git a/gcc/config.gcc b/gcc/config.gcc
index b108697cfc7b1c9c6dc1f30cca6fd1158182c29e..3e77f9df6ad6ca55fccca50387eab4b2501af647 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -317,7 +317,7 @@ aarch64*-*-*)
 	c_target_objs="aarch64-c.o"
 	cxx_target_objs="aarch64-c.o"
 	d_target_objs="aarch64-d.o"
-	extra_objs="aarch64-builtins.o aarch-common.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o"
+	extra_objs="aarch64-builtins.o aarch-common.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o aarch64-bti-insert.o"
 	target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.c"
 	target_has_targetm_common=yes
 	;;
diff --git a/gcc/config/aarch64/aarch64-bti-insert.c b/gcc/config/aarch64/aarch64-bti-insert.c
new file mode 100644
index ..be604fb2fd5df052971cc81b7e6d7760880a6b79
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-bti-insert.c
@@ -0,0 +1,236 @@
+/* Branch Target Identification for AArch64 architecture.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   Contributed by Arm Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GN

Re: [PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Sudakshina Das]

Hi

On 02/11/18 18:38, Sudakshina Das wrote:
> Hi
> 
> This patch is part of a series that enables ARMv8.5-A in GCC and
> adds Branch Target Identification Mechanism.
> (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)
> 
> This patch adds a new pass called "bti" which is triggered by the
> command line argument -mbranch-protection whenever "bti" is turned on.
> 
> The pass iterates through the instructions and adds appropriated BTI
> instructions based on the following:
>  * Add a new "BTI C" at the beginning of a function, unless its already
>protected by a "PACIASP/PACIBSP". We exempt the functions that are
>only called directly.
>  * Add a new "BTI J" for every target of an indirect jump, jump table
>targets, non-local goto targets or labels that might be referenced
>by variables, constant pools, etc (NOTE_INSN_DELETED_LABEL)
> 
> Since we have already changed the use of indirect tail calls to only x16
> and x17, we do not have to use "BTI JC".
> (check patch 3/6).
> 

I missed out on the explanation for the changes to the trampoline code.
The patch also updates the trampoline code in case BTI is enabled. Since
the trampoline code is a target of an indirect branch, we need to add an
appropriate BTI instruction at the beginning of it to avoid a branch
target exception.

> Bootstrapped and regression tested with aarch64-none-linux-gnu. Added
> new tests.
> Is this ok for trunk?
> 
> Thanks
> Sudi
> 
> *** gcc/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  
>   Ramana Radhakrishnan  
> 
>   * config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
>   * gcc/config/aarch64/aarch64.h: Update comment for
>   TRAMPOLINE_SIZE.
>   * config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
>   Update if bti is enabled.
>   * config/aarch64/aarch64-bti-insert.c: New file.
>   * config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
>   bti pass.
>   * config/aarch64/aarch64-protos.h (make_pass_insert_bti):
>   Declare the new bti pass.
>   * config/aarch64/aarch64.md (bti_nop): Define.
>   * config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.
> 
> *** gcc/testsuite/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  
> 
>   * gcc.target/aarch64/bti-1.c: New test.
>   * gcc.target/aarch64/bti-2.c: New test.
>   * lib/target-supports.exp
>   (check_effective_target_aarch64_bti_hw): Add new check for
>   BTI hw.
>

Updated patch attached with more comments and a bit of simplification
in aarch64-bti-insert.c. ChangeLog still applies.

Thanks
Sudi

diff --git a/gcc/config.gcc b/gcc/config.gcc
index b108697cfc7b1c9c6dc1f30cca6fd1158182c29e..3e77f9df6ad6ca55fccca50387eab4b2501af647 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -317,7 +317,7 @@ aarch64*-*-*)
 	c_target_objs="aarch64-c.o"
 	cxx_target_objs="aarch64-c.o"
 	d_target_objs="aarch64-d.o"
-	extra_objs="aarch64-builtins.o aarch-common.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o"
+	extra_objs="aarch64-builtins.o aarch-common.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o aarch64-bti-insert.o"
 	target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.c"
 	target_has_targetm_common=yes
 	;;
diff --git a/gcc/config/aarch64/aarch64-bti-insert.c b/gcc/config/aarch64/aarch64-bti-insert.c
new file mode 100644
index ..15202e0def3b514bdbd1564b39a121e43e01a67f
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-bti-insert.c
@@ -0,0 +1,226 @@
+/* Branch Target Identification for AArch64 architecture.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   Contributed by Arm Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   .  */
+
+#define IN_TARGET_CODE 1
+
+#include "config.h"
+#define INCLUDE_STRING
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "target.h"
+#include "rtl.h"
+#include "tree.h"
+#include "memmodel.h"
+#include "gimple.h"
+#include "tm_p.h"
+#include "stringpool.h"
+#include "attribs.h"
+#include "emit-rtl.h"
+#include "gimplify.h"
+#include "gimple-iterator.h"
+#include "dumpfile.h"
+#include "rtl-iter.h"
+#include "cfgrtl.h"
+#include "tree-pass.h"
+#include "cgraph.h"
+
+/* This pass enables the support for Branch Target I

[PATCH, GCC, AARCH64, 5/6] Enable BTI : Add new pass for BTI.

[Sudakshina Das]

Hi

This patch is part of a series that enables ARMv8.5-A in GCC and
adds Branch Target Identification Mechanism.
(https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)

This patch adds a new pass called "bti" which is triggered by the
command line argument -mbranch-protection whenever "bti" is turned on.

The pass iterates through the instructions and adds appropriated BTI 
instructions based on the following:
* Add a new "BTI C" at the beginning of a function, unless its already
  protected by a "PACIASP/PACIBSP". We exempt the functions that are
  only called directly.
* Add a new "BTI J" for every target of an indirect jump, jump table
  targets, non-local goto targets or labels that might be referenced
  by variables, constant pools, etc (NOTE_INSN_DELETED_LABEL)

Since we have already changed the use of indirect tail calls to only x16 
and x17, we do not have to use "BTI JC".
(check patch 3/6).

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

Thanks
Sudi

*** gcc/ChangeLog ***

2018-xx-xx  Sudakshina Das  
Ramana Radhakrishnan  

* config.gcc (aarch64*-*-*): Add aarch64-bti-insert.o.
* gcc/config/aarch64/aarch64.h: Update comment for
TRAMPOLINE_SIZE.
* config/aarch64/aarch64.c (aarch64_asm_trampoline_template):
Update if bti is enabled.
* config/aarch64/aarch64-bti-insert.c: New file.
* config/aarch64/aarch64-passes.def (INSERT_PASS_BEFORE): Insert
bti pass.
* config/aarch64/aarch64-protos.h (make_pass_insert_bti):
Declare the new bti pass.
* config/aarch64/aarch64.md (bti_nop): Define.
* config/aarch64/t-aarch64: Add rule for aarch64-bti-insert.o.

*** gcc/testsuite/ChangeLog ***

2018-xx-xx  Sudakshina Das  

* gcc.target/aarch64/bti-1.c: New test.
* gcc.target/aarch64/bti-2.c: New test.
* lib/target-supports.exp
(check_effective_target_aarch64_bti_hw): Add new check for
BTI hw.

diff --git a/gcc/config.gcc b/gcc/config.gcc
index b108697cfc7b1c9c6dc1f30cca6fd1158182c29e..3e77f9df6ad6ca55fccca50387eab4b2501af647 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -317,7 +317,7 @@ aarch64*-*-*)
 	c_target_objs="aarch64-c.o"
 	cxx_target_objs="aarch64-c.o"
 	d_target_objs="aarch64-d.o"
-	extra_objs="aarch64-builtins.o aarch-common.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o"
+	extra_objs="aarch64-builtins.o aarch-common.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o aarch64-bti-insert.o"
 	target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.c"
 	target_has_targetm_common=yes
 	;;
diff --git a/gcc/config/aarch64/aarch64-bti-insert.c b/gcc/config/aarch64/aarch64-bti-insert.c
new file mode 100644
index ..efd57620d8803302e03ca643b9f2495e188dc19b
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-bti-insert.c
@@ -0,0 +1,195 @@
+/* Branch Target Identification for AArch64 architecture.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   Contributed by Arm Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   .  */
+
+#define IN_TARGET_CODE 1
+
+#include "config.h"
+#define INCLUDE_STRING
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "target.h"
+#include "rtl.h"
+#include "tree.h"
+#include "memmodel.h"
+#include "gimple.h"
+#include "tm_p.h"
+#include "stringpool.h"
+#include "attribs.h"
+#include "emit-rtl.h"
+#include "gimplify.h"
+#include "gimple-iterator.h"
+#include "dumpfile.h"
+#include "rtl-iter.h"
+#include "cfgrtl.h"
+#include "tree-pass.h"
+#include "cgraph.h"
+
+namespace {
+
+const pass_data pass_data_insert_bti =
+{
+  RTL_PASS, /* type.  */
+  "bti", /* name.  */
+  OPTGROUP_NONE, /* optinfo_flags.  */
+  TV_MACH_DEP, /* tv_id.  */
+  0, /* properties_required.  */
+  0, /* properties_provided.  */
+  0, /* properties_destroyed.  */
+  0, /* todo_flags_start.  */
+  0, /* todo_flags_finish.  */
+};
+
+/* Check if X (or any sub-rtx of X) is a PACIASP/PACIBSP instruction.  */
+static bool
+aarch64_pac_insn_p (rtx x)
+{
+  if (!INSN_P (x))
+return x;
+
+  subrtx_var_iterator::array_type array;
+  FOR_EACH_SUBRTX_VAR (iter, array, PATTERN (x), ALL)
+{
+  rtx