Fix a bunch of JDWP bugs.

Most of these were broken in dalvikvm too.

Bug: http://code.google.com/p/android/issues/detail?id=20856
Change-Id: I88bc89e00a19edc21953cd4f42833f35bb5456a8
diff --git a/src/debugger.cc b/src/debugger.cc
index ef3ac88..4c47e00 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -160,11 +160,11 @@
     return JDWP::JT_STRING;
   } else if (c->IsClassClass()) {
     return JDWP::JT_CLASS_OBJECT;
-  } else if (c->InstanceOf(class_linker->FindSystemClass("Ljava/lang/Thread;"))) {
+  } else if (class_linker->FindSystemClass("Ljava/lang/Thread;")->IsAssignableFrom(c)) {
     return JDWP::JT_THREAD;
-  } else if (c->InstanceOf(class_linker->FindSystemClass("Ljava/lang/ThreadGroup;"))) {
+  } else if (class_linker->FindSystemClass("Ljava/lang/ThreadGroup;")->IsAssignableFrom(c)) {
     return JDWP::JT_THREAD_GROUP;
-  } else if (c->InstanceOf(class_linker->FindSystemClass("Ljava/lang/ClassLoader;"))) {
+  } else if (class_linker->FindSystemClass("Ljava/lang/ClassLoader;")->IsAssignableFrom(c)) {
     return JDWP::JT_CLASS_LOADER;
   } else {
     return JDWP::JT_OBJECT;
@@ -464,13 +464,48 @@
   return true;
 }
 
