re PR libgcc/86213 (-fsplit-stack runtime may clobber SSE input param reg)
authorThan McIntosh <thanm@google.com>
Wed, 20 Jun 2018 21:11:23 +0000 (21:11 +0000)
committerIan Lance Taylor <ian@gcc.gnu.org>
Wed, 20 Jun 2018 21:11:23 +0000 (21:11 +0000)
libgcc/:
PR libgcc/86213
* generic-morestack.c (allocate_segment): Move calls to getenv and
getpagesize to __morestack_load_mmap.
(__morestack_load_mmap) Initialize static_pagesize and
use_guard_page here so as to avoid clobbering SSE regs during a
__morestack call.
gcc/testsuite/:
* gcc.dg/split-8.c: New.

From-SVN: r261823

gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/split-8.c [new file with mode: 0644]
libgcc/ChangeLog
libgcc/generic-morestack.c

index 0343e8e532490e282f0d01b4c113ebb4210e9fe6..2e333b41109cc66b76dfe88a043b4a81dcf1f2de 100644 (file)
@@ -1,3 +1,8 @@
+2018-06-20  Than McIntosh  <thanm@google.com>
+
+       PR libgcc/86213
+       * gcc.dg/split-8.c: New.
+
 2018-06-20  Kelvin Nilsen  <kelvin@gcc.gnu.org>
 
        * gcc.target/powerpc/builtins-1.c: Adjust dg directives to scan
diff --git a/gcc/testsuite/gcc.dg/split-8.c b/gcc/testsuite/gcc.dg/split-8.c
new file mode 100644 (file)
index 0000000..33662e2
--- /dev/null
@@ -0,0 +1,43 @@
+/* { dg-do run } */
+/* { dg-require-effective-target split_stack } */
+/* { dg-options "-fsplit-stack" } */
+
+/* Testcase for PR86213. On the first call to __morestack there is a live
+   value in xmm0, which was being clobbered by a call to getenv().  */
+
+#include <stdlib.h>
+
+double gd[8];
+int z;
+
+double bar(double q)  __attribute__ ((noinline));
+double foo(double q)  __attribute__ ((noinline));
+int ck(double q)  __attribute__ ((noinline));
+int main(int argc, char **argv) __attribute__ ((no_split_stack));
+
+double bar(double q)
+{
+  double d[8];
+  for (unsigned i = 0; i < 8; ++i)
+    d[i] = gd[8-i-1];
+  return q + d[z&3];
+}
+
+double foo(double d)
+{
+  return bar(d);
+}
+
+int ck(double d)
+{
+  if (d != 64.0)
+    abort();
+  return 0;
+}
+
+typedef double (*fp)(double);
+fp g = foo;
+
+int main(int argc, char **argv) {
+  return ck(g(64.0));
+}
index 65efe170755c1678ddc38e0a86d9ebefe6544012..238b08f366a3ca9083576b1b3e219e5a48ce8ad8 100644 (file)
@@ -1,3 +1,12 @@
+2018-06-20  Than McIntosh  <thanm@google.com>
+
+       PR libgcc/86213
+       * generic-morestack.c (allocate_segment): Move calls to getenv and
+       getpagesize to __morestack_load_mmap.
+       (__morestack_load_mmap) Initialize static_pagesize and
+       use_guard_page here so as to avoid clobbering SSE regs during a
+       __morestack call.
+
 2018-06-18  Michael Meissner  <meissner@linux.ibm.com>
 
        * config/rs6000/t-float128 (FP128_CFLAGS_SW): Compile float128
index 80bfd7feda7fc9d2f8cea1c4146f3f16d3d80b3c..d70ca0922ceac3c35be2fd513b3614a092cf89de 100644 (file)
@@ -243,6 +243,12 @@ __thread struct initial_sp __morestack_initial_sp
 
 static sigset_t __morestack_fullmask;
 
+/* Page size, as returned from getpagesize(). Set on startup. */
+static unsigned int static_pagesize;
+
+/* Set on startup to non-zero value if SPLIT_STACK_GUARD env var is set. */
+static int use_guard_page;
+
 /* Convert an integer to a decimal string without using much stack
    space.  Return a pointer to the part of the buffer to use.  We this
    instead of sprintf because sprintf will require too much stack
@@ -320,8 +326,6 @@ __morestack_fail (const char *msg, size_t len, int err)
 static struct stack_segment *
 allocate_segment (size_t frame_size)
 {
-  static unsigned int static_pagesize;
-  static int use_guard_page;
   unsigned int pagesize;
   unsigned int overhead;
   unsigned int allocate;
@@ -329,27 +333,6 @@ allocate_segment (size_t frame_size)
   struct stack_segment *pss;
 
   pagesize = static_pagesize;
-  if (pagesize == 0)
-    {
-      unsigned int p;
-
-      pagesize = getpagesize ();
-
-#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
-      p = __sync_val_compare_and_swap (&static_pagesize, 0, pagesize);
-#else
-      /* Just hope this assignment is atomic.  */
-      static_pagesize = pagesize;
-      p = 0;
-#endif
-
-      use_guard_page = getenv ("SPLIT_STACK_GUARD") != 0;
-
-      /* FIXME: I'm not sure this assert should be in the released
-        code.  */
-      assert (p == 0 || p == pagesize);
-    }
-
   overhead = sizeof (struct stack_segment);
 
   allocate = pagesize;
@@ -815,7 +798,10 @@ __generic_findstack (void *stack)
 /* This function is called at program startup time to make sure that
    mmap, munmap, and getpagesize are resolved if linking dynamically.
    We want to resolve them while we have enough stack for them, rather
-   than calling into the dynamic linker while low on stack space.  */
+   than calling into the dynamic linker while low on stack space.
+   Similarly, invoke getenv here to check for split-stack related control
+   variables, since doing do as part of the __morestack path can result
+   in unwanted use of SSE/AVX registers (see GCC PR 86213). */
 
 void
 __morestack_load_mmap (void)
@@ -825,7 +811,12 @@ __morestack_load_mmap (void)
      TLS accessor function is resolved.  */
   mmap (__morestack_current_segment, 0, PROT_READ, MAP_ANONYMOUS, -1, 0);
   mprotect (NULL, 0, 0);
-  munmap (0, getpagesize ());
+  munmap (0, static_pagesize);
+
+  /* Initialize these values here, so as to avoid dynamic linker
+     activity as part of a __morestack call. */
+  static_pagesize = getpagesize();
+  use_guard_page = getenv ("SPLIT_STACK_GUARD") != 0;
 }
 
 /* This function may be used to iterate over the stack segments.