| Message ID | 20200428155045.GO29015@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: don't emit bti j after NOTE_INSN_DELETED_LABEL [PR94748] | expand |
> -----Original Message----- > From: Szabolcs Nagy <Szabolcs.Nagy@arm.com> > Sent: 28 April 2020 16:51 > To: gcc-patches@gcc.gnu.org > Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; Kyrylo Tkachov > <Kyrylo.Tkachov@arm.com>; Sudakshina Das <Sudi.Das@arm.com> > Subject: [PATCH] aarch64: don't emit bti j after NOTE_INSN_DELETED_LABEL > [PR94748] > > It was previously discussed that indirect branches cannot go to > NOTE_INSN_DELETED_LABEL so inserting a landing pad is unnecessary. > See https://gcc.gnu.org/pipermail/gcc-patches/2019-May/522625.html > > Before the patch a bti j was inserted after the label in > > __attribute__((target("branch-protection=bti"))) > int foo (void) > { > label: > return 0; > } > > This is not necessary and weakens the security protection. Ok if testing is successful. Thanks, Kyrill > > gcc/ChangeLog: > > 2020-04-28 Szabolcs Nagy <szabolcs.nagy@arm.com> > > PR target/94748 > * config/aarch64/aarch64-bti-insert.c (rest_of_insert_bti): Remove > the check for NOTE_INSN_DELETED_LABEL. > > gcc/testsuite/ChangeLog: > > 2020-04-28 Szabolcs Nagy <szabolcs.nagy@arm.com> > > PR target/94748 > * gcc.target/aarch64/pr94748.c: New test. > --- > gcc/config/aarch64/aarch64-bti-insert.c | 8 ++------ > gcc/testsuite/gcc.target/aarch64/pr94748.c | 10 ++++++++++ > 2 files changed, 12 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94748.c > > diff --git a/gcc/config/aarch64/aarch64-bti-insert.c > b/gcc/config/aarch64/aarch64-bti-insert.c > index aa091c308f6..57663ee23b4 100644 > --- a/gcc/config/aarch64/aarch64-bti-insert.c > +++ b/gcc/config/aarch64/aarch64-bti-insert.c > @@ -139,14 +139,10 @@ rest_of_insert_bti (void) > insn = NEXT_INSN (insn)) > { > /* If a label is marked to be preserved or can be a non-local goto > - target, it must be protected with a BTI J. The same applies to > - NOTE_INSN_DELETED_LABEL since they are basically labels that > might > - be referenced via variables or constant pool. */ > - if ((LABEL_P (insn) > + target, it must be protected with a BTI J. */ > + if (LABEL_P (insn) > && (LABEL_PRESERVE_P (insn) > || bb->flags & BB_NON_LOCAL_GOTO_TARGET)) > - || (NOTE_P (insn) > - && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL)) > { > bti_insn = gen_bti_j (); > emit_insn_after (bti_insn, insn); > diff --git a/gcc/testsuite/gcc.target/aarch64/pr94748.c > b/gcc/testsuite/gcc.target/aarch64/pr94748.c > new file mode 100644 > index 00000000000..2a2850d2fac > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/pr94748.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile } */ > + > +__attribute__ ((target("branch-protection=bti"))) > +int foo () > +{ > +label: > + return 0; > +} > + > +/* { dg-final { scan-assembler-not {hint (36|38) // bti (j|jc)} } } */ > -- > 2.17.1
diff --git a/gcc/config/aarch64/aarch64-bti-insert.c b/gcc/config/aarch64/aarch64-bti-insert.c index aa091c308f6..57663ee23b4 100644 --- a/gcc/config/aarch64/aarch64-bti-insert.c +++ b/gcc/config/aarch64/aarch64-bti-insert.c @@ -139,14 +139,10 @@ rest_of_insert_bti (void) insn = NEXT_INSN (insn)) { /* If a label is marked to be preserved or can be a non-local goto - target, it must be protected with a BTI J. The same applies to - NOTE_INSN_DELETED_LABEL since they are basically labels that might - be referenced via variables or constant pool. */ - if ((LABEL_P (insn) + target, it must be protected with a BTI J. */ + if (LABEL_P (insn) && (LABEL_PRESERVE_P (insn) || bb->flags & BB_NON_LOCAL_GOTO_TARGET)) - || (NOTE_P (insn) - && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL)) { bti_insn = gen_bti_j (); emit_insn_after (bti_insn, insn); diff --git a/gcc/testsuite/gcc.target/aarch64/pr94748.c b/gcc/testsuite/gcc.target/aarch64/pr94748.c new file mode 100644 index 00000000000..2a2850d2fac --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/pr94748.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ + +__attribute__ ((target("branch-protection=bti"))) +int foo () +{ +label: + return 0; +} + +/* { dg-final { scan-assembler-not {hint (36|38) // bti (j|jc)} } } */
It was previously discussed that indirect branches cannot go to NOTE_INSN_DELETED_LABEL so inserting a landing pad is unnecessary. See https://gcc.gnu.org/pipermail/gcc-patches/2019-May/522625.html Before the patch a bti j was inserted after the label in __attribute__((target("branch-protection=bti"))) int foo (void) { label: return 0; } This is not necessary and weakens the security protection. gcc/ChangeLog: 2020-04-28 Szabolcs Nagy <szabolcs.nagy@arm.com> PR target/94748 * config/aarch64/aarch64-bti-insert.c (rest_of_insert_bti): Remove the check for NOTE_INSN_DELETED_LABEL. gcc/testsuite/ChangeLog: 2020-04-28 Szabolcs Nagy <szabolcs.nagy@arm.com> PR target/94748 * gcc.target/aarch64/pr94748.c: New test. --- gcc/config/aarch64/aarch64-bti-insert.c | 8 ++------ gcc/testsuite/gcc.target/aarch64/pr94748.c | 10 ++++++++++ 2 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94748.c