Fix breakpoints.
My manual testing in jdb worked because I only ever set breakpoints at
offset 0, which is the same whether you're using bytecode or 16bitcode.
Change-Id: I7aae1961d08fc008af275ff0d1ae1f1e4619fa18
diff --git a/src/debugger.cc b/src/debugger.cc
index 12d4034..8c28cd9 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -177,8 +177,9 @@
static bool IsBreakpoint(Method* m, uint32_t dex_pc) {
MutexLock mu(gBreakpointsLock);
+ uint32_t pc = dex_pc / 2; // dex bytecodes are twice the size JDWP expects.
for (size_t i = 0; i < gBreakpoints.size(); ++i) {
- if (gBreakpoints[i].method == m && gBreakpoints[i].pc == dex_pc) {
+ if (gBreakpoints[i].method == m && gBreakpoints[i].pc == pc) {
VLOG(jdwp) << "Hit breakpoint #" << i << ": " << gBreakpoints[i];
return true;
}
@@ -898,7 +899,7 @@
location.typeTag = c->IsInterface() ? JDWP::TT_INTERFACE : JDWP::TT_CLASS;
location.classId = gRegistry->Add(c);
location.methodId = ToMethodId(m);
- location.idx = m->IsNative() ? -1 : m->ToDexPC(native_pc);
+ location.idx = m->IsNative() ? -1 : m->ToDexPC(native_pc) / 2;
}
}
@@ -1610,7 +1611,7 @@
location.typeTag = c->IsInterface() ? JDWP::TT_INTERFACE : JDWP::TT_CLASS;
location.classId = gRegistry->Add(c);
location.methodId = ToMethodId(m);
- location.idx = m->IsNative() ? -1 : dex_pc;
+ location.idx = m->IsNative() ? -1 : dex_pc / 2;
// Note we use "NoReg" so we don't keep track of references that are
// never actually sent to the debugger. 'this_id' is only used to
@@ -1667,30 +1668,24 @@
}
void Dbg::UpdateDebugger(int32_t dex_pc, Thread* self, Method** sp) {
- if (!gDebuggerActive) {
+ if (!gDebuggerActive || dex_pc == -2 /* fake method exit */) {
return;
}
Frame f(sp);
f.Next(); // Skip callee save frame.
Method* m = f.GetMethod();
- int event_flags = 0;
- // Update xtra.currentPc on every instruction. We need to do this if
- // there's a chance that we could get suspended. This can happen if
- // event_flags != 0 here, or somebody manually requests a suspend
- // (which gets handled at PERIOD_CHECKS time). One place where this
- // needs to be correct is in dvmAddSingleStep().
- //dvmExportPC(pc, fp);
-
- // We use a pc of -1 to represent method entry, since we might branch back to pc 0 later.
if (dex_pc == -1) {
- event_flags |= kMethodEntry;
+ // We use a pc of -1 to represent method entry, since we might branch back to pc 0 later.
+ // This means that for this special notification, there can't be anything else interesting
+ // going on, so we're done already.
+ Dbg::PostLocationEvent(m, 0, GetThis(f), kMethodEntry);
+ return;
}
- // See if we have a breakpoint here.
- // Depending on the "mods" associated with event(s) on this address,
- // we may or may not actually send a message to the debugger.
+ int event_flags = 0;
+
if (IsBreakpoint(m, dex_pc)) {
event_flags |= kBreakpoint;
}
diff --git a/src/jdwp/jdwp.h b/src/jdwp/jdwp.h
index 7bbc54f..a3f4b69 100644
--- a/src/jdwp/jdwp.h
+++ b/src/jdwp/jdwp.h
@@ -79,6 +79,8 @@
uint64_t idx; // A Dex PC.
};
std::ostream& operator<<(std::ostream& os, const JdwpLocation& rhs);
+bool operator==(const JdwpLocation& lhs, const JdwpLocation& rhs);
+bool operator!=(const JdwpLocation& lhs, const JdwpLocation& rhs);
/*
* How we talk to the debugger.
diff --git a/src/jdwp/jdwp_event.cc b/src/jdwp/jdwp_event.cc
index 5101f2f..285caf3 100644
--- a/src/jdwp/jdwp_event.cc
+++ b/src/jdwp/jdwp_event.cc
@@ -326,10 +326,10 @@
* Run through the list and remove any entries with an expired "count" mod
* from the event list, then free the match list.
*/
-void JdwpState::CleanupMatchList(JdwpEvent** matchList, int matchCount) {
+void JdwpState::CleanupMatchList(JdwpEvent** matchList, int match_count) {
JdwpEvent** ppEvent = matchList;
- while (matchCount--) {
+ while (match_count--) {
JdwpEvent* pEvent = *ppEvent;
for (int i = 0; i < pEvent->modCount; i++) {
@@ -369,21 +369,6 @@
}
/*
- * See if two locations are equal.
- *
- * It's tempting to do a bitwise compare ("struct ==" or memcmp), but if
- * the storage wasn't zeroed out there could be undefined values in the
- * padding. Besides, the odds of "idx" being equal while the others aren't
- * is very small, so this is usually just a simple integer comparison.
- */
-static inline bool LocationMatch(const JdwpLocation* pLoc1, const JdwpLocation* pLoc2) {
- return pLoc1->idx == pLoc2->idx &&
- pLoc1->methodId == pLoc2->methodId &&
- pLoc1->classId == pLoc2->classId &&
- pLoc1->typeTag == pLoc2->typeTag;
-}
-
-/*
* See if the event's mods match up with the contents of "basket".
*
* If we find a Count mod before rejecting an event, we decrement it. We
@@ -422,7 +407,7 @@
}
break;
case MK_LOCATION_ONLY:
- if (!LocationMatch(&pMod->locationOnly.loc, basket->pLoc)) {
+ if (pMod->locationOnly.loc != *basket->pLoc) {
return false;
}
break;
@@ -486,10 +471,10 @@
* Scan through the list of matches and determine the most severe
* suspension policy.
*/
-static JdwpSuspendPolicy scanSuspendPolicy(JdwpEvent** matchList, int matchCount) {
+static JdwpSuspendPolicy scanSuspendPolicy(JdwpEvent** matchList, int match_count) {
JdwpSuspendPolicy policy = SP_NONE;
- while (matchCount--) {
+ while (match_count--) {
if ((*matchList)->suspendPolicy > policy) {
policy = (*matchList)->suspendPolicy;
}
@@ -757,40 +742,40 @@
}
JdwpEvent** matchList = AllocMatchList(numEvents);
- int matchCount = 0;
+ int match_count = 0;
ExpandBuf* pReq = NULL;
JdwpSuspendPolicy suspendPolicy = SP_NONE;
{
MutexLock mu(event_lock_);
if ((eventFlags & Dbg::kBreakpoint) != 0) {
- FindMatchingEvents(EK_BREAKPOINT, &basket, matchList, &matchCount);
+ FindMatchingEvents(EK_BREAKPOINT, &basket, matchList, &match_count);
}
if ((eventFlags & Dbg::kSingleStep) != 0) {
- FindMatchingEvents(EK_SINGLE_STEP, &basket, matchList, &matchCount);
+ FindMatchingEvents(EK_SINGLE_STEP, &basket, matchList, &match_count);
}
if ((eventFlags & Dbg::kMethodEntry) != 0) {
- FindMatchingEvents(EK_METHOD_ENTRY, &basket, matchList, &matchCount);
+ FindMatchingEvents(EK_METHOD_ENTRY, &basket, matchList, &match_count);
}
if ((eventFlags & Dbg::kMethodExit) != 0) {
- FindMatchingEvents(EK_METHOD_EXIT, &basket, matchList, &matchCount);
+ FindMatchingEvents(EK_METHOD_EXIT, &basket, matchList, &match_count);
// TODO: match EK_METHOD_EXIT_WITH_RETURN_VALUE too; we need to include the 'value', though.
- //FindMatchingEvents(EK_METHOD_EXIT_WITH_RETURN_VALUE, &basket, matchList, &matchCount);
+ //FindMatchingEvents(EK_METHOD_EXIT_WITH_RETURN_VALUE, &basket, matchList, &match_count);
}
- if (matchCount != 0) {
- VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << matchCount << " total) "
+ if (match_count != 0) {
+ VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << match_count << " total) "
<< basket.className << "." << Dbg::GetMethodName(pLoc->classId, pLoc->methodId)
<< " thread=" << (void*) basket.threadId << " code=" << (void*) pLoc->idx << ")";
- suspendPolicy = scanSuspendPolicy(matchList, matchCount);
+ suspendPolicy = scanSuspendPolicy(matchList, match_count);
VLOG(jdwp) << " suspendPolicy=" << suspendPolicy;
pReq = eventPrep();
expandBufAdd1(pReq, suspendPolicy);
- expandBufAdd4BE(pReq, matchCount);
+ expandBufAdd4BE(pReq, match_count);
- for (int i = 0; i < matchCount; i++) {
+ for (int i = 0; i < match_count; i++) {
expandBufAdd1(pReq, matchList[i]->eventKind);
expandBufAdd4BE(pReq, matchList[i]->requestId);
expandBufAdd8BE(pReq, basket.threadId);
@@ -798,7 +783,7 @@
}
}
- CleanupMatchList(matchList, matchCount);
+ CleanupMatchList(matchList, match_count);
}
/* send request and possibly suspend ourselves */
@@ -814,7 +799,7 @@
Dbg::ThreadContinuing(old_state);
}
- return matchCount != 0;
+ return match_count != 0;
}
/*
@@ -840,37 +825,37 @@
ExpandBuf* pReq = NULL;
JdwpSuspendPolicy suspendPolicy = SP_NONE;
- int matchCount = 0;
+ int match_count = 0;
{
// Don't allow the list to be updated while we scan it.
MutexLock mu(event_lock_);
JdwpEvent** matchList = AllocMatchList(numEvents);
if (start) {
- FindMatchingEvents(EK_THREAD_START, &basket, matchList, &matchCount);
+ FindMatchingEvents(EK_THREAD_START, &basket, matchList, &match_count);
} else {
- FindMatchingEvents(EK_THREAD_DEATH, &basket, matchList, &matchCount);
+ FindMatchingEvents(EK_THREAD_DEATH, &basket, matchList, &match_count);
}
- if (matchCount != 0) {
- VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << matchCount << " total) "
+ if (match_count != 0) {
+ VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << match_count << " total) "
<< "thread=" << (void*) basket.threadId << ")";
- suspendPolicy = scanSuspendPolicy(matchList, matchCount);
+ suspendPolicy = scanSuspendPolicy(matchList, match_count);
VLOG(jdwp) << " suspendPolicy=" << suspendPolicy;
pReq = eventPrep();
expandBufAdd1(pReq, suspendPolicy);
- expandBufAdd4BE(pReq, matchCount);
+ expandBufAdd4BE(pReq, match_count);
- for (int i = 0; i < matchCount; i++) {
+ for (int i = 0; i < match_count; i++) {
expandBufAdd1(pReq, matchList[i]->eventKind);
expandBufAdd4BE(pReq, matchList[i]->requestId);
expandBufAdd8BE(pReq, basket.threadId);
}
}
- CleanupMatchList(matchList, matchCount);
+ CleanupMatchList(matchList, match_count);
}
/* send request and possibly suspend ourselves */
@@ -885,7 +870,7 @@
Dbg::ThreadContinuing(old_state);
}
- return matchCount != 0;
+ return match_count != 0;
}
/*
@@ -939,14 +924,14 @@
}
JdwpEvent** matchList = AllocMatchList(numEvents);
- int matchCount = 0;
+ int match_count = 0;
ExpandBuf* pReq = NULL;
JdwpSuspendPolicy suspendPolicy = SP_NONE;
{
MutexLock mu(event_lock_);
- FindMatchingEvents(EK_EXCEPTION, &basket, matchList, &matchCount);
- if (matchCount != 0) {
- VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << matchCount << " total)"
+ 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
<< " caught=" << basket.caught << ")";
@@ -957,14 +942,14 @@
VLOG(jdwp) << " catch: " << *pCatchLoc;
}
- suspendPolicy = scanSuspendPolicy(matchList, matchCount);
+ suspendPolicy = scanSuspendPolicy(matchList, match_count);
VLOG(jdwp) << " suspendPolicy=" << suspendPolicy;
pReq = eventPrep();
expandBufAdd1(pReq, suspendPolicy);
- expandBufAdd4BE(pReq, matchCount);
+ expandBufAdd4BE(pReq, match_count);
- for (int i = 0; i < matchCount; i++) {
+ for (int i = 0; i < match_count; i++) {
expandBufAdd1(pReq, matchList[i]->eventKind);
expandBufAdd4BE(pReq, matchList[i]->requestId);
expandBufAdd8BE(pReq, basket.threadId);
@@ -979,7 +964,7 @@
Dbg::RegisterObjectId(exceptionId);
}
- CleanupMatchList(matchList, matchCount);
+ CleanupMatchList(matchList, match_count);
}
/* send request and possibly suspend ourselves */
@@ -995,7 +980,7 @@
Dbg::ThreadContinuing(old_state);
}
- return matchCount != 0;
+ return match_count != 0;
}
/*
@@ -1019,17 +1004,17 @@
}
JdwpEvent** matchList = AllocMatchList(numEvents);
- int matchCount = 0;
+ int match_count = 0;
ExpandBuf* pReq = NULL;
JdwpSuspendPolicy suspendPolicy = SP_NONE;
{
MutexLock mu(event_lock_);
- FindMatchingEvents(EK_CLASS_PREPARE, &basket, matchList, &matchCount);
- if (matchCount != 0) {
- VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << matchCount << " total) "
+ 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;
- suspendPolicy = scanSuspendPolicy(matchList, matchCount);
+ suspendPolicy = scanSuspendPolicy(matchList, match_count);
VLOG(jdwp) << " suspendPolicy=" << suspendPolicy;
if (basket.threadId == debugThreadId) {
@@ -1047,9 +1032,9 @@
pReq = eventPrep();
expandBufAdd1(pReq, suspendPolicy);
- expandBufAdd4BE(pReq, matchCount);
+ expandBufAdd4BE(pReq, match_count);
- for (int i = 0; i < matchCount; i++) {
+ for (int i = 0; i < match_count; i++) {
expandBufAdd1(pReq, matchList[i]->eventKind);
expandBufAdd4BE(pReq, matchList[i]->requestId);
expandBufAdd8BE(pReq, basket.threadId);
@@ -1060,7 +1045,7 @@
expandBufAdd4BE(pReq, status);
}
}
- CleanupMatchList(matchList, matchCount);
+ CleanupMatchList(matchList, match_count);
}
/* send request and possibly suspend ourselves */
@@ -1075,7 +1060,7 @@
Dbg::ThreadContinuing(old_state);
}
- return matchCount != 0;
+ return match_count != 0;
}
/*
diff --git a/src/jdwp/jdwp_main.cc b/src/jdwp/jdwp_main.cc
index a7e82aa..0fad510 100644
--- a/src/jdwp/jdwp_main.cc
+++ b/src/jdwp/jdwp_main.cc
@@ -457,6 +457,15 @@
return os;
}
+bool operator==(const JdwpLocation& lhs, const JdwpLocation& rhs) {
+ return lhs.idx == rhs.idx && lhs.methodId == rhs.methodId &&
+ lhs.classId == rhs.classId && lhs.typeTag == rhs.typeTag;
+}
+
+bool operator!=(const JdwpLocation& lhs, const JdwpLocation& rhs) {
+ return !(lhs == rhs);
+}
+
std::ostream& operator<<(std::ostream& os, const JdwpTag& value) {
switch (value) {
case JT_ARRAY: os << "JT_ARRAY"; break;
