diff options
27 files changed, 279 insertions, 61 deletions
diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc index 2f8e33f941..0e6485be9f 100644 --- a/compiler/optimizing/intrinsics_arm64.cc +++ b/compiler/optimizing/intrinsics_arm64.cc @@ -3045,6 +3045,14 @@ void IntrinsicCodeGeneratorARM64::VisitThreadInterrupted(HInvoke* invoke) { __ Bind(&done); } +void IntrinsicLocationsBuilderARM64::VisitReachabilityFence(HInvoke* invoke) { + LocationSummary* locations = + new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified); + locations->SetInAt(0, Location::Any()); +} + +void IntrinsicCodeGeneratorARM64::VisitReachabilityFence(HInvoke* invoke ATTRIBUTE_UNUSED) { } + UNIMPLEMENTED_INTRINSIC(ARM64, ReferenceGetReferent) UNIMPLEMENTED_INTRINSIC(ARM64, StringStringIndexOf); diff --git a/compiler/optimizing/intrinsics_arm_vixl.cc b/compiler/optimizing/intrinsics_arm_vixl.cc index 830d0403e4..97a145664c 100644 --- a/compiler/optimizing/intrinsics_arm_vixl.cc +++ b/compiler/optimizing/intrinsics_arm_vixl.cc @@ -3363,6 +3363,14 @@ void IntrinsicCodeGeneratorARMVIXL::VisitThreadInterrupted(HInvoke* invoke) { } } +void IntrinsicLocationsBuilderARMVIXL::VisitReachabilityFence(HInvoke* invoke) { + LocationSummary* locations = + new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified); + locations->SetInAt(0, Location::Any()); +} + +void IntrinsicCodeGeneratorARMVIXL::VisitReachabilityFence(HInvoke* invoke ATTRIBUTE_UNUSED) { } + UNIMPLEMENTED_INTRINSIC(ARMVIXL, MathRoundDouble) // Could be done by changing rounding mode, maybe? UNIMPLEMENTED_INTRINSIC(ARMVIXL, UnsafeCASLong) // High register pressure. UNIMPLEMENTED_INTRINSIC(ARMVIXL, SystemArrayCopyChar) diff --git a/compiler/optimizing/intrinsics_mips.cc b/compiler/optimizing/intrinsics_mips.cc index cafa5228d9..b7936b9c8e 100644 --- a/compiler/optimizing/intrinsics_mips.cc +++ b/compiler/optimizing/intrinsics_mips.cc @@ -3239,6 +3239,14 @@ void IntrinsicCodeGeneratorMIPS::VisitThreadInterrupted(HInvoke* invoke) { __ Bind(&done); } +void IntrinsicLocationsBuilderMIPS::VisitReachabilityFence(HInvoke* invoke) { + LocationSummary* locations = + new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified); + locations->SetInAt(0, Location::Any()); +} + +void IntrinsicCodeGeneratorMIPS::VisitReachabilityFence(HInvoke* invoke ATTRIBUTE_UNUSED) { } + // Unimplemented intrinsics. UNIMPLEMENTED_INTRINSIC(MIPS, MathCeil) diff --git a/compiler/optimizing/intrinsics_mips64.cc b/compiler/optimizing/intrinsics_mips64.cc index 89f1818be2..4668c561ed 100644 --- a/compiler/optimizing/intrinsics_mips64.cc +++ b/compiler/optimizing/intrinsics_mips64.cc @@ -2622,6 +2622,14 @@ void IntrinsicCodeGeneratorMIPS64::VisitThreadInterrupted(HInvoke* invoke) { __ Bind(&done); } +void IntrinsicLocationsBuilderMIPS64::VisitReachabilityFence(HInvoke* invoke) { + LocationSummary* locations = + new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified); + locations->SetInAt(0, Location::Any()); +} + +void IntrinsicCodeGeneratorMIPS64::VisitReachabilityFence(HInvoke* invoke ATTRIBUTE_UNUSED) { } + UNIMPLEMENTED_INTRINSIC(MIPS64, ReferenceGetReferent) UNIMPLEMENTED_INTRINSIC(MIPS64, SystemArrayCopy) diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc index 46b7f3f1ce..0763ef2352 100644 --- a/compiler/optimizing/intrinsics_x86.cc +++ b/compiler/optimizing/intrinsics_x86.cc @@ -3359,6 +3359,13 @@ void IntrinsicCodeGeneratorX86::VisitThreadInterrupted(HInvoke* invoke) { __ Bind(&done); } +void IntrinsicLocationsBuilderX86::VisitReachabilityFence(HInvoke* invoke) { + LocationSummary* locations = + new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified); + locations->SetInAt(0, Location::Any()); +} + +void IntrinsicCodeGeneratorX86::VisitReachabilityFence(HInvoke* invoke ATTRIBUTE_UNUSED) { } UNIMPLEMENTED_INTRINSIC(X86, MathRoundDouble) UNIMPLEMENTED_INTRINSIC(X86, ReferenceGetReferent) diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc index 6483b7cb2a..91a505ede1 100644 --- a/compiler/optimizing/intrinsics_x86_64.cc +++ b/compiler/optimizing/intrinsics_x86_64.cc @@ -3035,6 +3035,14 @@ void IntrinsicCodeGeneratorX86_64::VisitThreadInterrupted(HInvoke* invoke) { __ Bind(&done); } +void IntrinsicLocationsBuilderX86_64::VisitReachabilityFence(HInvoke* invoke) { + LocationSummary* locations = + new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified); + locations->SetInAt(0, Location::Any()); +} + +void IntrinsicCodeGeneratorX86_64::VisitReachabilityFence(HInvoke* invoke ATTRIBUTE_UNUSED) { } + UNIMPLEMENTED_INTRINSIC(X86_64, ReferenceGetReferent) UNIMPLEMENTED_INTRINSIC(X86_64, FloatIsInfinite) UNIMPLEMENTED_INTRINSIC(X86_64, DoubleIsInfinite) diff --git a/openjdkjvmti/deopt_manager.cc b/openjdkjvmti/deopt_manager.cc index 6d84ffa53f..a6f1207f34 100644 --- a/openjdkjvmti/deopt_manager.cc +++ b/openjdkjvmti/deopt_manager.cc @@ -53,20 +53,29 @@ namespace openjdkjvmti { // TODO We should make this much more selective in the future so we only return true when we -// actually care about the method (i.e. had locals changed, have breakpoints, etc.). For now though -// we can just assume that we care we are loaded at all. -// -// Even if we don't keep track of this at the method level we might want to keep track of it at the -// level of enabled capabilities. -bool JvmtiMethodInspectionCallback::IsMethodBeingInspected( - art::ArtMethod* method ATTRIBUTE_UNUSED) { - return true; +// actually care about the method at this time (ie active frames had locals changed). For now we +// just assume that if anything has changed any frame's locals we care about all methods. If nothing +// has we only care about methods with active breakpoints on them. In the future we should probably +// rewrite all of this to instead do this at the ShadowFrame or thread granularity. +bool JvmtiMethodInspectionCallback::IsMethodBeingInspected(art::ArtMethod* method) { + // Non-java-debuggable runtimes we need to assume that any method might not be debuggable and + // therefore potentially being inspected (due to inlines). If we are debuggable we rely hard on + // inlining not being done since we don't keep track of which methods get inlined where and simply + // look to see if the method is breakpointed. + return !art::Runtime::Current()->IsJavaDebuggable() || + manager_->HaveLocalsChanged() || + manager_->MethodHasBreakpoints(method); } bool JvmtiMethodInspectionCallback::IsMethodSafeToJit(art::ArtMethod* method) { return !manager_->MethodHasBreakpoints(method); } +bool JvmtiMethodInspectionCallback::MethodNeedsDebugVersion( + art::ArtMethod* method ATTRIBUTE_UNUSED) { + return true; +} + DeoptManager::DeoptManager() : deoptimization_status_lock_("JVMTI_DeoptimizationStatusLock", static_cast<art::LockLevel>( @@ -75,7 +84,10 @@ DeoptManager::DeoptManager() performing_deoptimization_(false), global_deopt_count_(0), deopter_count_(0), - inspection_callback_(this) { } + breakpoint_status_lock_("JVMTI_BreakpointStatusLock", + static_cast<art::LockLevel>(art::LockLevel::kAbortLock + 1)), + inspection_callback_(this), + set_local_variable_called_(false) { } void DeoptManager::Setup() { art::ScopedThreadStateChange stsc(art::Thread::Current(), @@ -121,14 +133,11 @@ void DeoptManager::FinishSetup() { } bool DeoptManager::MethodHasBreakpoints(art::ArtMethod* method) { - art::MutexLock lk(art::Thread::Current(), deoptimization_status_lock_); + art::MutexLock lk(art::Thread::Current(), breakpoint_status_lock_); return MethodHasBreakpointsLocked(method); } bool DeoptManager::MethodHasBreakpointsLocked(art::ArtMethod* method) { - if (deopter_count_ == 0) { - return false; - } auto elem = breakpoint_status_.find(method); return elem != breakpoint_status_.end() && elem->second != 0; } @@ -158,18 +167,23 @@ void DeoptManager::AddMethodBreakpoint(art::ArtMethod* method) { art::ScopedThreadSuspension sts(self, art::kSuspended); deoptimization_status_lock_.ExclusiveLock(self); - - DCHECK_GT(deopter_count_, 0u) << "unexpected deotpimization request"; - - if (MethodHasBreakpointsLocked(method)) { - // Don't need to do anything extra. - breakpoint_status_[method]++; - // Another thread might be deoptimizing the very method we just added new breakpoints for. Wait - // for any deopts to finish before moving on. - WaitForDeoptimizationToFinish(self); - return; + { + breakpoint_status_lock_.ExclusiveLock(self); + + DCHECK_GT(deopter_count_, 0u) << "unexpected deotpimization request"; + + if (MethodHasBreakpointsLocked(method)) { + // Don't need to do anything extra. + breakpoint_status_[method]++; + // Another thread might be deoptimizing the very method we just added new breakpoints for. + // Wait for any deopts to finish before moving on. + breakpoint_status_lock_.ExclusiveUnlock(self); + WaitForDeoptimizationToFinish(self); + return; + } + breakpoint_status_[method] = 1; + breakpoint_status_lock_.ExclusiveUnlock(self); } - breakpoint_status_[method] = 1; auto instrumentation = art::Runtime::Current()->GetInstrumentation(); if (instrumentation->IsForcedInterpretOnly()) { // We are already interpreting everything so no need to do anything. @@ -196,17 +210,22 @@ void DeoptManager::RemoveMethodBreakpoint(art::ArtMethod* method) { // need but since that is very heavy we will instead just use a condition variable to make sure we // don't race with ourselves. deoptimization_status_lock_.ExclusiveLock(self); - - DCHECK_GT(deopter_count_, 0u) << "unexpected deotpimization request"; - DCHECK(MethodHasBreakpointsLocked(method)) << "Breakpoint on a method was removed without " - << "breakpoints present!"; + bool is_last_breakpoint; + { + art::MutexLock mu(self, breakpoint_status_lock_); + + DCHECK_GT(deopter_count_, 0u) << "unexpected deotpimization request"; + DCHECK(MethodHasBreakpointsLocked(method)) << "Breakpoint on a method was removed without " + << "breakpoints present!"; + breakpoint_status_[method] -= 1; + is_last_breakpoint = (breakpoint_status_[method] == 0); + } auto instrumentation = art::Runtime::Current()->GetInstrumentation(); - breakpoint_status_[method] -= 1; if (UNLIKELY(instrumentation->IsForcedInterpretOnly())) { // We don't need to do anything since we are interpreting everything anyway. deoptimization_status_lock_.ExclusiveUnlock(self); return; - } else if (breakpoint_status_[method] == 0) { + } else if (is_last_breakpoint) { if (UNLIKELY(is_default)) { RemoveDeoptimizeAllMethodsLocked(self); } else { diff --git a/openjdkjvmti/deopt_manager.h b/openjdkjvmti/deopt_manager.h index a495b6835c..6e991dee3d 100644 --- a/openjdkjvmti/deopt_manager.h +++ b/openjdkjvmti/deopt_manager.h @@ -32,6 +32,7 @@ #ifndef ART_OPENJDKJVMTI_DEOPT_MANAGER_H_ #define ART_OPENJDKJVMTI_DEOPT_MANAGER_H_ +#include <atomic> #include <unordered_map> #include "jni.h" @@ -62,6 +63,9 @@ struct JvmtiMethodInspectionCallback : public art::MethodInspectionCallback { bool IsMethodSafeToJit(art::ArtMethod* method) OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_); + bool MethodNeedsDebugVersion(art::ArtMethod* method) + OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_); + private: DeoptManager* manager_; }; @@ -107,9 +111,17 @@ class DeoptManager { static DeoptManager* Get(); + bool HaveLocalsChanged() const { + return set_local_variable_called_.load(); + } + + void SetLocalsUpdated() { + set_local_variable_called_.store(true); + } + private: bool MethodHasBreakpointsLocked(art::ArtMethod* method) - REQUIRES(deoptimization_status_lock_); + REQUIRES(breakpoint_status_lock_); // Wait until nothing is currently in the middle of deoptimizing/undeoptimizing something. This is // needed to ensure that everything is synchronized since threads need to drop the @@ -156,13 +168,20 @@ class DeoptManager { // Number of users of deoptimization there currently are. uint32_t deopter_count_ GUARDED_BY(deoptimization_status_lock_); + // A mutex that just protects the breakpoint-status map. This mutex should always be at the + // bottom of the lock hierarchy. Nothing more should be locked if we hold this. + art::Mutex breakpoint_status_lock_ ACQUIRED_BEFORE(art::Locks::abort_lock_); // A map from methods to the number of breakpoints in them from all envs. std::unordered_map<art::ArtMethod*, uint32_t> breakpoint_status_ - GUARDED_BY(deoptimization_status_lock_); + GUARDED_BY(breakpoint_status_lock_); // The MethodInspectionCallback we use to tell the runtime if we care about particular methods. JvmtiMethodInspectionCallback inspection_callback_; + // Set to true if anything calls SetLocalVariables on any thread since we need to be careful about + // OSR after this. + std::atomic<bool> set_local_variable_called_; + // Helper for setting up/tearing-down for deoptimization. friend class ScopedDeoptimizationContext; }; diff --git a/openjdkjvmti/ti_method.cc b/openjdkjvmti/ti_method.cc index bf2e6cd104..b83310dc85 100644 --- a/openjdkjvmti/ti_method.cc +++ b/openjdkjvmti/ti_method.cc @@ -915,6 +915,9 @@ jvmtiError MethodUtil::SetLocalVariableGeneric(jvmtiEnv* env ATTRIBUTE_UNUSED, if (depth < 0) { return ERR(ILLEGAL_ARGUMENT); } + // Make sure that we know not to do any OSR anymore. + // TODO We should really keep track of this at the Frame granularity. + DeoptManager::Get()->SetLocalsUpdated(); art::Thread* self = art::Thread::Current(); // Suspend JIT since it can get confused if we deoptimize methods getting jitted. art::jit::ScopedJitSuspend suspend_jit; diff --git a/runtime/debugger.cc b/runtime/debugger.cc index 99a4c77979..0481c03d52 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -364,6 +364,11 @@ bool DebuggerActiveMethodInspectionCallback::IsMethodSafeToJit(ArtMethod* m) { return !Dbg::MethodHasAnyBreakpoints(m); } +bool DebuggerActiveMethodInspectionCallback::MethodNeedsDebugVersion( + ArtMethod* m ATTRIBUTE_UNUSED) { + return Dbg::IsDebuggerActive(); +} + void InternalDebuggerControlCallback::StartDebugger() { // Release the mutator lock. ScopedThreadStateChange stsc(art::Thread::Current(), kNative); diff --git a/runtime/debugger.h b/runtime/debugger.h index 74018137a0..e1de991812 100644 --- a/runtime/debugger.h +++ b/runtime/debugger.h @@ -56,6 +56,7 @@ class Thread; struct DebuggerActiveMethodInspectionCallback : public MethodInspectionCallback { bool IsMethodBeingInspected(ArtMethod* method) OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_); bool IsMethodSafeToJit(ArtMethod* method) OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_); + bool MethodNeedsDebugVersion(ArtMethod* method) OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_); }; struct DebuggerDdmCallback : public DdmCallback { diff --git a/runtime/fault_handler.cc b/runtime/fault_handler.cc index 3015b10103..671079b128 100644 --- a/runtime/fault_handler.cc +++ b/runtime/fault_handler.cc @@ -37,7 +37,9 @@ namespace art { // Static fault manger object accessed by signal handler. FaultManager fault_manager; -extern "C" __attribute__((visibility("default"))) void art_sigsegv_fault() { +// This needs to be NO_INLINE since some debuggers do not read the inline-info to set a breakpoint +// if it isn't. +extern "C" NO_INLINE __attribute__((visibility("default"))) void art_sigsegv_fault() { // Set a breakpoint here to be informed when a SIGSEGV is unhandled by ART. VLOG(signals)<< "Caught unknown SIGSEGV in ART fault handler - chaining to next handler."; } diff --git a/runtime/image.cc b/runtime/image.cc index 0955c3a3ca..3b13d05d80 100644 --- a/runtime/image.cc +++ b/runtime/image.cc @@ -26,7 +26,7 @@ namespace art { const uint8_t ImageHeader::kImageMagic[] = { 'a', 'r', 't', '\n' }; -const uint8_t ImageHeader::kImageVersion[] = { '0', '5', '5', '\0' }; // Bitstring type check off. +const uint8_t ImageHeader::kImageVersion[] = { '0', '5', '6', '\0' }; // ReachabilityFence. ImageHeader::ImageHeader(uint32_t image_begin, uint32_t image_size, diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc index 84a148f21c..d7f33d5e43 100644 --- a/runtime/instrumentation.cc +++ b/runtime/instrumentation.cc @@ -139,10 +139,13 @@ static void UpdateEntrypoints(ArtMethod* method, const void* quick_code) bool Instrumentation::NeedDebugVersionFor(ArtMethod* method) const REQUIRES_SHARED(Locks::mutator_lock_) { - return Runtime::Current()->IsJavaDebuggable() && + art::Runtime* runtime = Runtime::Current(); + // If anything says we need the debug version or we are debuggable we will need the debug version + // of the method. + return (runtime->GetRuntimeCallbacks()->MethodNeedsDebugVersion(method) || + runtime->IsJavaDebuggable()) && !method->IsNative() && - !method->IsProxyMethod() && - Runtime::Current()->GetRuntimeCallbacks()->IsMethodBeingInspected(method); + !method->IsProxyMethod(); } void Instrumentation::InstallStubsForMethod(ArtMethod* method) { diff --git a/runtime/interpreter/interpreter_intrinsics.cc b/runtime/interpreter/interpreter_intrinsics.cc index 681a582b5d..f0cf2af7c7 100644 --- a/runtime/interpreter/interpreter_intrinsics.cc +++ b/runtime/interpreter/interpreter_intrinsics.cc @@ -399,6 +399,16 @@ VAR_HANDLE_ACCESSOR_INTRINSIC(VarHandleWeakCompareAndSetAcquire) VAR_HANDLE_ACCESSOR_INTRINSIC(VarHandleWeakCompareAndSetPlain) VAR_HANDLE_ACCESSOR_INTRINSIC(VarHandleWeakCompareAndSetRelease) +static ALWAYS_INLINE bool MterpReachabilityFence(ShadowFrame* shadow_frame ATTRIBUTE_UNUSED, + const Instruction* inst ATTRIBUTE_UNUSED, + uint16_t inst_data ATTRIBUTE_UNUSED, + JValue* result_register ATTRIBUTE_UNUSED) + REQUIRES_SHARED(Locks::mutator_lock_) { + // Do nothing; Its only purpose is to keep the argument reference live + // at preceding suspend points. That's automatic in the interpreter. + return true; +} + // Macro to help keep track of what's left to implement. #define UNIMPLEMENTED_CASE(name) \ case Intrinsics::k##name: \ @@ -499,6 +509,7 @@ bool MterpHandleIntrinsic(ShadowFrame* shadow_frame, UNIMPLEMENTED_CASE(MemoryPokeIntNative /* (JI)V */) UNIMPLEMENTED_CASE(MemoryPokeLongNative /* (JJ)V */) UNIMPLEMENTED_CASE(MemoryPokeShortNative /* (JS)V */) + INTRINSIC_CASE(ReachabilityFence /* (Ljava/lang/Object;)V */) INTRINSIC_CASE(StringCharAt) INTRINSIC_CASE(StringCompareTo) INTRINSIC_CASE(StringEquals) diff --git a/runtime/intrinsics_list.h b/runtime/intrinsics_list.h index da08793f59..2f91f5dfe0 100644 --- a/runtime/intrinsics_list.h +++ b/runtime/intrinsics_list.h @@ -218,6 +218,7 @@ V(VarHandleReleaseFence, kStatic, kNeedsEnvironmentOrCache, kWriteSideEffects, kNoThrow, "Ljava/lang/invoke/VarHandle;", "releaseFence", "()V") \ V(VarHandleLoadLoadFence, kStatic, kNeedsEnvironmentOrCache, kWriteSideEffects, kNoThrow, "Ljava/lang/invoke/VarHandle;", "loadLoadFence", "()V") \ V(VarHandleStoreStoreFence, kStatic, kNeedsEnvironmentOrCache, kReadSideEffects, kNoThrow, "Ljava/lang/invoke/VarHandle;", "storeStoreFence", "()V") \ + V(ReachabilityFence, kStatic, kNeedsEnvironmentOrCache, kWriteSideEffects, kNoThrow, "Ljava/lang/ref/Reference;", "reachabilityFence", "(Ljava/lang/Object;)V") \ SIGNATURE_POLYMORPHIC_INTRINSICS_LIST(V) #endif // ART_RUNTIME_INTRINSICS_LIST_H_ diff --git a/runtime/runtime_callbacks.cc b/runtime/runtime_callbacks.cc index cd3c0b7c88..758917cf7e 100644 --- a/runtime/runtime_callbacks.cc +++ b/runtime/runtime_callbacks.cc @@ -106,6 +106,15 @@ bool RuntimeCallbacks::IsMethodBeingInspected(ArtMethod* m) { return false; } +bool RuntimeCallbacks::MethodNeedsDebugVersion(ArtMethod* m) { + for (MethodInspectionCallback* cb : method_inspection_callbacks_) { + if (cb->MethodNeedsDebugVersion(m)) { + return true; + } + } + return false; +} + void RuntimeCallbacks::AddThreadLifecycleCallback(ThreadLifecycleCallback* cb) { thread_callbacks_.push_back(cb); } diff --git a/runtime/runtime_callbacks.h b/runtime/runtime_callbacks.h index 24386ba14a..9f0410d102 100644 --- a/runtime/runtime_callbacks.h +++ b/runtime/runtime_callbacks.h @@ -130,6 +130,10 @@ class MethodInspectionCallback { // Note that '!IsMethodSafeToJit(m) implies IsMethodBeingInspected(m)'. That is that if this // method returns false IsMethodBeingInspected must return true. virtual bool IsMethodSafeToJit(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) = 0; + + // Returns true if we expect the method to be debuggable but are not doing anything unusual with + // it currently. + virtual bool MethodNeedsDebugVersion(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) = 0; }; class RuntimeCallbacks { @@ -198,6 +202,11 @@ class RuntimeCallbacks { // entrypoint should not be changed to JITed code. bool IsMethodSafeToJit(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_); + // Returns true if some MethodInspectionCallback indicates the method needs to use a debug + // version. This allows later code to set breakpoints or perform other actions that could be + // broken by some optimizations. + bool MethodNeedsDebugVersion(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_); + void AddMethodInspectionCallback(MethodInspectionCallback* cb) REQUIRES_SHARED(Locks::mutator_lock_); void RemoveMethodInspectionCallback(MethodInspectionCallback* cb) diff --git a/test/036-finalizer/src/Main.java b/test/036-finalizer/src/Main.java index ff6186b240..9bfd74f91b 100644 --- a/test/036-finalizer/src/Main.java +++ b/test/036-finalizer/src/Main.java @@ -14,6 +14,7 @@ * limitations under the License. */ +import java.lang.ref.Reference; import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.List; @@ -74,6 +75,7 @@ public class Main { // Reference ft so we are sure the WeakReference cannot be cleared. FinalizerTest keepLive = wimp.get(); System.out.println("wimp: " + wimpString(wimp)); + Reference.reachabilityFence(keepLive); } public static void main(String[] args) { diff --git a/test/072-reachability-fence/expected.txt b/test/072-reachability-fence/expected.txt new file mode 100644 index 0000000000..fdd0d7bd59 --- /dev/null +++ b/test/072-reachability-fence/expected.txt @@ -0,0 +1,5 @@ +Starting +Reference 0 was live. +Reference 3 was live. +Reference 4 was live. +Finished diff --git a/test/072-reachability-fence/info.txt b/test/072-reachability-fence/info.txt new file mode 100644 index 0000000000..21b6d6a39f --- /dev/null +++ b/test/072-reachability-fence/info.txt @@ -0,0 +1,4 @@ +Check that reachabilityFence() prevents garbage collection of objects only referred to by a dead +reference. + +This is not very convincing, since we currently usually keep such objects around anyway. diff --git a/test/072-reachability-fence/src/Main.java b/test/072-reachability-fence/src/Main.java new file mode 100644 index 0000000000..ac1e131d99 --- /dev/null +++ b/test/072-reachability-fence/src/Main.java @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.lang.ref.Reference; +import java.lang.ref.WeakReference; + +public class Main { + public static void main(String[] args) { + System.out.println("Starting"); + WeakReference wrefs[] = new WeakReference[5]; + String str0 = generateString("String", 0); + String str1 = generateString("String", 1); + String str2 = generateString("String", 2); + String str3 = generateString("String", 3); + String str4 = generateString("String", 4); + wrefs[0] = new WeakReference(str0); + wrefs[1] = new WeakReference(str1); + wrefs[2] = new WeakReference(str2); + wrefs[3] = new WeakReference(str3); + wrefs[4] = new WeakReference(str4); + // Clear a couple as a sanity check. + str1 = null; + str2 = null; + // str<n> dead here; in the future we will possibly reuse the registers. + // Give the compiler something to fill the registers with. + String str5 = generateString("String", 5); + String str6 = generateString("String", 6); + String str7 = generateString("String", 7); + String str8 = generateString("String", 8); + String str9 = generateString("String", 9); + Runtime.getRuntime().gc(); + for (int i = 0; i < 5; ++i) { + if (wrefs[i].get() != null) { + System.out.println("Reference " + i + " was live."); + } + } + Reference.reachabilityFence(str0); + Reference.reachabilityFence(str1); + Reference.reachabilityFence(str2); + Reference.reachabilityFence(str3); + Reference.reachabilityFence(str4); + System.out.println("Finished"); + } + + private static String generateString(String base, int num) { + return base + num; + } +} diff --git a/test/1935-get-set-current-frame-jit/expected.txt b/test/1935-get-set-current-frame-jit/expected.txt index fed993cc1a..a685891775 100644 --- a/test/1935-get-set-current-frame-jit/expected.txt +++ b/test/1935-get-set-current-frame-jit/expected.txt @@ -1,7 +1,5 @@ JNI_OnLoad called From GetLocalInt(), value is 42 -isInterpreted? true Value is '42' Setting TARGET to 1337 -isInterpreted? true Value is '1337' diff --git a/test/1935-get-set-current-frame-jit/run b/test/1935-get-set-current-frame-jit/run index 51875a7e86..e569d08ffd 100755 --- a/test/1935-get-set-current-frame-jit/run +++ b/test/1935-get-set-current-frame-jit/run @@ -14,5 +14,5 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Ask for stack traces to be dumped to a file rather than to stdout. -./default-run "$@" --jvmti +# Ensure the test is not subject to code collection +./default-run "$@" --jvmti --runtime-option -Xjitinitialsize:32M diff --git a/test/1935-get-set-current-frame-jit/src/Main.java b/test/1935-get-set-current-frame-jit/src/Main.java index eb0a6374d2..378aaf7a94 100644 --- a/test/1935-get-set-current-frame-jit/src/Main.java +++ b/test/1935-get-set-current-frame-jit/src/Main.java @@ -21,6 +21,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Executable; import java.lang.reflect.Method; import java.nio.ByteBuffer; +import java.time.Instant; import java.util.concurrent.Semaphore; import java.util.Arrays; import java.util.Collection; @@ -49,9 +50,11 @@ public class Main { public static class IntRunner implements Runnable { private volatile boolean continueBusyLoop; private volatile boolean inBusyLoop; - public IntRunner() { + private final boolean expectOsr; + public IntRunner(boolean expectOsr) { this.continueBusyLoop = true; this.inBusyLoop = false; + this.expectOsr = expectOsr; } public void run() { int TARGET = 42; @@ -59,14 +62,23 @@ public class Main { while (continueBusyLoop) { inBusyLoop = true; } - int i = 0; - while (Main.isInterpreted() && i < 10000) { - Main.ensureJitCompiled(IntRunner.class, "run"); - i++; + // Wait up to 300 seconds for OSR to kick in if we expect it. If we don't give up after only + // 3 seconds. + Instant osrDeadline = Instant.now().plusSeconds(expectOsr ? 600 : 3); + do { + // Don't actually do anything here. + inBusyLoop = true; + } while (hasJit() && !Main.isInOsrCode("run") && osrDeadline.compareTo(Instant.now()) > 0); + // We shouldn't be doing OSR since we are using JVMTI and the set prevents OSR. + // Set local will also push us to interpreter but the get local may remain in compiled code. + if (hasJit()) { + boolean inOsr = Main.isInOsrCode("run"); + if (expectOsr && !inOsr) { + throw new Error("Expected to be in OSR but was not."); + } else if (!expectOsr && inOsr) { + throw new Error("Expected not to be in OSR but was."); + } } - // We shouldn't be doing OSR since we are using JVMTI and the get/set local will push us to - // interpreter. - System.out.println("isInterpreted? " + Main.isInterpreted()); reportValue(TARGET); } public void waitForBusyLoopStart() { while (!inBusyLoop) {} } @@ -78,7 +90,7 @@ public class Main { public static void runGet() throws Exception { Method target = IntRunner.class.getDeclaredMethod("run"); // Get Int - IntRunner int_runner = new IntRunner(); + IntRunner int_runner = new IntRunner(true); Thread target_get = new Thread(int_runner, "GetLocalInt - Target"); target_get.start(); int_runner.waitForBusyLoopStart(); @@ -108,7 +120,7 @@ public class Main { public static void runSet() throws Exception { Method target = IntRunner.class.getDeclaredMethod("run"); // Set Int - IntRunner int_runner = new IntRunner(); + IntRunner int_runner = new IntRunner(false); Thread target_set = new Thread(int_runner, "SetLocalInt - Target"); target_set.start(); int_runner.waitForBusyLoopStart(); @@ -157,6 +169,7 @@ public class Main { throw new Error("Unable to find stack frame in method " + target + " on thread " + thr); } - public static native void ensureJitCompiled(Class k, String f); public static native boolean isInterpreted(); + public static native boolean isInOsrCode(String methodName); + public static native boolean hasJit(); } diff --git a/tools/veridex/hidden_api.cc b/tools/veridex/hidden_api.cc index 93f921a25f..17fa1b8513 100644 --- a/tools/veridex/hidden_api.cc +++ b/tools/veridex/hidden_api.cc @@ -61,6 +61,11 @@ void HiddenApi::FillList(const char* filename, std::set<std::string>& entries) { // Add the class->method name (so stripping the signature). entries.insert(str.substr(0, pos)); } + pos = str.find(':'); + if (pos != std::string::npos) { + // Add the class->field name (so stripping the type). + entries.insert(str.substr(0, pos)); + } } } } diff --git a/tools/veridex/hidden_api_finder.cc b/tools/veridex/hidden_api_finder.cc index d611f78eed..4885e02769 100644 --- a/tools/veridex/hidden_api_finder.cc +++ b/tools/veridex/hidden_api_finder.cc @@ -58,6 +58,16 @@ void HiddenApiFinder::CheckField(uint32_t field_id, void HiddenApiFinder::CollectAccesses(VeridexResolver* resolver) { const DexFile& dex_file = resolver->GetDexFile(); + // Look at all types referenced in this dex file. Any of these + // types can lead to being used through reflection. + for (uint32_t i = 0; i < dex_file.NumTypeIds(); ++i) { + std::string name(dex_file.StringByTypeIdx(dex::TypeIndex(i))); + if (hidden_api_.IsInRestrictionList(name)) { + classes_.insert(name); + } + } + // Note: we collect strings constants only referenced in code items as the string table + // contains other kind of strings (eg types). size_t class_def_count = dex_file.NumClassDefs(); for (size_t class_def_index = 0; class_def_index < class_def_count; ++class_def_index) { const DexFile::ClassDef& class_def = dex_file.GetClassDef(class_def_index); @@ -76,15 +86,6 @@ void HiddenApiFinder::CollectAccesses(VeridexResolver* resolver) { CodeItemDataAccessor code_item_accessor(dex_file, code_item); for (const DexInstructionPcPair& inst : code_item_accessor) { switch (inst->Opcode()) { - case Instruction::CONST_CLASS: { - dex::TypeIndex type_index(inst->VRegB_21c()); - std::string name = dex_file.StringByTypeIdx(type_index); - // Only keep classes that are in a restriction list. - if (hidden_api_.IsInRestrictionList(name)) { - classes_.insert(name); - } - break; - } case Instruction::CONST_STRING: { dex::StringIndex string_index(inst->VRegB_21c()); std::string name = std::string(dex_file.StringDataByIdx(string_index)); |