From f67fa382f2d5bbed405017d3652437e3f8c8d87a Mon Sep 17 00:00:00 2001 From: rbackman Date: Wed, 12 Jun 2013 11:17:39 +0200 Subject: [PATCH] 8016131: nsk/sysdict/vm/stress/chain tests crash the VM in 'entry_frame_is_first()' Reviewed-by: jrose, kvn, mgronlun --- src/cpu/sparc/vm/frame_sparc.inline.hpp | 4 ++-- src/cpu/x86/vm/frame_x86.inline.hpp | 5 ++--- src/share/vm/prims/forte.cpp | 9 +++++++-- src/share/vm/runtime/frame.cpp | 13 ++++++++++++- src/share/vm/runtime/frame.hpp | 4 +++- src/share/vm/runtime/javaCalls.hpp | 2 ++ src/share/vm/runtime/thread.cpp | 8 ++++++++ src/share/vm/runtime/thread.hpp | 3 +++ 8 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/cpu/sparc/vm/frame_sparc.inline.hpp b/src/cpu/sparc/vm/frame_sparc.inline.hpp index 71655f6d7..774e8f3f0 100644 --- a/src/cpu/sparc/vm/frame_sparc.inline.hpp +++ b/src/cpu/sparc/vm/frame_sparc.inline.hpp @@ -240,10 +240,10 @@ inline ConstantPoolCache** frame::interpreter_frame_cache_addr() const { #endif // CC_INTERP -inline JavaCallWrapper* frame::entry_frame_call_wrapper() const { +inline JavaCallWrapper** frame::entry_frame_call_wrapper_addr() const { // note: adjust this code if the link argument in StubGenerator::call_stub() changes! const Argument link = Argument(0, false); - return (JavaCallWrapper*)sp()[link.as_in().as_register()->sp_offset_in_saved_window()]; + return (JavaCallWrapper**)&sp()[link.as_in().as_register()->sp_offset_in_saved_window()]; } diff --git a/src/cpu/x86/vm/frame_x86.inline.hpp b/src/cpu/x86/vm/frame_x86.inline.hpp index b15b00be5..3c5c225c3 100644 --- a/src/cpu/x86/vm/frame_x86.inline.hpp +++ b/src/cpu/x86/vm/frame_x86.inline.hpp @@ -272,11 +272,10 @@ inline jint frame::interpreter_frame_expression_stack_direction() { return -1; } // Entry frames -inline JavaCallWrapper* frame::entry_frame_call_wrapper() const { - return (JavaCallWrapper*)at(entry_frame_call_wrapper_offset); +inline JavaCallWrapper** frame::entry_frame_call_wrapper_addr() const { + return (JavaCallWrapper**)addr_at(entry_frame_call_wrapper_offset); } - // Compiled frames inline int frame::local_offset_for_compiler(int local_index, int nof_args, int max_nof_locals, int max_nof_monitors) { diff --git a/src/share/vm/prims/forte.cpp b/src/share/vm/prims/forte.cpp index 43da74944..4b1fbebc1 100644 --- a/src/share/vm/prims/forte.cpp +++ b/src/share/vm/prims/forte.cpp @@ -31,6 +31,7 @@ #include "oops/oop.inline.hpp" #include "oops/oop.inline2.hpp" #include "prims/forte.hpp" +#include "runtime/javaCalls.hpp" #include "runtime/thread.hpp" #include "runtime/vframe.hpp" #include "runtime/vframeArray.hpp" @@ -308,10 +309,14 @@ static bool find_initial_Java_frame(JavaThread* thread, for (loop_count = 0; loop_count < loop_max; loop_count++) { - if (candidate.is_first_frame()) { + if (candidate.is_entry_frame()) { + // jcw is NULL if the java call wrapper couldn't be found + JavaCallWrapper *jcw = candidate.entry_frame_call_wrapper_if_safe(thread); // If initial frame is frame from StubGenerator and there is no // previous anchor, there are no java frames associated with a method - return false; + if (jcw == NULL || jcw->is_first_frame()) { + return false; + } } if (candidate.is_interpreted_frame()) { diff --git a/src/share/vm/runtime/frame.cpp b/src/share/vm/runtime/frame.cpp index 92af92e92..9ea7421cf 100644 --- a/src/share/vm/runtime/frame.cpp +++ b/src/share/vm/runtime/frame.cpp @@ -221,9 +221,20 @@ bool frame::is_first_java_frame() const { bool frame::entry_frame_is_first() const { - return entry_frame_call_wrapper()->anchor()->last_Java_sp() == NULL; + return entry_frame_call_wrapper()->is_first_frame(); } +JavaCallWrapper* frame::entry_frame_call_wrapper_if_safe(JavaThread* thread) const { + JavaCallWrapper** jcw = entry_frame_call_wrapper_addr(); + address addr = (address) jcw; + + // addr must be within the usable part of the stack + if (thread->is_in_usable_stack(addr)) { + return *jcw; + } + + return NULL; +} bool frame::should_be_deoptimized() const { if (_deopt_state == is_deoptimized || diff --git a/src/share/vm/runtime/frame.hpp b/src/share/vm/runtime/frame.hpp index 87581ab8d..096b467b5 100644 --- a/src/share/vm/runtime/frame.hpp +++ b/src/share/vm/runtime/frame.hpp @@ -353,7 +353,9 @@ class frame VALUE_OBJ_CLASS_SPEC { public: // Entry frames - JavaCallWrapper* entry_frame_call_wrapper() const; + JavaCallWrapper* entry_frame_call_wrapper() const { return *entry_frame_call_wrapper_addr(); } + JavaCallWrapper* entry_frame_call_wrapper_if_safe(JavaThread* thread) const; + JavaCallWrapper** entry_frame_call_wrapper_addr() const; intptr_t* entry_frame_argument_at(int offset) const; // tells whether there is another chunk of Delta stack above diff --git a/src/share/vm/runtime/javaCalls.hpp b/src/share/vm/runtime/javaCalls.hpp index 08881fcbb..7c397d9f4 100644 --- a/src/share/vm/runtime/javaCalls.hpp +++ b/src/share/vm/runtime/javaCalls.hpp @@ -80,6 +80,8 @@ class JavaCallWrapper: StackObj { oop receiver() { return _receiver; } void oops_do(OopClosure* f); + bool is_first_frame() const { return _anchor.last_Java_sp() == NULL; } + }; diff --git a/src/share/vm/runtime/thread.cpp b/src/share/vm/runtime/thread.cpp index 3325479c0..3cb61cfdb 100644 --- a/src/share/vm/runtime/thread.cpp +++ b/src/share/vm/runtime/thread.cpp @@ -954,6 +954,14 @@ bool Thread::is_in_stack(address adr) const { } +bool Thread::is_in_usable_stack(address adr) const { + size_t stack_guard_size = os::uses_stack_guard_pages() ? (StackYellowPages + StackRedPages) * os::vm_page_size() : 0; + size_t usable_stack_size = _stack_size - stack_guard_size; + + return ((adr < stack_base()) && (adr >= stack_base() - usable_stack_size)); +} + + // We had to move these methods here, because vm threads get into ObjectSynchronizer::enter // However, there is a note in JavaThread::is_lock_owned() about the VM threads not being // used for compilation in the future. If that change is made, the need for these methods diff --git a/src/share/vm/runtime/thread.hpp b/src/share/vm/runtime/thread.hpp index 13950ec16..cbb68ca67 100644 --- a/src/share/vm/runtime/thread.hpp +++ b/src/share/vm/runtime/thread.hpp @@ -521,6 +521,9 @@ public: // Check if address is in the stack of the thread (not just for locks). // Warning: the method can only be used on the running thread bool is_in_stack(address adr) const; + // Check if address is in the usable part of the stack (excludes protected + // guard pages) + bool is_in_usable_stack(address adr) const; // Sets this thread as starting thread. Returns failure if thread // creation fails due to lack of memory, too many threads etc. -- GitLab