6929067: Stack guard pages should be removed when thread is detached

Summary: Add code to unmap stack guard area when thread is detached.
Reviewed-by: coleenp, kamg

src/os/linux/vm/os_linux.cpp

@@ -22,6 +22,8 @@
  *
  */
 
+# define __STDC_FORMAT_MACROS
+
 // do not include  precompiled  header file
 # include "incls/_os_linux.cpp.incl"
 
@@ -53,6 +55,8 @@
 # include <sys/ipc.h>
 # include <sys/shm.h>
 # include <link.h>
+# include <stdint.h>
+# include <inttypes.h>
 
 #define MAX_PATH    (2 * K)
 
@@ -2492,6 +2496,90 @@ bool os::uncommit_memory(char* addr, size_t size) {
     != MAP_FAILED;
 }
 
+// Linux uses a growable mapping for the stack, and if the mapping for
+// the stack guard pages is not removed when we detach a thread the
+// stack cannot grow beyond the pages where the stack guard was
+// mapped.  If at some point later in the process the stack expands to
+// that point, the Linux kernel cannot expand the stack any further
+// because the guard pages are in the way, and a segfault occurs.
+//
+// However, it's essential not to split the stack region by unmapping
+// a region (leaving a hole) that's already part of the stack mapping,
+// so if the stack mapping has already grown beyond the guard pages at
+// the time we create them, we have to truncate the stack mapping.
+// So, we need to know the extent of the stack mapping when
+// create_stack_guard_pages() is called.
+
+// Find the bounds of the stack mapping.  Return true for success.
+//
+// We only need this for stacks that are growable: at the time of
+// writing thread stacks don't use growable mappings (i.e. those
+// creeated with MAP_GROWSDOWN), and aren't marked "[stack]", so this
+// only applies to the main thread.
+static bool
+get_stack_bounds(uintptr_t *bottom, uintptr_t *top)
+{
+  FILE *f = fopen("/proc/self/maps", "r");
+  if (f == NULL)
+    return false;
+
+  while (!feof(f)) {
+    size_t dummy;
+    char *str = NULL;
+    ssize_t len = getline(&str, &dummy, f);
+    if (len == -1) {
+      return false;
+    }
+
+    if (len > 0 && str[len-1] == '\n') {
+      str[len-1] = 0;
+      len--;
+    }
+
+    static const char *stack_str = "[stack]";
+    if (len > (ssize_t)strlen(stack_str)
+       && (strcmp(str + len - strlen(stack_str), stack_str) == 0)) {
+      if (sscanf(str, "%" SCNxPTR "-%" SCNxPTR, bottom, top) == 2) {
+        uintptr_t sp = (uintptr_t)__builtin_frame_address(0);
+        if (sp >= *bottom && sp <= *top) {
+          free(str);
+          return true;
+        }
+      }
+    }
+
+    free(str);
+  }
+
+  return false;
+}
+
+// If the (growable) stack mapping already extends beyond the point
+// where we're going to put our guard pages, truncate the mapping at
+// that point by munmap()ping it.  This ensures that when we later
+// munmap() the guard pages we don't leave a hole in the stack
+// mapping.
+bool os::create_stack_guard_pages(char* addr, size_t size) {
+  uintptr_t stack_extent, stack_base;
+  if (get_stack_bounds(&stack_extent, &stack_base)) {
+    if (stack_extent < (uintptr_t)addr)
+      ::munmap((void*)stack_extent, (uintptr_t)addr - stack_extent);
+  }
+
+  return os::commit_memory(addr, size);
+}
+
+// If this is a growable mapping, remove the guard pages entirely by
+// munmap()ping them.  If not, just call uncommit_memory().
+bool os::remove_stack_guard_pages(char* addr, size_t size) {
+  uintptr_t stack_extent, stack_base;
+  if (get_stack_bounds(&stack_extent, &stack_base)) {
+    return ::munmap(addr, size) == 0;
+  }
+
+  return os::uncommit_memory(addr, size);
+}
+
 static address _highest_vm_reserved_address = NULL;
 
 // If 'fixed' is true, anon_mmap() will attempt to reserve anonymous memory

