From 26f728661a08062a373a3203b72dc2555c2aed2d Mon Sep 17 00:00:00 2001 From: Sebastien Hertz Date: Tue, 15 Sep 2015 09:52:07 +0200 Subject: [PATCH] Cleanup thread access in StackVisitor Adds method StackVisitor::GetThread to give access to the visited Thread* so we no longer need to copy that pointer in subclasses. Also adds a few missing const and DISALLOW_COPY_AND_ASSIGN. Change-Id: I57649ee7742ef4ef1e01447ac2fbb66f977b22eb --- runtime/debugger.cc | 6 +++--- runtime/profiler.cc | 7 +++++-- runtime/quick_exception_handler.cc | 23 +++++++++++------------ runtime/stack.h | 4 ++++ runtime/thread.cc | 10 ++++++---- runtime/trace.cc | 2 ++ 6 files changed, 31 insertions(+), 21 deletions(-) diff --git a/runtime/debugger.cc b/runtime/debugger.cc index d5691af80..ad69676ea 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -2443,6 +2443,8 @@ class FindFrameVisitor FINAL : public StackVisitor { private: const JDWP::FrameId frame_id_; JDWP::JdwpError error_; + + DISALLOW_COPY_AND_ASSIGN(FindFrameVisitor); }; JDWP::JdwpError Dbg::GetLocalValues(JDWP::Request* request, JDWP::ExpandBuf* pReply) { @@ -2822,7 +2824,6 @@ class CatchLocationFinder : public StackVisitor { CatchLocationFinder(Thread* self, const Handle& exception, Context* context) SHARED_REQUIRES(Locks::mutator_lock_) : StackVisitor(self, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames), - self_(self), exception_(exception), handle_scope_(self), this_at_throw_(handle_scope_.NewHandle(nullptr)), @@ -2852,7 +2853,7 @@ class CatchLocationFinder : public StackVisitor { } if (dex_pc != DexFile::kDexNoIndex) { - StackHandleScope<1> hs(self_); + StackHandleScope<1> hs(GetThread()); uint32_t found_dex_pc; Handle exception_class(hs.NewHandle(exception_->GetClass())); bool unused_clear_exception; @@ -2887,7 +2888,6 @@ class CatchLocationFinder : public StackVisitor { } private: - Thread* const self_; const Handle& exception_; StackHandleScope<1> handle_scope_; MutableHandle this_at_throw_; diff --git a/runtime/profiler.cc b/runtime/profiler.cc index 7e8c551ab..6a77a9ed8 100644 --- a/runtime/profiler.cc +++ b/runtime/profiler.cc @@ -57,7 +57,8 @@ volatile bool BackgroundMethodSamplingProfiler::shutting_down_ = false; class BoundedStackVisitor : public StackVisitor { public: BoundedStackVisitor(std::vector>* stack, - Thread* thread, uint32_t max_depth) + Thread* thread, + uint32_t max_depth) SHARED_REQUIRES(Locks::mutator_lock_) : StackVisitor(thread, nullptr, StackVisitor::StackWalkKind::kIncludeInlinedFrames), stack_(stack), @@ -80,9 +81,11 @@ class BoundedStackVisitor : public StackVisitor { } private: - std::vector>* stack_; + std::vector>* const stack_; const uint32_t max_depth_; uint32_t depth_; + + DISALLOW_COPY_AND_ASSIGN(BoundedStackVisitor); }; // This is called from either a thread list traversal or from a checkpoint. Regardless diff --git a/runtime/quick_exception_handler.cc b/runtime/quick_exception_handler.cc index 60defbaaa..57c6866aa 100644 --- a/runtime/quick_exception_handler.cc +++ b/runtime/quick_exception_handler.cc @@ -47,7 +47,6 @@ class CatchBlockStackVisitor FINAL : public StackVisitor { QuickExceptionHandler* exception_handler) SHARED_REQUIRES(Locks::mutator_lock_) : StackVisitor(self, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames), - self_(self), exception_(exception), exception_handler_(exception_handler) { } @@ -90,7 +89,7 @@ class CatchBlockStackVisitor FINAL : public StackVisitor { } if (dex_pc != DexFile::kDexNoIndex) { bool clear_exception = false; - StackHandleScope<1> hs(self_); + StackHandleScope<1> hs(GetThread()); Handle to_find(hs.NewHandle((*exception_)->GetClass())); uint32_t found_dex_pc = method->FindCatchBlock(to_find, dex_pc, &clear_exception); exception_handler_->SetClearException(clear_exception); @@ -105,7 +104,6 @@ class CatchBlockStackVisitor FINAL : public StackVisitor { return true; // Continue stack walk. } - Thread* const self_; // The exception we're looking for the catch block of. Handle* exception_; // The quick exception handler we're visiting for. @@ -154,7 +152,6 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor { DeoptimizeStackVisitor(Thread* self, Context* context, QuickExceptionHandler* exception_handler) SHARED_REQUIRES(Locks::mutator_lock_) : StackVisitor(self, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames), - self_(self), exception_handler_(exception_handler), prev_shadow_frame_(nullptr), stacked_shadow_frame_pushed_(false) { @@ -171,7 +168,8 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor { // In case there is no deoptimized shadow frame for this upcall, we still // need to push a nullptr to the stack since there is always a matching pop after // the long jump. - self_->PushStackedShadowFrame(nullptr, StackedShadowFrameType::kDeoptimizationShadowFrame); + GetThread()->PushStackedShadowFrame(nullptr, + StackedShadowFrameType::kDeoptimizationShadowFrame); stacked_shadow_frame_pushed_ = true; } return false; // End stack walk. @@ -200,18 +198,19 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor { CHECK(code_item != nullptr) << "No code item for " << PrettyMethod(m); uint16_t num_regs = code_item->registers_size_; uint32_t dex_pc = GetDexPc(); - StackHandleScope<2> hs(self_); // Dex cache, class loader and method. + StackHandleScope<2> hs(GetThread()); // Dex cache, class loader and method. mirror::Class* declaring_class = m->GetDeclaringClass(); Handle h_dex_cache(hs.NewHandle(declaring_class->GetDexCache())); Handle h_class_loader(hs.NewHandle(declaring_class->GetClassLoader())); - verifier::MethodVerifier verifier(self_, h_dex_cache->GetDexFile(), h_dex_cache, h_class_loader, - &m->GetClassDef(), code_item, m->GetDexMethodIndex(), - m, m->GetAccessFlags(), true, true, true, true); + verifier::MethodVerifier verifier(GetThread(), h_dex_cache->GetDexFile(), h_dex_cache, + h_class_loader, &m->GetClassDef(), code_item, + m->GetDexMethodIndex(), m, m->GetAccessFlags(), true, true, + true, true); bool verifier_success = verifier.Verify(); CHECK(verifier_success) << PrettyMethod(m); ShadowFrame* new_frame = ShadowFrame::CreateDeoptimizedFrame(num_regs, nullptr, m, dex_pc); { - ScopedStackedShadowFramePusher pusher(self_, new_frame, + ScopedStackedShadowFramePusher pusher(GetThread(), new_frame, StackedShadowFrameType::kShadowFrameUnderConstruction); const std::vector kinds(verifier.DescribeVRegs(dex_pc)); @@ -318,13 +317,13 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor { // Will be popped after the long jump after DeoptimizeStack(), // right before interpreter::EnterInterpreterFromDeoptimize(). stacked_shadow_frame_pushed_ = true; - self_->PushStackedShadowFrame(new_frame, StackedShadowFrameType::kDeoptimizationShadowFrame); + GetThread()->PushStackedShadowFrame(new_frame, + StackedShadowFrameType::kDeoptimizationShadowFrame); } prev_shadow_frame_ = new_frame; return true; } - Thread* const self_; QuickExceptionHandler* const exception_handler_; ShadowFrame* prev_shadow_frame_; bool stacked_shadow_frame_pushed_; diff --git a/runtime/stack.h b/runtime/stack.h index 25627383f..5bbf003e5 100644 --- a/runtime/stack.h +++ b/runtime/stack.h @@ -441,6 +441,10 @@ class StackVisitor { void WalkStack(bool include_transitions = false) SHARED_REQUIRES(Locks::mutator_lock_); + Thread* GetThread() const { + return thread_; + } + ArtMethod* GetMethod() const SHARED_REQUIRES(Locks::mutator_lock_); bool IsShadowFrame() const { diff --git a/runtime/thread.cc b/runtime/thread.cc index 86ac1407d..6e10368c8 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -1185,7 +1185,6 @@ struct StackDumpVisitor : public StackVisitor { SHARED_REQUIRES(Locks::mutator_lock_) : StackVisitor(thread_in, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames), os(os_in), - thread(thread_in), can_allocate(can_allocate_in), last_method(nullptr), last_line_number(0), @@ -1233,7 +1232,7 @@ struct StackDumpVisitor : public StackVisitor { } os << "\n"; if (frame_count == 0) { - Monitor::DescribeWait(os, thread); + Monitor::DescribeWait(os, GetThread()); } if (can_allocate) { // Visit locks, but do not abort on errors. This would trigger a nested abort. @@ -1269,7 +1268,6 @@ struct StackDumpVisitor : public StackVisitor { } std::ostream& os; - const Thread* thread; const bool can_allocate; ArtMethod* last_method; int last_line_number; @@ -1825,6 +1823,8 @@ class CountStackDepthVisitor : public StackVisitor { uint32_t depth_; uint32_t skip_depth_; bool skipping_; + + DISALLOW_COPY_AND_ASSIGN(CountStackDepthVisitor); }; template @@ -1891,7 +1891,9 @@ class BuildInternalStackTraceVisitor : public StackVisitor { // An array of the methods on the stack, the last entries are the dex PCs. mirror::PointerArray* trace_; // For cross compilation. - size_t pointer_size_; + const size_t pointer_size_; + + DISALLOW_COPY_AND_ASSIGN(BuildInternalStackTraceVisitor); }; template diff --git a/runtime/trace.cc b/runtime/trace.cc index 5a44947d3..e2743ceb1 100644 --- a/runtime/trace.cc +++ b/runtime/trace.cc @@ -73,6 +73,8 @@ class BuildStackTraceVisitor : public StackVisitor { private: std::vector* const method_trace_; + + DISALLOW_COPY_AND_ASSIGN(BuildStackTraceVisitor); }; static const char kTraceTokenChar = '*'; -- 2.11.0