Merge "Implement floating point compare and long compare." into dalvik-dev
diff --git a/src/debugger.cc b/src/debugger.cc
index 6f381d3..cd1081d 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -22,6 +22,7 @@
 
 #include "class_linker.h"
 #include "class_loader.h"
+#include "dex_verifier.h" // For Instruction.
 #include "context.h"
 #include "object_utils.h"
 #include "ScopedLocalRef.h"
@@ -112,6 +113,31 @@
   }
 };
 
+struct Breakpoint {
+  Method* method;
+  uint32_t pc;
+  Breakpoint(Method* method, uint32_t pc) : method(method), pc(pc) {}
+};
+
+static std::ostream& operator<<(std::ostream& os, const Breakpoint& rhs) {
+  os << "Breakpoint[" << PrettyMethod(rhs.method) << " @" << rhs.pc << "]";
+  return os;
+}
+
+struct SingleStepControl {
+  // Are we single-stepping right now?
+  bool is_active;
+  Thread* thread;
+
+  JDWP::JdwpStepSize step_size;
+  JDWP::JdwpStepDepth step_depth;
+
+  const Method* method;
+  int line; // May be -1.
+  //const AddressSet* pAddressSet;    /* if non-null, address set for line */
+  int stack_depth;
+};
+
 // JDWP is allowed unless the Zygote forbids it.
 static bool gJdwpAllowed = true;
 
@@ -125,6 +151,7 @@
 static JDWP::JdwpState* gJdwpState = NULL;
 static bool gDebuggerConnected;  // debugger or DDMS is connected.
 static bool gDebuggerActive;     // debugger is making requests.
+static bool gDisposed;           // debugger called VirtualMachine.Dispose, so we should drop the connection.
 
 static bool gDdmThreadNotification = false;
 
@@ -143,6 +170,22 @@
 static size_t gAllocRecordHead = 0;
 static size_t gAllocRecordCount = 0;
 
+// Breakpoints and single-stepping.
+static Mutex gBreakpointsLock("breakpoints lock");
+static std::vector<Breakpoint> gBreakpoints;
+static SingleStepControl gSingleStepControl;
+
+static bool IsBreakpoint(Method* m, uint32_t dex_pc) {
+  MutexLock mu(gBreakpointsLock);
+  for (size_t i = 0; i < gBreakpoints.size(); ++i) {
+    if (gBreakpoints[i].method == m && gBreakpoints[i].pc == dex_pc) {
+      VLOG(jdwp) << "Hit breakpoint #" << i << ": " << gBreakpoints[i];
+      return true;
+    }
+  }
+  return false;
+}
+
 static JDWP::JdwpTag BasicTagFromDescriptor(const char* descriptor) {
   // JDWP deliberately uses the descriptor characters' ASCII values for its enum.
   // Note that by "basic" we mean that we don't get more specific than JT_OBJECT.
@@ -379,6 +422,15 @@
   CHECK(!gDebuggerConnected);
   VLOG(jdwp) << "JDWP has attached";
   gDebuggerConnected = true;
+  gDisposed = false;
+}
+
+void Dbg::Disposed() {
+  gDisposed = true;
+}
+
+bool Dbg::IsDisposed() {
+  return gDisposed;
 }
 
 void Dbg::GoActive() {
@@ -493,6 +545,12 @@
   return o->AsClass();
 }
 
+static Thread* DecodeThread(JDWP::ObjectId threadId) {
+  Object* thread_peer = gRegistry->Get<Object*>(threadId);
+  CHECK(thread_peer != NULL);
+  return Thread::FromManagedThread(thread_peer);
+}
+
 JDWP::JdwpError Dbg::GetSuperclass(JDWP::RefTypeId id, JDWP::RefTypeId& superclassId) {
   JDWP::JdwpError status;
   Class* c = DecodeClass(id, status);
@@ -557,10 +615,6 @@
   Runtime::Current()->GetClassLinker()->VisitClasses(ClassListCreator::Visit, &clc);
 }
 
