From 89f435cfae0f00c8f4f5705230f8bcd495f62d67 Mon Sep 17 00:00:00 2001 From: coleenp Date: Fri, 12 Mar 2010 10:42:16 -0500 Subject: [PATCH] 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 | 88 ++++++++++++++++++++++++++++ src/os/solaris/vm/os_solaris.cpp | 8 +++ src/os/windows/vm/os_windows.cpp | 8 +++ src/share/vm/runtime/os.hpp | 3 + src/share/vm/runtime/thread.cpp | 4 +- test/runtime/6929067/T.java | 12 ++++ test/runtime/6929067/Test6929067.sh | 60 +++++++++++++++++++ test/runtime/6929067/invoke.c | 90 +++++++++++++++++++++++++++++ 8 files changed, 271 insertions(+), 2 deletions(-) create mode 100644 test/runtime/6929067/T.java create mode 100644 test/runtime/6929067/Test6929067.sh create mode 100644 test/runtime/6929067/invoke.c diff --git a/src/os/linux/vm/os_linux.cpp b/src/os/linux/vm/os_linux.cpp index a4c544565..02399b287 100644 --- a/src/os/linux/vm/os_linux.cpp +++ b/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 # include # include +# include +# include #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 diff --git a/src/os/solaris/vm/os_solaris.cpp b/src/os/solaris/vm/os_solaris.cpp index 2ef213498..db56befcb 100644 --- a/src/os/solaris/vm/os_solaris.cpp +++ b/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."); diff --git a/src/os/windows/vm/os_windows.cpp b/src/os/windows/vm/os_windows.cpp index 6314294e0..217d2c38f 100644 --- a/src/os/windows/vm/os_windows.cpp +++ b/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) { diff --git a/src/share/vm/runtime/os.hpp b/src/share/vm/runtime/os.hpp index 8f2921c07..8b45e9b6c 100644 --- a/src/share/vm/runtime/os.hpp +++ b/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); diff --git a/src/share/vm/runtime/thread.cpp b/src/share/vm/runtime/thread.cpp index e05be5e41..97822aa5e 100644 --- a/src/share/vm/runtime/thread.cpp +++ b/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."); diff --git a/test/runtime/6929067/T.java b/test/runtime/6929067/T.java new file mode 100644 index 000000000..48c48857b --- /dev/null +++ b/test/runtime/6929067/T.java @@ -0,0 +1,12 @@ +public class T +{ + public static boolean foo(boolean bar) + { + return bar; + } + + public static void printIt() + { + System.out.println("Hello"); + } +} diff --git a/test/runtime/6929067/Test6929067.sh b/test/runtime/6929067/Test6929067.sh new file mode 100644 index 000000000..51af19139 --- /dev/null +++ b/test/runtime/6929067/Test6929067.sh @@ -0,0 +1,60 @@ +#!/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 $? diff --git a/test/runtime/6929067/invoke.c b/test/runtime/6929067/invoke.c new file mode 100644 index 000000000..8dde2cd67 --- /dev/null +++ b/test/runtime/6929067/invoke.c @@ -0,0 +1,90 @@ +#include +#include +#include + +#include + +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; +} -- GitLab