Fix a couple of JDWP bugs.
A couple of JDWP tests started failing when we made "thread suspend
count is already zero" into a fatal error. This patch makes the debugger
a bit more careful about doing silly things (the JDWP protocol defines
these silly things as no-ops, and it's surprisingly easy for an end
user to get us into this state; just sit typing "resume" at the jdb(1)
prompt, for example).
I've also tidied up GetObjectTag to return an error on failure, but
that isn't the fix to any bug the tests find. (The only remaining JDWP
bugs are all caused by an inability to cope with register promotion,
and we're already working on the proper fix for that.)
Change-Id: I671f62234b5cb41aa9d86c6b947c518d7c068800
diff --git a/src/debugger.cc b/src/debugger.cc
index eb5efa2..29cb965 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -732,9 +732,13 @@
return JDWP::ERR_NONE;
}
-uint8_t Dbg::GetObjectTag(JDWP::ObjectId objectId) {
+JDWP::JdwpError Dbg::GetObjectTag(JDWP::ObjectId objectId, uint8_t& tag) {
Object* o = gRegistry->Get<Object*>(objectId);
- return TagFromObject(o);
+ if (o == kInvalidObject) {
+ return JDWP::ERR_INVALID_OBJECT;
+ }
+ tag = TagFromObject(o);
+ return JDWP::ERR_NONE;
}
size_t Dbg::GetTagWidth(JDWP::JdwpTag tag) {
@@ -1543,7 +1547,9 @@
LOG(WARNING) << "No such thread for resume: " << peer;
return;
}
- Runtime::Current()->GetThreadList()->Resume(thread, true);
+ if (thread->GetSuspendCount() > 0) {
+ Runtime::Current()->GetThreadList()->Resume(thread, true);
+ }
}
void Dbg::SuspendSelf() {
diff --git a/src/debugger.h b/src/debugger.h
index 89aac8e..9dffa7c 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -141,7 +141,7 @@
static JDWP::JdwpError GetReferenceType(JDWP::ObjectId objectId, JDWP::ExpandBuf* pReply);
static JDWP::JdwpError GetSignature(JDWP::RefTypeId refTypeId, std::string& signature);
static JDWP::JdwpError GetSourceFile(JDWP::RefTypeId refTypeId, std::string& source_file);
- static uint8_t GetObjectTag(JDWP::ObjectId objectId);
+ static JDWP::JdwpError GetObjectTag(JDWP::ObjectId objectId, uint8_t& tag);
static size_t GetTagWidth(JDWP::JdwpTag tag);
static JDWP::JdwpError GetArrayLength(JDWP::ObjectId arrayId, int& length);
diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index c49e9da..05fc979 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -974,16 +974,16 @@
expandBufAdd4BE(pReply, length);
for (uint32_t i = start_frame; i < (start_frame + length); ++i) {
- FrameId frameId;
+ FrameId frame_id;
JdwpLocation loc;
// TODO: switch to GetThreadFrames so we don't have to search for each frame
// even though we only want them in order.
- Dbg::GetThreadFrame(threadId, i, &frameId, &loc);
+ Dbg::GetThreadFrame(threadId, i, &frame_id, &loc);
- expandBufAdd8BE(pReply, frameId);
+ expandBufAdd8BE(pReply, frame_id);
AddLocation(pReply, &loc);
- VLOG(jdwp) << StringPrintf(" Frame %d: id=%#llx ", i, frameId) << loc;
+ VLOG(jdwp) << StringPrintf(" Frame %d: id=%#llx ", i, frame_id) << loc;
}
return ERR_NONE;
@@ -1339,10 +1339,10 @@
*/
static JdwpError handleSF_GetValues(JdwpState*, const uint8_t* buf, int, ExpandBuf* pReply) {
ObjectId threadId = ReadObjectId(&buf);
- FrameId frameId = ReadFrameId(&buf);
+ FrameId frame_id = ReadFrameId(&buf);
uint32_t slots = Read4BE(&buf);
- VLOG(jdwp) << StringPrintf(" Req for %d slots in threadId=%#llx frameId=%#llx", slots, threadId, frameId);
+ VLOG(jdwp) << StringPrintf(" Req for %d slots in threadId=%#llx frame_id=%#llx", slots, threadId, frame_id);
expandBufAdd4BE(pReply, slots); /* "int values" */
for (uint32_t i = 0; i < slots; i++) {
@@ -1353,7 +1353,7 @@
size_t width = Dbg::GetTagWidth(reqSigByte);
uint8_t* ptr = expandBufAddSpace(pReply, width+1);
- Dbg::GetLocalValue(threadId, frameId, slot, reqSigByte, ptr, width);
+ Dbg::GetLocalValue(threadId, frame_id, slot, reqSigByte, ptr, width);
}
return ERR_NONE;
@@ -1364,10 +1364,10 @@
*/
static JdwpError handleSF_SetValues(JdwpState*, const uint8_t* buf, int, ExpandBuf*) {
ObjectId threadId = ReadObjectId(&buf);
- FrameId frameId = ReadFrameId(&buf);
+ FrameId frame_id = ReadFrameId(&buf);
uint32_t slots = Read4BE(&buf);
- VLOG(jdwp) << StringPrintf(" Req to set %d slots in threadId=%#llx frameId=%#llx", slots, threadId, frameId);
+ VLOG(jdwp) << StringPrintf(" Req to set %d slots in threadId=%#llx frame_id=%#llx", slots, threadId, frame_id);
for (uint32_t i = 0; i < slots; i++) {
uint32_t slot = Read4BE(&buf);
@@ -1376,7 +1376,7 @@
uint64_t value = jdwpReadValue(&buf, width);
VLOG(jdwp) << " --> slot " << slot << " " << sigByte << " " << value;
- Dbg::SetLocalValue(threadId, frameId, slot, sigByte, value, width);
+ Dbg::SetLocalValue(threadId, frame_id, slot, sigByte, value, width);
}
return ERR_NONE;
@@ -1387,16 +1387,20 @@
*/
static JdwpError handleSF_ThisObject(JdwpState*, const uint8_t* buf, int, ExpandBuf* pReply) {
ReadObjectId(&buf); // Skip thread id.
- FrameId frameId = ReadFrameId(&buf);
+ FrameId frame_id = ReadFrameId(&buf);
- ObjectId objectId;
- Dbg::GetThisObject(frameId, &objectId);
+ ObjectId id;
+ Dbg::GetThisObject(frame_id, &id);
- uint8_t objectTag = Dbg::GetObjectTag(objectId);
- VLOG(jdwp) << StringPrintf(" Req for 'this' in frame=%#llx --> %#llx '%c'", frameId, objectId, (char)objectTag);
+ uint8_t tag;
+ JdwpError result = Dbg::GetObjectTag(id, tag);
+ if (result != ERR_NONE) {
+ return result;
+ }
- expandBufAdd1(pReply, objectTag);
- expandBufAddObjectId(pReply, objectId);
+ VLOG(jdwp) << StringPrintf(" Req for 'this' in frame=%#llx --> %#llx '%c'", frame_id, id, static_cast<char>(tag));
+ expandBufAdd1(pReply, tag);
+ expandBufAddObjectId(pReply, id);
return ERR_NONE;
}
diff --git a/src/thread_list.cc b/src/thread_list.cc
index 186ddbc..951a2be 100644
--- a/src/thread_list.cc
+++ b/src/thread_list.cc
@@ -274,7 +274,7 @@
MutexLock mu(thread_suspend_count_lock_);
for (It it = list_.begin(), end = list_.end(); it != end; ++it) {
Thread* thread = *it;
- if (thread == self || (for_debugger && thread == debug_thread)) {
+ if (thread == self || (for_debugger && thread == debug_thread) || thread->suspend_count_ == 0) {
continue;
}
ModifySuspendCount(thread, -1, for_debugger);