Stack guard pages
Andrew John Hughes
gnu_andrew at member.fsf.org
Tue Feb 23 12:36:51 PST 2010
On 23 February 2010 19:50, Andrew Haley <aph at redhat.com> wrote:
> On 02/23/2010 07:15 PM, Andrew John Hughes wrote:
>> On 23 February 2010 16:16, Andrew Haley <aph at redhat.com> wrote:
>>> I've been working on a bug in OpenOffice where using Java causes a
>>> later crash. https://bugzilla.novell.com/show_bug.cgi?id=578802
>>>
>>> It's a very strange bug: Java is called, does something, and then the
>>> thread is detached from the Java VM. Long after, a segfault occurs.
>>>
>>> The reason for this crash is the way that stack guard pages work.
>>> When DetachCurrentThread() calls
>>> JavaThread::remove_stack_guard_pages(), the guard pages are not
>>> removed. So, if at some point later in the process the stack grows
>>> beyond the point where the guard pages were mapped, the Linux kernel
>>> cannot expand the stack any further because the guard pages are in the
>>> way, and a segfault occurs.
>>>
>>> I've attached a reproducer for this bug to the end of this message.
>>> It crashes on many of the Linux systems I've tried.
>>>
>>> The right way to fix this is for remove_stack_guard_pages() to
>>> munmap() the guard pages. However, it's essential not to split the
>>> stack region, so if the stack has already grown beyond the guard
>>> pages, we have to unmap() it. Like so:
>>>
>>> bool os::create_stack_guard_pages(char* addr, size_t size) {
>>> uintptr_t stack_extent, stack_base;
>>> // get_stack_bounds() returns the memory extent mapped for the stack
>>> // by the kernel.
>>> 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);
>>> }
>>>
>>> bool os::remove_stack_guard_pages(char* addr, size_t size) {
>>> return ::munmap(addr, size) == 0;
>>> }
>>>
>>> This fixes the bug. Unfortunately, there is no os:: interface for
>>> create_stack_guard_pages() and remove_stack_guard_pages(), so this
>>> solution doesn't fit well with the current organization of the VM. I
>>> can't see any way of making this work only by touching os_linux.?pp .
>>>
>>> I could simply patch OpenJDK locally for the Linux distros, but I
>>> think we should have this discussion first.
>>
>> Segfaults here with the latest OpenJDK7 b84:
>>
>> $ /mnt/builder/jdk7/j2sdk-image/bin/java -version
>> openjdk version "1.7.0-internal"
>> OpenJDK Runtime Environment (build 1.7.0-internal-andrew_2010_02_23_18_45-b00)
>> OpenJDK 64-Bit Server VM (build 17.0-b09, mixed mode)
>>
>> $ LD_LIBRARY_PATH=/mnt/builder/jdk7/j2sdk-image/jre/lib/amd64/server ./invoke
>> Hello
>> Segmentation fault
>>
>> I think it would it would be clearer what the suggested fix entails if
>> you posted a diff or webrev of the suggested change.
>
> Maybe, but I don't ATM have a suggested change. Everything I've tried is
> rather intrusive, and potentially affects other targets.
>
> However, here's what I've been testing, for information only.
>
Thanks. That helps a lot. I was confusing the functions you
introduce with the ones of the same name in
src/share/vm/runtime/thread.{c,h}pp
For the other platforms, would it not be sufficient for them to just
have simple implementations that call os::commti_memory and
os::uncommit_memory as before? The HotSpot developers can run the
change through JPRT to test it on all platforms.
> Andrew.
>
>
> --- ./src/share/vm/runtime/thread.cpp~ 2008-07-10 21:04:36.000000000 +0100
> +++ ./src/share/vm/runtime/thread.cpp 2010-02-23 14:43:20.452135932 +0000
> @@ -2075,7 +2075,7 @@
> 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;
> }
> @@ -2096,7 +2096,7 @@
> 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.");
> --- ./src/share/vm/runtime/os.hpp~ 2008-07-10 21:04:36.000000000 +0100
> +++ ./src/share/vm/runtime/os.hpp 2010-02-23 14:49:27.512732036 +0000
> @@ -168,6 +168,9 @@
> static bool protect_memory(char* addr, size_t bytes);
> 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/os/linux/vm/os_linux.cpp~ 2010-02-23 11:42:40.005073691 +0000
> +++ ./src/os/linux/vm/os_linux.cpp 2010-02-23 15:44:47.112387815 +0000
> @@ -57,6 +57,10 @@
> # include <sys/shm.h>
> # include <link.h>
>
> +#include <sstream>
> +#include <iostream>
> +#include <fstream>
> +
> #define MAX_PATH (2 * K)
>
> // for timer info max values which include all bits
> @@ -2292,6 +2296,53 @@
> != MAP_FAILED;
> }
>
> +static bool
> +get_stack_bounds(uintptr_t *bottom, uintptr_t *top)
> +{
> + using namespace std;
> +
> + ostringstream oss;
> + oss << "/proc/" << syscall(SYS_gettid) << "/maps";
> + ifstream cin(oss.str().c_str());
> + while (!cin.eof())
> + {
> + string str;
> + getline(cin,str);
> + const string stack_str = "[stack]";
> + if (str.length() > stack_str.length()
> + && (str.compare(str.length() - stack_str.length(),
> + stack_str.length(), stack_str)
> + == 0))
> + {
> + istringstream iss(str);
> + iss.flags(ios::hex);
> + iss >> *bottom;
> + char c;
> + iss >> c;
> + iss >> *top;
> + uintptr_t sp = (intptr_t)__builtin_frame_address(0);
> + if (sp >= *bottom && sp <= *top)
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +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);
> +}
> +
> +bool os::remove_stack_guard_pages(char* addr, size_t size) {
> + return ::munmap(addr, size) == 0;
> +}
> +
> static address _highest_vm_reserved_address = NULL;
>
> // If 'fixed' is true, anon_mmap() will attempt to reserve anonymous memory
>
--
Andrew :-)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
More information about the hotspot-dev
mailing list