Fix detaching a debugger while threads are suspended.
The interesting part of this change is in "thread_list.cc".
I've done a TODO in TagFromClass, but haven't seen it make any practical
difference in a debugger. I also tightened up the types in GetThreadStatus
while investigating the fact that we report some threads as "RUNNING, SUSPENDED",
which makes no sense until you realize that TS_RUNNING corresponds to both
our kRunnable thread state and our kNative thread state, the latter of which
may actually be a suspended thread.
I've also made us fail faster in the "address in use" jdwp failure case,
and tidied up a bunch of the capitalization in logging.
Change-Id: I0fe705791d07db31c4615addce44da4fdfbfd0d1
diff --git a/src/check_jni.cc b/src/check_jni.cc
index 570e937..423bc2f 100644
--- a/src/check_jni.cc
+++ b/src/check_jni.cc
@@ -555,7 +555,7 @@
va_arg(ap, long); // Skip this argument.
} else if (ch == '.') {
} else {
- LOG(FATAL) << "unknown check format specifier: " << ch;
+ LOG(FATAL) << "Unknown check format specifier: " << ch;
}
}
va_end(ap);
@@ -718,7 +718,7 @@
}
break;
default:
- LOG(FATAL) << "bad flags (internal error): " << flags;
+ LOG(FATAL) << "Bad flags (internal error): " << flags;
}
/*
diff --git a/src/dalvik_system_Zygote.cc b/src/dalvik_system_Zygote.cc
index cd3f656..e3a2cdd 100644
--- a/src/dalvik_system_Zygote.cc
+++ b/src/dalvik_system_Zygote.cc
@@ -174,10 +174,7 @@
return 0;
}
-// Set Linux capability flags.
-//
-// Returns 0 on success, errno on failure.
-int SetCapabilities(int64_t permitted, int64_t effective) {
+void SetCapabilities(int64_t permitted, int64_t effective) {
#ifdef HAVE_ANDROID_OS
struct __user_cap_header_struct capheader;
struct __user_cap_data_struct capdata;
@@ -192,11 +189,9 @@
capdata.permitted = permitted;
if (capset(&capheader, &capdata) != 0) {
- return errno;
+ PLOG(FATAL) << "capset(" << permitted << ", " << effective << ") failed";
}
#endif /*HAVE_ANDROID_OS*/
-
- return 0;
}
void EnableDebugFeatures(uint32_t debug_flags) {
@@ -300,29 +295,25 @@
int err = SetGids(env, javaGids);
if (err < 0) {
- PLOG(FATAL) << "cannot setgroups()";
+ PLOG(FATAL) << "setgroups failed";
}
err = SetRLimits(env, javaRlimits);
if (err < 0) {
- PLOG(FATAL) << "cannot setrlimit()";
+ PLOG(FATAL) << "setrlimit failed";
}
err = setgid(gid);
if (err < 0) {
- PLOG(FATAL) << "cannot setgid(" << gid << ")";
+ PLOG(FATAL) << "setgid(" << gid << ") failed";
}
err = setuid(uid);
if (err < 0) {
- PLOG(FATAL) << "cannot setuid(" << uid << ")";
+ PLOG(FATAL) << "setuid(" << uid << ") failed";
}
- err = SetCapabilities(permittedCapabilities, effectiveCapabilities);
- if (err != 0) {
- PLOG(FATAL) << "cannot set capabilities ("
- << permittedCapabilities << "," << effectiveCapabilities << ")";
- }
+ SetCapabilities(permittedCapabilities, effectiveCapabilities);
// Our system thread ID, etc, has changed so reset Thread state.
self->InitAfterFork();
diff --git a/src/debugger.cc b/src/debugger.cc
index 5840655..f31ae58 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -154,18 +154,17 @@
return JDWP::JT_ARRAY;
}
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
if (c->IsStringClass()) {
return JDWP::JT_STRING;
} else if (c->IsClassClass()) {
return JDWP::JT_CLASS_OBJECT;
-#if 0 // TODO
- } else if (dvmInstanceof(clazz, gDvm.classJavaLangThread)) {
+ } else if (c->InstanceOf(class_linker->FindSystemClass("Ljava/lang/Thread;"))) {
return JDWP::JT_THREAD;
- } else if (dvmInstanceof(clazz, gDvm.classJavaLangThreadGroup)) {
+ } else if (c->InstanceOf(class_linker->FindSystemClass("Ljava/lang/ThreadGroup;"))) {
return JDWP::JT_THREAD_GROUP;
- } else if (dvmInstanceof(clazz, gDvm.classJavaLangClassLoader)) {
+ } else if (c->InstanceOf(class_linker->FindSystemClass("Ljava/lang/ClassLoader;"))) {
return JDWP::JT_CLASS_LOADER;
-#endif
} else {
return JDWP::JT_OBJECT;
}
@@ -325,7 +324,7 @@
// We probably failed because some other process has the port already, which means that
// if we don't abort the user is likely to think they're talking to us when they're actually
// talking to that other process.
- LOG(FATAL) << "debugger thread failed to initialize";
+ LOG(FATAL) << "Debugger thread failed to initialize";
}
// If a debugger has already attached, send the "welcome" message.
@@ -333,7 +332,7 @@
if (gJdwpState->IsActive()) {
//ScopedThreadStateChange tsc(Thread::Current(), Thread::kRunnable);
if (!gJdwpState->PostVMStart()) {
- LOG(WARNING) << "failed to post 'start' message to debugger";
+ LOG(WARNING) << "Failed to post 'start' message to debugger";
}
}
}
@@ -597,7 +596,7 @@
case JDWP::JT_LONG:
return 8;
default:
- LOG(FATAL) << "unknown tag " << tag;
+ LOG(FATAL) << "Unknown tag " << tag;
return -1;
}
}
@@ -987,7 +986,7 @@
} else if (tag == JDWP::JT_DOUBLE || tag == JDWP::JT_LONG) {
expandBufAdd8BE(pReply, f->Get64(o));
} else {
- LOG(FATAL) << "unknown tag: " << tag;
+ LOG(FATAL) << "Unknown tag: " << tag;
}
} else {
Object* value = f->GetObject(o);
@@ -1097,7 +1096,7 @@
return gRegistry->Add(GetStaticThreadGroup("mMain"));
}
-bool Dbg::GetThreadStatus(JDWP::ObjectId threadId, uint32_t* pThreadStatus, uint32_t* pSuspendStatus) {
+bool Dbg::GetThreadStatus(JDWP::ObjectId threadId, JDWP::JdwpThreadStatus* pThreadStatus, JDWP::JdwpSuspendStatus* pSuspendStatus) {
ScopedThreadListLock thread_list_lock;
Thread* thread = DecodeThread(threadId);
@@ -1117,10 +1116,10 @@
case Thread::kVmWait: *pThreadStatus = JDWP::TS_WAIT; break;
case Thread::kSuspended: *pThreadStatus = JDWP::TS_RUNNING; break;
default:
- LOG(FATAL) << "unknown thread state " << thread->GetState();
+ LOG(FATAL) << "Unknown thread state " << thread->GetState();
}
- *pSuspendStatus = (thread->IsSuspended() ? JDWP::SUSPEND_STATUS_SUSPENDED : 0);
+ *pSuspendStatus = (thread->IsSuspended() ? JDWP::SUSPEND_STATUS_SUSPENDED : JDWP::SUSPEND_STATUS_NOT_SUSPENDED);
return true;
}
@@ -1297,7 +1296,7 @@
const VmapTable vmap_table(m->GetVmapTableRaw());
uint32_t vmap_offset;
if (vmap_table.IsInContext(reg, vmap_offset)) {
- UNIMPLEMENTED(FATAL) << "don't know how to pull locals from callee save frames: " << vmap_offset;
+ UNIMPLEMENTED(FATAL) << "Don't know how to pull locals from callee save frames: " << vmap_offset;
}
switch (tag) {
@@ -1341,7 +1340,7 @@
Object* o = reinterpret_cast<Object*>(f.GetVReg(m, reg));
LOG(VERBOSE) << "get array local " << reg << " = " << o;
if (o != NULL && !Heap::IsHeapAddress(o)) {
- LOG(FATAL) << "reg " << reg << " expected to hold array: " << o;
+ LOG(FATAL) << "Register " << reg << " expected to hold array: " << o;
}
JDWP::SetObjectId(buf+1, gRegistry->Add(o));
}
@@ -1352,7 +1351,7 @@
Object* o = reinterpret_cast<Object*>(f.GetVReg(m, reg));
LOG(VERBOSE) << "get object local " << reg << " = " << o;
if (o != NULL && !Heap::IsHeapAddress(o)) {
- LOG(FATAL) << "reg " << reg << " expected to hold object: " << o;
+ LOG(FATAL) << "Register " << reg << " expected to hold object: " << o;
}
tag = TagFromObject(o);
JDWP::SetObjectId(buf+1, gRegistry->Add(o));
@@ -1370,7 +1369,7 @@
}
break;
default:
- LOG(FATAL) << "unknown tag " << tag;
+ LOG(FATAL) << "Unknown tag " << tag;
break;
}
@@ -1388,7 +1387,7 @@
const VmapTable vmap_table(m->GetVmapTableRaw());
uint32_t vmap_offset;
if (vmap_table.IsInContext(reg, vmap_offset)) {
- UNIMPLEMENTED(FATAL) << "don't know how to pull locals from callee save frames: " << vmap_offset;
+ UNIMPLEMENTED(FATAL) << "Don't know how to pull locals from callee save frames: " << vmap_offset;
}
switch (tag) {
@@ -1423,7 +1422,7 @@
f.SetVReg(m, reg + 1, static_cast<uint32_t>(value >> 32));
break;
default:
- LOG(FATAL) << "unknown tag " << tag;
+ LOG(FATAL) << "Unknown tag " << tag;
break;
}
}
@@ -1466,7 +1465,7 @@
return;
}
- // TODO - we currently always send both "verified" and "prepared" since
+ // OLD-TODO - we currently always send both "verified" and "prepared" since
// debuggers seem to like that. There might be some advantage to honesty,
// since the class may not yet be verified.
int state = JDWP::CS_VERIFIED | JDWP::CS_PREPARED;
@@ -1531,7 +1530,7 @@
}
/*
- * TODO: ought to screen the various IDs, and verify that the argument
+ * OLD-TODO: ought to screen the various IDs, and verify that the argument
* list is valid.
*/
req->receiver_ = gRegistry->Get<Object*>(objectId);
@@ -1679,7 +1678,7 @@
* Returns "true" if we have a reply. The reply buffer is newly allocated,
* and includes the chunk type/length, followed by the data.
*
- * TODO: we currently assume that the request and reply include a single
+ * OLD-TODO: we currently assume that the request and reply include a single
* chunk. If this becomes inconvenient we will need to adapt.
*/
bool Dbg::DdmHandlePacket(const uint8_t* buf, int dataLen, uint8_t** pReplyBuf, int* pReplyLen) {
@@ -2072,7 +2071,7 @@
bytesLeft = buf.size() - (size_t)(p - &buf[0]);
if (bytesLeft < needed) {
- LOG(WARNING) << "chunk is too big to transmit (chunk_len=" << chunk_len << ", " << needed << " bytes)";
+ LOG(WARNING) << "Chunk is too big to transmit (chunk_len=" << chunk_len << ", " << needed << " bytes)";
return;
}
}
@@ -2117,7 +2116,7 @@
}
if (!Heap::IsHeapAddress(c)) {
- LOG(WARNING) << "invalid class for managed heap object: " << o << " " << c;
+ LOG(WARNING) << "Invalid class for managed heap object: " << o << " " << c;
return HPSG_STATE(SOLIDITY_HARD, KIND_UNKNOWN);
}
diff --git a/src/debugger.h b/src/debugger.h
index 381f1db..0798635 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -182,7 +182,7 @@
static JDWP::ObjectId GetSystemThreadGroupId();
static JDWP::ObjectId GetMainThreadGroupId();
- static bool GetThreadStatus(JDWP::ObjectId threadId, uint32_t* threadStatus, uint32_t* suspendStatus);
+ static bool GetThreadStatus(JDWP::ObjectId threadId, JDWP::JdwpThreadStatus* pThreadStatus, JDWP::JdwpSuspendStatus* pSuspendStatus);
static uint32_t GetThreadSuspendCount(JDWP::ObjectId threadId);
static bool ThreadExists(JDWP::ObjectId threadId);
static bool IsSuspended(JDWP::ObjectId threadId);
diff --git a/src/jdwp/jdwp_adb.cc b/src/jdwp/jdwp_adb.cc
index f6b977e..eb6eec2 100644
--- a/src/jdwp/jdwp_adb.cc
+++ b/src/jdwp/jdwp_adb.cc
@@ -170,7 +170,7 @@
if (ret <= 0) {
if (ret < 0) {
- PLOG(WARNING) << "receiving file descriptor from ADB failed (socket " << netState->controlSock << ")";
+ PLOG(WARNING) << "Receiving file descriptor from ADB failed (socket " << netState->controlSock << ")";
}
close(netState->controlSock);
netState->controlSock = -1;
diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index b30a6a2..a11c805 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -925,13 +925,13 @@
LOG(VERBOSE) << StringPrintf(" Req for status of thread 0x%llx", threadId);
- uint32_t threadStatus;
- uint32_t suspendStatus;
+ JDWP::JdwpThreadStatus threadStatus;
+ JDWP::JdwpSuspendStatus suspendStatus;
if (!Dbg::GetThreadStatus(threadId, &threadStatus, &suspendStatus)) {
return ERR_INVALID_THREAD;
}
- LOG(VERBOSE) << " --> " << JdwpThreadStatus(threadStatus) << ", " << JdwpSuspendStatus(suspendStatus);
+ LOG(VERBOSE) << " --> " << threadStatus << ", " << suspendStatus;
expandBufAdd4BE(pReply, threadStatus);
expandBufAdd4BE(pReply, suspendStatus);
diff --git a/src/jdwp/jdwp_main.cc b/src/jdwp/jdwp_main.cc
index 98490e2..ad0610d 100644
--- a/src/jdwp/jdwp_main.cc
+++ b/src/jdwp/jdwp_main.cc
@@ -211,7 +211,7 @@
* mid-request, though, we could see this.
*/
if (eventThreadId != 0) {
- LOG(WARNING) << "resetting state while event in progress";
+ LOG(WARNING) << "Resetting state while event in progress";
DCHECK(false);
}
}
diff --git a/src/jdwp/jdwp_socket.cc b/src/jdwp/jdwp_socket.cc
index 351e456..b832133 100644
--- a/src/jdwp/jdwp_socket.cc
+++ b/src/jdwp/jdwp_socket.cc
@@ -80,7 +80,7 @@
}
};
-static JdwpNetState* netStartup(short port);
+static JdwpNetState* netStartup(short port, bool probe);
/*
* Set up some stuff for transport=dt_socket.
@@ -92,11 +92,11 @@
if (options->port != 0) {
/* try only the specified port */
port = options->port;
- state->netState = netStartup(port);
+ state->netState = netStartup(port, false);
} else {
/* scan through a range of ports, binding to the first available */
for (port = kBasePort; port <= kMaxPort; port++) {
- state->netState = netStartup(port);
+ state->netState = netStartup(port, true);
if (state->netState != NULL) {
break;
}
@@ -108,7 +108,7 @@
}
} else {
port = options->port; // used in a debug msg later
- state->netState = netStartup(-1);
+ state->netState = netStartup(-1, false);
}
if (options->suspend) {
@@ -139,10 +139,9 @@
*
* Returns 0 on success.
*/
-static JdwpNetState* netStartup(short port) {
+static JdwpNetState* netStartup(short port, bool probe) {
int one = 1;
JdwpNetState* netState = new JdwpNetState;
-
if (port < 0) {
return netState;
}
@@ -151,13 +150,13 @@
netState->listenSock = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
if (netState->listenSock < 0) {
- PLOG(ERROR) << "Socket create failed";
+ PLOG(probe ? ERROR : FATAL) << "Socket create failed";
goto fail;
}
/* allow immediate re-use */
if (setsockopt(netState->listenSock, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)) < 0) {
- PLOG(ERROR) << "setsockopt(SO_REUSEADDR) failed";
+ PLOG(probe ? ERROR : FATAL) << "setsockopt(SO_REUSEADDR) failed";
goto fail;
}
@@ -170,15 +169,14 @@
inet_aton("127.0.0.1", &addr.addrInet.sin_addr);
if (bind(netState->listenSock, &addr.addrPlain, sizeof(addr)) != 0) {
- PLOG(VERBOSE) << "attempt to bind to port " << port << " failed";
+ PLOG(probe ? ERROR : FATAL) << "Attempt to bind to port " << port << " failed";
goto fail;
}
netState->listenPort = port;
- LOG(VERBOSE) << "+++ bound to port " << netState->listenPort;
if (listen(netState->listenSock, 5) != 0) {
- PLOG(ERROR) << "Listen failed";
+ PLOG(probe ? ERROR : FATAL) << "Listen failed";
goto fail;
}
diff --git a/src/thread.cc b/src/thread.cc
index cc6c7e8..0a23d46 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -348,7 +348,7 @@
stack_base_ = reinterpret_cast<byte*>(temp_stack_base);
if (stack_size_ <= kStackOverflowReservedBytes) {
- LOG(FATAL) << "attempt to attach a thread with a too-small stack (" << stack_size_ << " bytes)";
+ LOG(FATAL) << "Attempt to attach a thread with a too-small stack (" << stack_size_ << " bytes)";
}
// Set stack_end_ to the bottom of the stack saving space of stack overflows
@@ -676,7 +676,7 @@
// Double-check the TLS slot allocation.
if (pthread_getspecific(pthread_key_self_) != NULL) {
- LOG(FATAL) << "newly-created pthread TLS slot is not NULL";
+ LOG(FATAL) << "Newly-created pthread TLS slot is not NULL";
}
}
diff --git a/src/thread_list.cc b/src/thread_list.cc
index e646231..03998b6 100644
--- a/src/thread_list.cc
+++ b/src/thread_list.cc
@@ -101,8 +101,8 @@
void ThreadList::ModifySuspendCount(Thread* thread, int delta, bool for_debugger) {
#ifndef NDEBUG
- DCHECK(delta == -1 || delta == +1 || delta == thread->debug_suspend_count_) << delta << " "
- << *thread;
+ DCHECK(delta == -1 || delta == +1 || delta == -thread->debug_suspend_count_)
+ << delta << " " << thread->debug_suspend_count_ << " " << *thread;
DCHECK_GE(thread->suspend_count_, thread->debug_suspend_count_) << *thread;
#endif
if (delta == -1 && thread->suspend_count_ <= 0) {