8226690: SIGSEGV in MetadataOnStackClosure::do_metadata
Summary: Dont create nmethod if classes have been redefined since compilation start.
Reviewed-by: sspitsyn, dlong, eosterlund, gdub
--- a/src/hotspot/share/ci/ciEnv.cpp Thu Sep 26 14:04:25 2019 +0200
+++ b/src/hotspot/share/ci/ciEnv.cpp Thu Sep 26 09:22:49 2019 -0400
@@ -154,6 +154,7 @@
_the_null_string = NULL;
_the_min_jint_string = NULL;
+ _jvmti_redefinition_count = 0;
_jvmti_can_hotswap_or_post_breakpoint = false;
_jvmti_can_access_local_variables = false;
_jvmti_can_post_on_exceptions = false;
@@ -209,6 +210,7 @@
_the_null_string = NULL;
_the_min_jint_string = NULL;
+ _jvmti_redefinition_count = 0;
_jvmti_can_hotswap_or_post_breakpoint = false;
_jvmti_can_access_local_variables = false;
_jvmti_can_post_on_exceptions = false;
@@ -231,6 +233,7 @@
VM_ENTRY_MARK;
// Get Jvmti capabilities under lock to get consistant values.
MutexLocker mu(JvmtiThreadState_lock);
+ _jvmti_redefinition_count = JvmtiExport::redefinition_count();
_jvmti_can_hotswap_or_post_breakpoint = JvmtiExport::can_hotswap_or_post_breakpoint();
_jvmti_can_access_local_variables = JvmtiExport::can_access_local_variables();
_jvmti_can_post_on_exceptions = JvmtiExport::can_post_on_exceptions();
@@ -238,6 +241,11 @@
}
bool ciEnv::jvmti_state_changed() const {
+ // Some classes were redefined
+ if (_jvmti_redefinition_count != JvmtiExport::redefinition_count()) {
+ return true;
+ }
+
if (!_jvmti_can_access_local_variables &&
JvmtiExport::can_access_local_variables()) {
return true;
@@ -254,6 +262,7 @@
JvmtiExport::can_pop_frame()) {
return true;
}
+
return false;
}
--- a/src/hotspot/share/ci/ciEnv.hpp Thu Sep 26 14:04:25 2019 +0200
+++ b/src/hotspot/share/ci/ciEnv.hpp Thu Sep 26 09:22:49 2019 -0400
@@ -68,6 +68,7 @@
int _name_buffer_len;
// Cache Jvmti state
+ uint64_t _jvmti_redefinition_count;
bool _jvmti_can_hotswap_or_post_breakpoint;
bool _jvmti_can_access_local_variables;
bool _jvmti_can_post_on_exceptions;
--- a/src/hotspot/share/code/nmethod.cpp Thu Sep 26 14:04:25 2019 +0200
+++ b/src/hotspot/share/code/nmethod.cpp Thu Sep 26 09:22:49 2019 -0400
@@ -2192,6 +2192,17 @@
virtual void do_oop(narrowOop* p) { ShouldNotReachHere(); }
};
+class VerifyMetadataClosure: public MetadataClosure {
+ public:
+ void do_metadata(Metadata* md) {
+ if (md->is_method()) {
+ Method* method = (Method*)md;
+ assert(!method->is_old(), "Should not be installing old methods");
+ }
+ }
+};
+
+
void nmethod::verify() {
// Hmm. OSR methods can be deopted but not marked as zombie or not_entrant
@@ -2255,6 +2266,10 @@
Universe::heap()->verify_nmethod(this);
verify_scopes();
+
+ CompiledICLocker nm_verify(this);
+ VerifyMetadataClosure vmc;
+ metadata_do(&vmc);
}
--- a/src/hotspot/share/jvmci/jvmciEnv.cpp Thu Sep 26 14:04:25 2019 +0200
+++ b/src/hotspot/share/jvmci/jvmciEnv.cpp Thu Sep 26 09:22:49 2019 -0400
@@ -44,6 +44,7 @@
_failure_reason_on_C_heap(false) {
// Get Jvmti capabilities under lock to get consistent values.
MutexLocker mu(JvmtiThreadState_lock);
+ _jvmti_redefinition_count = JvmtiExport::redefinition_count();
_jvmti_can_hotswap_or_post_breakpoint = JvmtiExport::can_hotswap_or_post_breakpoint() ? 1 : 0;
_jvmti_can_access_local_variables = JvmtiExport::can_access_local_variables() ? 1 : 0;
_jvmti_can_post_on_exceptions = JvmtiExport::can_post_on_exceptions() ? 1 : 0;
@@ -51,6 +52,10 @@
}
bool JVMCICompileState::jvmti_state_changed() const {
+ // Some classes were redefined
+ if (jvmti_redefinition_count() != JvmtiExport::redefinition_count()) {
+ return true;
+ }
if (!jvmti_can_access_local_variables() &&
JvmtiExport::can_access_local_variables()) {
return true;
--- a/src/hotspot/share/jvmci/jvmciEnv.hpp Thu Sep 26 14:04:25 2019 +0200
+++ b/src/hotspot/share/jvmci/jvmciEnv.hpp Thu Sep 26 09:22:49 2019 -0400
@@ -94,6 +94,7 @@
// Cache JVMTI state. Defined as bytes so that reading them from Java
// via Unsafe is well defined (the C++ type for bool is implementation
// defined and may not be the same as a Java boolean).
+ uint64_t _jvmti_redefinition_count;
jbyte _jvmti_can_hotswap_or_post_breakpoint;
jbyte _jvmti_can_access_local_variables;
jbyte _jvmti_can_post_on_exceptions;
@@ -113,6 +114,7 @@
CompileTask* task() { return _task; }
bool jvmti_state_changed() const;
+ uint64_t jvmti_redefinition_count() const { return _jvmti_redefinition_count; }
bool jvmti_can_hotswap_or_post_breakpoint() const { return _jvmti_can_hotswap_or_post_breakpoint != 0; }
bool jvmti_can_access_local_variables() const { return _jvmti_can_access_local_variables != 0; }
bool jvmti_can_post_on_exceptions() const { return _jvmti_can_post_on_exceptions != 0; }
--- a/src/hotspot/share/prims/jvmtiExport.cpp Thu Sep 26 14:04:25 2019 +0200
+++ b/src/hotspot/share/prims/jvmtiExport.cpp Thu Sep 26 09:22:49 2019 -0400
@@ -304,7 +304,7 @@
bool JvmtiExport::_can_modify_any_class = false;
bool JvmtiExport::_can_walk_any_space = false;
-bool JvmtiExport::_has_redefined_a_class = false;
+uint64_t JvmtiExport::_redefinition_count = 0;
bool JvmtiExport::_all_dependencies_are_recorded = false;
//
--- a/src/hotspot/share/prims/jvmtiExport.hpp Thu Sep 26 14:04:25 2019 +0200
+++ b/src/hotspot/share/prims/jvmtiExport.hpp Thu Sep 26 09:22:49 2019 -0400
@@ -173,10 +173,10 @@
// one or more classes during the lifetime of the VM. The flag should
// only be set by the friend class and can be queried by other sub
// systems as needed to relax invariant checks.
- static bool _has_redefined_a_class;
+ static uint64_t _redefinition_count;
friend class VM_RedefineClasses;
- inline static void set_has_redefined_a_class() {
- JVMTI_ONLY(_has_redefined_a_class = true;)
+ inline static void increment_redefinition_count() {
+ JVMTI_ONLY(_redefinition_count++;)
}
// Flag to indicate if the compiler has recorded all dependencies. When the
// can_redefine_classes capability is enabled in the OnLoad phase then the compiler
@@ -188,10 +188,13 @@
public:
inline static bool has_redefined_a_class() {
- JVMTI_ONLY(return _has_redefined_a_class);
+ JVMTI_ONLY(return _redefinition_count != 0);
NOT_JVMTI(return false);
}
+ // Only set in safepoint, so no memory ordering needed.
+ inline static uint64_t redefinition_count() { return _redefinition_count; }
+
inline static bool all_dependencies_are_recorded() {
return _all_dependencies_are_recorded;
}
--- a/src/hotspot/share/prims/jvmtiRedefineClasses.cpp Thu Sep 26 14:04:25 2019 +0200
+++ b/src/hotspot/share/prims/jvmtiRedefineClasses.cpp Thu Sep 26 09:22:49 2019 -0400
@@ -232,9 +232,9 @@
ResolvedMethodTable::adjust_method_entries(&trace_name_printed);
}
- // Set flag indicating that some invariants are no longer true.
+ // Increment flag indicating that some invariants are no longer true.
// See jvmtiExport.hpp for detailed explanation.
- JvmtiExport::set_has_redefined_a_class();
+ JvmtiExport::increment_redefinition_count();
// check_class() is optionally called for product bits, but is
// always called for non-product bits.
--- a/src/hotspot/share/runtime/sharedRuntime.cpp Thu Sep 26 14:04:25 2019 +0200
+++ b/src/hotspot/share/runtime/sharedRuntime.cpp Thu Sep 26 09:22:49 2019 -0400
@@ -1269,6 +1269,7 @@
// will be supported.
if (!callee_method->is_old() &&
(callee == NULL || (callee->is_in_use() && callee_method->code() == callee))) {
+ NoSafepointVerifier nsv;
#ifdef ASSERT
// We must not try to patch to jump to an already unloaded method.
if (dest_entry_point != 0) {