More debugger robustness.
This fixes about half the remaining test failures, including almost all of the
native crashes.
Change-Id: I9c3d994bfafaecca17597e442477c973e57f5979
diff --git a/src/debugger.cc b/src/debugger.cc
index bbdff71..61118eb 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -1170,9 +1170,15 @@
return BasicTagFromDescriptor(FieldHelper(FromFieldId(fieldId)).GetTypeDescriptor());
}
-void Dbg::GetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply) {
+static JDWP::JdwpError GetFieldValueImpl(JDWP::ObjectId objectId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply, bool is_static) {
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) {
+ return JDWP::ERR_INVALID_FIELDID;
+ }
JDWP::JdwpTag tag = BasicTagFromDescriptor(FieldHelper(f).GetTypeDescriptor());
@@ -1194,11 +1200,26 @@
expandBufAdd1(pReply, TagFromObject(value));
expandBufAddObjectId(pReply, gRegistry->Add(value));
}
+ return JDWP::ERR_NONE;
}
-JDWP::JdwpError Dbg::SetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, uint64_t value, int width) {
+JDWP::JdwpError Dbg::GetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply) {
+ return GetFieldValueImpl(objectId, fieldId, pReply, false);
+}
+
+JDWP::JdwpError Dbg::GetStaticFieldValue(JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply) {
+ return GetFieldValueImpl(0, fieldId, pReply, true);
+}
+
+static JDWP::JdwpError SetFieldValueImpl(JDWP::ObjectId objectId, JDWP::FieldId fieldId, uint64_t value, int width, bool is_static) {
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) {
+ return JDWP::ERR_INVALID_FIELDID;
+ }
JDWP::JdwpTag tag = BasicTagFromDescriptor(FieldHelper(f).GetTypeDescriptor());
@@ -1210,22 +1231,27 @@
}
} else {
Object* v = gRegistry->Get<Object*>(value);
- Class* field_type = FieldHelper(f).GetType();
- if (!field_type->IsAssignableFrom(v->GetClass())) {
+ if (v == kInvalidObject) {
return JDWP::ERR_INVALID_OBJECT;
}
+ if (v != NULL) {
+ 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);
+JDWP::JdwpError Dbg::SetFieldValue(JDWP::ObjectId objectId, JDWP::FieldId fieldId, uint64_t value, int width) {
+ return SetFieldValueImpl(objectId, fieldId, value, width, false);
}
JDWP::JdwpError Dbg::SetStaticFieldValue(JDWP::FieldId fieldId, uint64_t value, int width) {
- return SetFieldValue(0, fieldId, value, width);
+ return SetFieldValueImpl(0, fieldId, value, width, true);
}
std::string Dbg::StringToUtf8(JDWP::ObjectId strId) {
@@ -1239,7 +1265,7 @@
if (thread == NULL) {
return false;
}
- StringAppendF(&name, "<%d> %s", thread->GetThinLockId(), thread->GetThreadName()->ToModifiedUtf8().c_str());
+ name = thread->GetThreadName()->ToModifiedUtf8();
return true;
}
@@ -1999,15 +2025,22 @@
return JDWP::ERR_THREAD_SUSPENDED; // Probably not expected here.
}
- /*
- * OLD-TODO: ought to screen the various IDs, and verify that the argument
- * list is valid.
- */
+ JDWP::JdwpError status;
req->receiver_ = gRegistry->Get<Object*>(objectId);
- req->thread_ = gRegistry->Get<Object*>(threadId);
- req->class_ = gRegistry->Get<Class*>(classId);
+ if (req->receiver_ == kInvalidObject) {
+ return JDWP::ERR_INVALID_OBJECT;
+ }
+ req->thread_ = gRegistry->Get<Object*>(threadId); // TODO: check that this is actually a thread!
+ if (req->thread_ == kInvalidObject) {
+ return JDWP::ERR_INVALID_OBJECT;
+ }
+ req->class_ = DecodeClass(classId, status);
+ if (req->class_ == NULL) {
+ return status;
+ }
req->method_ = FromMethodId(methodId);
req->num_args_ = numArgs;
+ // TODO: check that the argument list is valid.
req->arg_array_ = argArray;
req->options_ = options;
req->invoke_needed_ = true;
diff --git a/src/debugger.h b/src/debugger.h
index 2397b52..1e5ca8f 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -168,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 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 void GetStaticFieldValue(JDWP::FieldId fieldId, JDWP::ExpandBuf* pReply);
+ static JDWP::JdwpError GetStaticFieldValue(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_handler.cc b/src/jdwp/jdwp_handler.cc
index 9b5761a..4640d98 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -485,7 +485,10 @@
expandBufAdd4BE(pReply, numFields);
for (uint32_t i = 0; i < numFields; i++) {
FieldId fieldId = ReadFieldId(&buf);
- Dbg::GetStaticFieldValue(fieldId, pReply);
+ JdwpError status = Dbg::GetStaticFieldValue(fieldId, pReply);
+ if (status != ERR_NONE) {
+ return status;
+ }
}
return ERR_NONE;
@@ -784,7 +787,10 @@
for (uint32_t i = 0; i < numFields; i++) {
FieldId fieldId = ReadFieldId(&buf);
- Dbg::GetFieldValue(objectId, fieldId, pReply);
+ JdwpError status = Dbg::GetFieldValue(objectId, fieldId, pReply);
+ if (status != ERR_NONE) {
+ return status;
+ }
}
return ERR_NONE;
@@ -807,8 +813,10 @@
uint64_t value = jdwpReadValue(&buf, width);
VLOG(jdwp) << " --> fieldId=" << fieldId << " tag=" << fieldTag << "(" << width << ") value=" << value;
-
- Dbg::SetFieldValue(objectId, fieldId, value, width);
+ JdwpError status = Dbg::SetFieldValue(objectId, fieldId, value, width);
+ if (status != ERR_NONE) {
+ return status;
+ }
}
return ERR_NONE;
@@ -967,7 +975,7 @@
*/
static JdwpError handleTR_Frames(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
ObjectId threadId = ReadObjectId(&buf);
- uint32_t startFrame = Read4BE(&buf);
+ uint32_t start_frame = Read4BE(&buf);
uint32_t length = Read4BE(&buf);
if (!Dbg::ThreadExists(threadId)) {
@@ -978,25 +986,27 @@
return ERR_THREAD_NOT_SUSPENDED;
}
- size_t frameCount = Dbg::GetThreadFrameCount(threadId);
+ size_t actual_frame_count = Dbg::GetThreadFrameCount(threadId);
- VLOG(jdwp) << StringPrintf(" Request for frames: threadId=%llx start=%d length=%d [count=%zd]", threadId, startFrame, length, frameCount);
- if (frameCount <= 0) {
+ VLOG(jdwp) << StringPrintf(" Request for frames: threadId=%llx start=%d length=%d [count=%zd]", threadId, start_frame, length, actual_frame_count);
+ if (actual_frame_count <= 0) {
return ERR_THREAD_NOT_SUSPENDED; /* == 0 means 100% native */
}
- if (length == (uint32_t) -1) {
- length = frameCount;
- }
- CHECK_GE(startFrame, 0U);
- CHECK_LT(startFrame, frameCount);
- CHECK_LE(startFrame + length, frameCount);
- uint32_t frames = length;
- expandBufAdd4BE(pReply, frames);
- for (uint32_t i = startFrame; i < (startFrame+length); i++) {
+ if (start_frame > actual_frame_count) {
+ return ERR_INVALID_INDEX;
+ }
+ if (length == static_cast<uint32_t>(-1)) {
+ length = actual_frame_count - start_frame;
+ }
+ if (start_frame + length > actual_frame_count) {
+ return ERR_INVALID_LENGTH;
+ }
+
+ expandBufAdd4BE(pReply, length);
+ for (uint32_t i = start_frame; i < (start_frame + length); ++i) {
FrameId frameId;
JdwpLocation loc;
-
Dbg::GetThreadFrame(threadId, i, &frameId, &loc);
expandBufAdd8BE(pReply, frameId);
@@ -1022,11 +1032,11 @@
return ERR_THREAD_NOT_SUSPENDED;
}
- int frameCount = Dbg::GetThreadFrameCount(threadId);
- if (frameCount < 0) {
+ int frame_count = Dbg::GetThreadFrameCount(threadId);
+ if (frame_count < 0) {
return ERR_INVALID_THREAD;
}
- expandBufAdd4BE(pReply, (uint32_t)frameCount);
+ expandBufAdd4BE(pReply, static_cast<uint32_t>(frame_count));
return ERR_NONE;
}