-bool Dbg::GetSuperclass(JDWP::RefTypeId id, JDWP::RefTypeId& superclassId) {
+static Array* DecodeArray(JDWP::RefTypeId id, JDWP::JdwpError& status) {
   Object* o = gRegistry->Get<Object*>(id);
-  if (o == NULL || !o->IsClass()) {
-    return false;
+  if (o == NULL) {
+    status = JDWP::ERR_INVALID_OBJECT;
+    return NULL;
   }
-  superclassId = gRegistry->Add(o->AsClass()->GetSuperClass());
-  return true;
+  if (!o->IsArrayInstance()) {
+    status = JDWP::ERR_INVALID_ARRAY;
+    return NULL;
+  }
+  status = JDWP::ERR_NONE;
+  return o->AsArray();
+}
+
+// TODO: this should probably be used everywhere we're converting a RefTypeId to a Class*.
+static Class* DecodeClass(JDWP::RefTypeId id, JDWP::JdwpError& status) {
+  Object* o = gRegistry->Get<Object*>(id);
+  if (o == NULL) {
+    status = JDWP::ERR_INVALID_OBJECT;
+    return NULL;
+  }
+  if (!o->IsClass()) {
+    status = JDWP::ERR_INVALID_CLASS;
+    return NULL;
+  }
+  status = JDWP::ERR_NONE;
+  return o->AsClass();
+}
+
+JDWP::JdwpError Dbg::GetSuperclass(JDWP::RefTypeId id, JDWP::RefTypeId& superclassId) {
+  JDWP::JdwpError status;
+  Class* c = DecodeClass(id, status);
+  if (c == NULL) {
+    return status;
+  }
+  if (c->IsInterface()) {
+    // http://code.google.com/p/android/issues/detail?id=20856
+    superclassId = NULL;
+  } else {
+    superclassId = gRegistry->Add(c->GetSuperClass());
+  }
+  return JDWP::ERR_NONE;
 }
 
 JDWP::ObjectId Dbg::GetClassLoader(JDWP::RefTypeId id) {
@@ -630,34 +665,33 @@
   }
 }
 
-int Dbg::GetArrayLength(JDWP::ObjectId arrayId) {
-  Object* o = gRegistry->Get<Object*>(arrayId);
-  Array* a = o->AsArray();
-  return a->GetLength();
-}
-
-uint8_t Dbg::GetArrayElementTag(JDWP::ObjectId arrayId) {
-  Object* o = gRegistry->Get<Object*>(arrayId);
-  Array* a = o->AsArray();
-  std::string descriptor(ClassHelper(a->GetClass()).GetDescriptor());
-  JDWP::JdwpTag tag = BasicTagFromDescriptor(descriptor.c_str() + 1);
-  if (!IsPrimitiveTag(tag)) {
-    tag = TagFromClass(a->GetClass()->GetComponentType());
+JDWP::JdwpError Dbg::GetArrayLength(JDWP::ObjectId arrayId, int& length) {
+  JDWP::JdwpError status;
+  Array* a = DecodeArray(arrayId, status);
+  if (a == NULL) {
+    return status;
   }
-  return tag;
+  length = a->GetLength();
+  return JDWP::ERR_NONE;
 }
 
-bool Dbg::OutputArray(JDWP::ObjectId arrayId, int offset, int count, JDWP::ExpandBuf* pReply) {
-  Object* o = gRegistry->Get<Object*>(arrayId);
-  Array* a = o->AsArray();
+JDWP::JdwpError Dbg::OutputArray(JDWP::ObjectId arrayId, int offset, int count, JDWP::ExpandBuf* pReply) {
+  JDWP::JdwpError status;
+  Array* a = DecodeArray(arrayId, status);
+  if (a == NULL) {
+    return status;
+  }
 
   if (offset < 0 || count < 0 || offset > a->GetLength() || a->GetLength() - offset < count) {
     LOG(WARNING) << __FUNCTION__ << " access out of bounds: offset=" << offset << "; count=" << count;
-    return false;
+    return JDWP::ERR_INVALID_LENGTH;
   }
   std::string descriptor(ClassHelper(a->GetClass()).GetDescriptor());
   JDWP::JdwpTag tag = BasicTagFromDescriptor(descriptor.c_str() + 1);
 
+  expandBufAdd1(pReply, tag);
+  expandBufAdd4BE(pReply, count);
+
   if (IsPrimitiveTag(tag)) {
     size_t width = GetTagWidth(tag);
     const uint8_t* src = reinterpret_cast<uint8_t*>(a->GetRawData());
@@ -684,16 +718,19 @@
     }
   }
 
-  return true;
+  return JDWP::ERR_NONE;
 }
 
-bool Dbg::SetArrayElements(JDWP::ObjectId arrayId, int offset, int count, const uint8_t* src) {
-  Object* o = gRegistry->Get<Object*>(arrayId);
-  Array* a = o->AsArray();
+JDWP::JdwpError Dbg::SetArrayElements(JDWP::ObjectId arrayId, int offset, int count, const uint8_t* src) {
+  JDWP::JdwpError status;
+  Array* a = DecodeArray(arrayId, status);
+  if (a == NULL) {
+    return status;
+  }
 
   if (offset < 0 || count < 0 || offset > a->GetLength() || a->GetLength() - offset < count) {
     LOG(WARNING) << __FUNCTION__ << " access out of bounds: offset=" << offset << "; count=" << count;
-    return false;
+    return JDWP::ERR_INVALID_LENGTH;
   }
   std::string descriptor(ClassHelper(a->GetClass()).GetDescriptor());
   JDWP::JdwpTag tag = BasicTagFromDescriptor(descriptor.c_str() + 1);
@@ -726,7 +763,7 @@
     }
   }
 
-  return true;
+  return JDWP::ERR_NONE;
 }
 
 JDWP::ObjectId Dbg::CreateString(const std::string& str) {
@@ -1047,7 +1084,7 @@
   }
 }
 
-void Dbg::SetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, uint64_t value, int width) {
+JDWP::JdwpError Dbg::SetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, uint64_t value, int width) {
   Object* o = gRegistry->Get<Object*>(objectId);
   Field* f = FromFieldId(fieldId);
 
@@ -1060,16 +1097,23 @@
       f->Set32(o, value);
     }
   } else {
-    f->SetObject(o, gRegistry->Get<Object*>(value));
+    Object* v = gRegistry->Get<Object*>(value);
+    Class* field_type = FieldHelper(f).GetType();
+    if (!field_type->IsAssignableFrom(v->GetClass())) {
+      return JDWP::ERR_INVALID_OBJECT;
+    }
+    f->SetObject(o, v);
   }
+
+  return JDWP::ERR_NONE;
 }
 
 void Dbg::GetStaticFieldValue(JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply) {
   GetFieldValue(0, fieldId, pReply);
 }
 
