Revert "Fix stack-walking race"
This reverts commit 721e40283793649b4750c05da4fe972bd372f7f9.
Reason for revert: Wanted more discussion.
Test: none
Bug: 72608560
Bug: 29259363
Bug: 148166031
Change-Id: Id0fb201018a0d0bdca11d51ec156a41d6d7fe4ae
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index 6639d4b..7ff9b73 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -458,9 +458,9 @@
TEST_DISABLED_FOR_MEMORY_TOOL();
// The `native_alloc_2_ >= native_alloc_1_` assertion below may not
- // hold true on some x86 or x86_64 systems; disable this test while we
+ // hold true on some x86 systems; disable this test while we
// investigate (b/29259363).
- TEST_DISABLED();
+ TEST_DISABLED_FOR_X86();
RunTest(/*use_fd=*/ false,
/*expect_use=*/ false);
diff --git a/runtime/base/locks.h b/runtime/base/locks.h
index 806e84e..c1667f3 100644
--- a/runtime/base/locks.h
+++ b/runtime/base/locks.h
@@ -97,7 +97,6 @@
kTracingStreamingLock,
kClassLoaderClassesLock,
kDefaultMutexLevel,
- kInstrumentationInstallLock,
kDexLock,
kMarkSweepLargeObjectLock,
kJdwpObjectRegistryLock,
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index c9f3247..011d947 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -298,12 +298,6 @@
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) {
@@ -402,14 +396,8 @@
break;
}
}
- {
- // 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();
- }
+ instrumentation_stack_->insert(it, instrumentation_frame);
+ SetReturnPc(instrumentation_exit_pc_);
}
uint32_t dex_pc = dex::kDexNoIndex;
if (last_return_pc_ != 0 && GetCurrentOatQuickMethodHeader() != nullptr) {
@@ -429,7 +417,6 @@
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);
@@ -549,7 +536,6 @@
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);
@@ -557,11 +543,7 @@
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 bc8f8cc..20e97dc 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -21,11 +21,9 @@
#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"
@@ -73,7 +71,6 @@
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),
@@ -848,277 +845,186 @@
DCHECK(thread_ == Thread::Current() || thread_->IsSuspended());
}
CHECK_EQ(cur_depth_, 0U);
- 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;
+ 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;
+ 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);
} 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);
+ const void* code = method->GetOatMethodQuickCode(class_linker->GetImagePointerSize());
+ if (code != nullptr) {
+ cur_oat_quick_method_header_ = OatQuickMethodHeader::FromEntryPoint(code);
} else {
- 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);
- }
+ // 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;
}
- 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++;
- }
- }
- }
-
- if (!skip_to_depth) {
- bool should_continue = VisitFrame();
- if (UNLIKELY(!should_continue)) {
- return;
- }
- }
- 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);
- // }
- }
- }
-
- 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_;
- }
-
- method = *cur_quick_frame_;
- }
- // We want to start again. Break right now.
- if (retry) {
- break; // From for (const ManagedStack* current_fragment = thread_->GetManagedStack();
- }
- } 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);
+ header_retrieved = true;
}
- if (!skip_to_depth && include_transitions) {
+ 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;
+ }
+ cur_depth_++;
+ inlined_frames_count++;
+ }
+ }
+ }
+
bool should_continue = VisitFrame();
- if (!should_continue) {
+ 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 (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);
+ }
+ 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_;
+ }
+
+ if (kCount == CountTransitions::kYes || !method->IsRuntimeMethod()) {
+ cur_depth_++;
+ }
+ method = *cur_quick_frame_;
}
- if (kCount == CountTransitions::kYes) {
- increment_depth();
+ } else if (cur_shadow_frame_ != nullptr) {
+ do {
+ SanityCheckFrame();
+ bool should_continue = VisitFrame();
+ if (UNLIKELY(!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 (retry) {
- continue; // Real retry.
+ if (kCount == CountTransitions::kYes) {
+ cur_depth_++;
}
- if (num_frames_ != 0) {
- CHECK_EQ(cur_depth_, num_frames_);
- }
- } while (retry);
+ }
+ if (num_frames_ != 0) {
+ CHECK_EQ(cur_depth_, num_frames_);
+ }
}
template void StackVisitor::WalkStack<StackVisitor::CountTransitions::kYes>(bool);
diff --git a/runtime/stack.h b/runtime/stack.h
index 6f6c365..ad73e75 100644
--- a/runtime/stack.h
+++ b/runtime/stack.h
@@ -129,10 +129,6 @@
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;
@@ -364,8 +360,6 @@
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 be9bced..c3e4afe 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -2295,15 +2295,8 @@
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 0bee7e5..34434cf 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -607,22 +607,6 @@
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_;
}
@@ -1889,15 +1873,6 @@
// 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
deleted file mode 100644
index 77a1486..0000000
--- a/test/2011-stack-walk-concurrent-instrument/expected.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-JNI_OnLoad called
-Done
diff --git a/test/2011-stack-walk-concurrent-instrument/info.txt b/test/2011-stack-walk-concurrent-instrument/info.txt
deleted file mode 100644
index 91f0106..0000000
--- a/test/2011-stack-walk-concurrent-instrument/info.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-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
deleted file mode 100644
index 8f96f93..0000000
--- a/test/2011-stack-walk-concurrent-instrument/src/Main.java
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * 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
deleted file mode 100644
index 68e58f4..0000000
--- a/test/2011-stack-walk-concurrent-instrument/stack_walk_concurrent.cc
+++ /dev/null
@@ -1,97 +0,0 @@
-/*
- * 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 53ed6ab..89538c6 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -589,7 +589,6 @@
"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",
],