[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 Uroš Bizjak changed: What|Removed |Added Target Milestone|--- |8.5 Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #12 from Uroš Bizjak --- Fixed everywhere.
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 --- Comment #11 from CVS Commits --- The releases/gcc-8 branch has been updated by Uros Bizjak : https://gcc.gnu.org/g:bf7b9330982165e051de0962c5bc231e2d1242d9 commit r8-10410-gbf7b9330982165e051de0962c5bc231e2d1242d9 Author: Uros Bizjak 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 gcc/ChangeLog: PR target/96536 * config/i386/i386.md (restore_stack_nonlocal): Add missing compare RTX.
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 --- Comment #10 from CVS Commits --- The releases/gcc-9 branch has been updated by Uros Bizjak : https://gcc.gnu.org/g:65f460db575eb004172e75b88f5a76724f04e255 commit r9-8813-g65f460db575eb004172e75b88f5a76724f04e255 Author: Uros Bizjak 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 gcc/ChangeLog: PR target/96536 * config/i386/i386.md (restore_stack_nonlocal): Add missing compare RTX.
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 --- Comment #9 from CVS Commits --- The releases/gcc-10 branch has been updated by Uros Bizjak : https://gcc.gnu.org/g:6342cee8801f191466b71116d004e8ccb812caaa commit r10-8638-g6342cee8801f191466b71116d004e8ccb812caaa Author: Uros Bizjak 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 gcc/ChangeLog: PR target/96536 * config/i386/i386.md (restore_stack_nonlocal): Add missing compare RTX.
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 --- Comment #8 from CVS Commits --- The master branch has been updated by Uros Bizjak : https://gcc.gnu.org/g:f8104bb9dc2365d268ca93e43a24f42e8314fcc1 commit r11-2739-gf8104bb9dc2365d268ca93e43a24f42e8314fcc1 Author: Uros Bizjak 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 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.
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 --- Comment #7 from Hongtao.liu --- (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.)
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536
--- Comment #6 from Uroš Bizjak ---
(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] )
#(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] )
#(const_int 16 [0x10]))) [2 S8 A8])))
#]) "cet-sjlj-1.c":16:3 262 {*subdi_2}
# (nil))
subqbuf+16(%rip), %rax # 15[c=8 l=7] *subdi_2/1
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 --- Comment #5 from Hongtao.liu --- (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.
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 Uroš Bizjak 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 --- Created attachment 49060 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49060=edit Proposed patch Attached patch completely rewrites restore_stack_nonlocal expander. Can someone please test the patch on a CET enabled target?
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 --- Comment #3 from Hongtao.liu --- (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.
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 --- Comment #2 from Uroš Bizjak --- (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.
[Bug target/96536] -fcf-protection code in i386.md:restore_stack_nonlocal uses invalid compare-and-jump rtl
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96536 --- Comment #1 from Hongtao.liu --- 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));