-void Dbg::GetVisibleClassList(JDWP::ObjectId classLoaderId, uint32_t* pNumClasses, JDWP::RefTypeId** pClassRefBuf) {
-  UNIMPLEMENTED(FATAL);
-}
-
 bool Dbg::GetClassInfo(JDWP::RefTypeId classId, JDWP::JdwpTypeTag* pTypeTag, uint32_t* pStatus, std::string* pDescriptor) {
   Object* o = gRegistry->Get<Object*>(classId);
   if (o == NULL || !o->IsClass()) {
@@ -607,11 +661,6 @@
   *pRefTypeId = gRegistry->Add(o->GetClass());
 }
 
-uint8_t Dbg::GetClassObjectType(JDWP::RefTypeId refTypeId) {
-  UNIMPLEMENTED(FATAL);
-  return 0;
-}
-
 JDWP::JdwpError Dbg::GetSignature(JDWP::RefTypeId refTypeId, std::string& signature) {
   JDWP::JdwpError status;
   Class* c = DecodeClass(refTypeId, status);
@@ -797,7 +846,7 @@
   return gRegistry->Get<Class*>(instClassId)->InstanceOf(gRegistry->Get<Class*>(classId));
 }
 
-JDWP::FieldId ToFieldId(const Field* f) {
+static JDWP::FieldId ToFieldId(const Field* f) {
 #ifdef MOVING_GARBAGE_COLLECTOR
   UNIMPLEMENTED(FATAL);
 #else
@@ -805,7 +854,7 @@
 #endif
 }
 
-JDWP::MethodId ToMethodId(const Method* m) {
+static JDWP::MethodId ToMethodId(const Method* m) {
 #ifdef MOVING_GARBAGE_COLLECTOR
   UNIMPLEMENTED(FATAL);
 #else
@@ -813,7 +862,7 @@
 #endif
 }
 
-Field* FromFieldId(JDWP::FieldId fid) {
+static Field* FromFieldId(JDWP::FieldId fid) {
 #ifdef MOVING_GARBAGE_COLLECTOR
   UNIMPLEMENTED(FATAL);
 #else
@@ -821,7 +870,7 @@
 #endif
 }
 
-Method* FromMethodId(JDWP::MethodId mid) {
+static Method* FromMethodId(JDWP::MethodId mid) {
 #ifdef MOVING_GARBAGE_COLLECTOR
   UNIMPLEMENTED(FATAL);
 #else
@@ -829,7 +878,7 @@
 #endif
 }
 
-void SetLocation(JDWP::JdwpLocation& location, Method* m, uintptr_t native_pc) {
+static void SetLocation(JDWP::JdwpLocation& location, Method* m, uintptr_t native_pc) {
   if (m == NULL) {
     memset(&location, 0, sizeof(location));
   } else {
@@ -1127,12 +1176,6 @@
   return s->ToModifiedUtf8();
 }
 
-Thread* DecodeThread(JDWP::ObjectId threadId) {
-  Object* thread_peer = gRegistry->Get<Object*>(threadId);
-  CHECK(thread_peer != NULL);
-  return Thread::FromManagedThread(thread_peer);
-}
-
 bool Dbg::GetThreadName(JDWP::ObjectId threadId, std::string& name) {
   ScopedThreadListLock thread_list_lock;
   Thread* thread = DecodeThread(threadId);
@@ -1286,8 +1329,7 @@
   GetThreadGroupThreadsImpl(NULL, ppThreadIds, pThreadCount);
 }
 
-int Dbg::GetThreadFrameCount(JDWP::ObjectId threadId) {
-  ScopedThreadListLock thread_list_lock;
+static int GetStackDepth(Thread* thread) {
   struct CountStackDepthVisitor : public Thread::StackVisitor {
     CountStackDepthVisitor() : depth(0) {}
     virtual void VisitFrame(const Frame& f, uintptr_t) {
@@ -1299,10 +1341,15 @@
     size_t depth;
   };
   CountStackDepthVisitor visitor;
-  DecodeThread(threadId)->WalkStack(&visitor);
+  thread->WalkStack(&visitor);
   return visitor.depth;
 }
 
+int Dbg::GetThreadFrameCount(JDWP::ObjectId threadId) {
+  ScopedThreadListLock thread_list_lock;
+  return GetStackDepth(DecodeThread(threadId));
+}
+
 bool Dbg::GetThreadFrame(JDWP::ObjectId threadId, int desired_frame_number, JDWP::FrameId* pFrameId, JDWP::JdwpLocation* pLoc) {
   ScopedThreadListLock thread_list_lock;
   struct GetFrameVisitor : public Thread::StackVisitor {
@@ -1597,6 +1644,9 @@
     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
@@ -1611,85 +1661,73 @@
     event_flags |= kMethodEntry;
   }
 
-  /*
   // 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.
-  if (GET_OPCODE(*pc) == OP_BREAKPOINT) {
-    ALOGV("+++ breakpoint hit at %p", pc);
-    event_flags |= kBreakPoint;
+  if (IsBreakpoint(m, dex_pc)) {
+    event_flags |= kBreakpoint;
   }
-  */
 
-  /*
   // If the debugger is single-stepping one of our threads, check to
   // see if we're that thread and we've reached a step point.
-  const StepControl* pCtrl = &gDvm.stepControl;
-  if (pCtrl->active && pCtrl->thread == self) {
-    int frameDepth;
-    bool doStop = false;
-    const char* msg = NULL;
-
-    assert(!dvmIsNativeMethod(method));
-
-    if (pCtrl->depth == SD_INTO) {
+  if (gSingleStepControl.is_active && gSingleStepControl.thread == self) {
+    CHECK(!m->IsNative());
+    if (gSingleStepControl.step_depth == JDWP::SD_INTO) {
       // Step into method calls.  We break when the line number
       // or method pointer changes.  If we're in SS_MIN mode, we
       // always stop.
-      if (pCtrl->method != method) {
-        doStop = true;
-        msg = "new method";
-      } else if (pCtrl->size == SS_MIN) {
-        doStop = true;
-        msg = "new instruction";
-      } else if (!dvmAddressSetGet(pCtrl->pAddressSet, pc - method->insns)) {
-        doStop = true;
-        msg = "new line";
+      if (gSingleStepControl.method != m) {
+        event_flags |= kSingleStep;
+        VLOG(jdwp) << "SS new method";
+      } else if (gSingleStepControl.step_size == JDWP::SS_MIN) {
+        event_flags |= kSingleStep;
+        VLOG(jdwp) << "SS new instruction";
+//      } else if (!dvmAddressSetGet(gSingleStepControl.pAddressSet, pc - method->insns)) {
+//        event_flags |= kSingleStep;
+//        VLOG(jdwp) << "SS new line";
       }
-    } else if (pCtrl->depth == SD_OVER) {
+    } else if (gSingleStepControl.step_depth == JDWP::SD_OVER) {
       // Step over method calls.  We break when the line number is
       // different and the frame depth is <= the original frame
       // depth.  (We can't just compare on the method, because we
       // might get unrolled past it by an exception, and it's tricky
       // to identify recursion.)
-      frameDepth = dvmComputeVagueFrameDepth(self, fp);
-      if (frameDepth < pCtrl->frameDepth) {
+
+      // TODO: can we just use the value of 'sp'?
+      int stack_depth = GetStackDepth(self);
+
+      if (stack_depth < gSingleStepControl.stack_depth) {
         // popped up one or more frames, always trigger
-        doStop = true;
-        msg = "method pop";
-      } else if (frameDepth == pCtrl->frameDepth) {
+        event_flags |= kSingleStep;
+        VLOG(jdwp) << "SS method pop";
+      } else if (stack_depth == gSingleStepControl.stack_depth) {
         // same depth, see if we moved
-        if (pCtrl->size == SS_MIN) {
-          doStop = true;
-          msg = "new instruction";
-        } else if (!dvmAddressSetGet(pCtrl->pAddressSet, pc - method->insns)) {
-          doStop = true;
-          msg = "new line";
+        if (gSingleStepControl.step_size == JDWP::SS_MIN) {
+          event_flags |= kSingleStep;
+          VLOG(jdwp) << "SS new instruction";
+//        } else if (!dvmAddressSetGet(gSingleStepControl.pAddressSet, pc - method->insns)) {
+//          event_flags |= kSingleStep;
+//          VLOG(jdwp) << "SS new line";
         }
       }
     } else {
-      assert(pCtrl->depth == SD_OUT);
+      CHECK_EQ(gSingleStepControl.step_depth, JDWP::SD_OUT);
       // Return from the current method.  We break when the frame
       // depth pops up.
 
       // This differs from the "method exit" break in that it stops
       // with the PC at the next instruction in the returned-to
       // function, rather than the end of the returning function.
-      frameDepth = dvmComputeVagueFrameDepth(self, fp);
-      if (frameDepth < pCtrl->frameDepth) {
-        doStop = true;
-        msg = "method pop";
+
+      // TODO: can we just use the value of 'sp'?
+      int stack_depth = GetStackDepth(self);
+      if (stack_depth < gSingleStepControl.stack_depth) {
+        event_flags |= kSingleStep;
+        VLOG(jdwp) << "SS method pop";
       }
     }
-
-    if (doStop) {
-      ALOGV("#####S %s", msg);
-      event_flags |= kSingleStep;
-    }
   }
-  */
 
-  /*
   // Check to see if this is a "return" instruction.  JDWP says we should
   // send the event *after* the code has been executed, but it also says
   // the location we provide is the last instruction.  Since the "return"
@@ -1698,37 +1736,80 @@
   // we potentially need to combine it with other events.)
   // We're also not supposed to generate a method exit event if the method
   // terminates "with a thrown exception".
-  u2 opcode = GET_OPCODE(*pc);
-  if (opcode == OP_RETURN_VOID || opcode == OP_RETURN || opcode == OP_RETURN_WIDE ||opcode == OP_RETURN_OBJECT) {
-    event_flags |= kMethodExit;
+  if (dex_pc >= 0) {
+    const DexFile::CodeItem* code_item = MethodHelper(m).GetCodeItem();
+    CHECK(code_item != NULL);
+    CHECK_LT(dex_pc, static_cast<int32_t>(code_item->insns_size_in_code_units_));
+    if (Instruction::At(&code_item->insns_[dex_pc])->IsReturn()) {
+      event_flags |= kMethodExit;
+    }
   }
-  */
 
   // If there's something interesting going on, see if it matches one
   // of the debugger filters.
   if (event_flags != 0) {
-    Frame f(sp);
-    f.Next(); // Skip callee save frame.
-    Dbg::PostLocationEvent(f.GetMethod(), dex_pc, GetThis(f), event_flags);
+    Dbg::PostLocationEvent(m, dex_pc, GetThis(f), event_flags);
   }
 }
 
-bool Dbg::WatchLocation(const JDWP::JdwpLocation* pLoc) {
-  UNIMPLEMENTED(FATAL);
-  return false;
+void Dbg::WatchLocation(const JDWP::JdwpLocation* location) {
+  MutexLock mu(gBreakpointsLock);
+  Method* m = FromMethodId(location->methodId);
+  gBreakpoints.push_back(Breakpoint(m, location->idx));
+  VLOG(jdwp) << "Set breakpoint #" << (gBreakpoints.size() - 1) << ": " << gBreakpoints[gBreakpoints.size() - 1];
 }
 
-void Dbg::UnwatchLocation(const JDWP::JdwpLocation* pLoc) {
-  UNIMPLEMENTED(FATAL);
+void Dbg::UnwatchLocation(const JDWP::JdwpLocation* location) {
+  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) {
+      VLOG(jdwp) << "Removed breakpoint #" << i << ": " << gBreakpoints[i];
+      gBreakpoints.erase(gBreakpoints.begin() + i);
+      return;
+    }
+  }
 }
 
-bool Dbg::ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize size, JDWP::JdwpStepDepth depth) {
-  UNIMPLEMENTED(FATAL);
-  return false;
+bool Dbg::ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize step_size, JDWP::JdwpStepDepth step_depth) {
+  Thread* thread = DecodeThread(threadId);
+
+  // TODO: there's no theoretical reason why we couldn't support single-stepping
+  // of multiple threads at once, but we never did so historically.
+  if (gSingleStepControl.thread != NULL && thread != gSingleStepControl.thread) {
+    LOG(WARNING) << "single-step already active for " << *gSingleStepControl.thread
+                 << "; switching to " << *thread;
+  }
+
+  struct SingleStepStackVisitor : public Thread::StackVisitor {
+    SingleStepStackVisitor() {
+      gSingleStepControl.method = NULL;
+      gSingleStepControl.stack_depth = 0;
+    }
+    virtual void VisitFrame(const Frame& f, uintptr_t) {
+      // TODO: we'll need to skip callee-save frames too.
+      if (f.HasMethod()) {
+        ++gSingleStepControl.stack_depth;
+        if (gSingleStepControl.method == NULL) {
+          gSingleStepControl.method = f.GetMethod();
+        }
+      }
+    }
+  };
+  SingleStepStackVisitor visitor;
+  thread->WalkStack(&visitor);
+
+  gSingleStepControl.thread = thread;
+  gSingleStepControl.step_size = step_size;
+  gSingleStepControl.step_depth = step_depth;
+  gSingleStepControl.is_active = true;
+
+  return true;
 }
 
 void Dbg::UnconfigureStep(JDWP::ObjectId threadId) {
-  UNIMPLEMENTED(FATAL);
+  gSingleStepControl.is_active = false;
+  gSingleStepControl.thread = NULL;
 }
 
 JDWP::JdwpError Dbg::InvokeMethod(JDWP::ObjectId threadId, JDWP::ObjectId objectId, JDWP::RefTypeId classId, JDWP::MethodId methodId, uint32_t numArgs, uint64_t* argArray, uint32_t options, JDWP::JdwpTag* pResultTag, uint64_t* pResultValue, JDWP::ObjectId* pExceptionId) {
diff --git a/src/debugger.h b/src/debugger.h
index dbb99ff..5089995 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -93,6 +93,7 @@
   static void Connected();
   static void GoActive();
   static void Disconnected();
+  static void Disposed();
 
   /*
    * Returns "true" if a debugger is connected.  Returns "false" if it's
@@ -102,6 +103,8 @@
 
   static bool IsDebuggingEnabled();
 
+  static bool IsDisposed();
+
   /*
    * Time, in milliseconds, since the last debugger activity.  Does not
    * include DDMS activity.  Returns -1 if there has been no activity.
@@ -135,11 +138,9 @@
   static bool GetAccessFlags(JDWP::RefTypeId id, uint32_t& access_flags);
   static bool IsInterface(JDWP::RefTypeId classId, bool& is_interface);
   static void GetClassList(std::vector<JDWP::RefTypeId>& classes);
-  static void GetVisibleClassList(JDWP::ObjectId classLoaderId, uint32_t* pNumClasses, JDWP::RefTypeId** pClassRefBuf);
   static bool GetClassInfo(JDWP::RefTypeId classId, JDWP::JdwpTypeTag* pTypeTag, uint32_t* pStatus, std::string* pDescriptor);
   static void FindLoadedClassBySignature(const char* descriptor, std::vector<JDWP::RefTypeId>& ids);
   static void GetObjectType(JDWP::ObjectId objectId, JDWP::JdwpTypeTag* pRefTypeTag, JDWP::RefTypeId* pRefTypeId);
-  static uint8_t GetClassObjectType(JDWP::RefTypeId refTypeId);
   static JDWP::JdwpError GetSignature(JDWP::RefTypeId refTypeId, std::string& signature);
   static bool GetSourceFile(JDWP::RefTypeId refTypeId, std::string& source_file);
   static uint8_t GetObjectTag(JDWP::ObjectId objectId);
@@ -209,7 +210,7 @@
    * Debugger notification
    */
   enum {
-    kBreakPoint     = 0x01,
+    kBreakpoint     = 0x01,
     kSingleStep     = 0x02,
     kMethodEntry    = 0x04,
     kMethodExit     = 0x08,
@@ -222,7 +223,7 @@
 
   static void UpdateDebugger(int32_t dex_pc, Thread* self, Method** sp);
 
-  static bool WatchLocation(const JDWP::JdwpLocation* pLoc);
+  static void WatchLocation(const JDWP::JdwpLocation* pLoc);
   static void UnwatchLocation(const JDWP::JdwpLocation* pLoc);
   static bool ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize size, JDWP::JdwpStepDepth depth);
   static void UnconfigureStep(JDWP::ObjectId threadId);
diff --git a/src/jdwp/jdwp_event.cc b/src/jdwp/jdwp_event.cc
index c0e6295..d68d011 100644
--- a/src/jdwp/jdwp_event.cc
+++ b/src/jdwp/jdwp_event.cc
@@ -760,7 +760,7 @@
 
   {
     MutexLock mu(event_lock_);
-    if ((eventFlags & Dbg::kBreakPoint) != 0) {
+    if ((eventFlags & Dbg::kBreakpoint) != 0) {
       FindMatchingEvents(EK_BREAKPOINT, &basket, matchList, &matchCount);
     }
     if ((eventFlags & Dbg::kSingleStep) != 0) {
@@ -771,11 +771,14 @@
     }
     if ((eventFlags & Dbg::kMethodExit) != 0) {
       FindMatchingEvents(EK_METHOD_EXIT, &basket, matchList, &matchCount);
+
+      // 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);
     }
     if (matchCount != 0) {
       VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << matchCount << " total) "
-                   << basket.className << "." << Dbg::GetMethodName(pLoc->classId, pLoc->methodId)
-                   << " thread=" << (void*) basket.threadId << " code=" << (void*) pLoc->idx << ")";
+                 << basket.className << "." << Dbg::GetMethodName(pLoc->classId, pLoc->methodId)
+                 << " thread=" << (void*) basket.threadId << " code=" << (void*) pLoc->idx << ")";
 
       suspendPolicy = scanSuspendPolicy(matchList, matchCount);
       VLOG(jdwp) << "  suspendPolicy=" << suspendPolicy;
diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index 3dbf437..d49454d 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -275,13 +275,8 @@
   return ERR_NONE;
 }
 
-/*
- * The debugger is politely asking to disconnect.  We're good with that.
- *
- * We could resume threads and clean up pinned references, but we can do
- * that when the TCP connection drops.
- */
 static JdwpError handleVM_Dispose(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
+  Dbg::Disposed();
   return ERR_NONE;
 }
 
@@ -416,7 +411,7 @@
   return ERR_NONE;
 }
 
-static JdwpError handleVM_AllClasses(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply, bool generic) {
+static JdwpError handleVM_AllClasses(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply, bool descriptor_and_status, bool generic) {
   std::vector<JDWP::RefTypeId> classes;
   Dbg::GetClassList(classes);
 
@@ -433,22 +428,24 @@
 
     expandBufAdd1(pReply, refTypeTag);
     expandBufAddRefTypeId(pReply, classes[i]);
-    expandBufAddUtf8String(pReply, descriptor);
-    if (generic) {
-      expandBufAddUtf8String(pReply, genericSignature);
+    if (descriptor_and_status) {
+      expandBufAddUtf8String(pReply, descriptor);
+      if (generic) {
+        expandBufAddUtf8String(pReply, genericSignature);
+      }
+      expandBufAdd4BE(pReply, status);
     }
-    expandBufAdd4BE(pReply, status);
   }
 
   return ERR_NONE;
 }
 
 static JdwpError handleVM_AllClasses(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
-  return handleVM_AllClasses(state, buf, dataLen, pReply, false);
+  return handleVM_AllClasses(state, buf, dataLen, pReply, true, false);
 }
 
 static JdwpError handleVM_AllClassesWithGeneric(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
-  return handleVM_AllClasses(state, buf, dataLen, pReply, true);
+  return handleVM_AllClasses(state, buf, dataLen, pReply, true, true);
 }
 
 /*
@@ -1187,56 +1184,13 @@
   return Dbg::SetArrayElements(arrayId, firstIndex, values, buf);
 }
 
-/*
- * Return the set of classes visible to a class loader.  All classes which
- * have the class loader as a defining or initiating loader are returned.
- */
 static JdwpError handleCLR_VisibleClasses(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
   ObjectId classLoaderObject;
-  uint32_t numClasses = 0;
-  RefTypeId* classRefBuf = NULL;
-  int i;
-
   classLoaderObject = ReadObjectId(&buf);
-
-  Dbg::GetVisibleClassList(classLoaderObject, &numClasses, &classRefBuf);
-
-  expandBufAdd4BE(pReply, numClasses);
-  for (i = 0; i < (int) numClasses; i++) {
-    uint8_t refTypeTag = Dbg::GetClassObjectType(classRefBuf[i]);
-
-    expandBufAdd1(pReply, refTypeTag);
-    expandBufAddRefTypeId(pReply, classRefBuf[i]);
-  }
-
-  return ERR_NONE;
-}
-
-/*
- * Return a newly-allocated string in which all occurrences of '.' have
- * been changed to '/'.  If we find a '/' in the original string, NULL
- * is returned to avoid ambiguity.
- */
-char* dvmDotToSlash(const char* str) {
-  char* newStr = strdup(str);
-  char* cp = newStr;
-
-  if (newStr == NULL) {
-    return NULL;
-  }
-
-  while (*cp != '\0') {
-    if (*cp == '/') {
-      CHECK(false);
-      return NULL;
-    }
-    if (*cp == '.') {
-      *cp = '/';
-    }
-    cp++;
-  }
-
-  return newStr;
+  // TODO: we should only return classes which have the given class loader as a defining or
+  // initiating loader. The former would be easy; the latter is hard, because we don't have
+  // any such notion.
+  return handleVM_AllClasses(state, buf, dataLen, pReply, false, false);
 }
 
 /*
@@ -1305,17 +1259,20 @@
       break;
     case MK_CLASS_MATCH:    /* restrict events to matching classes */
       {
+        // pattern is "java.foo.*", we want "java/foo/*".
         std::string pattern(ReadNewUtf8String(&buf));
+        std::replace(pattern.begin(), pattern.end(), '.', '/');
         VLOG(jdwp) << StringPrintf("    ClassMatch: '%s'", pattern.c_str());
-        /* pattern is "java.foo.*", we want "java/foo/ *" */
-        pEvent->mods[idx].classMatch.classPattern = dvmDotToSlash(pattern.c_str());
+        pEvent->mods[idx].classMatch.classPattern = strdup(pattern.c_str());
       }
       break;
     case MK_CLASS_EXCLUDE:  /* restrict events to non-matching classes */
       {
+        // pattern is "java.foo.*", we want "java/foo/*".
         std::string pattern(ReadNewUtf8String(&buf));
+        std::replace(pattern.begin(), pattern.end(), '.', '/');
         VLOG(jdwp) << StringPrintf("    ClassExclude: '%s'", pattern.c_str());
-        pEvent->mods[idx].classExclude.classPattern = dvmDotToSlash(pattern.c_str());
+        pEvent->mods[idx].classExclude.classPattern = strdup(pattern.c_str());
       }
       break;
     case MK_LOCATION_ONLY:  /* restrict certain events based on loc */
diff --git a/src/jdwp/jdwp_main.cc b/src/jdwp/jdwp_main.cc
index 72aff5c..e408a76 100644
--- a/src/jdwp/jdwp_main.cc
+++ b/src/jdwp/jdwp_main.cc
@@ -325,7 +325,7 @@
 
     /* process requests until the debugger drops */
     bool first = true;
-    while (true) {
+    while (!Dbg::IsDisposed()) {
       // sanity check -- shouldn't happen?
       if (Thread::Current()->GetState() != Thread::kVmWait) {
         LOG(ERROR) << "JDWP thread no longer in VMWAIT (now " << Thread::Current()->GetState() << "); resetting";