commit a656a0f6fbcf2ba3f15cae54a773b9c636dd32c1
author Elliott Hughes <enh@google.com> Tue Feb 21 18:03:44 2012 -0800
committer Elliott Hughes <enh@google.com> Tue Feb 21 18:11:42 2012 -0800
tree 94efe4a413f4aa71fe5a088d8e7f53ac0ef37ef7
parent 153f448a75d82f45894e57e5ef5bf88679a2101d

Fix a few more JDWP bugs.

My previous division by 2 of dex PCs was wrong-headed. I need to fix the test
instead to not try to set a breakpoint in the middle of an instruction.

Also, we weren't reporting some class-specific events that we should have done
because of an InstanceOf/IsAssignableFrom mixup.

Change-Id: I187fc0977955038270866c9d7d28b0a709408aa0

src/debugger.cc

diff --git a/src/debugger.cc b/src/debugger.cc
index 494ee73..d2c05b7 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -122,12 +122,12 @@
 
 struct Breakpoint {
   Method* method;
-  uint32_t pc;
-  Breakpoint(Method* method, uint32_t pc) : method(method), pc(pc) {}
+  uint32_t dex_pc;
+  Breakpoint(Method* method, uint32_t dex_pc) : method(method), dex_pc(dex_pc) {}
 };
 
 static std::ostream& operator<<(std::ostream& os, const Breakpoint& rhs) {
-  os << "Breakpoint[" << PrettyMethod(rhs.method) << " @" << rhs.pc << "]";
+  os << "Breakpoint[" << PrettyMethod(rhs.method) << " @" << rhs.dex_pc << "]";
   return os;
 }
 
@@ -184,9 +184,8 @@
 
 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 == pc) {
+    if (gBreakpoints[i].method == m && gBreakpoints[i].dex_pc == dex_pc) {
       VLOG(jdwp) << "Hit breakpoint #" << i << ": " << gBreakpoints[i];
       return true;
     }
@@ -893,12 +892,10 @@
 bool Dbg::MatchType(JDWP::RefTypeId instClassId, JDWP::RefTypeId classId) {
   JDWP::JdwpError status;
   Class* c1 = DecodeClass(instClassId, status);
+  CHECK(c1 != NULL);
   Class* c2 = DecodeClass(classId, status);
-  if (c1 == NULL || c2 == NULL) {
-    // TODO: it doesn't seem like we can do any better here?
-    return false;
-  }
-  return c1->InstanceOf(c2);
+  CHECK(c2 != NULL);
+  return c1->IsAssignableFrom(c2);
 }
 
 static JDWP::FieldId ToFieldId(const Field* f) {
@@ -941,7 +938,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) / 2;
+    location.idx = m->IsNative() ? -1 : m->ToDexPC(native_pc);
   }
 }
 
@@ -1455,6 +1452,10 @@
         : found(false), depth(0), desired_frame_number(desired_frame_number), pFrameId(pFrameId), pLoc(pLoc) {
     }
     virtual void VisitFrame(const Frame& f, uintptr_t pc) {
+      if (found) {
+        return;
+      }
+
       // TODO: we'll need to skip callee-save frames too.
       if (!f.HasMethod()) {
         return; // The debugger can't do anything useful with a frame that has no Method*.
@@ -1681,7 +1682,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 / 2;
+  location.idx = m->IsNative() ? -1 : dex_pc;
 
   // 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
@@ -1855,7 +1856,7 @@
   MutexLock mu(gBreakpointsLock);
   Method* m = FromMethodId(location->methodId);
   for (size_t i = 0; i < gBreakpoints.size(); ++i) {
-    if (gBreakpoints[i].method == m && gBreakpoints[i].pc == location->idx) {
+    if (gBreakpoints[i].method == m && gBreakpoints[i].dex_pc == location->idx) {
       VLOG(jdwp) << "Removed breakpoint #" << i << ": " << gBreakpoints[i];
       gBreakpoints.erase(gBreakpoints.begin() + i);
       return;