From 5d1c1bbd4c8a1727027d0ae69277af6f6f6badf3 Mon Sep 17 00:00:00 2001 From: Sebastien Hertz Date: Mon, 15 Sep 2014 19:21:30 +0200 Subject: [PATCH] Check for errors in ThreadGroupReference JDWP commands Returns INVALID_OBJECT error for null or invalid object. Also returns INVALID_THREAD_GROUP error when the object is not a java.lang.ThreadGroup. Removes unused Dbg::GetMainThreadGroupId method. Bug: 17503230 (cherry picked from commit a06430c76981d545b5f2b64a7ef53c44c030cf73) Change-Id: Ic39d3d2c45bf288fc22d908a3c90a3ca24f1c4d4 --- runtime/debugger.cc | 143 ++++++++++++++++++++++++++++++------------- runtime/debugger.h | 15 +++-- runtime/jdwp/jdwp_handler.cc | 30 ++------- 3 files changed, 113 insertions(+), 75 deletions(-) diff --git a/runtime/debugger.cc b/runtime/debugger.cc index 7fb199c0e..e074fc17a 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -1979,7 +1979,7 @@ JDWP::JdwpError Dbg::GetThreadName(JDWP::ObjectId thread_id, std::string& name) } JDWP::JdwpError Dbg::GetThreadGroup(JDWP::ObjectId thread_id, JDWP::ExpandBuf* pReply) { - ScopedObjectAccess soa(Thread::Current()); + ScopedObjectAccessUnchecked soa(Thread::Current()); mirror::Object* thread_object = gRegistry->Get(thread_id); if (thread_object == ObjectRegistry::kInvalidObject) { return JDWP::ERR_INVALID_OBJECT; @@ -2010,24 +2010,51 @@ JDWP::JdwpError Dbg::GetThreadGroup(JDWP::ObjectId thread_id, JDWP::ExpandBuf* p return error; } -std::string Dbg::GetThreadGroupName(JDWP::ObjectId thread_group_id) { - ScopedObjectAccess soa(Thread::Current()); - mirror::Object* thread_group = gRegistry->Get(thread_group_id); - CHECK(thread_group != nullptr); - const char* old_cause = soa.Self()->StartAssertNoThreadSuspension("Debugger: GetThreadGroupName"); +static mirror::Object* DecodeThreadGroup(ScopedObjectAccessUnchecked& soa, + JDWP::ObjectId thread_group_id, JDWP::JdwpError* error) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + mirror::Object* thread_group = Dbg::GetObjectRegistry()->Get(thread_group_id); + if (thread_group == nullptr || thread_group == ObjectRegistry::kInvalidObject) { + *error = JDWP::ERR_INVALID_OBJECT; + return nullptr; + } mirror::Class* c = soa.Decode(WellKnownClasses::java_lang_ThreadGroup); CHECK(c != nullptr); + if (!c->IsAssignableFrom(thread_group->GetClass())) { + // This is not a java.lang.ThreadGroup. + *error = JDWP::ERR_INVALID_THREAD_GROUP; + return nullptr; + } + *error = JDWP::ERR_NONE; + return thread_group; +} + +JDWP::JdwpError Dbg::GetThreadGroupName(JDWP::ObjectId thread_group_id, JDWP::ExpandBuf* pReply) { + ScopedObjectAccessUnchecked soa(Thread::Current()); + JDWP::JdwpError error; + mirror::Object* thread_group = DecodeThreadGroup(soa, thread_group_id, &error); + if (error != JDWP::ERR_NONE) { + return error; + } + const char* old_cause = soa.Self()->StartAssertNoThreadSuspension("Debugger: GetThreadGroupName"); + mirror::Class* c = soa.Decode(WellKnownClasses::java_lang_ThreadGroup); mirror::ArtField* f = c->FindInstanceField("name", "Ljava/lang/String;"); CHECK(f != NULL); mirror::String* s = reinterpret_cast(f->GetObject(thread_group)); soa.Self()->EndAssertNoThreadSuspension(old_cause); - return s->ToModifiedUtf8(); + + std::string thread_group_name(s->ToModifiedUtf8()); + expandBufAddUtf8String(pReply, thread_group_name); + return JDWP::ERR_NONE; } -JDWP::ObjectId Dbg::GetThreadGroupParent(JDWP::ObjectId thread_group_id) { +JDWP::JdwpError Dbg::GetThreadGroupParent(JDWP::ObjectId thread_group_id, JDWP::ExpandBuf* pReply) { ScopedObjectAccessUnchecked soa(Thread::Current()); - mirror::Object* thread_group = gRegistry->Get(thread_group_id); - CHECK(thread_group != nullptr); + JDWP::JdwpError error; + mirror::Object* thread_group = DecodeThreadGroup(soa, thread_group_id, &error); + if (error != JDWP::ERR_NONE) { + return error; + } const char* old_cause = soa.Self()->StartAssertNoThreadSuspension("Debugger: GetThreadGroupParent"); mirror::Class* c = soa.Decode(WellKnownClasses::java_lang_ThreadGroup); CHECK(c != nullptr); @@ -2035,19 +2062,70 @@ JDWP::ObjectId Dbg::GetThreadGroupParent(JDWP::ObjectId thread_group_id) { CHECK(f != NULL); mirror::Object* parent = f->GetObject(thread_group); soa.Self()->EndAssertNoThreadSuspension(old_cause); - return gRegistry->Add(parent); + + JDWP::ObjectId parent_group_id = gRegistry->Add(parent); + expandBufAddObjectId(pReply, parent_group_id); + return JDWP::ERR_NONE; } -JDWP::ObjectId Dbg::GetSystemThreadGroupId() { +static void GetChildThreadGroups(ScopedObjectAccessUnchecked& soa, mirror::Object* thread_group, + std::vector* child_thread_group_ids) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + CHECK(thread_group != nullptr); + + // Get the ArrayList "groups" out of this thread group... + mirror::ArtField* groups_field = thread_group->GetClass()->FindInstanceField("groups", "Ljava/util/List;"); + mirror::Object* groups_array_list = groups_field->GetObject(thread_group); + + // Get the array and size out of the ArrayList... + mirror::ArtField* array_field = groups_array_list->GetClass()->FindInstanceField("array", "[Ljava/lang/Object;"); + mirror::ArtField* size_field = groups_array_list->GetClass()->FindInstanceField("size", "I"); + mirror::ObjectArray* groups_array = + array_field->GetObject(groups_array_list)->AsObjectArray(); + const int32_t size = size_field->GetInt(groups_array_list); + + // Copy the first 'size' elements out of the array into the result. + ObjectRegistry* registry = Dbg::GetObjectRegistry(); + for (int32_t i = 0; i < size; ++i) { + child_thread_group_ids->push_back(registry->Add(groups_array->Get(i))); + } +} + +JDWP::JdwpError Dbg::GetThreadGroupChildren(JDWP::ObjectId thread_group_id, + JDWP::ExpandBuf* pReply) { ScopedObjectAccessUnchecked soa(Thread::Current()); - mirror::ArtField* f = soa.DecodeField(WellKnownClasses::java_lang_ThreadGroup_systemThreadGroup); - mirror::Object* group = f->GetObject(f->GetDeclaringClass()); - return gRegistry->Add(group); + JDWP::JdwpError error; + mirror::Object* thread_group = DecodeThreadGroup(soa, thread_group_id, &error); + if (error != JDWP::ERR_NONE) { + return error; + } + + // Add child threads. + { + std::vector child_thread_ids; + GetThreads(thread_group, &child_thread_ids); + expandBufAdd4BE(pReply, child_thread_ids.size()); + for (JDWP::ObjectId child_thread_id : child_thread_ids) { + expandBufAddObjectId(pReply, child_thread_id); + } + } + + // Add child thread groups. + { + std::vector child_thread_groups_ids; + GetChildThreadGroups(soa, thread_group, &child_thread_groups_ids); + expandBufAdd4BE(pReply, child_thread_groups_ids.size()); + for (JDWP::ObjectId child_thread_group_id : child_thread_groups_ids) { + expandBufAddObjectId(pReply, child_thread_group_id); + } + } + + return JDWP::ERR_NONE; } -JDWP::ObjectId Dbg::GetMainThreadGroupId() { - ScopedObjectAccess soa(Thread::Current()); - mirror::ArtField* f = soa.DecodeField(WellKnownClasses::java_lang_ThreadGroup_mainThreadGroup); +JDWP::ObjectId Dbg::GetSystemThreadGroupId() { + ScopedObjectAccessUnchecked soa(Thread::Current()); + mirror::ArtField* f = soa.DecodeField(WellKnownClasses::java_lang_ThreadGroup_systemThreadGroup); mirror::Object* group = f->GetObject(f->GetDeclaringClass()); return gRegistry->Add(group); } @@ -2149,9 +2227,8 @@ static bool IsInDesiredThreadGroup(ScopedObjectAccessUnchecked& soa, return (group == desired_thread_group); } -void Dbg::GetThreads(JDWP::ObjectId thread_group_id, std::vector& thread_ids) { +void Dbg::GetThreads(mirror::Object* thread_group, std::vector* thread_ids) { ScopedObjectAccessUnchecked soa(Thread::Current()); - mirror::Object* thread_group = gRegistry->Get(thread_group_id); std::list all_threads_list; { MutexLock mu(Thread::Current(), *Locks::thread_list_lock_); @@ -2178,34 +2255,12 @@ void Dbg::GetThreads(JDWP::ObjectId thread_group_id, std::vector continue; } if (IsInDesiredThreadGroup(soa, thread_group, peer)) { - thread_ids.push_back(gRegistry->Add(peer)); + thread_ids->push_back(gRegistry->Add(peer)); } } } -void Dbg::GetChildThreadGroups(JDWP::ObjectId thread_group_id, std::vector& child_thread_group_ids) { - ScopedObjectAccess soa(Thread::Current()); - mirror::Object* thread_group = gRegistry->Get(thread_group_id); - - // Get the ArrayList "groups" out of this thread group... - mirror::ArtField* groups_field = thread_group->GetClass()->FindInstanceField("groups", "Ljava/util/List;"); - mirror::Object* groups_array_list = groups_field->GetObject(thread_group); - - // Get the array and size out of the ArrayList... - mirror::ArtField* array_field = groups_array_list->GetClass()->FindInstanceField("array", "[Ljava/lang/Object;"); - mirror::ArtField* size_field = groups_array_list->GetClass()->FindInstanceField("size", "I"); - mirror::ObjectArray* groups_array = - array_field->GetObject(groups_array_list)->AsObjectArray(); - const int32_t size = size_field->GetInt(groups_array_list); - - // Copy the first 'size' elements out of the array into the result. - for (int32_t i = 0; i < size; ++i) { - child_thread_group_ids.push_back(gRegistry->Add(groups_array->Get(i))); - } -} - -static int GetStackDepth(Thread* thread) - SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { +static int GetStackDepth(Thread* thread) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { struct CountStackDepthVisitor : public StackVisitor { explicit CountStackDepthVisitor(Thread* thread) : StackVisitor(thread, NULL), depth(0) {} diff --git a/runtime/debugger.h b/runtime/debugger.h index 5dc39d848..2c81c8d75 100644 --- a/runtime/debugger.h +++ b/runtime/debugger.h @@ -414,13 +414,19 @@ class Dbg { SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(Locks::thread_list_lock_); static JDWP::JdwpError GetThreadGroup(JDWP::ObjectId thread_id, JDWP::ExpandBuf* pReply) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(Locks::thread_list_lock_); - static std::string GetThreadGroupName(JDWP::ObjectId thread_group_id); - static JDWP::ObjectId GetThreadGroupParent(JDWP::ObjectId thread_group_id) + static JDWP::JdwpError GetThreadGroupName(JDWP::ObjectId thread_group_id, + JDWP::ExpandBuf* pReply) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + static JDWP::JdwpError GetThreadGroupParent(JDWP::ObjectId thread_group_id, + JDWP::ExpandBuf* pReply) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + static JDWP::JdwpError GetThreadGroupChildren(JDWP::ObjectId thread_group_id, + JDWP::ExpandBuf* pReply) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); static JDWP::ObjectId GetSystemThreadGroupId() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - static JDWP::ObjectId GetMainThreadGroupId(); static JDWP::JdwpThreadStatus ToJdwpThreadStatus(ThreadState state); static JDWP::JdwpError GetThreadStatus(JDWP::ObjectId thread_id, @@ -435,10 +441,9 @@ class Dbg { // Fills 'thread_ids' with the threads in the given thread group. If thread_group_id == 0, // returns all threads. - static void GetThreads(JDWP::ObjectId thread_group_id, std::vector& thread_ids) + static void GetThreads(mirror::Object* thread_group, std::vector* thread_ids) LOCKS_EXCLUDED(Locks::thread_list_lock_) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - static void GetChildThreadGroups(JDWP::ObjectId thread_group_id, std::vector& child_thread_group_ids); static JDWP::JdwpError GetThreadFrameCount(JDWP::ObjectId thread_id, size_t& result) LOCKS_EXCLUDED(Locks::thread_list_lock_); diff --git a/runtime/jdwp/jdwp_handler.cc b/runtime/jdwp/jdwp_handler.cc index 1d56fe773..6888241c7 100644 --- a/runtime/jdwp/jdwp_handler.cc +++ b/runtime/jdwp/jdwp_handler.cc @@ -225,7 +225,7 @@ static JdwpError VM_ClassesBySignature(JdwpState*, Request& request, ExpandBuf* static JdwpError VM_AllThreads(JdwpState*, Request&, ExpandBuf* pReply) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { std::vector thread_ids; - Dbg::GetThreads(0, thread_ids); + Dbg::GetThreads(nullptr /* all thread groups */, &thread_ids); expandBufAdd4BE(pReply, thread_ids.size()); for (uint32_t i = 0; i < thread_ids.size(); ++i) { @@ -1150,10 +1150,7 @@ static JdwpError TR_DebugSuspendCount(JdwpState*, Request& request, ExpandBuf* p static JdwpError TGR_Name(JdwpState*, Request& request, ExpandBuf* pReply) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { ObjectId thread_group_id = request.ReadThreadGroupId(); - - expandBufAddUtf8String(pReply, Dbg::GetThreadGroupName(thread_group_id)); - - return ERR_NONE; + return Dbg::GetThreadGroupName(thread_group_id, pReply); } /* @@ -1163,11 +1160,7 @@ static JdwpError TGR_Name(JdwpState*, Request& request, ExpandBuf* pReply) static JdwpError TGR_Parent(JdwpState*, Request& request, ExpandBuf* pReply) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { ObjectId thread_group_id = request.ReadThreadGroupId(); - - ObjectId parentGroup = Dbg::GetThreadGroupParent(thread_group_id); - expandBufAddObjectId(pReply, parentGroup); - - return ERR_NONE; + return Dbg::GetThreadGroupParent(thread_group_id, pReply); } /* @@ -1177,22 +1170,7 @@ static JdwpError TGR_Parent(JdwpState*, Request& request, ExpandBuf* pReply) static JdwpError TGR_Children(JdwpState*, Request& request, ExpandBuf* pReply) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { ObjectId thread_group_id = request.ReadThreadGroupId(); - - std::vector thread_ids; - Dbg::GetThreads(thread_group_id, thread_ids); - expandBufAdd4BE(pReply, thread_ids.size()); - for (uint32_t i = 0; i < thread_ids.size(); ++i) { - expandBufAddObjectId(pReply, thread_ids[i]); - } - - std::vector child_thread_groups_ids; - Dbg::GetChildThreadGroups(thread_group_id, child_thread_groups_ids); - expandBufAdd4BE(pReply, child_thread_groups_ids.size()); - for (uint32_t i = 0; i < child_thread_groups_ids.size(); ++i) { - expandBufAddObjectId(pReply, child_thread_groups_ids[i]); - } - - return ERR_NONE; + return Dbg::GetThreadGroupChildren(thread_group_id, pReply); } /* -- 2.11.0