-void Dbg::SetStaticFieldValue(JDWP::FieldId fieldId, uint64_t value, int width) {
-  SetFieldValue(0, fieldId, value, width);
+JDWP::JdwpError Dbg::SetStaticFieldValue(JDWP::FieldId fieldId, uint64_t value, int width) {
+  return SetFieldValue(0, fieldId, value, width);
 }
 
 std::string Dbg::StringToUtf8(JDWP::ObjectId strId) {
diff --git a/src/debugger.h b/src/debugger.h
index 04066cd..b92fbfe 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -130,7 +130,7 @@
    */
   static std::string GetClassDescriptor(JDWP::RefTypeId id);
   static bool GetClassObject(JDWP::RefTypeId id, JDWP::ObjectId& classObjectId);
-  static bool GetSuperclass(JDWP::RefTypeId id, JDWP::RefTypeId& superclassId);
+  static JDWP::JdwpError GetSuperclass(JDWP::RefTypeId id, JDWP::RefTypeId& superclassId);
   static JDWP::ObjectId GetClassLoader(JDWP::RefTypeId id);
   static bool GetAccessFlags(JDWP::RefTypeId id, uint32_t& access_flags);
   static bool IsInterface(JDWP::RefTypeId classId, bool& is_interface);
@@ -145,10 +145,9 @@
   static uint8_t GetObjectTag(JDWP::ObjectId objectId);
   static size_t GetTagWidth(JDWP::JdwpTag tag);
 
-  static int GetArrayLength(JDWP::ObjectId arrayId);
-  static uint8_t GetArrayElementTag(JDWP::ObjectId arrayId);
-  static bool OutputArray(JDWP::ObjectId arrayId, int firstIndex, int count, JDWP::ExpandBuf* pReply);
-  static bool SetArrayElements(JDWP::ObjectId arrayId, int firstIndex, int count, const uint8_t* buf);
+  static JDWP::JdwpError GetArrayLength(JDWP::ObjectId arrayId, int& length);
+  static JDWP::JdwpError OutputArray(JDWP::ObjectId arrayId, int firstIndex, int count, JDWP::ExpandBuf* pReply);
+  static JDWP::JdwpError SetArrayElements(JDWP::ObjectId arrayId, int firstIndex, int count, const uint8_t* buf);
 
   static JDWP::ObjectId CreateString(const std::string& str);
   static bool CreateObject(JDWP::RefTypeId classId, JDWP::ObjectId& new_object);
@@ -169,9 +168,9 @@
   static JDWP::JdwpTag GetFieldBasicTag(JDWP::FieldId fieldId);
   static JDWP::JdwpTag GetStaticFieldBasicTag(JDWP::FieldId fieldId);
   static void GetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply);
-  static void SetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, uint64_t value, int width);
+  static JDWP::JdwpError SetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, uint64_t value, int width);
   static void GetStaticFieldValue(JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply);
-  static void SetStaticFieldValue(JDWP::FieldId fieldId, uint64_t value, int width);
+  static JDWP::JdwpError SetStaticFieldValue(JDWP::FieldId fieldId, uint64_t value, int width);
 
   static std::string StringToUtf8(JDWP::ObjectId strId);
 
diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index aabdcb8..5e4487c 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -630,8 +630,9 @@
 static JdwpError handleCT_Superclass(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
   RefTypeId classId = ReadRefTypeId(&buf);
   RefTypeId superClassId;
-  if (!Dbg::GetSuperclass(classId, superClassId)) {
-    return ERR_INVALID_CLASS;
+  JdwpError status = Dbg::GetSuperclass(classId, superClassId);
+  if (status != ERR_NONE) {
+    return status;
   }
   expandBufAddRefTypeId(pReply, superClassId);
   return ERR_NONE;
@@ -653,7 +654,10 @@
     uint64_t value = jdwpReadValue(&buf, width);
 
     VLOG(jdwp) << StringPrintf("    --> field=%x tag=%c -> %lld", fieldId, fieldTag, value);
-    Dbg::SetStaticFieldValue(fieldId, value, width);
+    JdwpError status = Dbg::SetStaticFieldValue(fieldId, value, width);
+    if (status != ERR_NONE) {
+      return status;
+    }
   }
 
   return ERR_NONE;
@@ -730,26 +734,28 @@
   return ERR_NONE;
 }
 
