Bug 94697 - aarch64: bti j at function start instead of bti c
Summary: aarch64: bti j at function start instead of bti c
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: 10.2
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-21 15:13 UTC by nsz
Modified: 2020-05-14 15:53 UTC (History)
1 user (show)

See Also:
Host:
Target: aarch64*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-04-22 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description nsz 2020-04-21 15:13:58 UTC
function that may be indirectly called does not start with bti c:

void bar(int *);
void *addr;
int foo(int x)
{
label:
  addr=&&label;
  bar(&x);
  return x;
} 

with -O2 -mbranch-protection=bti+pac-ret

foo:
.L2:
        hint    36 // bti j
        hint    25 // paciasp
        adrp    x1, .L2
        stp     x29, x30, [sp, -32]!
        add     x1, x1, :lo12:.L2
        adrp    x2, .LANCHOR0
        mov     x29, sp
        str     x1, [x2, #:lo12:.LANCHOR0]
        str     w0, [sp, 28]
        add     x0, sp, 28
        bl      bar
        ldr     w0, [sp, 28]
        ldp     x29, x30, [sp], 32
        hint    29 // autiasp
        ret

        .set    .LANCHOR0,. + 0
addr:
        .zero   8

happens if function starts with a label that may be indirect
jump target so a bti j is inserted, but there is a paciasp
at the beginning which would normally act as implicit bti c
when it's the first instruction.
Comment 1 Richard Earnshaw 2020-04-22 10:01:30 UTC
Jumping to the label is not the same as calling the function indirectly.  But yes, the code here is clearly incorrect.
Comment 2 CVS Commits 2020-04-23 15:15:03 UTC
The master branch has been updated by Szabolcs Nagy <nsz@gcc.gnu.org>:

https://gcc.gnu.org/g:f7e4641afba7c348a7e7c8655e537a953c416bb3

commit r10-7918-gf7e4641afba7c348a7e7c8655e537a953c416bb3
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Fri Apr 17 16:54:12 2020 +0100

    aarch64: ensure bti c is emitted at function start [PR94697]
    
    The bti pass currently first emits bti c at function start
    if there is no paciasp (which also acts as indirect call
    landing pad), then bti j is emitted at jump labels, however
    if there is a label right before paciasp then the function
    start can end up like
    
      foo:
      label:
        bti j
        paciasp
        ...
    
    This patch is a minimal fix that just moves the bti c handling
    after the bti j handling so we end up with
    
      foo:
        bti c
      label:
        bti j
        paciasp
        ...
    
    This could be improved by emitting bti jc in this case, or by
    detecting that the label is not in fact an indirect jump target
    and then this situation would be much less common.
    
    Needs to be backported to gcc-9 branch.
    
    gcc/ChangeLog:
    
            PR target/94697
            * config/aarch64/aarch64-bti-insert.c (rest_of_insert_bti): Swap
            bti c and bti j handling.
    
    gcc/testsuite/ChangeLog:
    
            PR target/94697
            * gcc.target/aarch64/pr94697.c: New test.
Comment 3 Christophe Lyon 2020-04-27 11:13:44 UTC
     
>             PR target/94697
>             * gcc.target/aarch64/pr94697.c: New test.

The new test fails with -mabi=ilp32
Comment 4 CVS Commits 2020-04-27 17:18:16 UTC
The master branch has been updated by Szabolcs Nagy <nsz@gcc.gnu.org>:

https://gcc.gnu.org/g:562bfb1f0e64aa6398bdf4baa0a8b205f4b617ab

commit r10-7992-g562bfb1f0e64aa6398bdf4baa0a8b205f4b617ab
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Mon Apr 27 18:07:59 2020 +0100

    aarch64: disable test on ilp32 [PR94697]
    
    branch-protection=pac-ret is not supported on ilp32 now and
    the test requires it via branch-protection=standard.
    
    committed as obvious.
    
    gcc/testsuite/ChangeLog:
    
            PR target/94697
            * gcc.target/aarch64/pr94697.c: Require lp64.
Comment 5 Jakub Jelinek 2020-05-07 11:56:24 UTC
GCC 10.1 has been released.
Comment 6 nsz 2020-05-07 12:01:33 UTC
this is fixed for gcc 10.1, just not backported yet so i kept the bug open
Comment 7 CVS Commits 2020-05-14 15:17:57 UTC
The releases/gcc-9 branch has been updated by Szabolcs Nagy <nsz@gcc.gnu.org>:

https://gcc.gnu.org/g:f6e42cdee5de2b3441afc88c8888c1166bdffe57

commit r9-8594-gf6e42cdee5de2b3441afc88c8888c1166bdffe57
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Fri Apr 17 16:54:12 2020 +0100

    aarch64: ensure bti c is emitted at function start [PR94697]
    
    The bti pass currently first emits bti c at function start
    if there is no paciasp (which also acts as indirect call
    landing pad), then bti j is emitted at jump labels, however
    if there is a label right before paciasp then the function
    start can end up like
    
      foo:
      label:
        bti j
        paciasp
        ...
    
    This patch is a minimal fix that just moves the bti c handling
    after the bti j handling so we end up with
    
      foo:
        bti c
      label:
        bti j
        paciasp
        ...
    
    This could be improved by emitting bti jc in this case, or by
    detecting that the label is not in fact an indirect jump target
    and then this situation would be much less common.
    
    Needs to be backported to gcc-9 branch.
    
    Backported without the testcase because of missing infrastructure
    for check-function-bodies.
    
    gcc/ChangeLog:
    
            Backport from mainline.
            2020-04-23  Szabolcs Nagy  <szabolcs.nagy@arm.com>
    
            PR target/94697
            * config/aarch64/aarch64-bti-insert.c (rest_of_insert_bti): Swap
            bti c and bti j handling.
Comment 8 nsz 2020-05-14 15:53:34 UTC
fixed for gcc-10.1 and on the gcc-9 branch.