* [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
@ 2019-02-15 4:13 H.J. Lu
2019-02-16 15:02 ` V2 " H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2019-02-15 4:13 UTC (permalink / raw)
To: gcc-patches; +Cc: Uros Bizjak, Jan Hubicka
NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
but was not used for other purposes than taking its address and was
transformed to mark that no code jumps to it. NOTE_INSN_DELETED_LABEL
is generated only in 3 places:
1. When delete_insn sees an unused label which is an explicit label in
the input source code or its address is taken, it turns the label into
a NOTE_INSN_DELETED_LABEL note.
2. When rtl_tidy_fallthru_edge deletes a tablejump, it turns the
tablejump into a NOTE_INSN_DELETED_LABEL note.
3. ix86_init_large_pic_reg creats a NOTE_INSN_DELETED_LABEL note, .L2,
to initialize large model PIC register:
L2:
movabsq $_GLOBAL_OFFSET_TABLE_-.L2, %r11
leaq .L2(%rip), %rax
movabsq $val@GOT, %rdx
addq %r11, %rax
Among of them, ENDBR is needed only when the label address is taken.
rest_of_insert_endbranch has
if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
|| (NOTE_P (insn)
&& NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
/* TODO. Check /s bit also. */
{
cet_eb = gen_nop_endbr ();
emit_insn_after (cet_eb, insn);
continue;
}
For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
if its address is taken. Also ix86_init_large_pic_reg shouldn't set
LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
to keep the label.
gcc/
PR target/89355
* config/i386/i386.c (rest_of_insert_endbranch): Check
forced_labels to see if the address of NOTE_INSN_DELETED_LABEL
is taken.
(ix86_init_large_pic_reg): Don't set LABEL_PRESERVE_P.
gcc/testsuite/
PR target/89355
* gcc.target/i386/cet-label-3.c: New test.
* gcc.target/i386/cet-label-4.c: Likewise.
* gcc.target/i386/cet-label-5.c: Likewise.
---
gcc/config/i386/i386.c | 9 +++++---
gcc/testsuite/gcc.target/i386/cet-label-3.c | 23 +++++++++++++++++++++
gcc/testsuite/gcc.target/i386/cet-label-4.c | 12 +++++++++++
gcc/testsuite/gcc.target/i386/cet-label-5.c | 13 ++++++++++++
4 files changed, 54 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-3.c
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-4.c
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-5.c
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index fd05873ba39..ed53fbea9ae 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2734,10 +2734,14 @@ rest_of_insert_endbranch (void)
continue;
}
+ /* NB: Add an ENDBR for NOTE_INSN_DELETED_LABEL only if its
+ addresss is taken. */
if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
|| (NOTE_P (insn)
- && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
- /* TODO. Check /s bit also. */
+ && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL
+ && vec_safe_contains<rtx_insn *>
+ (forced_labels,
+ static_cast<rtx_code_label *> (insn))))
{
cet_eb = gen_nop_endbr ();
emit_insn_after (cet_eb, insn);
@@ -7002,7 +7006,6 @@ ix86_init_large_pic_reg (unsigned int tmp_regno)
gcc_assert (Pmode == DImode);
label = gen_label_rtx ();
emit_label (label);
- LABEL_PRESERVE_P (label) = 1;
tmp_reg = gen_rtx_REG (Pmode, tmp_regno);
gcc_assert (REGNO (pic_offset_table_rtx) != tmp_regno);
emit_insn (gen_set_rip_rex64 (pic_offset_table_rtx,
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-3.c b/gcc/testsuite/gcc.target/i386/cet-label-3.c
new file mode 100644
index 00000000000..9f427a866f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-3.c
@@ -0,0 +1,23 @@
+/* PR target/89355 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection" } */
+/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } } } } */
+int
+test (int* val)
+{
+ int status = 99;
+
+ if (!val)
+ {
+ status = 22;
+ goto end;
+ }
+
+ extern int x;
+ *val = x;
+
+ status = 0;
+end:
+ return status;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-4.c b/gcc/testsuite/gcc.target/i386/cet-label-4.c
new file mode 100644
index 00000000000..d743d2bf202
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-4.c
@@ -0,0 +1,12 @@
+/* PR target/89355 */
+/* { dg-do compile { target { fpic && lp64 } } } */
+/* { dg-options "-O2 -fcf-protection -fPIC -mcmodel=large" } */
+/* { dg-final { scan-assembler-times "endbr64" 1 } } */
+
+extern int val;
+
+int
+test (void)
+{
+ return val;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-5.c b/gcc/testsuite/gcc.target/i386/cet-label-5.c
new file mode 100644
index 00000000000..4d5ca816598
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-5.c
@@ -0,0 +1,13 @@
+/* PR target/89355 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -Wno-return-local-addr" } */
+/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */
+
+void *
+func (void)
+{
+ return &&bar;
+bar:
+ return 0;
+}
--
2.20.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
2019-02-15 4:13 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed H.J. Lu
@ 2019-02-16 15:02 ` H.J. Lu
2019-05-21 21:48 ` PING^1: " H.J. Lu
2019-05-28 9:22 ` Jakub Jelinek
0 siblings, 2 replies; 13+ messages in thread
From: H.J. Lu @ 2019-02-16 15:02 UTC (permalink / raw)
To: gcc-patches, Uros Bizjak, Jan Hubicka; +Cc: 刘袋鼠
On Thu, Feb 14, 2019 at 08:13:32PM -0800, H.J. Lu wrote:
> NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
> but was not used for other purposes than taking its address and was
> transformed to mark that no code jumps to it. NOTE_INSN_DELETED_LABEL
> is generated only in 3 places:
>
> 1. When delete_insn sees an unused label which is an explicit label in
> the input source code or its address is taken, it turns the label into
> a NOTE_INSN_DELETED_LABEL note.
> 2. When rtl_tidy_fallthru_edge deletes a tablejump, it turns the
> tablejump into a NOTE_INSN_DELETED_LABEL note.
> 3. ix86_init_large_pic_reg creats a NOTE_INSN_DELETED_LABEL note, .L2,
> to initialize large model PIC register:
>
> L2:
> movabsq $_GLOBAL_OFFSET_TABLE_-.L2, %r11
> leaq .L2(%rip), %rax
> movabsq $val@GOT, %rdx
> addq %r11, %rax
>
> Among of them, ENDBR is needed only when the label address is taken.
> rest_of_insert_endbranch has
>
> if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
> || (NOTE_P (insn)
> && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
> /* TODO. Check /s bit also. */
> {
> cet_eb = gen_nop_endbr ();
> emit_insn_after (cet_eb, insn);
> continue;
> }
>
> For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> if its address is taken. Also ix86_init_large_pic_reg shouldn't set
> LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> to keep the label.
>
> gcc/
>
> PR target/89355
> * config/i386/i386.c (rest_of_insert_endbranch): Check
> forced_labels to see if the address of NOTE_INSN_DELETED_LABEL
> is taken.
> (ix86_init_large_pic_reg): Don't set LABEL_PRESERVE_P.
>
Here is the updated patch. We should check LABEL_PRESERVE_P on
NOTE_INSN_DELETED_LABEL to see if its address is taken.
OK for trunk?
Thanks.
H.J.
---
NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
but was not used for other purposes than taking its address and was
transformed to mark that no code jumps to it. Since LABEL_PRESERVE_P is
true only if the label address was taken, check LABEL_PRESERVE_P before
inserting ENDBR.
2019-02-15 H.J. Lu <hongjiu.lu@intel.com>
Hongtao Liu <hongtao.liu@intel.com>
gcc/
PR target/89355
* config/i386/i386.c (rest_of_insert_endbranch): LABEL_PRESERVE_P
to see if the address of NOTE_INSN_DELETED_LABEL is taken.
(ix86_init_large_pic_reg): Don't set LABEL_PRESERVE_P.
gcc/testsuite/
PR target/89355
* gcc.target/i386/cet-label-3.c: New test.
* gcc.target/i386/cet-label-4.c: Likewise.
* gcc.target/i386/cet-label-5.c: Likewise.
---
gcc/config/i386/i386.c | 9 ++++----
gcc/testsuite/gcc.target/i386/cet-label-3.c | 23 +++++++++++++++++++++
gcc/testsuite/gcc.target/i386/cet-label-4.c | 12 +++++++++++
gcc/testsuite/gcc.target/i386/cet-label-5.c | 13 ++++++++++++
4 files changed, 52 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-3.c
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-4.c
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-5.c
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 609273e4fc4..acdc789b834 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2734,10 +2734,10 @@ rest_of_insert_endbranch (void)
continue;
}
- if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
- || (NOTE_P (insn)
- && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
- /* TODO. Check /s bit also. */
+ if ((LABEL_P (insn)
+ || (NOTE_P (insn)
+ && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
+ && LABEL_PRESERVE_P (insn))
{
cet_eb = gen_nop_endbr ();
emit_insn_after (cet_eb, insn);
@@ -6997,7 +6997,6 @@ ix86_init_large_pic_reg (unsigned int tmp_regno)
gcc_assert (Pmode == DImode);
label = gen_label_rtx ();
emit_label (label);
- LABEL_PRESERVE_P (label) = 1;
tmp_reg = gen_rtx_REG (Pmode, tmp_regno);
gcc_assert (REGNO (pic_offset_table_rtx) != tmp_regno);
emit_insn (gen_set_rip_rex64 (pic_offset_table_rtx,
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-3.c b/gcc/testsuite/gcc.target/i386/cet-label-3.c
new file mode 100644
index 00000000000..9f427a866f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-3.c
@@ -0,0 +1,23 @@
+/* PR target/89355 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection" } */
+/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } } } } */
+int
+test (int* val)
+{
+ int status = 99;
+
+ if (!val)
+ {
+ status = 22;
+ goto end;
+ }
+
+ extern int x;
+ *val = x;
+
+ status = 0;
+end:
+ return status;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-4.c b/gcc/testsuite/gcc.target/i386/cet-label-4.c
new file mode 100644
index 00000000000..d743d2bf202
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-4.c
@@ -0,0 +1,12 @@
+/* PR target/89355 */
+/* { dg-do compile { target { fpic && lp64 } } } */
+/* { dg-options "-O2 -fcf-protection -fPIC -mcmodel=large" } */
+/* { dg-final { scan-assembler-times "endbr64" 1 } } */
+
+extern int val;
+
+int
+test (void)
+{
+ return val;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-5.c b/gcc/testsuite/gcc.target/i386/cet-label-5.c
new file mode 100644
index 00000000000..4d5ca816598
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-5.c
@@ -0,0 +1,13 @@
+/* PR target/89355 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -Wno-return-local-addr" } */
+/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */
+
+void *
+func (void)
+{
+ return &&bar;
+bar:
+ return 0;
+}
--
2.20.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* PING^1: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
2019-02-16 15:02 ` V2 " H.J. Lu
@ 2019-05-21 21:48 ` H.J. Lu
2019-05-28 7:21 ` Uros Bizjak
2019-05-28 9:22 ` Jakub Jelinek
1 sibling, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2019-05-21 21:48 UTC (permalink / raw)
To: GCC Patches, Uros Bizjak, Jan Hubicka
Cc: 刘袋鼠, Jeffrey Law
On Sat, Feb 16, 2019 at 7:02 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Feb 14, 2019 at 08:13:32PM -0800, H.J. Lu wrote:
> > NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
> > but was not used for other purposes than taking its address and was
> > transformed to mark that no code jumps to it. NOTE_INSN_DELETED_LABEL
> > is generated only in 3 places:
> >
> > 1. When delete_insn sees an unused label which is an explicit label in
> > the input source code or its address is taken, it turns the label into
> > a NOTE_INSN_DELETED_LABEL note.
> > 2. When rtl_tidy_fallthru_edge deletes a tablejump, it turns the
> > tablejump into a NOTE_INSN_DELETED_LABEL note.
> > 3. ix86_init_large_pic_reg creats a NOTE_INSN_DELETED_LABEL note, .L2,
> > to initialize large model PIC register:
> >
> > L2:
> > movabsq $_GLOBAL_OFFSET_TABLE_-.L2, %r11
> > leaq .L2(%rip), %rax
> > movabsq $val@GOT, %rdx
> > addq %r11, %rax
> >
> > Among of them, ENDBR is needed only when the label address is taken.
> > rest_of_insert_endbranch has
> >
> > if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
> > || (NOTE_P (insn)
> > && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
> > /* TODO. Check /s bit also. */
> > {
> > cet_eb = gen_nop_endbr ();
> > emit_insn_after (cet_eb, insn);
> > continue;
> > }
> >
> > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > if its address is taken. Also ix86_init_large_pic_reg shouldn't set
> > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> > to keep the label.
> >
> > gcc/
> >
> > PR target/89355
> > * config/i386/i386.c (rest_of_insert_endbranch): Check
> > forced_labels to see if the address of NOTE_INSN_DELETED_LABEL
> > is taken.
> > (ix86_init_large_pic_reg): Don't set LABEL_PRESERVE_P.
> >
>
> Here is the updated patch. We should check LABEL_PRESERVE_P on
> NOTE_INSN_DELETED_LABEL to see if its address is taken.
>
> OK for trunk?
>
> Thanks.
>
> H.J.
> ---
> NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
> but was not used for other purposes than taking its address and was
> transformed to mark that no code jumps to it. Since LABEL_PRESERVE_P is
> true only if the label address was taken, check LABEL_PRESERVE_P before
> inserting ENDBR.
>
> 2019-02-15 H.J. Lu <hongjiu.lu@intel.com>
> Hongtao Liu <hongtao.liu@intel.com>
>
> gcc/
>
> PR target/89355
> * config/i386/i386.c (rest_of_insert_endbranch): LABEL_PRESERVE_P
> to see if the address of NOTE_INSN_DELETED_LABEL is taken.
> (ix86_init_large_pic_reg): Don't set LABEL_PRESERVE_P.
>
> gcc/testsuite/
>
> PR target/89355
> * gcc.target/i386/cet-label-3.c: New test.
> * gcc.target/i386/cet-label-4.c: Likewise.
> * gcc.target/i386/cet-label-5.c: Likewise.
PING:
https://gcc.gnu.org/ml/gcc-patches/2019-02/msg01327.html
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PING^1: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
2019-05-21 21:48 ` PING^1: " H.J. Lu
@ 2019-05-28 7:21 ` Uros Bizjak
0 siblings, 0 replies; 13+ messages in thread
From: Uros Bizjak @ 2019-05-28 7:21 UTC (permalink / raw)
To: H.J. Lu; +Cc: GCC Patches, Jan Hubicka, 刘袋鼠, Jeffrey Law
On Tue, May 21, 2019 at 11:48 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Feb 16, 2019 at 7:02 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Thu, Feb 14, 2019 at 08:13:32PM -0800, H.J. Lu wrote:
> > > NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
> > > but was not used for other purposes than taking its address and was
> > > transformed to mark that no code jumps to it. NOTE_INSN_DELETED_LABEL
> > > is generated only in 3 places:
> > >
> > > 1. When delete_insn sees an unused label which is an explicit label in
> > > the input source code or its address is taken, it turns the label into
> > > a NOTE_INSN_DELETED_LABEL note.
> > > 2. When rtl_tidy_fallthru_edge deletes a tablejump, it turns the
> > > tablejump into a NOTE_INSN_DELETED_LABEL note.
> > > 3. ix86_init_large_pic_reg creats a NOTE_INSN_DELETED_LABEL note, .L2,
> > > to initialize large model PIC register:
> > >
> > > L2:
> > > movabsq $_GLOBAL_OFFSET_TABLE_-.L2, %r11
> > > leaq .L2(%rip), %rax
> > > movabsq $val@GOT, %rdx
> > > addq %r11, %rax
> > >
> > > Among of them, ENDBR is needed only when the label address is taken.
> > > rest_of_insert_endbranch has
> > >
> > > if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
> > > || (NOTE_P (insn)
> > > && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
> > > /* TODO. Check /s bit also. */
> > > {
> > > cet_eb = gen_nop_endbr ();
> > > emit_insn_after (cet_eb, insn);
> > > continue;
> > > }
> > >
> > > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > > if its address is taken. Also ix86_init_large_pic_reg shouldn't set
> > > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> > > to keep the label.
> > >
> > > gcc/
> > >
> > > PR target/89355
> > > * config/i386/i386.c (rest_of_insert_endbranch): Check
> > > forced_labels to see if the address of NOTE_INSN_DELETED_LABEL
> > > is taken.
> > > (ix86_init_large_pic_reg): Don't set LABEL_PRESERVE_P.
> > >
> >
> > Here is the updated patch. We should check LABEL_PRESERVE_P on
> > NOTE_INSN_DELETED_LABEL to see if its address is taken.
> >
> > OK for trunk?
> >
> > Thanks.
> >
> > H.J.
> > ---
> > NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
> > but was not used for other purposes than taking its address and was
> > transformed to mark that no code jumps to it. Since LABEL_PRESERVE_P is
> > true only if the label address was taken, check LABEL_PRESERVE_P before
> > inserting ENDBR.
> >
> > 2019-02-15 H.J. Lu <hongjiu.lu@intel.com>
> > Hongtao Liu <hongtao.liu@intel.com>
> >
> > gcc/
> >
> > PR target/89355
> > * config/i386/i386.c (rest_of_insert_endbranch): LABEL_PRESERVE_P
> > to see if the address of NOTE_INSN_DELETED_LABEL is taken.
> > (ix86_init_large_pic_reg): Don't set LABEL_PRESERVE_P.
> >
> > gcc/testsuite/
> >
> > PR target/89355
> > * gcc.target/i386/cet-label-3.c: New test.
> > * gcc.target/i386/cet-label-4.c: Likewise.
> > * gcc.target/i386/cet-label-5.c: Likewise.
>
> PING:
>
> https://gcc.gnu.org/ml/gcc-patches/2019-02/msg01327.html
This patch goes deep into RTL details, so IMO RTL expert (Jeff ?)
should approve it.
Uros.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
2019-02-16 15:02 ` V2 " H.J. Lu
2019-05-21 21:48 ` PING^1: " H.J. Lu
@ 2019-05-28 9:22 ` Jakub Jelinek
2019-05-28 15:16 ` H.J. Lu
1 sibling, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2019-05-28 9:22 UTC (permalink / raw)
To: H.J. Lu; +Cc: gcc-patches, Uros Bizjak, Jan Hubicka, 刘袋鼠
On Sat, Feb 16, 2019 at 07:02:11AM -0800, H.J. Lu wrote:
> > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > if its address is taken. Also ix86_init_large_pic_reg shouldn't set
> > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> > to keep the label.
Can you explain when is it ever needed to emit ENDBR on
NOTE_INSN_DELETED_LABEL? Only labels that are proven not to be ever
branched to are turned into deleted labels, so I think the right answer is
never emit any ENDBR on those...
Jakub
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
2019-05-28 9:22 ` Jakub Jelinek
@ 2019-05-28 15:16 ` H.J. Lu
2019-05-28 15:31 ` Jakub Jelinek
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2019-05-28 15:16 UTC (permalink / raw)
To: Jakub Jelinek
Cc: GCC Patches, Uros Bizjak, Jan Hubicka, 刘袋鼠
On Tue, May 28, 2019 at 1:57 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Sat, Feb 16, 2019 at 07:02:11AM -0800, H.J. Lu wrote:
> > > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > > if its address is taken. Also ix86_init_large_pic_reg shouldn't set
> > > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> > > to keep the label.
>
> Can you explain when is it ever needed to emit ENDBR on
> NOTE_INSN_DELETED_LABEL? Only labels that are proven not to be ever
> branched to are turned into deleted labels, so I think the right answer is
> never emit any ENDBR on those...
>
This condition is checked by gcc.target/i386/cet-label-5.c in the
patch. For
void *
func (void)
{
return &&bar;
bar:
return 0;
}
we generate:
(note/s 4 2 10 2 ("bar") NOTE_INSN_DELETED_LABEL 2)
(insn:TI 10 4 11 2 (set (reg/i:DI 0 ax)
(label_ref:DI [4 deleted]))
"/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
66 {*movdi_internal}
(nil))
(insn 11 10 20 2 (use (reg/i:DI 0 ax))
"/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
-1
(nil))
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
2019-05-28 15:16 ` H.J. Lu
@ 2019-05-28 15:31 ` Jakub Jelinek
2019-05-28 15:34 ` H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2019-05-28 15:31 UTC (permalink / raw)
To: H.J. Lu; +Cc: GCC Patches, Uros Bizjak, Jan Hubicka, 刘袋鼠
On Tue, May 28, 2019 at 08:10:19AM -0700, H.J. Lu wrote:
> On Tue, May 28, 2019 at 1:57 AM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Sat, Feb 16, 2019 at 07:02:11AM -0800, H.J. Lu wrote:
> > > > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > > > if its address is taken. Also ix86_init_large_pic_reg shouldn't set
> > > > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> > > > to keep the label.
> >
> > Can you explain when is it ever needed to emit ENDBR on
> > NOTE_INSN_DELETED_LABEL? Only labels that are proven not to be ever
> > branched to are turned into deleted labels, so I think the right answer is
> > never emit any ENDBR on those...
> >
>
> This condition is checked by gcc.target/i386/cet-label-5.c in the
> patch. For
>
> void *
> func (void)
> {
> return &&bar;
> bar:
> return 0;
> }
>
> we generate:
>
> (note/s 4 2 10 2 ("bar") NOTE_INSN_DELETED_LABEL 2)
> (insn:TI 10 4 11 2 (set (reg/i:DI 0 ax)
> (label_ref:DI [4 deleted]))
> "/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
> 66 {*movdi_internal}
> (nil))
> (insn 11 10 20 2 (use (reg/i:DI 0 ax))
> "/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
> -1
> (nil))
We shouldn't generate ENDBR in that case, nothing can goto to bar (otherwise
it would remain a normal label, not a deleted label).
Jakub
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
2019-05-28 15:31 ` Jakub Jelinek
@ 2019-05-28 15:34 ` H.J. Lu
2019-05-28 16:11 ` Jakub Jelinek
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2019-05-28 15:34 UTC (permalink / raw)
To: Jakub Jelinek
Cc: GCC Patches, Uros Bizjak, Jan Hubicka, 刘袋鼠
On Tue, May 28, 2019 at 8:16 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, May 28, 2019 at 08:10:19AM -0700, H.J. Lu wrote:
> > On Tue, May 28, 2019 at 1:57 AM Jakub Jelinek <jakub@redhat.com> wrote:
> > >
> > > On Sat, Feb 16, 2019 at 07:02:11AM -0800, H.J. Lu wrote:
> > > > > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > > > > if its address is taken. Also ix86_init_large_pic_reg shouldn't set
> > > > > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> > > > > to keep the label.
> > >
> > > Can you explain when is it ever needed to emit ENDBR on
> > > NOTE_INSN_DELETED_LABEL? Only labels that are proven not to be ever
> > > branched to are turned into deleted labels, so I think the right answer is
> > > never emit any ENDBR on those...
> > >
> >
> > This condition is checked by gcc.target/i386/cet-label-5.c in the
> > patch. For
> >
> > void *
> > func (void)
> > {
> > return &&bar;
> > bar:
> > return 0;
> > }
> >
> > we generate:
> >
> > (note/s 4 2 10 2 ("bar") NOTE_INSN_DELETED_LABEL 2)
> > (insn:TI 10 4 11 2 (set (reg/i:DI 0 ax)
> > (label_ref:DI [4 deleted]))
> > "/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
> > 66 {*movdi_internal}
> > (nil))
> > (insn 11 10 20 2 (use (reg/i:DI 0 ax))
> > "/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
> > -1
> > (nil))
>
> We shouldn't generate ENDBR in that case, nothing can goto to bar (otherwise
> it would remain a normal label, not a deleted label).
>
But return value of func () may be used with indirect jump.
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
2019-05-28 15:34 ` H.J. Lu
@ 2019-05-28 16:11 ` Jakub Jelinek
2019-05-28 17:37 ` Jeff Law
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2019-05-28 16:11 UTC (permalink / raw)
To: H.J. Lu; +Cc: GCC Patches, Uros Bizjak, Jan Hubicka, 刘袋鼠
On Tue, May 28, 2019 at 08:30:59AM -0700, H.J. Lu wrote:
> > We shouldn't generate ENDBR in that case, nothing can goto to bar (otherwise
> > it would remain a normal label, not a deleted label).
> >
>
> But return value of func () may be used with indirect jump.
No, it may be used say to print that address, but computed goto can't be
used to jump from one function to a different function, see
https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
"You may not use this mechanism to jump to code in a different function.
If you do that, totally unpredictable things happen."
NOTE_INSN_DELETED_LABEL is not guaranteed to be followed by any sensible
code, the only reason it is kept is that there is or might be something
referencing the label and so you want to emit the label somewhere in the
function, but don't care much where in the function.
Let's look at weirdo testcases where we warn about by default with
-Wreturn-local-addr, but still the indirect jump is within the same
function:
void *
foo (void *x)
{
if (!x)
{
return &&bar;
bar:
return 0;
}
goto *x;
}
void *
qux (void *x, int y)
{
if (y == 1)
{
return &&bar;
bar:
return (void *) 5;
}
else if (y == 2)
{
return &&baz;
baz:
return (void *) 6;
}
goto *x;
}
if called with foo (foo (0)) or qux (qux (0, 1)) or qux (qux (0, 2)).
In foo we emit NOTE_INSN_DELETED_LABEL, but just return that and optimize
away the actual goto *x, because we can prove what it will do. In qux
we can't and so there is no NOTE_INSN_DELETED_LABEL.
Jakub
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed
2019-05-28 16:11 ` Jakub Jelinek
@ 2019-05-28 17:37 ` Jeff Law
2019-05-31 17:39 ` [PATCH] Don't insert ENDBR after NOTE_INSN_DELETED_LABEL H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Law @ 2019-05-28 17:37 UTC (permalink / raw)
To: Jakub Jelinek, H.J. Lu
Cc: GCC Patches, Uros Bizjak, Jan Hubicka, 刘袋鼠
On 5/28/19 9:48 AM, Jakub Jelinek wrote:
> On Tue, May 28, 2019 at 08:30:59AM -0700, H.J. Lu wrote:
>>> We shouldn't generate ENDBR in that case, nothing can goto to bar (otherwise
>>> it would remain a normal label, not a deleted label).
>>>
>>
>> But return value of func () may be used with indirect jump.
>
> No, it may be used say to print that address, but computed goto can't be
> used to jump from one function to a different function, see
> https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
> "You may not use this mechanism to jump to code in a different function.
> If you do that, totally unpredictable things happen."
Right. We disallowed this case some time ago IIRC. It's essentially
undefined behavior. I would even claim that in a CET world that we
*want* a CET fault if something tried to use the deleted label as a jump
target and thus an ENDBR is undesirable.
>
> NOTE_INSN_DELETED_LABEL is not guaranteed to be followed by any sensible
> code, the only reason it is kept is that there is or might be something
> referencing the label and so you want to emit the label somewhere in the
> function, but don't care much where in the function.
Exactly.
Jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] Don't insert ENDBR after NOTE_INSN_DELETED_LABEL
2019-05-28 17:37 ` Jeff Law
@ 2019-05-31 17:39 ` H.J. Lu
2019-05-31 18:03 ` Jakub Jelinek
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2019-05-31 17:39 UTC (permalink / raw)
To: Jeff Law
Cc: Jakub Jelinek, GCC Patches, Uros Bizjak, Jan Hubicka,
刘袋鼠
[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]
On Tue, May 28, 2019 at 10:33 AM Jeff Law <law@redhat.com> wrote:
>
> On 5/28/19 9:48 AM, Jakub Jelinek wrote:
> > On Tue, May 28, 2019 at 08:30:59AM -0700, H.J. Lu wrote:
> >>> We shouldn't generate ENDBR in that case, nothing can goto to bar (otherwise
> >>> it would remain a normal label, not a deleted label).
> >>>
> >>
> >> But return value of func () may be used with indirect jump.
> >
> > No, it may be used say to print that address, but computed goto can't be
> > used to jump from one function to a different function, see
> > https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
> > "You may not use this mechanism to jump to code in a different function.
> > If you do that, totally unpredictable things happen."
> Right. We disallowed this case some time ago IIRC. It's essentially
> undefined behavior. I would even claim that in a CET world that we
> *want* a CET fault if something tried to use the deleted label as a jump
> target and thus an ENDBR is undesirable.
>
>
> >
> > NOTE_INSN_DELETED_LABEL is not guaranteed to be followed by any sensible
> > code, the only reason it is kept is that there is or might be something
> > referencing the label and so you want to emit the label somewhere in the
> > function, but don't care much where in the function.
> Exactly.
>
> Jeff
Here is the updated patch not to insert ENDBR after NOTE_INSN_DELETED_LABEL.
Tested on Linux/x86-64 with -fcf-protection.
OK for trunk?
Thanks.
--
H.J.
[-- Attachment #2: 0001-i386-Don-t-insert-ENDBR-after-NOTE_INSN_DELETED_LABE.patch --]
[-- Type: text/x-patch, Size: 4534 bytes --]
From 221484c1c6871d6fa93d4338df52d03d76a436dc Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 14 Feb 2019 09:12:57 -0800
Subject: [PATCH] i386: Don't insert ENDBR after NOTE_INSN_DELETED_LABEL
NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
but was not used for other purposes than taking its address which cannot
be used as target for indirect jumps.
Since LABEL_PRESERVE_P is true only if the label address was taken, don't
set LABEL_PRESERVE_P on label used to initialize PIC register.
Tested on Linux/x86-64 with -fcf-protection.
For x86-64 libc.so on glibc master branch (commit f43b8dd55588c3),
Before: 2961 endbr64
After: 2943 endbr64
2019-05-31 H.J. Lu <hongjiu.lu@intel.com>
Hongtao Liu <hongtao.liu@intel.com>
gcc/
PR target/89355
* config/i386/i386-features.c (rest_of_insert_endbranch): Remove
NOTE_INSN_DELETED_LABEL check.
* config/i386/i386.c (ix86_init_large_pic_reg): Don't set
LABEL_PRESERVE_P.
gcc/testsuite/
PR target/89355
* gcc.target/i386/cet-label-3.c: New test.
* gcc.target/i386/cet-label-4.c: Likewise.
* gcc.target/i386/cet-label-5.c: Likewise.
---
gcc/config/i386/i386-features.c | 5 +----
gcc/config/i386/i386.c | 1 -
gcc/testsuite/gcc.target/i386/cet-label-3.c | 23 +++++++++++++++++++++
gcc/testsuite/gcc.target/i386/cet-label-4.c | 12 +++++++++++
gcc/testsuite/gcc.target/i386/cet-label-5.c | 13 ++++++++++++
5 files changed, 49 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-3.c
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-4.c
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-5.c
diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index 60a120f4df7..c8de5261240 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1911,10 +1911,7 @@ rest_of_insert_endbranch (void)
continue;
}
- if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
- || (NOTE_P (insn)
- && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
- /* TODO. Check /s bit also. */
+ if (LABEL_P (insn) && LABEL_PRESERVE_P (insn))
{
cet_eb = gen_nop_endbr ();
emit_insn_after (cet_eb, insn);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 79fcb5c4e57..bb4df4760b6 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1651,7 +1651,6 @@ ix86_init_large_pic_reg (unsigned int tmp_regno)
gcc_assert (Pmode == DImode);
label = gen_label_rtx ();
emit_label (label);
- LABEL_PRESERVE_P (label) = 1;
tmp_reg = gen_rtx_REG (Pmode, tmp_regno);
gcc_assert (REGNO (pic_offset_table_rtx) != tmp_regno);
emit_insn (gen_set_rip_rex64 (pic_offset_table_rtx,
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-3.c b/gcc/testsuite/gcc.target/i386/cet-label-3.c
new file mode 100644
index 00000000000..9f427a866f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-3.c
@@ -0,0 +1,23 @@
+/* PR target/89355 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection" } */
+/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } } } } */
+int
+test (int* val)
+{
+ int status = 99;
+
+ if (!val)
+ {
+ status = 22;
+ goto end;
+ }
+
+ extern int x;
+ *val = x;
+
+ status = 0;
+end:
+ return status;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-4.c b/gcc/testsuite/gcc.target/i386/cet-label-4.c
new file mode 100644
index 00000000000..d743d2bf202
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-4.c
@@ -0,0 +1,12 @@
+/* PR target/89355 */
+/* { dg-do compile { target { fpic && lp64 } } } */
+/* { dg-options "-O2 -fcf-protection -fPIC -mcmodel=large" } */
+/* { dg-final { scan-assembler-times "endbr64" 1 } } */
+
+extern int val;
+
+int
+test (void)
+{
+ return val;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-5.c b/gcc/testsuite/gcc.target/i386/cet-label-5.c
new file mode 100644
index 00000000000..862c79b058d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-5.c
@@ -0,0 +1,13 @@
+/* PR target/89355 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -Wno-return-local-addr" } */
+/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } } } } */
+
+void *
+func (void)
+{
+ return &&bar;
+bar:
+ return 0;
+}
--
2.20.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Don't insert ENDBR after NOTE_INSN_DELETED_LABEL
2019-05-31 17:39 ` [PATCH] Don't insert ENDBR after NOTE_INSN_DELETED_LABEL H.J. Lu
@ 2019-05-31 18:03 ` Jakub Jelinek
2019-05-31 18:26 ` H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2019-05-31 18:03 UTC (permalink / raw)
To: H.J. Lu
Cc: Jeff Law, GCC Patches, Uros Bizjak, Jan Hubicka,
刘袋鼠
On Fri, May 31, 2019 at 09:53:42AM -0700, H.J. Lu wrote:
> 2019-05-31 H.J. Lu <hongjiu.lu@intel.com>
> Hongtao Liu <hongtao.liu@intel.com>
>
> gcc/
>
> PR target/89355
> * config/i386/i386.c (ix86_init_large_pic_reg): Don't set
> LABEL_PRESERVE_P.
Why this change? It is both not needed and undesirable, the
large pic reg label is used magically, not as a normal label, so
LABEL_PRESERVE_P on it is desirable.
Otherwise LGTM.
Jakub
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Don't insert ENDBR after NOTE_INSN_DELETED_LABEL
2019-05-31 18:03 ` Jakub Jelinek
@ 2019-05-31 18:26 ` H.J. Lu
0 siblings, 0 replies; 13+ messages in thread
From: H.J. Lu @ 2019-05-31 18:26 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Jeff Law, GCC Patches, Uros Bizjak, Jan Hubicka,
刘袋鼠
[-- Attachment #1: Type: text/plain, Size: 646 bytes --]
On Fri, May 31, 2019 at 10:43 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, May 31, 2019 at 09:53:42AM -0700, H.J. Lu wrote:
> > 2019-05-31 H.J. Lu <hongjiu.lu@intel.com>
> > Hongtao Liu <hongtao.liu@intel.com>
> >
> > gcc/
> >
> > PR target/89355
> > * config/i386/i386.c (ix86_init_large_pic_reg): Don't set
> > LABEL_PRESERVE_P.
>
> Why this change? It is both not needed and undesirable, the
> large pic reg label is used magically, not as a normal label, so
> LABEL_PRESERVE_P on it is desirable.
Removed.
> Otherwise LGTM.
>
> Jakub
This is the patch I am checking in.
Thanks.
--
H.J.
[-- Attachment #2: 0001-i386-Don-t-insert-ENDBR-after-NOTE_INSN_DELETED_LABE.patch --]
[-- Type: text/x-patch, Size: 3761 bytes --]
From 57c4c30a5eda286800fd6c4e0101b82d8cfb604f Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 14 Feb 2019 09:12:57 -0800
Subject: [PATCH] i386: Don't insert ENDBR after NOTE_INSN_DELETED_LABEL
NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
but was not used for other purposes than taking its address which cannot
be used as target for indirect jumps.
Tested on Linux/x86-64 with -fcf-protection.
For x86-64 libc.so on glibc master branch (commit f43b8dd55588c3),
Before: 2961 endbr64
After: 2943 endbr64
2019-05-31 H.J. Lu <hongjiu.lu@intel.com>
Hongtao Liu <hongtao.liu@intel.com>
gcc/
PR target/89355
* config/i386/i386-features.c (rest_of_insert_endbranch): Remove
NOTE_INSN_DELETED_LABEL check.
gcc/testsuite/
PR target/89355
* gcc.target/i386/cet-label-3.c: New test.
* gcc.target/i386/cet-label-4.c: Likewise.
* gcc.target/i386/cet-label-5.c: Likewise.
---
gcc/config/i386/i386-features.c | 5 +----
gcc/testsuite/gcc.target/i386/cet-label-3.c | 23 +++++++++++++++++++++
gcc/testsuite/gcc.target/i386/cet-label-4.c | 12 +++++++++++
gcc/testsuite/gcc.target/i386/cet-label-5.c | 13 ++++++++++++
4 files changed, 49 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-3.c
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-4.c
create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-5.c
diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index 60a120f4df7..c8de5261240 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1911,10 +1911,7 @@ rest_of_insert_endbranch (void)
continue;
}
- if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
- || (NOTE_P (insn)
- && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
- /* TODO. Check /s bit also. */
+ if (LABEL_P (insn) && LABEL_PRESERVE_P (insn))
{
cet_eb = gen_nop_endbr ();
emit_insn_after (cet_eb, insn);
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-3.c b/gcc/testsuite/gcc.target/i386/cet-label-3.c
new file mode 100644
index 00000000000..9f427a866f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-3.c
@@ -0,0 +1,23 @@
+/* PR target/89355 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection" } */
+/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } } } } */
+int
+test (int* val)
+{
+ int status = 99;
+
+ if (!val)
+ {
+ status = 22;
+ goto end;
+ }
+
+ extern int x;
+ *val = x;
+
+ status = 0;
+end:
+ return status;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-4.c b/gcc/testsuite/gcc.target/i386/cet-label-4.c
new file mode 100644
index 00000000000..d743d2bf202
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-4.c
@@ -0,0 +1,12 @@
+/* PR target/89355 */
+/* { dg-do compile { target { fpic && lp64 } } } */
+/* { dg-options "-O2 -fcf-protection -fPIC -mcmodel=large" } */
+/* { dg-final { scan-assembler-times "endbr64" 1 } } */
+
+extern int val;
+
+int
+test (void)
+{
+ return val;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-5.c b/gcc/testsuite/gcc.target/i386/cet-label-5.c
new file mode 100644
index 00000000000..862c79b058d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-5.c
@@ -0,0 +1,13 @@
+/* PR target/89355 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -Wno-return-local-addr" } */
+/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } } } } */
+
+void *
+func (void)
+{
+ return &&bar;
+bar:
+ return 0;
+}
--
2.20.1
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-05-31 18:03 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-15 4:13 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed H.J. Lu
2019-02-16 15:02 ` V2 " H.J. Lu
2019-05-21 21:48 ` PING^1: " H.J. Lu
2019-05-28 7:21 ` Uros Bizjak
2019-05-28 9:22 ` Jakub Jelinek
2019-05-28 15:16 ` H.J. Lu
2019-05-28 15:31 ` Jakub Jelinek
2019-05-28 15:34 ` H.J. Lu
2019-05-28 16:11 ` Jakub Jelinek
2019-05-28 17:37 ` Jeff Law
2019-05-31 17:39 ` [PATCH] Don't insert ENDBR after NOTE_INSN_DELETED_LABEL H.J. Lu
2019-05-31 18:03 ` Jakub Jelinek
2019-05-31 18:26 ` H.J. Lu
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).