-/*
- * Pull out the LocalVariableTable goodies.
- */
-static JdwpError handleM_VariableTableWithGeneric(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
+static JdwpError handleM_VariableTable(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply, bool generic) {
   RefTypeId classId = ReadRefTypeId(&buf);
   MethodId methodId = ReadMethodId(&buf);
 
   VLOG(jdwp) << StringPrintf("  Req for LocalVarTab in class=%s method=%s", Dbg::GetClassDescriptor(classId).c_str(), Dbg::GetMethodName(classId, methodId).c_str());
 
-  /*
-   * We could return ERR_ABSENT_INFORMATION here if the DEX file was
-   * built without local variable information.  That will cause Eclipse
-   * to make a best-effort attempt at displaying local variables
-   * anonymously.  However, the attempt isn't very good, so we're probably
-   * better off just not showing anything.
-   */
-  Dbg::OutputVariableTable(classId, methodId, true, pReply);
+  // We could return ERR_ABSENT_INFORMATION here if the DEX file was built without local variable
+  // information. That will cause Eclipse to make a best-effort attempt at displaying local
+  // variables anonymously. However, the attempt isn't very good, so we're probably better off just
+  // not showing anything.
+  Dbg::OutputVariableTable(classId, methodId, generic, pReply);
   return ERR_NONE;
 }
 
+static JdwpError handleM_VariableTable(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
+  return handleM_VariableTable(state, buf, dataLen, pReply, false);
+}
+
+static JdwpError handleM_VariableTableWithGeneric(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
+  return handleM_VariableTable(state, buf, dataLen, pReply, true);
+}
+
 /*
  * Given an object reference, return the runtime type of the object
  * (class or array).
@@ -1133,11 +1139,14 @@
   ObjectId arrayId = ReadObjectId(&buf);
   VLOG(jdwp) << StringPrintf("  Req for length of array 0x%llx", arrayId);
 
-  uint32_t arrayLength = Dbg::GetArrayLength(arrayId);
+  int length;
+  JdwpError status = Dbg::GetArrayLength(arrayId, length);
+  if (status != ERR_NONE) {
+    return status;
+  }
+  VLOG(jdwp) << StringPrintf("    --> %d", length);
 
-  VLOG(jdwp) << StringPrintf("    --> %d", arrayLength);
-
-  expandBufAdd4BE(pReply, arrayLength);
+  expandBufAdd4BE(pReply, length);
 
   return ERR_NONE;
 }
@@ -1149,18 +1158,9 @@
   ObjectId arrayId = ReadObjectId(&buf);
   uint32_t firstIndex = Read4BE(&buf);
   uint32_t length = Read4BE(&buf);
+  VLOG(jdwp) << StringPrintf("  Req for array values 0x%llx first=%d len=%d", arrayId, firstIndex, length);
 
-  uint8_t tag = Dbg::GetArrayElementTag(arrayId);
-  VLOG(jdwp) << StringPrintf("  Req for array values 0x%llx first=%d len=%d (elem tag=%c)", arrayId, firstIndex, length, tag);
-
-  expandBufAdd1(pReply, tag);
-  expandBufAdd4BE(pReply, length);
-
-  if (!Dbg::OutputArray(arrayId, firstIndex, length, pReply)) {
-    return ERR_INVALID_LENGTH;
-  }
-
-  return ERR_NONE;
+  return Dbg::OutputArray(arrayId, firstIndex, length, pReply);
 }
 
 /*
@@ -1173,11 +1173,7 @@
 
   VLOG(jdwp) << StringPrintf("  Req to set array values 0x%llx first=%d count=%d", arrayId, firstIndex, values);
 
-  if (!Dbg::SetArrayElements(arrayId, firstIndex, values, buf)) {
-    return ERR_INVALID_LENGTH;
-  }
-
-  return ERR_NONE;
+  return Dbg::SetArrayElements(arrayId, firstIndex, values, buf);
 }
 
 /*
@@ -1620,7 +1616,7 @@
 
   /* Method command set (6) */
   { 6,    1,  handleM_LineTable,      "Method.LineTable" },
-  { 6,    2,  NULL, "Method.VariableTable" },
+  { 6,    2,  handleM_VariableTable,  "Method.VariableTable" },
   { 6,    3,  NULL, "Method.Bytecodes" },
   { 6,    4,  NULL, "Method.IsObsolete" },
   { 6,    5,  handleM_VariableTableWithGeneric, "Method.VariableTableWithGeneric" },