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
Change-Id: Ifae39b0280633836655b22fc212018cb06b65c6c
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index c8bb537..efd7cc2 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -1939,7 +1939,7 @@
 }
 
 JDWP::JdwpError Dbg::GetThreadGroup(JDWP::ObjectId thread_id, JDWP::ExpandBuf* pReply) {
-  ScopedObjectAccess soa(Thread::Current());
+  ScopedObjectAccessUnchecked soa(Thread::Current());
   JDWP::JdwpError error;
   mirror::Object* thread_object = gRegistry->Get<mirror::Object*>(thread_id, &error);
   if (error != JDWP::ERR_NONE) {
@@ -1970,26 +1970,54 @@
   return error;
 }
 
-std::string Dbg::GetThreadGroupName(JDWP::ObjectId thread_group_id) {
-  ScopedObjectAccess soa(Thread::Current());
-  JDWP::JdwpError error;
-  mirror::Object* thread_group = gRegistry->Get<mirror::Object*>(thread_group_id, &error);
-  CHECK(thread_group != nullptr) << error;
-  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 = gRegistry->Get<mirror::Object*>(thread_group_id, error);
+  if (*error != JDWP::ERR_NONE) {
+    return nullptr;
+  }
+  if (thread_group == nullptr) {
+    *error = JDWP::ERR_INVALID_OBJECT;
+    return nullptr;
+  }
   mirror::Class* c = soa.Decode<mirror::Class*>(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<mirror::Class*>(WellKnownClasses::java_lang_ThreadGroup);
   mirror::ArtField* f = c->FindInstanceField("name", "Ljava/lang/String;");
   CHECK(f != nullptr);
   mirror::String* s = reinterpret_cast<mirror::String*>(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());
   JDWP::JdwpError error;
-  mirror::Object* thread_group = gRegistry->Get<mirror::Object*>(thread_group_id, &error);
-  CHECK(thread_group != nullptr) << 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<mirror::Class*>(WellKnownClasses::java_lang_ThreadGroup);
   CHECK(c != nullptr);
@@ -1997,7 +2025,64 @@
   CHECK(f != nullptr);
   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;
+}
+
+static void GetChildThreadGroups(ScopedObjectAccessUnchecked& soa, mirror::Object* thread_group,
+                                 std::vector<JDWP::ObjectId>* child_thread_group_ids)
+    SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+  CHECK(thread_group != nullptr);
+
+  // Get the ArrayList<ThreadGroup> "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<ThreadGroup>...
+  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<mirror::Object>* groups_array =
+      array_field->GetObject(groups_array_list)->AsObjectArray<mirror::Object>();
+  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)));
+  }
+}
+
+JDWP::JdwpError Dbg::GetThreadGroupChildren(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;
+  }
+
+  // Add child threads.
+  {
+    std::vector<JDWP::ObjectId> 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<JDWP::ObjectId> 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::GetSystemThreadGroupId() {
@@ -2007,13 +2092,6 @@
   return gRegistry->Add(group);
 }
 
-JDWP::ObjectId Dbg::GetMainThreadGroupId() {
-  ScopedObjectAccess soa(Thread::Current());
-  mirror::ArtField* f = soa.DecodeField(WellKnownClasses::java_lang_ThreadGroup_mainThreadGroup);
-  mirror::Object* group = f->GetObject(f->GetDeclaringClass());
-  return gRegistry->Add(group);
-}
-
 JDWP::JdwpThreadStatus Dbg::ToJdwpThreadStatus(ThreadState state) {
   switch (state) {
     case kBlocked:
@@ -2111,11 +2189,8 @@
   return (group == desired_thread_group);
 }
 
-void Dbg::GetThreads(JDWP::ObjectId thread_group_id, std::vector<JDWP::ObjectId>* thread_ids) {
+void Dbg::GetThreads(mirror::Object* thread_group, std::vector<JDWP::ObjectId>* thread_ids) {
   ScopedObjectAccessUnchecked soa(Thread::Current());
-  JDWP::JdwpError error;
-  mirror::Object* thread_group = gRegistry->Get<mirror::Object*>(thread_group_id, &error);
-  CHECK_EQ(error, JDWP::ERR_NONE);
   std::list<Thread*> all_threads_list;
   {
     MutexLock mu(Thread::Current(), *Locks::thread_list_lock_);
@@ -2147,30 +2222,6 @@
   }
 }
 
-void Dbg::GetChildThreadGroups(JDWP::ObjectId thread_group_id,
-                               std::vector<JDWP::ObjectId>* child_thread_group_ids) {
-  ScopedObjectAccess soa(Thread::Current());
-  JDWP::JdwpError error;
-  mirror::Object* thread_group = gRegistry->Get<mirror::Object*>(thread_group_id, &error);
-  CHECK(thread_group != nullptr) << error;
-
-  // Get the ArrayList<ThreadGroup> "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<ThreadGroup>...
-  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<mirror::Object>* groups_array =
-      array_field->GetObject(groups_array_list)->AsObjectArray<mirror::Object>();
-  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_) {
   struct CountStackDepthVisitor : public StackVisitor {
     explicit CountStackDepthVisitor(Thread* thread)
diff --git a/runtime/debugger.h b/runtime/debugger.h
index 219210e..ab758ca 100644
--- a/runtime/debugger.h
+++ b/runtime/debugger.h
@@ -393,13 +393,19 @@
       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,
@@ -414,11 +420,9 @@
 
   // 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<JDWP::ObjectId>* thread_ids)
+  static void GetThreads(mirror::Object* thread_group, std::vector<JDWP::ObjectId>* thread_ids)
       LOCKS_EXCLUDED(Locks::thread_list_lock_)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
-  static void GetChildThreadGroups(JDWP::ObjectId thread_group_id,
-                                   std::vector<JDWP::ObjectId>* 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 35095f9..8560cb5 100644
--- a/runtime/jdwp/jdwp_handler.cc
+++ b/runtime/jdwp/jdwp_handler.cc
@@ -220,7 +220,7 @@
 static JdwpError VM_AllThreads(JdwpState*, Request*, ExpandBuf* pReply)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   std::vector<ObjectId> 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) {
@@ -1141,10 +1141,7 @@
 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);
 }
 
 /*
@@ -1154,11 +1151,7 @@
 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);
 }
 
 /*
@@ -1168,22 +1161,7 @@
 static JdwpError TGR_Children(JdwpState*, Request* request, ExpandBuf* pReply)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   ObjectId thread_group_id = request->ReadThreadGroupId();
-
-  std::vector<ObjectId> 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<ObjectId> 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);
 }
 
 /*