Revert "Remove Instrumentation::can_use_instrumentation_trampolines_."
This reverts commit 4ac8cfe0f0a53b3ecec371d0e4e702a906bb0e2b.
Reason for revert: Fails on target.
Change-Id: Ia3f4492063cdab548862355f79ac484d44d12c87
diff --git a/openjdkjvmti/deopt_manager.cc b/openjdkjvmti/deopt_manager.cc
index 6377050..f30a21d 100644
--- a/openjdkjvmti/deopt_manager.cc
+++ b/openjdkjvmti/deopt_manager.cc
@@ -450,8 +450,6 @@
return OK;
}
-static constexpr const char* kInstrumentationKey = "JVMTI_DeoptRequester";
-
void DeoptManager::RemoveDeoptimizationRequester() {
art::Thread* self = art::Thread::Current();
art::ScopedThreadStateChange sts(self, art::ThreadState::kSuspended);
@@ -460,7 +458,8 @@
deopter_count_--;
if (deopter_count_ == 0) {
ScopedDeoptimizationContext sdc(self, this);
- art::Runtime::Current()->GetInstrumentation()->DisableDeoptimization(kInstrumentationKey);
+ // TODO Give this a real key.
+ art::Runtime::Current()->GetInstrumentation()->DisableDeoptimization("");
return;
} else {
deoptimization_status_lock_.ExclusiveUnlock(self);
@@ -479,7 +478,7 @@
// Enable deoptimization
instrumentation->EnableDeoptimization();
// Tell instrumentation we will be deopting single threads.
- instrumentation->EnableSingleThreadDeopt(kInstrumentationKey);
+ instrumentation->EnableSingleThreadDeopt();
} else {
deoptimization_status_lock_.ExclusiveUnlock(self);
}
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index b7f2c3c..8ef5ef4 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -185,7 +185,8 @@
deoptimization_enabled_(false),
interpreter_handler_table_(kMainHandlerTable),
quick_alloc_entry_points_instrumentation_counter_(0),
- alloc_entrypoints_instrumented_(false) {
+ alloc_entrypoints_instrumented_(false),
+ can_use_instrumentation_trampolines_(true) {
}
void Instrumentation::InstallStubsForClass(ObjPtr<mirror::Class> klass) {
@@ -754,6 +755,19 @@
return GetCurrentInstrumentationLevel() != new_level;
}
+void Instrumentation::UpdateInstrumentationLevels(InstrumentationLevel level) {
+ if (level == InstrumentationLevel::kInstrumentWithInterpreter) {
+ can_use_instrumentation_trampolines_ = false;
+ }
+ if (UNLIKELY(!can_use_instrumentation_trampolines_)) {
+ for (auto& p : requested_instrumentation_levels_) {
+ if (p.second == InstrumentationLevel::kInstrumentWithInstrumentationStubs) {
+ p.second = InstrumentationLevel::kInstrumentWithInterpreter;
+ }
+ }
+ }
+}
+
void Instrumentation::ConfigureStubs(const char* key, InstrumentationLevel desired_level) {
// Store the instrumentation level for this key or remove it.
if (desired_level == InstrumentationLevel::kInstrumentNothing) {
@@ -764,12 +778,15 @@
requested_instrumentation_levels_.Overwrite(key, desired_level);
}
+ UpdateInstrumentationLevels(desired_level);
UpdateStubs();
}
-void Instrumentation::EnableSingleThreadDeopt(const char* key) {
+void Instrumentation::EnableSingleThreadDeopt() {
// Single-thread deopt only uses interpreter.
- ConfigureStubs(key, InstrumentationLevel::kInstrumentWithInterpreter);
+ can_use_instrumentation_trampolines_ = false;
+ UpdateInstrumentationLevels(InstrumentationLevel::kInstrumentWithInterpreter);
+ UpdateStubs();
}
void Instrumentation::UpdateInstrumentationLevel(InstrumentationLevel requested_level) {
@@ -783,6 +800,11 @@
requested_level = std::max(requested_level, v.second);
}
+ DCHECK(can_use_instrumentation_trampolines_ ||
+ requested_level != InstrumentationLevel::kInstrumentWithInstrumentationStubs)
+ << "Use trampolines: " << can_use_instrumentation_trampolines_ << " level "
+ << requested_level;
+
if (!RequiresInstrumentationInstallation(requested_level)) {
// We're already set.
return;
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 1f7ee7f..a505ce3 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -545,7 +545,7 @@
REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!GetDeoptimizedMethodsLock());
// Sets up instrumentation to allow single thread deoptimization using ForceInterpreterCount.
- void EnableSingleThreadDeopt(const char* key)
+ void EnableSingleThreadDeopt()
REQUIRES(Locks::mutator_lock_, Roles::uninterruptible_)
REQUIRES(!Locks::thread_list_lock_,
!Locks::classlinker_classes_lock_,
@@ -603,6 +603,11 @@
REQUIRES(!GetDeoptimizedMethodsLock(),
!Locks::thread_list_lock_,
!Locks::classlinker_classes_lock_);
+ void UpdateInstrumentationLevels(InstrumentationLevel level)
+ REQUIRES(Locks::mutator_lock_, Roles::uninterruptible_)
+ REQUIRES(!GetDeoptimizedMethodsLock(),
+ !Locks::thread_list_lock_,
+ !Locks::classlinker_classes_lock_);
void UpdateInterpreterHandlerTable() REQUIRES(Locks::mutator_lock_) {
/*
@@ -766,6 +771,11 @@
// alloc_entrypoints_instrumented_ change during suspend points.
bool alloc_entrypoints_instrumented_;
+ // If we can use instrumentation trampolines. After the first time we instrument something with
+ // the interpreter we can no longer use trampolines because it can lead to stack corruption.
+ // TODO Figure out a way to remove the need for this.
+ bool can_use_instrumentation_trampolines_;
+
friend class InstrumentationTest; // For GetCurrentInstrumentationLevel and ConfigureStubs.
friend class InstrumentationStackPopper; // For popping instrumentation frames.
friend void InstrumentationInstallStack(Thread*, void*, bool);
diff --git a/runtime/instrumentation_test.cc b/runtime/instrumentation_test.cc
index d3fa5c8..cfca51e 100644
--- a/runtime/instrumentation_test.cc
+++ b/runtime/instrumentation_test.cc
@@ -837,7 +837,8 @@
// Configure stubs with instrumentation stubs.
CheckConfigureStubs(kClientOneKey,
Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs);
- CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs,
+ // Make sure we are still interpreter since going from interpreter->instrumentation is dangerous.
+ CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInterpreter,
1U);
// Check we can disable instrumentation.
@@ -863,7 +864,7 @@
// Configure stubs with instrumentation stubs again.
CheckConfigureStubs(kClientOneKey,
Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs);
- CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs,
+ CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInterpreter,
1U);
// Check we can disable instrumentation.
@@ -967,9 +968,9 @@
CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInterpreter, 2U);
// 1st client requests instrumentation deactivation but 2nd client still needs
- // instrumentation stubs.
+ // instrumentation stubs. Since we already got interpreter stubs we need to stay there.
CheckConfigureStubs(kClientOneKey, Instrumentation::InstrumentationLevel::kInstrumentNothing);
- CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs,
+ CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInterpreter,
1U);
// 2nd client requests instrumentation deactivation