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
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());
diff --git a/src/debugger.h b/src/debugger.h
index b88edbb..659843a 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -170,7 +170,7 @@
static JDWP::JdwpTag GetStaticFieldBasicTag(JDWP::FieldId fieldId);
static JDWP::JdwpError GetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply);
static JDWP::JdwpError SetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, uint64_t value, int width);
- static JDWP::JdwpError GetStaticFieldValue(JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply);
+ static JDWP::JdwpError GetStaticFieldValue(JDWP::RefTypeId refTypeId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply);
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_event.cc b/src/jdwp/jdwp_event.cc
index b92884f..6476c3c 100644
--- a/src/jdwp/jdwp_event.cc
+++ b/src/jdwp/jdwp_event.cc
@@ -839,7 +839,7 @@
if (match_count != 0) {
VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << match_count << " total) "
- << "thread=" << (void*) basket.threadId << ")";
+ << StringPrintf("thread=%#llx", basket.threadId) << ")";
suspendPolicy = scanSuspendPolicy(matchList, match_count);
VLOG(jdwp) << " suspendPolicy=" << suspendPolicy;
@@ -932,8 +932,8 @@
FindMatchingEvents(EK_EXCEPTION, &basket, matchList, &match_count);
if (match_count != 0) {
VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << match_count << " total)"
- << " thread=" << (void*) basket.threadId
- << " exceptId=" << (void*) exceptionId
+ << StringPrintf(" thread=%#llx", basket.threadId)
+ << StringPrintf(" exceptId=%#llx", exceptionId)
<< " caught=" << basket.caught << ")"
<< " throw: " << *pThrowLoc;
if (pCatchLoc->classId == 0) {
@@ -1012,7 +1012,7 @@
FindMatchingEvents(EK_CLASS_PREPARE, &basket, matchList, &match_count);
if (match_count != 0) {
VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << match_count << " total) "
- << "thread=" << (void*) basket.threadId << ") " << signature;
+ << StringPrintf("thread=%#llx", basket.threadId) << ") " << signature;
suspendPolicy = scanSuspendPolicy(matchList, match_count);
VLOG(jdwp) << " suspendPolicy=" << suspendPolicy;
diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index ef40817..c977979 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -471,20 +471,16 @@
* Get values from static fields in a reference type.
*/
static JdwpError handleRT_GetValues(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
- ReadRefTypeId(&buf); // We don't need this, but we need to skip over it in the request.
- uint32_t numFields = Read4BE(&buf);
-
- VLOG(jdwp) << " RT_GetValues " << numFields << ":";
-
- expandBufAdd4BE(pReply, numFields);
- for (uint32_t i = 0; i < numFields; i++) {
+ RefTypeId refTypeId = ReadRefTypeId(&buf);
+ uint32_t field_count = Read4BE(&buf);
+ expandBufAdd4BE(pReply, field_count);
+ for (uint32_t i = 0; i < field_count; i++) {
FieldId fieldId = ReadFieldId(&buf);
- JdwpError status = Dbg::GetStaticFieldValue(fieldId, pReply);
+ JdwpError status = Dbg::GetStaticFieldValue(refTypeId, fieldId, pReply);
if (status != ERR_NONE) {
return status;
}
}
-
return ERR_NONE;
}
@@ -565,7 +561,7 @@
expandBufAddUtf8String(pReply, signature);
} else {
LOG(WARNING) << StringPrintf("No signature for refTypeId=%#llx", refTypeId);
- expandBufAddUtf8String(pReply, "Lunknown;");
+ expandBufAddUtf8String(pReply, "");
}
expandBufAddUtf8String(pReply, genericSignature);
@@ -773,13 +769,13 @@
*/
static JdwpError handleOR_GetValues(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
ObjectId objectId = ReadObjectId(&buf);
- uint32_t numFields = Read4BE(&buf);
+ uint32_t field_count = Read4BE(&buf);
- VLOG(jdwp) << StringPrintf(" Req for %d fields from objectId=%#llx", numFields, objectId);
+ VLOG(jdwp) << StringPrintf(" Req for %d fields from objectId=%#llx", field_count, objectId);
- expandBufAdd4BE(pReply, numFields);
+ expandBufAdd4BE(pReply, field_count);
- for (uint32_t i = 0; i < numFields; i++) {
+ for (uint32_t i = 0; i < field_count; i++) {
FieldId fieldId = ReadFieldId(&buf);
JdwpError status = Dbg::GetFieldValue(objectId, fieldId, pReply);
if (status != ERR_NONE) {
@@ -795,11 +791,11 @@
*/
static JdwpError handleOR_SetValues(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
ObjectId objectId = ReadObjectId(&buf);
- uint32_t numFields = Read4BE(&buf);
+ uint32_t field_count = Read4BE(&buf);
- VLOG(jdwp) << StringPrintf(" Req to set %d fields in objectId=%#llx", numFields, objectId);
+ VLOG(jdwp) << StringPrintf(" Req to set %d fields in objectId=%#llx", field_count, objectId);
- for (uint32_t i = 0; i < numFields; i++) {
+ for (uint32_t i = 0; i < field_count; i++) {
FieldId fieldId = ReadFieldId(&buf);
JDWP::JdwpTag fieldTag = Dbg::GetFieldBasicTag(fieldId);