src/os/solaris/vm/os_solaris.cpp

@@ -2698,6 +2698,14 @@ void os::free_memory(char* addr, size_t bytes) {
   }
 }
 
+bool os::create_stack_guard_pages(char* addr, size_t size) {
+  return os::commit_memory(addr, size);
+}
+
+bool os::remove_stack_guard_pages(char* addr, size_t size) {
+  return os::uncommit_memory(addr, size);
+}
+
 // Change the page size in a given range.
 void os::realign_memory(char *addr, size_t bytes, size_t alignment_hint) {
   assert((intptr_t)addr % alignment_hint == 0, "Address should be aligned.");

src/os/windows/vm/os_windows.cpp

@@ -2803,6 +2803,14 @@ bool os::release_memory(char* addr, size_t bytes) {
   return VirtualFree(addr, 0, MEM_RELEASE) != 0;
 }
 
+bool os::create_stack_guard_pages(char* addr, size_t size) {
+  return os::commit_memory(addr, size);
+}
+
+bool os::remove_stack_guard_pages(char* addr, size_t size) {
+  return os::uncommit_memory(addr, size);
+}
+
 // Set protections specified
 bool os::protect_memory(char* addr, size_t bytes, ProtType prot,
                         bool is_committed) {

src/share/vm/runtime/os.hpp

@@ -218,6 +218,9 @@ class os: AllStatic {
 
   static bool   guard_memory(char* addr, size_t bytes);
   static bool   unguard_memory(char* addr, size_t bytes);
+  static bool   create_stack_guard_pages(char* addr, size_t bytes);
+  static bool   remove_stack_guard_pages(char* addr, size_t bytes);
+
   static char*  map_memory(int fd, const char* file_name, size_t file_offset,
                            char *addr, size_t bytes, bool read_only = false,
                            bool allow_exec = false);

src/share/vm/runtime/thread.cpp

@@ -2137,7 +2137,7 @@ void JavaThread::create_stack_guard_pages() {
   int allocate = os::allocate_stack_guard_pages();
   // warning("Guarding at " PTR_FORMAT " for len " SIZE_FORMAT "\n", low_addr, len);
 
-  if (allocate && !os::commit_memory((char *) low_addr, len)) {
+  if (allocate && !os::create_stack_guard_pages((char *) low_addr, len)) {
     warning("Attempt to allocate stack guard pages failed.");
     return;
   }
@@ -2158,7 +2158,7 @@ void JavaThread::remove_stack_guard_pages() {
   size_t len = (StackYellowPages + StackRedPages) * os::vm_page_size();
 
   if (os::allocate_stack_guard_pages()) {
-    if (os::uncommit_memory((char *) low_addr, len)) {
+    if (os::remove_stack_guard_pages((char *) low_addr, len)) {
       _stack_guard_state = stack_guard_unused;
     } else {
       warning("Attempt to deallocate stack guard pages failed.");

test/runtime/6929067/T.java

+public class T
+{
+  public static boolean foo(boolean bar)
+  {
+    return bar;
+  }
+
+  public static void printIt()
+  {
+    System.out.println("Hello");
+  }
+}

test/runtime/6929067/Test6929067.sh

+#!/bin/sh
+
+##
+## @test Test6929067.sh
+## @bug 6929067
+## @summary Stack guard pages should be removed when thread is detached
+## @run shell Test6929067.sh
+##
+
+if [ "${TESTSRC}" = "" ]
+then TESTSRC=.
+fi
+
+if [ "${TESTJAVA}" = "" ]
+then
+  PARENT=`dirname \`which java\``
+  TESTJAVA=`dirname ${PARENT}`
+  echo "TESTJAVA not set, selecting " ${TESTJAVA}
+  echo "If this is incorrect, try setting the variable manually."
+fi
+
+BIT_FLAG=""
+
+# set platform-dependent variables
+OS=`uname -s`
+case "$OS" in
+  Linux)
+    NULL=/dev/null
+    PS=":"
+    FS="/"
+    ;;
+  SunOS | Windows_* )
+    NULL=NUL
+    PS=";"
+    FS="\\"
+    echo "Test passed; only valid for Linux"
+    exit 0;
+    ;;
+  * )
+    echo "Unrecognized system!"
+    exit 1;
+    ;;
+esac
+
+LD_LIBRARY_PATH=.:${TESTJAVA}/jre/lib/i386/client:/usr/openwin/lib:/usr/dt/lib:/usr/lib:$LD_LIBRARY_PATH
+export LD_LIBRARY_PATH
+
+THIS_DIR=`pwd`
+
+cp ${TESTSRC}${FS}invoke.c ${THIS_DIR}
+cp ${TESTSRC}${FS}T.java ${THIS_DIR}
+
+
+${TESTJAVA}${FS}bin${FS}java ${BIT_FLAG} -fullversion
+
+${TESTJAVA}${FS}bin${FS}javac T.java
+
+gcc -o invoke -I${TESTJAVA}/include -I${TESTJAVA}/include/linux invoke.c ${TESTJAVA}/jre/lib/i386/client/libjvm.so
+./invoke
+exit $?

test/runtime/6929067/invoke.c

+#include <assert.h>
+#include <jni.h>
+#include <alloca.h>
+
+#include <pthread.h>
+
+union env_union
+{
+  void *void_env;
+  JNIEnv *jni_env;
+};
+
+union env_union tmp;
+JNIEnv* env;
+JavaVM* jvm;
+JavaVMInitArgs vm_args;
+JavaVMOption options[1];
+jclass class_id;
+jmethodID method_id;
+jint result;
+
+long product(unsigned long n, unsigned long m) {
+    if (m == 1) {
+      return n;
+    } else {
+      int *p = alloca(sizeof (int));
+      *p = n;
+      return product (n, m-1) + *p;
+    }
+}
+
+void *
+floobydust (void *p)
+{
+  (*jvm)->AttachCurrentThread(jvm, &tmp.void_env, NULL);
+  env = tmp.jni_env;
+
+  class_id = (*env)->FindClass (env, "T");
+  assert (class_id);
+
+  method_id = (*env)->GetStaticMethodID (env, class_id, "printIt", "()V");
+  assert (method_id);
+
+  (*env)->CallStaticVoidMethod (env, class_id, method_id, NULL);
+
+  (*jvm)->DetachCurrentThread(jvm);
+
+  printf("%ld\n", product(5000,5000));
+
+  (*jvm)->AttachCurrentThread(jvm, &tmp.void_env, NULL);
+  env = tmp.jni_env;
+
+  class_id = (*env)->FindClass (env, "T");
+  assert (class_id);
+
+  method_id = (*env)->GetStaticMethodID (env, class_id, "printIt", "()V");
+  assert (method_id);
+
+  (*env)->CallStaticVoidMethod (env, class_id, method_id, NULL);
+
+  (*jvm)->DetachCurrentThread(jvm);
+
+  printf("%ld\n", product(5000,5000));
+
+  return NULL;
+}
+
+int
+main (int argc, const char** argv)
+{
+  options[0].optionString = "-Xss320k";
+
+  vm_args.version = JNI_VERSION_1_2;
+  vm_args.ignoreUnrecognized = JNI_TRUE;
+  vm_args.options = options;
+  vm_args.nOptions = 1;
+
+  result = JNI_CreateJavaVM (&jvm, &tmp.void_env, &vm_args);
+  assert (result >= 0);
+
+  env = tmp.jni_env;
+
+  floobydust (NULL);
+
+  pthread_t thr;
+  pthread_create (&thr, NULL, floobydust, NULL);
+  pthread_join (thr, NULL);
+
+  return 0;
+}