Revert^4 "JVMTI PopFrame support"
This reverts commit 202b617acf477e8e8e11915f467120a0bd518e74.
This unreverts commit 202b617acf.
This unreverts commit 88a2a9d7a1.
There were several bugs with the implementation of pop-frame related
to interactions between the jit, exception handling, class-loading,
and deoptimization.
- We were instrumenting the target thread stack in cases where it was
unnecessary which caused the exception handler to incorrectly
determine that a method was not deoptimizable. This caused the
pop-frame to be ignored.
- We were incorrectly sending ExceptionCatch events if an exception
suppressed by pop-frame would have been caught in the current frame.
- We were allowing pop-frame to be used on threads suspended in the
ClassLoad or ClassPrepare events despite having surprising semantics
in that situation (see b/117615146).
Needed to modify test 1953 slightly for inclusion in CTS. I needed to
make the CTS entrypoint not run the class-load tests (since the cts
configuration means the classes are loaded by the verifier and not the
interpreter). I updated the expected.txt and check script to reflect
this.
Reason for revert: Fixed issue causing Exception events to sometimes
eat PopFrame and other issues.
Test: ./test.py --host
Test: ./art/tools/run-libjdwp-tests.sh --mode=host
Bug: 73255278
Bug: 111357976
Bug: 117533193
Bug: 117615146
Change-Id: I655c4fe769938cf41d7589f931d6710cf2001506
diff --git a/openjdkjvmti/events-inl.h b/openjdkjvmti/events-inl.h
index e98517f..ca66556 100644
--- a/openjdkjvmti/events-inl.h
+++ b/openjdkjvmti/events-inl.h
@@ -26,7 +26,9 @@
#include "jni/jni_internal.h"
#include "nativehelper/scoped_local_ref.h"
#include "scoped_thread_state_change-inl.h"
+#include "stack.h"
#include "ti_breakpoint.h"
+#include "ti_thread.h"
#include "art_jvmti.h"
@@ -359,6 +361,7 @@
// have to deal with use-after-free or the frames being reallocated later.
art::WriterMutexLock lk(art::Thread::Current(), env->event_info_mutex_);
return env->notify_frames.erase(frame) != 0 &&
+ !frame->GetForcePopFrame() &&
ShouldDispatchOnThread<ArtJvmtiEvent::kFramePop>(env, thread);
}
@@ -418,6 +421,67 @@
ExecuteCallback<ArtJvmtiEvent::kFramePop>(event, jnienv, jni_thread, jmethod, is_exception);
}
+struct ScopedDisablePopFrame {
+ public:
+ explicit ScopedDisablePopFrame(art::Thread* thread) : thread_(thread) {
+ art::Locks::mutator_lock_->AssertSharedHeld(thread_);
+ art::MutexLock mu(thread_, *art::Locks::thread_list_lock_);
+ JvmtiGlobalTLSData* data = ThreadUtil::GetOrCreateGlobalTLSData(thread_);
+ current_top_frame_ = art::StackVisitor::ComputeNumFrames(
+ thread_, art::StackVisitor::StackWalkKind::kIncludeInlinedFrames);
+ old_disable_frame_pop_depth_ = data->disable_pop_frame_depth;
+ data->disable_pop_frame_depth = current_top_frame_;
+ DCHECK(old_disable_frame_pop_depth_ == JvmtiGlobalTLSData::kNoDisallowedPopFrame ||
+ current_top_frame_ > old_disable_frame_pop_depth_)
+ << "old: " << old_disable_frame_pop_depth_ << " current: " << current_top_frame_;
+ }
+
+ ~ScopedDisablePopFrame() {
+ art::Locks::mutator_lock_->AssertSharedHeld(thread_);
+ art::MutexLock mu(thread_, *art::Locks::thread_list_lock_);
+ JvmtiGlobalTLSData* data = ThreadUtil::GetGlobalTLSData(thread_);
+ DCHECK_EQ(data->disable_pop_frame_depth, current_top_frame_);
+ data->disable_pop_frame_depth = old_disable_frame_pop_depth_;
+ }
+
+ private:
+ art::Thread* thread_;
+ size_t current_top_frame_;
+ size_t old_disable_frame_pop_depth_;
+};
+// We want to prevent the use of PopFrame when reporting either of these events.
+template <ArtJvmtiEvent kEvent>
+inline void EventHandler::DispatchClassLoadOrPrepareEvent(art::Thread* thread,
+ JNIEnv* jnienv,
+ jthread jni_thread,
+ jclass klass) const {
+ ScopedDisablePopFrame sdpf(thread);
+ art::ScopedThreadStateChange stsc(thread, art::ThreadState::kNative);
+ std::vector<impl::EventHandlerFunc<kEvent>> events = CollectEvents<kEvent>(thread,
+ jnienv,
+ jni_thread,
+ klass);
+
+ for (auto event : events) {
+ ExecuteCallback<kEvent>(event, jnienv, jni_thread, klass);
+ }
+}
+
+template <>
+inline void EventHandler::DispatchEvent<ArtJvmtiEvent::kClassLoad>(art::Thread* thread,
+ JNIEnv* jnienv,
+ jthread jni_thread,
+ jclass klass) const {
+ DispatchClassLoadOrPrepareEvent<ArtJvmtiEvent::kClassLoad>(thread, jnienv, jni_thread, klass);
+}
+template <>
+inline void EventHandler::DispatchEvent<ArtJvmtiEvent::kClassPrepare>(art::Thread* thread,
+ JNIEnv* jnienv,
+ jthread jni_thread,
+ jclass klass) const {
+ DispatchClassLoadOrPrepareEvent<ArtJvmtiEvent::kClassPrepare>(thread, jnienv, jni_thread, klass);
+}
+
// Need to give a custom specialization for NativeMethodBind since it has to deal with an out
// variable.
template <>
@@ -553,6 +617,7 @@
: ArtJvmtiEvent::kClassFileLoadHookRetransformable;
return (added && caps.can_access_local_variables == 1) ||
caps.can_generate_breakpoint_events == 1 ||
+ caps.can_pop_frame == 1 ||
(caps.can_retransform_classes == 1 &&
IsEventEnabledAnywhere(event) &&
env->event_masks.IsEnabledAnywhere(event));
@@ -573,6 +638,11 @@
if (caps.can_generate_breakpoint_events == 1) {
HandleBreakpointEventsChanged(added);
}
+ if (caps.can_pop_frame == 1 && added) {
+ // TODO We should keep track of how many of these have been enabled and remove it if there are
+ // no more possible users. This isn't expected to be too common.
+ art::Runtime::Current()->SetNonStandardExitsEnabled();
+ }
}
}