diff options
| -rw-r--r-- | dex2oat/dex2oat_test.cc | 4 | ||||
| -rw-r--r-- | runtime/base/locks.h | 1 | ||||
| -rw-r--r-- | runtime/instrumentation.cc | 22 | ||||
| -rw-r--r-- | runtime/stack.cc | 410 | ||||
| -rw-r--r-- | runtime/stack.h | 6 | ||||
| -rw-r--r-- | runtime/thread.cc | 7 | ||||
| -rw-r--r-- | runtime/thread.h | 25 | ||||
| -rw-r--r-- | test/2011-stack-walk-concurrent-instrument/expected.txt | 2 | ||||
| -rw-r--r-- | test/2011-stack-walk-concurrent-instrument/info.txt | 3 | ||||
| -rw-r--r-- | test/2011-stack-walk-concurrent-instrument/src/Main.java | 66 | ||||
| -rw-r--r-- | test/2011-stack-walk-concurrent-instrument/stack_walk_concurrent.cc | 97 | ||||
| -rw-r--r-- | test/Android.bp | 1 |
12 files changed, 482 insertions, 162 deletions
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc index 7ff9b734b1..6639d4b6c3 100644 --- a/dex2oat/dex2oat_test.cc +++ b/dex2oat/dex2oat_test.cc @@ -458,9 +458,9 @@ TEST_F(Dex2oatSwapUseTest, CheckSwapUsage) { TEST_DISABLED_FOR_MEMORY_TOOL(); // The `native_alloc_2_ >= native_alloc_1_` assertion below may not - // hold true on some x86 systems; disable this test while we + // hold true on some x86 or x86_64 systems; disable this test while we // investigate (b/29259363). - TEST_DISABLED_FOR_X86(); + TEST_DISABLED(); RunTest(/*use_fd=*/ false, /*expect_use=*/ false); diff --git a/runtime/base/locks.h b/runtime/base/locks.h index c1667f3ea7..806e84e773 100644 --- a/runtime/base/locks.h +++ b/runtime/base/locks.h @@ -97,6 +97,7 @@ enum LockLevel : uint8_t { kTracingStreamingLock, kClassLoaderClassesLock, kDefaultMutexLevel, + kInstrumentationInstallLock, kDexLock, kMarkSweepLargeObjectLock, kJdwpObjectRegistryLock, diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc index 011d947ea2..c9f3247002 100644 --- a/runtime/instrumentation.cc +++ b/runtime/instrumentation.cc @@ -298,6 +298,12 @@ void InstrumentationInstallStack(Thread* thread, void* arg) last_return_pc_(0), force_deopt_id_(force_deopt_id) {} + protected: + bool IsStackInstrumentWalk() const override { + return true; + } + + public: bool VisitFrame() override REQUIRES_SHARED(Locks::mutator_lock_) { ArtMethod* m = GetMethod(); if (m == nullptr) { @@ -396,8 +402,14 @@ void InstrumentationInstallStack(Thread* thread, void* arg) break; } } - instrumentation_stack_->insert(it, instrumentation_frame); - SetReturnPc(instrumentation_exit_pc_); + { + // Increment the version count of the instrumentation stack so other threads can see it. + MutexLock mu2(Thread::Current(), + *GetThread()->GetInstrumentationInstallSequenceNumberMutex()); + instrumentation_stack_->insert(it, instrumentation_frame); + SetReturnPc(instrumentation_exit_pc_); + GetThread()->IncrementInstrumentationInstallSequenceNumber(); + } } uint32_t dex_pc = dex::kDexNoIndex; if (last_return_pc_ != 0 && GetCurrentOatQuickMethodHeader() != nullptr) { @@ -417,6 +429,7 @@ void InstrumentationInstallStack(Thread* thread, void* arg) uintptr_t last_return_pc_; uint64_t force_deopt_id_; }; + MutexLock mu(Thread::Current(), *thread->GetInstrumentationInstallMutex()); if (kVerboseInstrumentation) { std::string thread_name; thread->GetThreadName(thread_name); @@ -536,6 +549,7 @@ static void InstrumentationRestoreStack(Thread* thread, void* arg) thread->GetThreadName(thread_name); LOG(INFO) << "Removing exit stubs in " << thread_name; } + MutexLock mu(Thread::Current(), *thread->GetInstrumentationInstallMutex()); std::deque<instrumentation::InstrumentationStackFrame>* stack = thread->GetInstrumentationStack(); if (stack->size() > 0) { Instrumentation* instrumentation = reinterpret_cast<Instrumentation*>(arg); @@ -543,7 +557,11 @@ static void InstrumentationRestoreStack(Thread* thread, void* arg) reinterpret_cast<uintptr_t>(GetQuickInstrumentationExitPc()); RestoreStackVisitor visitor(thread, instrumentation_exit_pc, instrumentation); visitor.WalkStack(true); + MutexLock mu2(Thread::Current(), *thread->GetInstrumentationInstallSequenceNumberMutex()); + thread->IncrementInstrumentationInstallSequenceNumber(); CHECK_EQ(visitor.frames_removed_, stack->size()); + // Since we restore the return-pcs first we don't really need to increment the instrumentation + // install sequence number, still good practice though. while (stack->size() > 0) { stack->pop_front(); } diff --git a/runtime/stack.cc b/runtime/stack.cc index 20e97dc968..bc8f8cc648 100644 --- a/runtime/stack.cc +++ b/runtime/stack.cc @@ -21,9 +21,11 @@ #include "arch/context.h" #include "art_method-inl.h" +#include "base/aborting.h" #include "base/callee_save_type.h" #include "base/enums.h" #include "base/hex_dump.h" +#include "base/mutex.h" #include "dex/dex_file_types.h" #include "entrypoints/entrypoint_utils-inl.h" #include "entrypoints/quick/callee_save_frame.h" @@ -71,6 +73,7 @@ StackVisitor::StackVisitor(Thread* thread, cur_oat_quick_method_header_(nullptr), num_frames_(num_frames), cur_depth_(0), + walk_started_(false), cur_inline_info_(nullptr, CodeInfo()), cur_stack_map_(0, StackMap()), context_(context), @@ -845,186 +848,277 @@ void StackVisitor::WalkStack(bool include_transitions) { DCHECK(thread_ == Thread::Current() || thread_->IsSuspended()); } CHECK_EQ(cur_depth_, 0U); - bool exit_stubs_installed = Runtime::Current()->GetInstrumentation()->AreExitStubsInstalled(); - uint32_t instrumentation_stack_depth = 0; - size_t inlined_frames_count = 0; - - for (const ManagedStack* current_fragment = thread_->GetManagedStack(); - current_fragment != nullptr; current_fragment = current_fragment->GetLink()) { - cur_shadow_frame_ = current_fragment->GetTopShadowFrame(); - cur_quick_frame_ = current_fragment->GetTopQuickFrame(); - cur_quick_frame_pc_ = 0; - cur_oat_quick_method_header_ = nullptr; - if (cur_quick_frame_ != nullptr) { // Handle quick stack frames. - // Can't be both a shadow and a quick fragment. - DCHECK(current_fragment->GetTopShadowFrame() == nullptr); - ArtMethod* method = *cur_quick_frame_; - DCHECK(method != nullptr); - bool header_retrieved = false; - if (method->IsNative()) { - // We do not have a PC for the first frame, so we cannot simply use - // ArtMethod::GetOatQuickMethodHeader() as we're unable to distinguish there - // between GenericJNI frame and JIT-compiled JNI stub; the entrypoint may have - // changed since the frame was entered. The top quick frame tag indicates - // GenericJNI here, otherwise it's either AOT-compiled or JNI-compiled JNI stub. - if (UNLIKELY(current_fragment->GetTopQuickFrameTag())) { - // The generic JNI does not have any method header. - cur_oat_quick_method_header_ = nullptr; - } else { - const void* existing_entry_point = method->GetEntryPointFromQuickCompiledCode(); - CHECK(existing_entry_point != nullptr); - Runtime* runtime = Runtime::Current(); - ClassLinker* class_linker = runtime->GetClassLinker(); - // Check whether we can quickly get the header from the current entrypoint. - if (!class_linker->IsQuickGenericJniStub(existing_entry_point) && - !class_linker->IsQuickResolutionStub(existing_entry_point) && - existing_entry_point != GetQuickInstrumentationEntryPoint()) { - cur_oat_quick_method_header_ = - OatQuickMethodHeader::FromEntryPoint(existing_entry_point); + CHECK(!walk_started_); + walk_started_ = true; + bool retry = false; + size_t depth; + bool skip_to_depth = false; + uint64_t current_sequence_number; + Thread* self = Thread::Current(); + // If the instrumentation stack changes due to DeoptimizeThreadStack or similar while we are + // running this stack-walk we need to restart and try again. + // + // This is because when we perform DeoptimizeThreadStack we replace the return-pc of the threads + // stack-frames with a trampoline, the actual return-address is placed in the threads + // InstrumentationStack. The instrumentation code will fill up this stack as it goes. Since there + // is no good way to map instrumentation stack frames to real ones (or vice versa) and we need to + // know which instrumentation stack frame we are on in order to find the real return-pc using that + // the next stack-frame's size we instead simply keep track of how many instrumentation frames we + // have used (as instrumentation_stack_depth). Each time we need a new instrumentation frame we + // simply increment this value. This works well enough but if InstrumentationInstallStack is + // called concurrently the InstrumentationStack might have frames inserted prior to the point this + // StackVisitor is currently at. This would cause the visitor to get the incorrect + // instrumentation_frame and start incorrectly walking the stack. + // + // To prevent this we keep track of when the last update that changes the indexs of + // instrumentation frames occurs and restart the stack walk if that changes. To simplify the + // process of building stack walkers we also keep track of how many times we have called + // VisitFrame and skip that many calls. This prevents any frames from being visited more than once + // per 'WalkStack' call. + do { + retry = false; + depth = 0; + // Keep track of how far in the stack-walk we are, making sure to also keep track of whether + // we've skipped enough to resume calling 'VisitFrame' after a retry. + auto increment_depth = [&]() { + DCHECK_LE(depth, cur_depth_); + ++depth; + if (skip_to_depth && depth == cur_depth_) { + skip_to_depth = false; + } else if (!skip_to_depth) { + ++cur_depth_; + } + }; + // Wait for any current instrumentation installs to finish, Skip this if we are aborting. + if (LIKELY(gAborting == 0)) { + MutexLock mu(self, *GetThread()->GetInstrumentationInstallMutex()); + } + // Get Current install state id. + { + MutexLock mu(self, *GetThread()->GetInstrumentationInstallSequenceNumberMutex()); + current_sequence_number = GetThread()->GetInstrumentationInstallSequenceNumber(); + } + bool exit_stubs_installed = Runtime::Current()->GetInstrumentation()->AreExitStubsInstalled(); + uint32_t instrumentation_stack_depth = 0; + size_t inlined_frames_count = 0; + + for (const ManagedStack* current_fragment = thread_->GetManagedStack(); + current_fragment != nullptr; + current_fragment = current_fragment->GetLink()) { + cur_shadow_frame_ = current_fragment->GetTopShadowFrame(); + cur_quick_frame_ = current_fragment->GetTopQuickFrame(); + cur_quick_frame_pc_ = 0; + cur_oat_quick_method_header_ = nullptr; + if (cur_quick_frame_ != nullptr) { // Handle quick stack frames. + // Can't be both a shadow and a quick fragment. + DCHECK(current_fragment->GetTopShadowFrame() == nullptr); + ArtMethod* method = *cur_quick_frame_; + DCHECK(method != nullptr); + bool header_retrieved = false; + if (method->IsNative()) { + // We do not have a PC for the first frame, so we cannot simply use + // ArtMethod::GetOatQuickMethodHeader() as we're unable to distinguish there + // between GenericJNI frame and JIT-compiled JNI stub; the entrypoint may have + // changed since the frame was entered. The top quick frame tag indicates + // GenericJNI here, otherwise it's either AOT-compiled or JNI-compiled JNI stub. + if (UNLIKELY(current_fragment->GetTopQuickFrameTag())) { + // The generic JNI does not have any method header. + cur_oat_quick_method_header_ = nullptr; } else { - const void* code = method->GetOatMethodQuickCode(class_linker->GetImagePointerSize()); - if (code != nullptr) { - cur_oat_quick_method_header_ = OatQuickMethodHeader::FromEntryPoint(code); + const void* existing_entry_point = method->GetEntryPointFromQuickCompiledCode(); + CHECK(existing_entry_point != nullptr); + Runtime* runtime = Runtime::Current(); + ClassLinker* class_linker = runtime->GetClassLinker(); + // Check whether we can quickly get the header from the current entrypoint. + if (!class_linker->IsQuickGenericJniStub(existing_entry_point) && + !class_linker->IsQuickResolutionStub(existing_entry_point) && + existing_entry_point != GetQuickInstrumentationEntryPoint()) { + cur_oat_quick_method_header_ = + OatQuickMethodHeader::FromEntryPoint(existing_entry_point); } else { - // This must be a JITted JNI stub frame. - CHECK(runtime->GetJit() != nullptr); - code = runtime->GetJit()->GetCodeCache()->GetJniStubCode(method); - CHECK(code != nullptr) << method->PrettyMethod(); - cur_oat_quick_method_header_ = OatQuickMethodHeader::FromCodePointer(code); + const void* code = method->GetOatMethodQuickCode(class_linker->GetImagePointerSize()); + if (code != nullptr) { + cur_oat_quick_method_header_ = OatQuickMethodHeader::FromEntryPoint(code); + } else { + // This must be a JITted JNI stub frame. + CHECK(runtime->GetJit() != nullptr); + code = runtime->GetJit()->GetCodeCache()->GetJniStubCode(method); + CHECK(code != nullptr) << method->PrettyMethod(); + cur_oat_quick_method_header_ = OatQuickMethodHeader::FromCodePointer(code); + } } } + header_retrieved = true; } - header_retrieved = true; - } - while (method != nullptr) { - if (!header_retrieved) { - cur_oat_quick_method_header_ = method->GetOatQuickMethodHeader(cur_quick_frame_pc_); - } - header_retrieved = false; // Force header retrieval in next iteration. - SanityCheckFrame(); - - if ((walk_kind_ == StackWalkKind::kIncludeInlinedFrames) - && (cur_oat_quick_method_header_ != nullptr) - && cur_oat_quick_method_header_->IsOptimized() - && !method->IsNative() // JNI methods cannot have any inlined frames. - && CodeInfo::HasInlineInfo(cur_oat_quick_method_header_->GetOptimizedCodeInfoPtr())) { - DCHECK_NE(cur_quick_frame_pc_, 0u); - CodeInfo* code_info = GetCurrentInlineInfo(); - StackMap* stack_map = GetCurrentStackMap(); - if (stack_map->IsValid() && stack_map->HasInlineInfo()) { - DCHECK_EQ(current_inline_frames_.size(), 0u); - for (current_inline_frames_ = code_info->GetInlineInfosOf(*stack_map); - !current_inline_frames_.empty(); - current_inline_frames_.pop_back()) { - bool should_continue = VisitFrame(); - if (UNLIKELY(!should_continue)) { - return; + while (method != nullptr) { + if (!header_retrieved) { + cur_oat_quick_method_header_ = method->GetOatQuickMethodHeader(cur_quick_frame_pc_); + } + header_retrieved = false; // Force header retrieval in next iteration. + SanityCheckFrame(); + + if ((walk_kind_ == StackWalkKind::kIncludeInlinedFrames) && + (cur_oat_quick_method_header_ != nullptr) && + cur_oat_quick_method_header_->IsOptimized() && + !method->IsNative() // JNI methods cannot have any inlined frames. + && CodeInfo::HasInlineInfo(cur_oat_quick_method_header_->GetOptimizedCodeInfoPtr())) { + DCHECK_NE(cur_quick_frame_pc_, 0u); + CodeInfo* code_info = GetCurrentInlineInfo(); + StackMap* stack_map = GetCurrentStackMap(); + if (stack_map->IsValid() && stack_map->HasInlineInfo()) { + DCHECK_EQ(current_inline_frames_.size(), 0u); + for (current_inline_frames_ = code_info->GetInlineInfosOf(*stack_map); + !current_inline_frames_.empty(); + current_inline_frames_.pop_back()) { + if (!skip_to_depth) { + bool should_continue = VisitFrame(); + if (UNLIKELY(!should_continue)) { + return; + } + } + increment_depth(); + inlined_frames_count++; } - cur_depth_++; - inlined_frames_count++; } } - } - bool should_continue = VisitFrame(); - if (UNLIKELY(!should_continue)) { - return; - } - - QuickMethodFrameInfo frame_info = GetCurrentQuickFrameInfo(); - if (context_ != nullptr) { - context_->FillCalleeSaves(reinterpret_cast<uint8_t*>(cur_quick_frame_), frame_info); - } - // Compute PC for next stack frame from return PC. - size_t frame_size = frame_info.FrameSizeInBytes(); - size_t return_pc_offset = frame_size - sizeof(void*); - uint8_t* return_pc_addr = reinterpret_cast<uint8_t*>(cur_quick_frame_) + return_pc_offset; - uintptr_t return_pc = *reinterpret_cast<uintptr_t*>(return_pc_addr); - - if (UNLIKELY(exit_stubs_installed || - reinterpret_cast<uintptr_t>(GetQuickInstrumentationExitPc()) == return_pc)) { - // While profiling, the return pc is restored from the side stack, except when walking - // the stack for an exception where the side stack will be unwound in VisitFrame. - if (reinterpret_cast<uintptr_t>(GetQuickInstrumentationExitPc()) == return_pc) { - CHECK_LT(instrumentation_stack_depth, thread_->GetInstrumentationStack()->size()); - const instrumentation::InstrumentationStackFrame& instrumentation_frame = - (*thread_->GetInstrumentationStack())[instrumentation_stack_depth]; - instrumentation_stack_depth++; - if (GetMethod() == - Runtime::Current()->GetCalleeSaveMethod(CalleeSaveType::kSaveAllCalleeSaves)) { - // Skip runtime save all callee frames which are used to deliver exceptions. - } else if (instrumentation_frame.interpreter_entry_) { - ArtMethod* callee = - Runtime::Current()->GetCalleeSaveMethod(CalleeSaveType::kSaveRefsAndArgs); - CHECK_EQ(GetMethod(), callee) << "Expected: " << ArtMethod::PrettyMethod(callee) - << " Found: " << ArtMethod::PrettyMethod(GetMethod()); - } else { - // Instrumentation generally doesn't distinguish between a method's obsolete and - // non-obsolete version. - CHECK_EQ(instrumentation_frame.method_->GetNonObsoleteMethod(), - GetMethod()->GetNonObsoleteMethod()) - << "Expected: " - << ArtMethod::PrettyMethod(instrumentation_frame.method_->GetNonObsoleteMethod()) - << " Found: " << ArtMethod::PrettyMethod(GetMethod()->GetNonObsoleteMethod()); + if (!skip_to_depth) { + bool should_continue = VisitFrame(); + if (UNLIKELY(!should_continue)) { + return; } - if (num_frames_ != 0) { - // Check agreement of frame Ids only if num_frames_ is computed to avoid infinite - // recursion. - size_t frame_id = instrumentation::Instrumentation::ComputeFrameId( - thread_, - cur_depth_, - inlined_frames_count); - CHECK_EQ(instrumentation_frame.frame_id_, frame_id); + } + if (kCount == CountTransitions::kYes || !method->IsRuntimeMethod()) { + increment_depth(); + } + + // NB This uses cur_oat_quick_frame_info_ (and hence cur_quick_frame_pc_ & the previous + // return_pc_) to determine the frame information. + QuickMethodFrameInfo frame_info = GetCurrentQuickFrameInfo(); + if (context_ != nullptr) { + context_->FillCalleeSaves(reinterpret_cast<uint8_t*>(cur_quick_frame_), frame_info); + } + // Compute PC for next stack frame from return PC. + size_t frame_size = frame_info.FrameSizeInBytes(); + size_t return_pc_offset = frame_size - sizeof(void*); + uint8_t* return_pc_addr = reinterpret_cast<uint8_t*>(cur_quick_frame_) + return_pc_offset; + uintptr_t return_pc = *reinterpret_cast<uintptr_t*>(return_pc_addr); + + if (UNLIKELY(exit_stubs_installed || + reinterpret_cast<uintptr_t>(GetQuickInstrumentationExitPc()) == return_pc)) { + // While profiling, the return pc is restored from the side stack, except when walking + // the stack for an exception where the side stack will be unwound in VisitFrame. + if (reinterpret_cast<uintptr_t>(GetQuickInstrumentationExitPc()) == return_pc) { + MutexLock mu(self, *GetThread()->GetInstrumentationInstallSequenceNumberMutex()); + // Instrument-install walks will change the instrumentation-stack but only put things + // at the end, so we can continue regardless. We hold the InstrumentationInstallMutex + // so we know nothing can mess up the stack. + if (!IsStackInstrumentWalk() && + GetThread()->GetInstrumentationInstallSequenceNumber() != + current_sequence_number) { + if (kDebugStackWalk) { + LOG(INFO) << "Retrying walk from begining due to instrumentation change. " + << "Skipping to depth " << cur_depth_; + } + skip_to_depth = true; + retry = true; + // Annoyingly c++ doesn't have a labled continue. If they respected RAII this would + // be a good time to use goto but as is we'll just break twice. + break; // From while (method != nullptr) + } else if (kIsDebugBuild && IsStackInstrumentWalk()) { + GetThread()->GetInstrumentationInstallMutex()->AssertExclusiveHeld(self); + } + CHECK_LT(instrumentation_stack_depth, thread_->GetInstrumentationStack()->size()); + const instrumentation::InstrumentationStackFrame& instrumentation_frame = + (*thread_->GetInstrumentationStack())[instrumentation_stack_depth]; + instrumentation_stack_depth++; + if (GetMethod() == + Runtime::Current()->GetCalleeSaveMethod(CalleeSaveType::kSaveAllCalleeSaves)) { + // Skip runtime save all callee frames which are used to deliver exceptions. + } else if (instrumentation_frame.interpreter_entry_) { + ArtMethod* callee = + Runtime::Current()->GetCalleeSaveMethod(CalleeSaveType::kSaveRefsAndArgs); + CHECK_EQ(GetMethod(), callee) + << "Expected: " << ArtMethod::PrettyMethod(callee) + << " Found: " << ArtMethod::PrettyMethod(GetMethod()); + } else { + // Instrumentation generally doesn't distinguish between a method's obsolete and + // non-obsolete version. + CHECK_EQ(instrumentation_frame.method_->GetNonObsoleteMethod(), + GetMethod()->GetNonObsoleteMethod()) + << "Expected: " + << ArtMethod::PrettyMethod( + instrumentation_frame.method_->GetNonObsoleteMethod()) + << " Found: " << ArtMethod::PrettyMethod(GetMethod()->GetNonObsoleteMethod()); + } + return_pc = instrumentation_frame.return_pc_; + // TODO It would be nice to do this check but unfortunately it leads to a race since + // we need to perform a recursive stack-walk to calculate the frame-id. + // size_t expected_id = instrumentation_frame.frame_id_; + // if (num_frames_ != 0) { + // // Check agreement of frame Ids only if num_frames_ is computed to avoid infinite + // // recursion. + // size_t frame_id = instrumentation::Instrumentation::ComputeFrameId( + // thread_, + // // We already incremented depth but we want to get the last visited frame. + // depth - 1, + // inlined_frames_count); + // CHECK_EQ(expected_id, frame_id); + // } } - return_pc = instrumentation_frame.return_pc_; } - } - cur_quick_frame_pc_ = return_pc; - uint8_t* next_frame = reinterpret_cast<uint8_t*>(cur_quick_frame_) + frame_size; - cur_quick_frame_ = reinterpret_cast<ArtMethod**>(next_frame); - - if (kDebugStackWalk) { - LOG(INFO) << ArtMethod::PrettyMethod(method) << "@" << method << " size=" << frame_size - << std::boolalpha - << " optimized=" << (cur_oat_quick_method_header_ != nullptr && - cur_oat_quick_method_header_->IsOptimized()) - << " native=" << method->IsNative() - << std::noboolalpha - << " entrypoints=" << method->GetEntryPointFromQuickCompiledCode() - << "," << (method->IsNative() ? method->GetEntryPointFromJni() : nullptr) - << " next=" << *cur_quick_frame_; - } + cur_quick_frame_pc_ = return_pc; + uint8_t* next_frame = reinterpret_cast<uint8_t*>(cur_quick_frame_) + frame_size; + cur_quick_frame_ = reinterpret_cast<ArtMethod**>(next_frame); + + if (kDebugStackWalk) { + LOG(INFO) << ArtMethod::PrettyMethod(method) << "@" << method << " size=" << frame_size + << std::boolalpha << " optimized=" + << (cur_oat_quick_method_header_ != nullptr && + cur_oat_quick_method_header_->IsOptimized()) + << " native=" << method->IsNative() << std::noboolalpha + << " entrypoints=" << method->GetEntryPointFromQuickCompiledCode() << "," + << (method->IsNative() ? method->GetEntryPointFromJni() : nullptr) + << " next=" << *cur_quick_frame_; + } - if (kCount == CountTransitions::kYes || !method->IsRuntimeMethod()) { - cur_depth_++; + method = *cur_quick_frame_; + } + // We want to start again. Break right now. + if (retry) { + break; // From for (const ManagedStack* current_fragment = thread_->GetManagedStack(); } - method = *cur_quick_frame_; + } else if (cur_shadow_frame_ != nullptr) { + do { + SanityCheckFrame(); + if (!skip_to_depth) { + bool should_continue = VisitFrame(); + if (UNLIKELY(!should_continue)) { + return; + } + } + increment_depth(); + cur_shadow_frame_ = cur_shadow_frame_->GetLink(); + } while (cur_shadow_frame_ != nullptr); } - } else if (cur_shadow_frame_ != nullptr) { - do { - SanityCheckFrame(); + if (!skip_to_depth && include_transitions) { bool should_continue = VisitFrame(); - if (UNLIKELY(!should_continue)) { + if (!should_continue) { return; } - cur_depth_++; - cur_shadow_frame_ = cur_shadow_frame_->GetLink(); - } while (cur_shadow_frame_ != nullptr); - } - if (include_transitions) { - bool should_continue = VisitFrame(); - if (!should_continue) { - return; + } + if (kCount == CountTransitions::kYes) { + increment_depth(); } } - if (kCount == CountTransitions::kYes) { - cur_depth_++; + if (retry) { + continue; // Real retry. } - } - if (num_frames_ != 0) { - CHECK_EQ(cur_depth_, num_frames_); - } + if (num_frames_ != 0) { + CHECK_EQ(cur_depth_, num_frames_); + } + } while (retry); } template void StackVisitor::WalkStack<StackVisitor::CountTransitions::kYes>(bool); diff --git a/runtime/stack.h b/runtime/stack.h index ad73e75fb5..6f6c365f5d 100644 --- a/runtime/stack.h +++ b/runtime/stack.h @@ -129,6 +129,10 @@ class StackVisitor { bool GetRegisterIfAccessible(uint32_t reg, VRegKind kind, uint32_t* val) const REQUIRES_SHARED(Locks::mutator_lock_); + virtual bool IsStackInstrumentWalk() const { + return false; + } + public: virtual ~StackVisitor() {} StackVisitor(const StackVisitor&) = default; @@ -360,6 +364,8 @@ class StackVisitor { size_t num_frames_; // Depth of the frame we're currently at. size_t cur_depth_; + // Whether we've received an initial WalkStack call. + bool walk_started_; // Current inlined frames of the method we are currently at. // We keep poping frames from the end as we visit the frames. BitTableRange<InlineInfo> current_inline_frames_; diff --git a/runtime/thread.cc b/runtime/thread.cc index c3e4afe768..be9bceddf7 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -2295,8 +2295,15 @@ void Thread::NotifyThreadGroup(ScopedObjectAccessAlreadyRunnable& soa, jobject t Thread::Thread(bool daemon) : tls32_(daemon), + instrumentation_install_sequence_number_(0), wait_monitor_(nullptr), is_runtime_thread_(false) { + instrumentation_install_mutex_ = new Mutex( + "a thread instrumentation install mutex", + LockLevel::kInstrumentationInstallLock, + /*recursive=*/true); + instrumentation_install_sequence_number_mutex_ = new Mutex( + "a thread instrumentation install sequence number mutex", LockLevel::kGenericBottomLock); wait_mutex_ = new Mutex("a thread wait mutex", LockLevel::kThreadWaitLock); wait_cond_ = new ConditionVariable("a thread wait condition variable", *wait_mutex_); tlsPtr_.instrumentation_stack = new std::deque<instrumentation::InstrumentationStackFrame>; diff --git a/runtime/thread.h b/runtime/thread.h index 34434cf68c..0bee7e5add 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -607,6 +607,22 @@ class Thread { void NotifyLocked(Thread* self) REQUIRES(wait_mutex_); public: + Mutex* GetInstrumentationInstallMutex() const LOCK_RETURNED(instrumentation_install_mutex_) { + return instrumentation_install_mutex_; + } + Mutex* GetInstrumentationInstallSequenceNumberMutex() const + LOCK_RETURNED(instrumentation_install_sequence_number_mutex_) { + return instrumentation_install_sequence_number_mutex_; + } + uint64_t GetInstrumentationInstallSequenceNumber() const + REQUIRES(instrumentation_install_sequence_number_mutex_) { + return instrumentation_install_sequence_number_; + } + void IncrementInstrumentationInstallSequenceNumber() + REQUIRES(instrumentation_install_sequence_number_mutex_) { + ++instrumentation_install_sequence_number_; + } + Mutex* GetWaitMutex() const LOCK_RETURNED(wait_mutex_) { return wait_mutex_; } @@ -1873,6 +1889,15 @@ class Thread { // All fields below this line should not be accessed by native code. This means these fields can // be modified, rearranged, added or removed without having to modify asm_support.h + // Mutex that guards instrumentation installation. + Mutex* instrumentation_install_mutex_ DEFAULT_MUTEX_ACQUIRED_AFTER; + + // Mutex and id to enable stack-walking to recognize that a concurrent instrumentation-install has + // occurred and stack-walking will need to retry. + Mutex* instrumentation_install_sequence_number_mutex_ BOTTOM_MUTEX_ACQUIRED_AFTER; + uint64_t instrumentation_install_sequence_number_ + GUARDED_BY(instrumentation_install_sequence_number_mutex_); + // Guards the 'wait_monitor_' members. Mutex* wait_mutex_ DEFAULT_MUTEX_ACQUIRED_AFTER; diff --git a/test/2011-stack-walk-concurrent-instrument/expected.txt b/test/2011-stack-walk-concurrent-instrument/expected.txt new file mode 100644 index 0000000000..77a1486479 --- /dev/null +++ b/test/2011-stack-walk-concurrent-instrument/expected.txt @@ -0,0 +1,2 @@ +JNI_OnLoad called +Done diff --git a/test/2011-stack-walk-concurrent-instrument/info.txt b/test/2011-stack-walk-concurrent-instrument/info.txt new file mode 100644 index 0000000000..91f0106279 --- /dev/null +++ b/test/2011-stack-walk-concurrent-instrument/info.txt @@ -0,0 +1,3 @@ +Tests concurrently instrumenting a thread while walking a stack doesn't crash/break. + +Bug: 72608560 diff --git a/test/2011-stack-walk-concurrent-instrument/src/Main.java b/test/2011-stack-walk-concurrent-instrument/src/Main.java new file mode 100644 index 0000000000..8f96f937c9 --- /dev/null +++ b/test/2011-stack-walk-concurrent-instrument/src/Main.java @@ -0,0 +1,66 @@ +/* + * Copyright 2020 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.util.concurrent.*; + +public class Main { + public Main() { + } + + void $noinline$f(Runnable r) throws Exception { + $noinline$g(r); + } + + void $noinline$g(Runnable r) { + $noinline$h(r); + } + + void $noinline$h(Runnable r) { + r.run(); + } + + public native void resetTest(); + public native void waitAndDeopt(Thread t); + public native void doSelfStackWalk(); + + void testConcurrent() throws Exception { + resetTest(); + final Thread current = Thread.currentThread(); + Thread t = new Thread(() -> { + try { + this.waitAndDeopt(current); + } catch (Exception e) { + throw new Error("Fail!", e); + } + }); + t.start(); + $noinline$f(() -> { + try { + this.doSelfStackWalk(); + } catch (Exception e) { + throw new Error("Fail!", e); + } + }); + t.join(); + } + + public static void main(String[] args) throws Exception { + System.loadLibrary(args[0]); + Main st = new Main(); + st.testConcurrent(); + System.out.println("Done"); + } +} diff --git a/test/2011-stack-walk-concurrent-instrument/stack_walk_concurrent.cc b/test/2011-stack-walk-concurrent-instrument/stack_walk_concurrent.cc new file mode 100644 index 0000000000..68e58f4143 --- /dev/null +++ b/test/2011-stack-walk-concurrent-instrument/stack_walk_concurrent.cc @@ -0,0 +1,97 @@ +/* + * Copyright 2020 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. + */ + +#include <atomic> +#include <string_view> + +#include "arch/context.h" +#include "art_method-inl.h" +#include "jni.h" +#include "scoped_thread_state_change.h" +#include "stack.h" +#include "thread.h" + +namespace art { +namespace StackWalkConcurrentInstrument { + +std::atomic<bool> instrument_waiting = false; +std::atomic<bool> instrumented = false; + +// Spin lock. +static void WaitForInstrument() REQUIRES_SHARED(Locks::mutator_lock_) { + ScopedThreadSuspension sts(Thread::Current(), ThreadState::kWaitingForDeoptimization); + instrument_waiting = true; + while (!instrumented) { + } +} + +class SelfStackWalkVisitor : public StackVisitor { + public: + explicit SelfStackWalkVisitor(Thread* thread) REQUIRES_SHARED(Locks::mutator_lock_) + : StackVisitor(thread, Context::Create(), StackWalkKind::kIncludeInlinedFrames) {} + + bool VisitFrame() override REQUIRES_SHARED(Locks::mutator_lock_) { + if (GetMethod()->GetNameView() == "$noinline$f") { + CHECK(!found_f_); + found_f_ = true; + } else if (GetMethod()->GetNameView() == "$noinline$g") { + CHECK(!found_g_); + found_g_ = true; + WaitForInstrument(); + } else if (GetMethod()->GetNameView() == "$noinline$h") { + CHECK(!found_h_); + found_h_ = true; + } + return true; + } + + bool found_f_ = false; + bool found_g_ = false; + bool found_h_ = false; +}; + +extern "C" JNIEXPORT void JNICALL Java_Main_resetTest(JNIEnv*, jobject) { + instrument_waiting = false; + instrumented = false; +} + +extern "C" JNIEXPORT void JNICALL Java_Main_doSelfStackWalk(JNIEnv*, jobject) { + ScopedObjectAccess soa(Thread::Current()); + SelfStackWalkVisitor sswv(Thread::Current()); + sswv.WalkStack(); + CHECK(sswv.found_f_); + CHECK(sswv.found_g_); + CHECK(sswv.found_h_); +} +extern "C" JNIEXPORT void JNICALL Java_Main_waitAndDeopt(JNIEnv*, jobject, jobject target) { + while (!instrument_waiting) { + } + bool timed_out = false; + Thread* other = Runtime::Current()->GetThreadList()->SuspendThreadByPeer( + target, true, SuspendReason::kInternal, &timed_out); + CHECK(!timed_out); + CHECK(other != nullptr); + ScopedObjectAccess soa(Thread::Current()); + Runtime::Current()->GetInstrumentation()->InstrumentThreadStack(other); + MutexLock mu(Thread::Current(), *Locks::thread_suspend_count_lock_); + bool updated = other->ModifySuspendCount(Thread::Current(), -1, nullptr, SuspendReason::kInternal); + CHECK(updated); + instrumented = true; + return; +} + +} // namespace StackWalkConcurrentInstrument +} // namespace art diff --git a/test/Android.bp b/test/Android.bp index 89538c6751..53ed6ab1ca 100644 --- a/test/Android.bp +++ b/test/Android.bp @@ -589,6 +589,7 @@ cc_defaults { "1947-breakpoint-redefine-deopt/check_deopt.cc", "1972-jni-id-swap-indices/jni_id.cc", "1985-structural-redefine-stack-scope/stack_scope.cc", + "2011-stack-walk-concurrent-instrument/stack_walk_concurrent.cc", "common/runtime_state.cc", "common/stack_inspect.cc", ], |