* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
@ 2020-08-10 8:43 ` crazylht at gmail dot com
2020-08-10 9:04 ` ubizjak at gmail dot com
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: crazylht at gmail dot com @ 2020-08-10 8:43 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #1 from Hongtao.liu <crazylht at gmail dot com> ---
I'm testing patch like
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b24a4557871..269c528c3ad 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -19132,15 +19132,15 @@
/* Compare through substraction the saved and the current ssp to decide
if ssp has to be adjusted. */
- tmp = gen_rtx_SET (reg_ssp, gen_rtx_MINUS (word_mode, reg_ssp,
- ssp_slot));
- clob = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (CCmode, FLAGS_REG));
- tmp = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, tmp, clob));
- emit_insn (tmp);
+ emit_insn ((word_mode == SImode)
+ ? gen_subsi3 (reg_ssp, reg_ssp, ssp_slot)
+ : gen_subdi3 (reg_ssp, reg_ssp, ssp_slot));
+ tmp = gen_rtx_COMPARE (CCZmode, reg_ssp, const0_rtx);
+ flags = gen_rtx_REG (CCZmode, FLAGS_REG);
+ emit_insn (gen_rtx_SET (flags, tmp));
/* Compare and jump over adjustment code. */
noadj_label = gen_label_rtx ();
- flags = gen_rtx_REG (CCZmode, FLAGS_REG);
tmp = gen_rtx_EQ (VOIDmode, flags, const0_rtx);
tmp = gen_rtx_IF_THEN_ELSE (VOIDmode, tmp,
gen_rtx_LABEL_REF (VOIDmode, noadj_label),
@@ -19184,14 +19184,10 @@
emit_insn ((word_mode == SImode)
? gen_incsspsi (reg_255)
: gen_incsspdi (reg_255));
- tmp = gen_rtx_SET (reg_adj, gen_rtx_MINUS (ptr_mode,
- reg_adj,
- GEN_INT (255)));
- clob = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (CCmode, FLAGS_REG));
- tmp = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, tmp, clob));
- emit_insn (tmp);
-
- tmp = gen_rtx_COMPARE (CCmode, reg_adj, GEN_INT (255));
+ emit_insn ((ptr_mode == SImode)
+ ? gen_subsi3 (reg_adj, reg_adj, GEN_INT (255))
+ : gen_subdi3 (reg_adj, reg_adj, GEN_INT (255)));
+ tmp = gen_rtx_COMPARE (CCmode, reg_adj, const0_rtx);
flags = gen_rtx_REG (CCmode, FLAGS_REG);
emit_insn (gen_rtx_SET (flags, tmp));
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
2020-08-10 8:43 ` [Bug target/96536] " crazylht at gmail dot com
@ 2020-08-10 9:04 ` ubizjak at gmail dot com
2020-08-10 9:32 ` crazylht at gmail dot com
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: ubizjak at gmail dot com @ 2020-08-10 9:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #2 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Hongtao.liu from comment #1)
> I'm testing patch like
You can probably use gen_sub2_insn here.
On a related note, "@" prefix can be used for rdssp, so one can pass mode to an
expander. This would eliminate e.g. "(word_mode == SImode) ? gen_rdsspsi
(reg_ssp) : gen_rdsspdi (reg_ssp)" constructs with just one call to the
expander.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
2020-08-10 8:43 ` [Bug target/96536] " crazylht at gmail dot com
2020-08-10 9:04 ` ubizjak at gmail dot com
@ 2020-08-10 9:32 ` crazylht at gmail dot com
2020-08-13 21:06 ` ubizjak at gmail dot com
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: crazylht at gmail dot com @ 2020-08-10 9:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #3 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Uroš Bizjak from comment #2)
> (In reply to Hongtao.liu from comment #1)
> > I'm testing patch like
>
> You can probably use gen_sub2_insn here.
>
> On a related note, "@" prefix can be used for rdssp, so one can pass mode to
> an expander. This would eliminate e.g. "(word_mode == SImode) ? gen_rdsspsi
> (reg_ssp) : gen_rdsspdi (reg_ssp)" constructs with just one call to the
> expander.
Yes, thanks for pointing out.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
` (2 preceding siblings ...)
2020-08-10 9:32 ` crazylht at gmail dot com
@ 2020-08-13 21:06 ` ubizjak at gmail dot com
2020-08-14 6:33 ` crazylht at gmail dot com
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: ubizjak at gmail dot com @ 2020-08-13 21:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
Uroš Bizjak <ubizjak at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |ubizjak at gmail dot com
Last reconfirmed| |2020-08-13
Status|UNCONFIRMED |ASSIGNED
Ever confirmed|0 |1
--- Comment #4 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 49060
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49060&action=edit
Proposed patch
Attached patch completely rewrites restore_stack_nonlocal expander.
Can someone please test the patch on a CET enabled target?
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
` (3 preceding siblings ...)
2020-08-13 21:06 ` ubizjak at gmail dot com
@ 2020-08-14 6:33 ` crazylht at gmail dot com
2020-08-17 14:39 ` ubizjak at gmail dot com
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: crazylht at gmail dot com @ 2020-08-14 6:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #5 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Uroš Bizjak from comment #4)
> Created attachment 49060 [details]
> Proposed patch
>
> Attached patch completely rewrites restore_stack_nonlocal expander.
>
> Can someone please test the patch on a CET enabled target?
I can help with this, we have CET enabled tigerlake.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
` (4 preceding siblings ...)
2020-08-14 6:33 ` crazylht at gmail dot com
@ 2020-08-17 14:39 ` ubizjak at gmail dot com
2020-08-18 8:54 ` crazylht at gmail dot com
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: ubizjak at gmail dot com @ 2020-08-17 14:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #6 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Hongtao.liu from comment #1)
> I'm testing patch like
>
> emit_insn ((word_mode == SImode)
> ? gen_incsspsi (reg_255)
> : gen_incsspdi (reg_255));
> - tmp = gen_rtx_SET (reg_adj, gen_rtx_MINUS (ptr_mode,
> - reg_adj,
> - GEN_INT (255)));
> - clob = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (CCmode, FLAGS_REG));
> - tmp = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, tmp, clob));
> - emit_insn (tmp);
> -
> - tmp = gen_rtx_COMPARE (CCmode, reg_adj, GEN_INT (255));
> + emit_insn ((ptr_mode == SImode)
> + ? gen_subsi3 (reg_adj, reg_adj, GEN_INT (255))
> + : gen_subdi3 (reg_adj, reg_adj, GEN_INT (255)));
> + tmp = gen_rtx_COMPARE (CCmode, reg_adj, const0_rtx);
> flags = gen_rtx_REG (CCmode, FLAGS_REG);
> emit_insn (gen_rtx_SET (flags, tmp));
The above part is not correct. The original code compares result with 255, your
patch compares result with 0.
So, the minimum patch (for backport) should just introduce:
--cut here--
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 292de142e90..6c207be3512 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -18695,6 +18695,10 @@
tmp = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, tmp, clob));
emit_insn (tmp);
+ tmp = gen_rtx_COMPARE (CCZmode, reg_ssp, const0_rtx);
+ flags = gen_rtx_REG (CCZmode, FLAGS_REG);
+ emit_insn (gen_rtx_SET (flags, tmp));
+
/* Compare and jump over adjustment code. */
noadj_label = gen_label_rtx ();
flags = gen_rtx_REG (CCZmode, FLAGS_REG);
--cut here--
The patch creates correct form of sub insn (tested with cet-sjlj-1.c testcase):
#(insn 15 14 16 2 (parallel [
# (set (reg:CCZ 17 flags)
# (compare:CCZ (minus:DI (reg:DI 0 ax [85])
# (mem:DI (const:DI (plus:DI (symbol_ref:DI ("buf")
[flags 0x2] <var_decl 0x7f41c308cb40 buf>)
# (const_int 16 [0x10]))) [2 S8 A8]))
# (const_int 0 [0])))
# (set (reg:DI 0 ax [85])
# (minus:DI (reg:DI 0 ax [85])
# (mem:DI (const:DI (plus:DI (symbol_ref:DI ("buf") [flags
0x2] <var_decl 0x7f41c308cb40 buf>)
# (const_int 16 [0x10]))) [2 S8 A8])))
# ]) "cet-sjlj-1.c":16:3 262 {*subdi_2}
# (nil))
subq buf+16(%rip), %rax # 15 [c=8 l=7] *subdi_2/1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
` (5 preceding siblings ...)
2020-08-17 14:39 ` ubizjak at gmail dot com
@ 2020-08-18 8:54 ` crazylht at gmail dot com
2020-08-18 15:32 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: crazylht at gmail dot com @ 2020-08-18 8:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #7 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Hongtao.liu from comment #5)
> (In reply to Uroš Bizjak from comment #4)
> > Created attachment 49060 [details]
> > Proposed patch
> >
> > Attached patch completely rewrites restore_stack_nonlocal expander.
> >
> > Can someone please test the patch on a CET enabled target?
>
> I can help with this, we have CET enabled tigerlake.
Bootstrap is ok, regression test for i386/x86-64 backend is ok.
(our cet-enabled tigerlake is pre-alpha version, test ran very slowly.)
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
` (6 preceding siblings ...)
2020-08-18 8:54 ` crazylht at gmail dot com
@ 2020-08-18 15:32 ` cvs-commit at gcc dot gnu.org
2020-08-18 15:36 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-18 15:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:
https://gcc.gnu.org/g:f8104bb9dc2365d268ca93e43a24f42e8314fcc1
commit r11-2739-gf8104bb9dc2365d268ca93e43a24f42e8314fcc1
Author: Uros Bizjak <ubizjak@gmail.com>
Date: Tue Aug 18 17:31:49 2020 +0200
i386: Rewrite restore_stack_nonlocal expander [PR96536].
-fcf-protection code in restore_stack_nonlocal uses a branch based on
a clobber result. The patch adds missing compare and completely
rewrites the expander to use high-level functions in RTL construction.
2020-08-18 Uroš Bizjak <ubizjak@gmail.com>
gcc/ChangeLog:
PR target/96536
* config/i386/i386.md (restore_stack_nonlocal): Add missing compare
RTX. Rewrite expander to use high-level functions in RTL
construction.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
` (7 preceding siblings ...)
2020-08-18 15:32 ` cvs-commit at gcc dot gnu.org
@ 2020-08-18 15:36 ` cvs-commit at gcc dot gnu.org
2020-08-18 16:49 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-18 15:36 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:
https://gcc.gnu.org/g:6342cee8801f191466b71116d004e8ccb812caaa
commit r10-8638-g6342cee8801f191466b71116d004e8ccb812caaa
Author: Uros Bizjak <ubizjak@gmail.com>
Date: Tue Aug 18 17:34:37 2020 +0200
i386: Fix restore_stack_nonlocal expander [PR96536].
-fcf-protection code in restore_stack_nonlocal uses a branch based on
a clobber result. The patch adds missing compare.
2020-08-18 Uroš Bizjak <ubizjak@gmail.com>
gcc/ChangeLog:
PR target/96536
* config/i386/i386.md (restore_stack_nonlocal):
Add missing compare RTX.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
` (8 preceding siblings ...)
2020-08-18 15:36 ` cvs-commit at gcc dot gnu.org
@ 2020-08-18 16:49 ` cvs-commit at gcc dot gnu.org
2020-08-18 17:49 ` cvs-commit at gcc dot gnu.org
2020-08-18 17:50 ` ubizjak at gmail dot com
11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-18 16:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:
https://gcc.gnu.org/g:65f460db575eb004172e75b88f5a76724f04e255
commit r9-8813-g65f460db575eb004172e75b88f5a76724f04e255
Author: Uros Bizjak <ubizjak@gmail.com>
Date: Tue Aug 18 18:47:47 2020 +0200
i386: Fix restore_stack_nonlocal expander [PR96536].
-fcf-protection code in restore_stack_nonlocal uses a branch based on
a clobber result. The patch adds missing compare.
2020-08-18 Uroš Bizjak <ubizjak@gmail.com>
gcc/ChangeLog:
PR target/96536
* config/i386/i386.md (restore_stack_nonlocal):
Add missing compare RTX.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
` (9 preceding siblings ...)
2020-08-18 16:49 ` cvs-commit at gcc dot gnu.org
@ 2020-08-18 17:49 ` cvs-commit at gcc dot gnu.org
2020-08-18 17:50 ` ubizjak at gmail dot com
11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-18 17:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:
https://gcc.gnu.org/g:bf7b9330982165e051de0962c5bc231e2d1242d9
commit r8-10410-gbf7b9330982165e051de0962c5bc231e2d1242d9
Author: Uros Bizjak <ubizjak@gmail.com>
Date: Tue Aug 18 19:48:51 2020 +0200
i386: Fix restore_stack_nonlocal expander [PR96536].
-fcf-protection code in restore_stack_nonlocal uses a branch based on
a clobber result. The patch adds missing compare.
2020-08-18 Uroš Bizjak <ubizjak@gmail.com>
gcc/ChangeLog:
PR target/96536
* config/i386/i386.md (restore_stack_nonlocal):
Add missing compare RTX.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
2020-08-08 10:36 [Bug target/96536] New: -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl rsandifo at gcc dot gnu.org
` (10 preceding siblings ...)
2020-08-18 17:49 ` cvs-commit at gcc dot gnu.org
@ 2020-08-18 17:50 ` ubizjak at gmail dot com
11 siblings, 0 replies; 13+ messages in thread
From: ubizjak at gmail dot com @ 2020-08-18 17:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
Uroš Bizjak <ubizjak at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |8.5
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #12 from Uroš Bizjak <ubizjak at gmail dot com> ---
Fixed everywhere.
^ permalink raw reply [flat|nested] 13+ messages in thread