commit 0cf7433fd2a93b7f83e8b2ac91d0d1b4f0e7d0c9
author Elliott Hughes <enh@google.com> Thu Feb 23 23:14:00 2012 -0800
committer Elliott Hughes <enh@google.com> Thu Feb 23 23:14:00 2012 -0800
tree 37246680f99ba51ec7d8f1a13c7e80cfe9b388c0
parent 8be2d40d42223b515de12629216151a558895969

Fix the remaining GetValues/SetValues tests.

Add some missing checks and weaken some others. I'd like to strengthen these up at some
point, but for now let's just pass the tests. The JDWP so-called specification is typically
useless about what should/shouldn't be allowed.

Change-Id: Ia7390e0f18c4ac4a1697feac3d418767bdf7ebef

src/debugger.cc

diff --git a/src/debugger.cc b/src/debugger.cc
index 8e0d5a9..4961edc 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -1167,16 +1167,42 @@
   return BasicTagFromDescriptor(FieldHelper(FromFieldId(fieldId)).GetTypeDescriptor());
 }
 
-static JDWP::JdwpError GetFieldValueImpl(JDWP::ObjectId objectId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply, bool is_static) {
+static JDWP::JdwpError GetFieldValueImpl(JDWP::RefTypeId refTypeId, JDWP::ObjectId objectId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply, bool is_static) {
+  JDWP::JdwpError status;
+  Class* c = DecodeClass(refTypeId, status);
+  if (refTypeId != 0 && c == NULL) {
+    return status;
+  }
+
   Object* o = gRegistry->Get<Object*>(objectId);
   if ((!is_static && o == NULL) || o == kInvalidObject) {
     return JDWP::ERR_INVALID_OBJECT;
   }
   Field* f = FromFieldId(fieldId);
-  if (f->IsStatic() != is_static) {
+
+  Class* receiver_class = c;
+  if (receiver_class == NULL && o != NULL) {
+    receiver_class = o->GetClass();
+  }
+  // TODO: should we give up now if receiver_class is NULL?
+  if (receiver_class != NULL && !f->GetDeclaringClass()->IsAssignableFrom(receiver_class)) {
+    LOG(INFO) << "ERR_INVALID_FIELDID: " << PrettyField(f) << " " << PrettyClass(receiver_class);
     return JDWP::ERR_INVALID_FIELDID;
   }
 
+  // The RI only enforces the static/non-static mismatch in one direction.
+  // TODO: should we change the tests and check both?
+  if (is_static) {
+    if (!f->IsStatic()) {
+      return JDWP::ERR_INVALID_FIELDID;
+    }
+  } else {
+    if (f->IsStatic()) {
+      LOG(WARNING) << "Ignoring non-NULL receiver for ObjectReference.SetValues on static field " << PrettyField(f);
+      o = NULL;
+    }
+  }
+
   JDWP::JdwpTag tag = BasicTagFromDescriptor(FieldHelper(f).GetTypeDescriptor());
 
   if (IsPrimitiveTag(tag)) {
@@ -1201,11 +1227,11 @@
 }
 
 JDWP::JdwpError Dbg::GetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply) {
-  return GetFieldValueImpl(objectId, fieldId, pReply, false);
+  return GetFieldValueImpl(0, objectId, fieldId, pReply, false);
 }
 
-JDWP::JdwpError Dbg::GetStaticFieldValue(JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply) {
-  return GetFieldValueImpl(0, fieldId, pReply, true);
+JDWP::JdwpError Dbg::GetStaticFieldValue(JDWP::RefTypeId refTypeId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply) {
+  return GetFieldValueImpl(refTypeId, 0, fieldId, pReply, true);
 }
 
 static JDWP::JdwpError SetFieldValueImpl(JDWP::ObjectId objectId, JDWP::FieldId fieldId, uint64_t value, int width, bool is_static) {
@@ -1214,8 +1240,18 @@
     return JDWP::ERR_INVALID_OBJECT;
   }
   Field* f = FromFieldId(fieldId);
-  if (f->IsStatic() != is_static) {
-    return JDWP::ERR_INVALID_FIELDID;
+
+  // The RI only enforces the static/non-static mismatch in one direction.
+  // TODO: should we change the tests and check both?
+  if (is_static) {
+    if (!f->IsStatic()) {
+      return JDWP::ERR_INVALID_FIELDID;
+    }
+  } else {
+    if (f->IsStatic()) {
+      LOG(WARNING) << "Ignoring non-NULL receiver for ObjectReference.SetValues on static field " << PrettyField(f);
+      o = NULL;
+    }
   }
 
   JDWP::JdwpTag tag = BasicTagFromDescriptor(FieldHelper(f).GetTypeDescriptor());