[Bug target/94697] New: aarch64: bti j at function start instead of bti c

[Bug target/94697] New: aarch64: bti j at function start instead of bti c

[nsz at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

            Bug ID: 94697
           Summary: aarch64: bti j at function start instead of bti c
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nsz at gcc dot gnu.org
  Target Milestone: ---

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.

[Bug target/94697] aarch64: bti j at function start instead of bti c

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-04-22
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
Jumping to the label is not the same as calling the function indirectly.  But
yes, the code here is clearly incorrect.

[Bug target/94697] aarch64: bti j at function start instead of bti c

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug target/94697] aarch64: bti j at function start instead of bti c

[nsz at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

nsz at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.0

[Bug target/94697] aarch64: bti j at function start instead of bti c

[clyon at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

Christophe Lyon <clyon at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clyon at gcc dot gnu.org

--- Comment #3 from Christophe Lyon <clyon at gcc dot gnu.org> ---

>             PR target/94697
>             * gcc.target/aarch64/pr94697.c: New test.

The new test fails with -mabi=ilp32

[Bug target/94697] aarch64: bti j at function start instead of bti c

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug target/94697] aarch64: bti j at function start instead of bti c

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.0                        |10.2

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.1 has been released.

[Bug target/94697] aarch64: bti j at function start instead of bti c

[nsz at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

--- Comment #6 from nsz at gcc dot gnu.org ---
this is fixed for gcc 10.1, just not backported yet so i kept the bug open

[Bug target/94697] aarch64: bti j at function start instead of bti c

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug target/94697] aarch64: bti j at function start instead of bti c

[nsz at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697

nsz at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from nsz at gcc dot gnu.org ---
fixed for gcc-10.1 and on the gcc-9 branch.