This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[committed][PATCH] Stack clash protection 05/08 - V4


This patch introduces a new note we can add to insns.  REG_STACK_CHECK
which is added to a stack allocation to tell to the optimizers that the
allocation should not be combined with other allocations or otherwise
modified.  Since V3 the scheduler was modified to honor REG_STACK_CHECK
as well.

I've run into two instances of this problem.  First in combine-stack-adj
on x86.  Essentially it merged two stack allocations and rewrote the
actual probes.  The net result was the code was vulnerable to problems
if an async signal was delivered between the allocation point and the
probing point.

The second instance which I saw on aarch64 was the scheduler doing
dependency breaking which had the same ultimate effect.

This patch addresses both by marking the relevant insn with the new note
and having the passes appropriately ignore insns with that note.

It includes a test for the combine-stack-adj instance of this problem.
THe aarch64 patches include a test for the scheduler instance.


Bootstrapped and regression tested on x86_64.  Installing on the trunk.

Note I'm stopping here for the night.  s390 is fully ack'd, but needs
another round of testing which will be running overnight.  ppc and
aarch64 are close, but not fully ack'd yet.

Jeff
commit e5b97fee1ab41413bf44623c397a9569dfc1f39e
Author: Jeff Law <law@torsion.usersys.redhat.com>
Date:   Fri Jul 21 01:03:08 2017 -0400

            * combine-stack-adj.c (combine_stack_adjustments_for_block): Do
            nothing for stack adjustments with REG_STACK_CHECK.
            * sched-deps.c (parse_add_or_inc): Reject insns with
            REG_STACK_CHECK from dependency breaking.
            * config/i386/i386.c (pro_epilogue_adjust_stack): Return insn.
            (ix86_adjust_satck_and_probe_stack_clash): Add REG_STACK_NOTEs.
            * reg-notes.def (STACK_CHECK): New note.
    
            * gcc.target/i386/stack-check-11.c: New test.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index f9d3a419f1a..8f3485bfc1d 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,13 @@
 2017-09-19  Jeff Law  <law@redhat.com>
 
+	* combine-stack-adj.c (combine_stack_adjustments_for_block): Do
+	nothing for stack adjustments with REG_STACK_CHECK.
+	* sched-deps.c (parse_add_or_inc): Reject insns with
+	REG_STACK_CHECK from dependency breaking.
+	* config/i386/i386.c (pro_epilogue_adjust_stack): Return insn.
+	(ix86_adjust_satck_and_probe_stack_clash): Add REG_STACK_NOTEs.
+	* reg-notes.def (STACK_CHECK): New note.
+
 	* config/i386/i386.c (ix86_adjust_stack_and_probe_stack_clash): New.
 	(ix86_expand_prologue): Dump stack clash info as needed.
 	Call ix86_adjust_stack_and_probe_stack_clash as needed.
diff --git a/gcc/combine-stack-adj.c b/gcc/combine-stack-adj.c
index 9ec14a3e443..82d6dba856f 100644
--- a/gcc/combine-stack-adj.c
+++ b/gcc/combine-stack-adj.c
@@ -508,6 +508,8 @@ combine_stack_adjustments_for_block (basic_block bb)
 	continue;
 
       set = single_set_for_csa (insn);
+      if (set && find_reg_note (insn, REG_STACK_CHECK, NULL_RTX))
+	set = NULL_RTX;
       if (set)
 	{
 	  rtx dest = SET_DEST (set);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index fdfe5951ca1..d19c770d4cb 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13547,7 +13547,7 @@ ix86_add_queued_cfa_restore_notes (rtx insn)
    zero if %r11 register is live and cannot be freely used and positive
    otherwise.  */
 
-static void
+static rtx
 pro_epilogue_adjust_stack (rtx dest, rtx src, rtx offset,
 			   int style, bool set_cfa)
 {
@@ -13638,6 +13638,7 @@ pro_epilogue_adjust_stack (rtx dest, rtx src, rtx offset,
       m->fs.sp_valid = valid;
       m->fs.sp_realigned = realigned;
     }
+  return insn;
 }
 
 /* Find an available register to be used as dynamic realign argument
@@ -13987,9 +13988,11 @@ ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size)
       for (i = probe_interval; i <= size; i += probe_interval)
 	{
 	  /* Allocate PROBE_INTERVAL bytes.  */
-	  pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
-				     GEN_INT (-probe_interval), -1,
-				     m->fs.cfa_reg == stack_pointer_rtx);
+	  rtx insn
+	    = pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
+					 GEN_INT (-PROBE_INTERVAL), -1,
+					 m->fs.cfa_reg == stack_pointer_rtx);
+	  add_reg_note (insn, REG_STACK_CHECK, const0_rtx);
 
 	  /* And probe at *sp.  */
 	  emit_stack_probe (stack_pointer_rtx);
diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def
index 943eff41d30..a542990cde2 100644
--- a/gcc/reg-notes.def
+++ b/gcc/reg-notes.def
@@ -224,6 +224,10 @@ REG_NOTE (ARGS_SIZE)
    pseudo reg.  */
 REG_NOTE (RETURNED)
 
+/* Indicates the instruction is a stack check probe that should not
+   be combined with other stack adjustments.  */
+REG_NOTE (STACK_CHECK)
+
 /* Used to mark a call with the function decl called by the call.
    The decl might not be available in the call due to splitting of the call
    insn.  This note is a SYMBOL_REF.  */
diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
index e7c4b879f55..a64e4e17bbc 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -4714,6 +4714,11 @@ parse_add_or_inc (struct mem_inc_info *mii, rtx_insn *insn, bool before_mem)
   if (RTX_FRAME_RELATED_P (insn) || !pat)
     return false;
 
+  /* Do not allow breaking data dependencies for insns that are marked
+     with REG_STACK_CHECK.  */
+  if (find_reg_note (insn, REG_STACK_CHECK, NULL))
+    return false;
+
   /* Result must be single reg.  */
   if (!REG_P (SET_DEST (pat)))
     return false;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 55510b120b2..d2eec19d4d0 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,5 +1,7 @@
 2017-09-19  Jeff Law  <law@redhat.com>
 
+	* gcc.target/i386/stack-check-11.c: New test.
+
 	* gcc.dg/stack-check-4.c: New test.
 	* gcc.dg/stack-check-5.c: New test.
 	* gcc.dg/stack-check-6.c: New test.
diff --git a/gcc/testsuite/gcc.target/i386/stack-check-11.c b/gcc/testsuite/gcc.target/i386/stack-check-11.c
new file mode 100644
index 00000000000..183103f01e5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/stack-check-11.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fstack-clash-protection" } */
+/* { dg-require-effective-target supports_stack_clash_protection } */
+
+extern void arf (unsigned long int *, unsigned long int *);
+void
+frob ()
+{
+  unsigned long int num[859];
+  unsigned long int den[859];
+  arf (den, num);
+}
+
+/* { dg-final { scan-assembler-times "subq" 4 } } */
+/* { dg-final { scan-assembler-times "orq" 3 } } */
+

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]