diff options
author | Sebastien Hertz <shertz@google.com> | 2015-04-08 09:36:07 +0200 |
---|---|---|
committer | Sebastien Hertz <shertz@google.com> | 2015-05-18 10:20:08 +0200 |
commit | 6650075fb831379d24cfafc3d56442e742418688 (patch) | |
tree | b69ebc7f02846c8b664380a4979fa2053386e51e /runtime/jdwp | |
parent | d8b19f1869f3cb1b7a6db3268ff2e2e6bbc6543b (diff) | |
download | art-6650075fb831379d24cfafc3d56442e742418688.tar.gz art-6650075fb831379d24cfafc3d56442e742418688.tar.bz2 art-6650075fb831379d24cfafc3d56442e742418688.zip |
JDWP: more GC safety
Ensures GC safety when keeping references that may be moved by GC:
- SingleStepControl: stores ArtMethod* in a GcRoot
- ModBasket: stores references in a StackHandleScope
Bug: 18166750
(cherry picked from commit 261bc044a3575512869586593e99e97cd8b1c321)
Change-Id: I35971a901537956739d1f089d61cb4ea9dc6c93d
Diffstat (limited to 'runtime/jdwp')
-rw-r--r-- | runtime/jdwp/jdwp_event.cc | 89 | ||||
-rw-r--r-- | runtime/jdwp/object_registry.cc | 39 | ||||
-rw-r--r-- | runtime/jdwp/object_registry.h | 16 |
3 files changed, 95 insertions, 49 deletions
diff --git a/runtime/jdwp/jdwp_event.cc b/runtime/jdwp/jdwp_event.cc index ab3f2e4ddd..ff75268daa 100644 --- a/runtime/jdwp/jdwp_event.cc +++ b/runtime/jdwp/jdwp_event.cc @@ -32,6 +32,8 @@ #include "scoped_thread_state_change.h" #include "thread-inl.h" +#include "handle_scope-inl.h" + /* General notes: @@ -108,20 +110,32 @@ namespace JDWP { * Stuff to compare against when deciding if a mod matches. Only the * values for mods valid for the event being evaluated will be filled in. * The rest will be zeroed. + * Must be allocated on the stack only. This is enforced by removing the + * operator new. */ struct ModBasket { - ModBasket() : pLoc(nullptr), thread(nullptr), locationClass(nullptr), exceptionClass(nullptr), - caught(false), field(nullptr), thisPtr(nullptr) { } - - const EventLocation* pLoc; /* LocationOnly */ - std::string className; /* ClassMatch/ClassExclude */ - Thread* thread; /* ThreadOnly */ - mirror::Class* locationClass; /* ClassOnly */ - mirror::Class* exceptionClass; /* ExceptionOnly */ - bool caught; /* ExceptionOnly */ - ArtField* field; /* FieldOnly */ - mirror::Object* thisPtr; /* InstanceOnly */ + explicit ModBasket(Thread* self) + : hs(self), pLoc(nullptr), thread(self), + locationClass(hs.NewHandle<mirror::Class>(nullptr)), + exceptionClass(hs.NewHandle<mirror::Class>(nullptr)), + caught(false), + field(nullptr), + thisPtr(hs.NewHandle<mirror::Object>(nullptr)) { } + + StackHandleScope<3> hs; + const EventLocation* pLoc; /* LocationOnly */ + std::string className; /* ClassMatch/ClassExclude */ + Thread* const thread; /* ThreadOnly */ + MutableHandle<mirror::Class> locationClass; /* ClassOnly */ + MutableHandle<mirror::Class> exceptionClass; /* ExceptionOnly */ + bool caught; /* ExceptionOnly */ + ArtField* field; /* FieldOnly */ + MutableHandle<mirror::Object> thisPtr; /* InstanceOnly */ /* nothing for StepOnly -- handled differently */ + + private: + DISALLOW_ALLOCATION(); // forbids allocation on the heap. + DISALLOW_IMPLICIT_CONSTRUCTORS(ModBasket); }; static bool NeedsFullDeoptimization(JdwpEventKind eventKind) { @@ -457,7 +471,7 @@ static bool ModsMatch(JdwpEvent* pEvent, const ModBasket& basket) } break; case MK_CLASS_ONLY: - if (!Dbg::MatchType(basket.locationClass, pMod->classOnly.refTypeId)) { + if (!Dbg::MatchType(basket.locationClass.Get(), pMod->classOnly.refTypeId)) { return false; } break; @@ -478,7 +492,7 @@ static bool ModsMatch(JdwpEvent* pEvent, const ModBasket& basket) break; case MK_EXCEPTION_ONLY: if (pMod->exceptionOnly.refTypeId != 0 && - !Dbg::MatchType(basket.exceptionClass, pMod->exceptionOnly.refTypeId)) { + !Dbg::MatchType(basket.exceptionClass.Get(), pMod->exceptionOnly.refTypeId)) { return false; } if ((basket.caught && !pMod->exceptionOnly.caught) || @@ -497,7 +511,7 @@ static bool ModsMatch(JdwpEvent* pEvent, const ModBasket& basket) } break; case MK_INSTANCE_ONLY: - if (!Dbg::MatchInstance(pMod->instanceOnly.objectId, basket.thisPtr)) { + if (!Dbg::MatchInstance(pMod->instanceOnly.objectId, basket.thisPtr.Get())) { return false; } break; @@ -825,12 +839,11 @@ void JdwpState::PostLocationEvent(const EventLocation* pLoc, mirror::Object* thi DCHECK(pLoc->method != nullptr); DCHECK_EQ(pLoc->method->IsStatic(), thisPtr == nullptr); - ModBasket basket; + ModBasket basket(Thread::Current()); basket.pLoc = pLoc; - basket.locationClass = pLoc->method->GetDeclaringClass(); - basket.thisPtr = thisPtr; - basket.thread = Thread::Current(); - basket.className = Dbg::GetClassName(basket.locationClass); + basket.locationClass.Assign(pLoc->method->GetDeclaringClass()); + basket.thisPtr.Assign(thisPtr); + basket.className = Dbg::GetClassName(basket.locationClass.Get()); /* * On rare occasions we may need to execute interpreted code in the VM @@ -924,16 +937,15 @@ void JdwpState::PostFieldEvent(const EventLocation* pLoc, ArtField* field, DCHECK_EQ(fieldValue != nullptr, is_modification); DCHECK_EQ(field->IsStatic(), this_object == nullptr); - ModBasket basket; + ModBasket basket(Thread::Current()); basket.pLoc = pLoc; - basket.locationClass = pLoc->method->GetDeclaringClass(); - basket.thisPtr = this_object; - basket.thread = Thread::Current(); - basket.className = Dbg::GetClassName(basket.locationClass); + basket.locationClass.Assign(pLoc->method->GetDeclaringClass()); + basket.thisPtr.Assign(this_object); + basket.className = Dbg::GetClassName(basket.locationClass.Get()); basket.field = field; if (InvokeInProgress()) { - VLOG(jdwp) << "Not posting field event during invoke"; + VLOG(jdwp) << "Not posting field event during invoke (" << basket.className << ")"; return; } @@ -975,7 +987,7 @@ void JdwpState::PostFieldEvent(const EventLocation* pLoc, ArtField* field, uint8_t tag; { ScopedObjectAccessUnchecked soa(Thread::Current()); - tag = Dbg::TagFromObject(soa, basket.thisPtr); + tag = Dbg::TagFromObject(soa, basket.thisPtr.Get()); } for (const JdwpEvent* pEvent : match_list) { @@ -1028,8 +1040,7 @@ void JdwpState::PostThreadChange(Thread* thread, bool start) { return; } - ModBasket basket; - basket.thread = thread; + ModBasket basket(thread); std::vector<JdwpEvent*> match_list; const JdwpEventKind match_kind = (start) ? EK_THREAD_START : EK_THREAD_DEATH; @@ -1106,18 +1117,15 @@ void JdwpState::PostException(const EventLocation* pThrowLoc, mirror::Throwable* VLOG(jdwp) << "Unexpected: exception event with empty throw location"; } - ModBasket basket; + ModBasket basket(Thread::Current()); basket.pLoc = pThrowLoc; if (pThrowLoc->method != nullptr) { - basket.locationClass = pThrowLoc->method->GetDeclaringClass(); - } else { - basket.locationClass = nullptr; + basket.locationClass.Assign(pThrowLoc->method->GetDeclaringClass()); } - basket.thread = Thread::Current(); - basket.className = Dbg::GetClassName(basket.locationClass); - basket.exceptionClass = exception_object->GetClass(); + basket.className = Dbg::GetClassName(basket.locationClass.Get()); + basket.exceptionClass.Assign(exception_object->GetClass()); basket.caught = (pCatchLoc->method != 0); - basket.thisPtr = thisPtr; + basket.thisPtr.Assign(thisPtr); /* don't try to post an exception caused by the debugger */ if (InvokeInProgress()) { @@ -1188,10 +1196,9 @@ void JdwpState::PostException(const EventLocation* pThrowLoc, mirror::Throwable* void JdwpState::PostClassPrepare(mirror::Class* klass) { DCHECK(klass != nullptr); - ModBasket basket; - basket.locationClass = klass; - basket.thread = Thread::Current(); - basket.className = Dbg::GetClassName(basket.locationClass); + ModBasket basket(Thread::Current()); + basket.locationClass.Assign(klass); + basket.className = Dbg::GetClassName(basket.locationClass.Get()); /* suppress class prep caused by debugger */ if (InvokeInProgress()) { @@ -1214,7 +1221,7 @@ void JdwpState::PostClassPrepare(mirror::Class* klass) { // debuggers seem to like that. There might be some advantage to honesty, // since the class may not yet be verified. int status = JDWP::CS_VERIFIED | JDWP::CS_PREPARED; - JDWP::JdwpTypeTag tag = Dbg::GetTypeTag(basket.locationClass); + JDWP::JdwpTypeTag tag = Dbg::GetTypeTag(basket.locationClass.Get()); std::string temp; std::string signature(basket.locationClass->GetDescriptor(&temp)); diff --git a/runtime/jdwp/object_registry.cc b/runtime/jdwp/object_registry.cc index a42a58f601..2b28f7df5a 100644 --- a/runtime/jdwp/object_registry.cc +++ b/runtime/jdwp/object_registry.cc @@ -36,17 +36,45 @@ ObjectRegistry::ObjectRegistry() } JDWP::RefTypeId ObjectRegistry::AddRefType(mirror::Class* c) { - return InternalAdd(c); + return Add(c); } -JDWP::ObjectId ObjectRegistry::Add(mirror::Object* o) { - return InternalAdd(o); +JDWP::RefTypeId ObjectRegistry::AddRefType(Handle<mirror::Class> c_h) { + return Add(c_h); } -JDWP::ObjectId ObjectRegistry::InternalAdd(mirror::Object* o) { +JDWP::ObjectId ObjectRegistry::Add(mirror::Object* o) { if (o == nullptr) { return 0; } + Thread* const self = Thread::Current(); + StackHandleScope<1> hs(self); + return InternalAdd(hs.NewHandle(o)); +} + +// Template instantiations must be declared below. +template<class T> +JDWP::ObjectId ObjectRegistry::Add(Handle<T> obj_h) { + if (obj_h.Get() == nullptr) { + return 0; + } + return InternalAdd(obj_h); +} + +// Explicit template instantiation. +template +SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) +LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::thread_suspend_count_lock_) +JDWP::ObjectId ObjectRegistry::Add(Handle<mirror::Object> obj_h); + +template +SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) +LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::thread_suspend_count_lock_) +JDWP::ObjectId ObjectRegistry::Add(Handle<mirror::Throwable> obj_h); + +template<class T> +JDWP::ObjectId ObjectRegistry::InternalAdd(Handle<T> obj_h) { + CHECK(obj_h.Get() != nullptr); Thread* const self = Thread::Current(); self->AssertNoPendingException(); @@ -55,9 +83,6 @@ JDWP::ObjectId ObjectRegistry::InternalAdd(mirror::Object* o) { Locks::thread_list_lock_->AssertNotHeld(self); Locks::thread_suspend_count_lock_->AssertNotHeld(self); - StackHandleScope<1> hs(self); - Handle<mirror::Object> obj_h(hs.NewHandle(o)); - // Call IdentityHashCode here to avoid a lock level violation between lock_ and monitor_lock. int32_t identity_hash_code = obj_h->IdentityHashCode(); diff --git a/runtime/jdwp/object_registry.h b/runtime/jdwp/object_registry.h index 27a4e55f41..4c149cdac7 100644 --- a/runtime/jdwp/object_registry.h +++ b/runtime/jdwp/object_registry.h @@ -23,6 +23,7 @@ #include <map> #include "base/casts.h" +#include "handle.h" #include "jdwp/jdwp.h" #include "safe_map.h" @@ -65,11 +66,23 @@ class ObjectRegistry { SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::thread_suspend_count_lock_); + JDWP::RefTypeId AddRefType(mirror::Class* c) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::thread_suspend_count_lock_); + template<class T> + JDWP::ObjectId Add(Handle<T> obj_h) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) + LOCKS_EXCLUDED(Locks::thread_list_lock_, + Locks::thread_suspend_count_lock_); + + JDWP::RefTypeId AddRefType(Handle<mirror::Class> c_h) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) + LOCKS_EXCLUDED(Locks::thread_list_lock_, + Locks::thread_suspend_count_lock_); + template<typename T> T Get(JDWP::ObjectId id, JDWP::JdwpError* error) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { if (id == 0) { @@ -98,7 +111,8 @@ class ObjectRegistry { jobject GetJObject(JDWP::ObjectId id) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); private: - JDWP::ObjectId InternalAdd(mirror::Object* o) + template<class T> + JDWP::ObjectId InternalAdd(Handle<T> obj_h) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(lock_, Locks::thread_list_lock_, |