Support for single-stepping by line.

Plus various other test fixes.

Change-Id: I2ef923e56a16a14380eda150685b5e3db944616e
diff --git a/src/debugger.cc b/src/debugger.cc
index 662757d..12d4034 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -133,8 +133,8 @@
   JDWP::JdwpStepDepth step_depth;
 
   const Method* method;
-  int line; // May be -1.
-  //const AddressSet* pAddressSet;    /* if non-null, address set for line */
+  int32_t line_number; // Or -1 for native methods.
+  std::set<uint32_t> dex_pcs;
   int stack_depth;
 };
 
@@ -547,7 +547,9 @@
 
 static Thread* DecodeThread(JDWP::ObjectId threadId) {
   Object* thread_peer = gRegistry->Get<Object*>(threadId);
-  CHECK(thread_peer != NULL);
+  if (thread_peer == NULL) {
+    return NULL;
+  }
   return Thread::FromManagedThread(thread_peer);
 }
 
@@ -649,16 +651,26 @@
   }
 }
 
-void Dbg::GetObjectType(JDWP::ObjectId objectId, JDWP::JdwpTypeTag* pRefTypeTag, JDWP::RefTypeId* pRefTypeId) {
+JDWP::JdwpError Dbg::GetReferenceType(JDWP::ObjectId objectId, JDWP::ExpandBuf* pReply) {
   Object* o = gRegistry->Get<Object*>(objectId);
-  if (o->GetClass()->IsArrayClass()) {
-    *pRefTypeTag = JDWP::TT_ARRAY;
-  } else if (o->GetClass()->IsInterface()) {
-    *pRefTypeTag = JDWP::TT_INTERFACE;
-  } else {
-    *pRefTypeTag = JDWP::TT_CLASS;
+  if (o == NULL) {
+    return JDWP::ERR_INVALID_OBJECT;
   }
-  *pRefTypeId = gRegistry->Add(o->GetClass());
+
+  JDWP::JdwpTypeTag type_tag;
+  if (o->GetClass()->IsArrayClass()) {
+    type_tag = JDWP::TT_ARRAY;
+  } else if (o->GetClass()->IsInterface()) {
+    type_tag = JDWP::TT_INTERFACE;
+  } else {
+    type_tag = JDWP::TT_CLASS;
+  }
+  JDWP::RefTypeId type_id = gRegistry->Add(o->GetClass());
+
+  expandBufAdd1(pReply, type_tag);
+  expandBufAddRefTypeId(pReply, type_id);
+
+  return JDWP::ERR_NONE;
 }
 
 JDWP::JdwpError Dbg::GetSignature(JDWP::RefTypeId refTypeId, std::string& signature) {
@@ -1017,10 +1029,10 @@
     int numItems;
     JDWP::ExpandBuf* pReply;
 
-    static bool Callback(void* context, uint32_t address, uint32_t lineNum) {
+    static bool Callback(void* context, uint32_t address, uint32_t line_number) {
       DebugCallbackContext* pContext = reinterpret_cast<DebugCallbackContext*>(context);
       expandBufAdd8BE(pContext->pReply, address);
-      expandBufAdd4BE(pContext->pReply, lineNum);
+      expandBufAdd4BE(pContext->pReply, line_number);
       pContext->numItems++;
       return true;
     }
@@ -1186,9 +1198,16 @@
   return true;
 }
 
-JDWP::ObjectId Dbg::GetThreadGroup(JDWP::ObjectId threadId) {
+JDWP::JdwpError Dbg::GetThreadGroup(JDWP::ObjectId threadId, JDWP::ExpandBuf* pReply) {
   Object* thread = gRegistry->Get<Object*>(threadId);
-  CHECK(thread != NULL);
+  if (thread != NULL) {
+    return JDWP::ERR_INVALID_OBJECT;
+  }
+
+  // Okay, so it's an object, but is it actually a thread?
+  if (DecodeThread(threadId)) {
+    return JDWP::ERR_INVALID_THREAD;
+  }
 
   Class* c = Runtime::Current()->GetClassLinker()->FindSystemClass("Ljava/lang/Thread;");
   CHECK(c != NULL);
@@ -1196,7 +1215,10 @@
   CHECK(f != NULL);
   Object* group = f->GetObject(thread);
   CHECK(group != NULL);
-  return gRegistry->Add(group);
+  JDWP::ObjectId thread_group_id = gRegistry->Add(group);
+
+  expandBufAddObjectId(pReply, thread_group_id);
+  return JDWP::ERR_NONE;
 }
 
 std::string Dbg::GetThreadGroupName(JDWP::ObjectId threadGroupId) {
@@ -1269,8 +1291,13 @@
   return true;
 }
 
-uint32_t Dbg::GetThreadSuspendCount(JDWP::ObjectId threadId) {
-  return DecodeThread(threadId)->GetSuspendCount();
+JDWP::JdwpError Dbg::GetThreadSuspendCount(JDWP::ObjectId threadId, JDWP::ExpandBuf* pReply) {
+  Thread* thread = DecodeThread(threadId);
+  if (thread == NULL) {
+    return JDWP::ERR_INVALID_THREAD;
+  }
+  expandBufAdd4BE(pReply, thread->GetSuspendCount());
+  return JDWP::ERR_NONE;
 }
 
 bool Dbg::ThreadExists(JDWP::ObjectId threadId) {
@@ -1682,9 +1709,9 @@
       } 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 (gSingleStepControl.dex_pcs.find(dex_pc) == gSingleStepControl.dex_pcs.end()) {
+        event_flags |= kSingleStep;
+        VLOG(jdwp) << "SS new line";
       }
     } else if (gSingleStepControl.step_depth == JDWP::SD_OVER) {
       // Step over method calls.  We break when the line number is
@@ -1705,9 +1732,9 @@
         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 (gSingleStepControl.dex_pcs.find(dex_pc) == gSingleStepControl.dex_pcs.end()) {
+          event_flags |= kSingleStep;
+          VLOG(jdwp) << "SS new line";
         }
       }
     } else {
@@ -1771,8 +1798,11 @@
   }
 }
 
-bool Dbg::ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize step_size, JDWP::JdwpStepDepth step_depth) {
+JDWP::JdwpError Dbg::ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize step_size, JDWP::JdwpStepDepth step_depth) {
   Thread* thread = DecodeThread(threadId);
+  if (thread == NULL) {
+    return JDWP::ERR_INVALID_THREAD;
+  }
 
   // TODO: there's no theoretical reason why we couldn't support single-stepping
   // of multiple threads at once, but we never did so historically.
@@ -1781,17 +1811,29 @@
                  << "; switching to " << *thread;
   }
 
+  //
+  // Work out what Method* we're in, the current line number, and how deep the stack currently
+  // is for step-out.
+  //
+
   struct SingleStepStackVisitor : public Thread::StackVisitor {
     SingleStepStackVisitor() {
       gSingleStepControl.method = NULL;
       gSingleStepControl.stack_depth = 0;
     }
-    virtual void VisitFrame(const Frame& f, uintptr_t) {
+    virtual void VisitFrame(const Frame& f, uintptr_t pc) {
       // TODO: we'll need to skip callee-save frames too.
       if (f.HasMethod()) {
         ++gSingleStepControl.stack_depth;
         if (gSingleStepControl.method == NULL) {
-          gSingleStepControl.method = f.GetMethod();
+          const Method* m = f.GetMethod();
+          const DexCache* dex_cache = m->GetDeclaringClass()->GetDexCache();
+          gSingleStepControl.method = m;
+          gSingleStepControl.line_number = -1;
+          if (dex_cache != NULL) {
+            const DexFile& dex_file = Runtime::Current()->GetClassLinker()->FindDexFile(dex_cache);
+            gSingleStepControl.line_number = dex_file.GetLineNumFromPC(m, m->ToDexPC(pc));
+          }
         }
       }
     }
@@ -1799,17 +1841,85 @@
   SingleStepStackVisitor visitor;
   thread->WalkStack(&visitor);
 
+  //
+  // Find the dex_pc values that correspond to the current line, for line-based single-stepping.
+  //
+
+  struct DebugCallbackContext {
+    DebugCallbackContext() {
+      last_pc_valid = false;
+      last_pc = 0;
+      gSingleStepControl.dex_pcs.clear();
+    }
+
+    static bool Callback(void* raw_context, uint32_t address, uint32_t line_number) {
+      DebugCallbackContext* context = reinterpret_cast<DebugCallbackContext*>(raw_context);
+      if (static_cast<int32_t>(line_number) == gSingleStepControl.line_number) {
+        if (!context->last_pc_valid) {
+          // Everything from this address until the next line change is ours.
+          context->last_pc = address;
+          context->last_pc_valid = true;
+        }
+        // Otherwise, if we're already in a valid range for this line,
+        // just keep going (shouldn't really happen)...
+      } else if (context->last_pc_valid) { // and the line number is new
+        // Add everything from the last entry up until here to the set
+        for (uint32_t dex_pc = context->last_pc; dex_pc < address; ++dex_pc) {
+          gSingleStepControl.dex_pcs.insert(dex_pc);
+        }
+        context->last_pc_valid = false;
+      }
+      return false; // There may be multiple entries for any given line.
+    }
+
+    ~DebugCallbackContext() {
+      // If the line number was the last in the position table...
+      if (last_pc_valid) {
+        size_t end = MethodHelper(gSingleStepControl.method).GetCodeItem()->insns_size_in_code_units_;
+        for (uint32_t dex_pc = last_pc; dex_pc < end; ++dex_pc) {
+          gSingleStepControl.dex_pcs.insert(dex_pc);
+        }
+      }
+    }
+
+    bool last_pc_valid;
+    uint32_t last_pc;
+  };
+  DebugCallbackContext context;
+  const Method* m = gSingleStepControl.method;
+  MethodHelper mh(m);
+  mh.GetDexFile().DecodeDebugInfo(mh.GetCodeItem(), m->IsStatic(), m->GetDexMethodIndex(),
+                                  DebugCallbackContext::Callback, NULL, &context);
+
+  //
+  // Everything else...
+  //
+
   gSingleStepControl.thread = thread;
   gSingleStepControl.step_size = step_size;
   gSingleStepControl.step_depth = step_depth;
   gSingleStepControl.is_active = true;
 
-  return true;
+  if (VLOG_IS_ON(jdwp)) {
+    VLOG(jdwp) << "Single-step thread: " << *gSingleStepControl.thread;
+    VLOG(jdwp) << "Single-step step size: " << gSingleStepControl.step_size;
+    VLOG(jdwp) << "Single-step step depth: " << gSingleStepControl.step_depth;
+    VLOG(jdwp) << "Single-step current method: " << PrettyMethod(gSingleStepControl.method);
+    VLOG(jdwp) << "Single-step current line: " << gSingleStepControl.line_number;
+    VLOG(jdwp) << "Single-step current stack depth: " << gSingleStepControl.stack_depth;
+    VLOG(jdwp) << "Single-step dex_pc values:";
+    for (std::set<uint32_t>::iterator it = gSingleStepControl.dex_pcs.begin() ; it != gSingleStepControl.dex_pcs.end(); ++it) {
+      VLOG(jdwp) << " " << *it;
+    }
+  }
+
+  return JDWP::ERR_NONE;
 }
 
 void Dbg::UnconfigureStep(JDWP::ObjectId threadId) {
   gSingleStepControl.is_active = false;
   gSingleStepControl.thread = NULL;
+  gSingleStepControl.dex_pcs.clear();
 }
 
 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 625f941..d8b9800 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -140,7 +140,7 @@
   static void GetClassList(std::vector<JDWP::RefTypeId>& classes);
   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 JDWP::JdwpError GetReferenceType(JDWP::ObjectId objectId, JDWP::ExpandBuf* pReply);
   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);
@@ -179,14 +179,14 @@
    * Thread, ThreadGroup, Frame
    */
   static bool GetThreadName(JDWP::ObjectId threadId, std::string& name);
-  static JDWP::ObjectId GetThreadGroup(JDWP::ObjectId threadId);
+  static JDWP::JdwpError GetThreadGroup(JDWP::ObjectId threadId, JDWP::ExpandBuf* pReply);
   static std::string GetThreadGroupName(JDWP::ObjectId threadGroupId);
   static JDWP::ObjectId GetThreadGroupParent(JDWP::ObjectId threadGroupId);
   static JDWP::ObjectId GetSystemThreadGroupId();
   static JDWP::ObjectId GetMainThreadGroupId();
 
   static bool GetThreadStatus(JDWP::ObjectId threadId, JDWP::JdwpThreadStatus* pThreadStatus, JDWP::JdwpSuspendStatus* pSuspendStatus);
-  static uint32_t GetThreadSuspendCount(JDWP::ObjectId threadId);
+  static JDWP::JdwpError GetThreadSuspendCount(JDWP::ObjectId threadId, JDWP::ExpandBuf* pReply);
   static bool ThreadExists(JDWP::ObjectId threadId);
   static bool IsSuspended(JDWP::ObjectId threadId);
   //static void WaitForSuspend(JDWP::ObjectId threadId);
@@ -225,7 +225,7 @@
 
   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 JDWP::JdwpError ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize size, JDWP::JdwpStepDepth depth);
   static void UnconfigureStep(JDWP::ObjectId threadId);
 
   static JDWP::JdwpError 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* pExceptObj);
diff --git a/src/dex_file.cc b/src/dex_file.cc
index 2a59ee3..159065e 100644
--- a/src/dex_file.cc
+++ b/src/dex_file.cc
@@ -624,8 +624,8 @@
 }
 
 void DexFile::DecodeDebugInfo0(const CodeItem* code_item, bool is_static, uint32_t method_idx,
-                               DexDebugNewPositionCb posCb, DexDebugNewLocalCb local_cb,
-                               void* cnxt, const byte* stream, LocalInfo* local_in_reg) const {
+                               DexDebugNewPositionCb position_cb, DexDebugNewLocalCb local_cb,
+                               void* context, const byte* stream, LocalInfo* local_in_reg) const {
   uint32_t line = DecodeUnsignedLeb128(&stream);
   uint32_t parameters_size = DecodeUnsignedLeb128(&stream);
   uint16_t arg_reg = code_item->registers_size_ - code_item->ins_size_;
@@ -713,7 +713,7 @@
 
         // Emit what was previously there, if anything
         if (need_locals) {
-          InvokeLocalCbIfLive(cnxt, reg, address, local_in_reg, local_cb);
+          InvokeLocalCbIfLive(context, reg, address, local_in_reg, local_cb);
 
           local_in_reg[reg].name_ = StringDataByIdx(name_idx);
           local_in_reg[reg].descriptor_ = StringByTypeIdx(descriptor_idx);
@@ -734,7 +734,7 @@
         }
 
         if (need_locals) {
-          InvokeLocalCbIfLive(cnxt, reg, address, local_in_reg, local_cb);
+          InvokeLocalCbIfLive(context, reg, address, local_in_reg, local_cb);
           local_in_reg[reg].is_live_ = false;
         }
         break;
@@ -773,8 +773,8 @@
         address += adjopcode / DBG_LINE_RANGE;
         line += DBG_LINE_BASE + (adjopcode % DBG_LINE_RANGE);
 
-        if (posCb != NULL) {
-          if (posCb(cnxt, address, line)) {
+        if (position_cb != NULL) {
+          if (position_cb(context, address, line)) {
             // early exit
             return;
           }
@@ -786,21 +786,21 @@
 }
 
 void DexFile::DecodeDebugInfo(const CodeItem* code_item, bool is_static, uint32_t method_idx,
-                                 DexDebugNewPositionCb posCb, DexDebugNewLocalCb local_cb,
-                                 void* cnxt) const {
+                              DexDebugNewPositionCb position_cb, DexDebugNewLocalCb local_cb,
+                              void* context) const {
   const byte* stream = GetDebugInfoStream(code_item);
   LocalInfo local_in_reg[code_item->registers_size_];
 
   if (stream != NULL) {
-    DecodeDebugInfo0(code_item, is_static, method_idx, posCb, local_cb, cnxt, stream, local_in_reg);
+    DecodeDebugInfo0(code_item, is_static, method_idx, position_cb, local_cb, context, stream, local_in_reg);
   }
   for (int reg = 0; reg < code_item->registers_size_; reg++) {
-    InvokeLocalCbIfLive(cnxt, reg, code_item->insns_size_in_code_units_, local_in_reg, local_cb);
+    InvokeLocalCbIfLive(context, reg, code_item->insns_size_in_code_units_, local_in_reg, local_cb);
   }
 }
 
-bool DexFile::LineNumForPcCb(void* cnxt, uint32_t address, uint32_t line_num) {
-  LineNumFromPcContext* context = reinterpret_cast<LineNumFromPcContext*>(cnxt);
+bool DexFile::LineNumForPcCb(void* raw_context, uint32_t address, uint32_t line_num) {
+  LineNumFromPcContext* context = reinterpret_cast<LineNumFromPcContext*>(raw_context);
 
   // We know that this callback will be called in
   // ascending address order, so keep going until we find
diff --git a/src/dex_file.h b/src/dex_file.h
index 1f87bca..d66677e 100644
--- a/src/dex_file.h
+++ b/src/dex_file.h
@@ -683,18 +683,18 @@
 
   // Callback for "new position table entry".
   // Returning true causes the decoder to stop early.
-  typedef bool (*DexDebugNewPositionCb)(void* cnxt, uint32_t address, uint32_t line_num);
+  typedef bool (*DexDebugNewPositionCb)(void* context, uint32_t address, uint32_t line_num);
 
   // Callback for "new locals table entry". "signature" is an empty string
   // if no signature is available for an entry.
-  typedef void (*DexDebugNewLocalCb)(void* cnxt, uint16_t reg,
+  typedef void (*DexDebugNewLocalCb)(void* context, uint16_t reg,
                                      uint32_t startAddress,
                                      uint32_t endAddress,
                                      const char* name,
                                      const char* descriptor,
                                      const char* signature);
 
-  static bool LineNumForPcCb(void* cnxt, uint32_t address, uint32_t line_num);
+  static bool LineNumForPcCb(void* context, uint32_t address, uint32_t line_num);
 
   // Debug info opcodes and constants
   enum {
@@ -736,10 +736,10 @@
     DISALLOW_COPY_AND_ASSIGN(LineNumFromPcContext);
   };
 
-  void InvokeLocalCbIfLive(void* cnxt, int reg, uint32_t end_address,
+  void InvokeLocalCbIfLive(void* context, int reg, uint32_t end_address,
                            LocalInfo* local_in_reg, DexDebugNewLocalCb local_cb) const {
     if (local_cb != NULL && local_in_reg[reg].is_live_) {
-      local_cb(cnxt, reg, local_in_reg[reg].start_address_, end_address,
+      local_cb(context, reg, local_in_reg[reg].start_address_, end_address,
           local_in_reg[reg].name_, local_in_reg[reg].descriptor_,
           local_in_reg[reg].signature_ != NULL ? local_in_reg[reg].signature_ : "");
     }
@@ -756,8 +756,8 @@
   int32_t GetLineNumFromPC(const Method* method, uint32_t rel_pc) const;
 
   void DecodeDebugInfo(const CodeItem* code_item, bool is_static, uint32_t method_idx,
-                       DexDebugNewPositionCb posCb, DexDebugNewLocalCb local_cb,
-                       void* cnxt) const;
+                       DexDebugNewPositionCb position_cb, DexDebugNewLocalCb local_cb,
+                       void* context) const;
 
   const char* GetSourceFile(const ClassDef& class_def) const {
     if (class_def.source_file_idx_ == 0xffffffff) {
@@ -840,8 +840,8 @@
   bool CheckMagicAndVersion() const;
 
   void DecodeDebugInfo0(const CodeItem* code_item, bool is_static, uint32_t method_idx,
-      DexDebugNewPositionCb posCb, DexDebugNewLocalCb local_cb,
-      void* cnxt, const byte* stream, LocalInfo* local_in_reg) const;
+      DexDebugNewPositionCb position_cb, DexDebugNewLocalCb local_cb,
+      void* context, const byte* stream, LocalInfo* local_in_reg) const;
 
   // The index of descriptors to class definition indexes (as opposed to type id indexes)
   typedef std::map<const StringPiece, uint32_t> Index;
diff --git a/src/jdwp/jdwp_event.cc b/src/jdwp/jdwp_event.cc
index b688993..5101f2f 100644
--- a/src/jdwp/jdwp_event.cc
+++ b/src/jdwp/jdwp_event.cc
@@ -160,7 +160,10 @@
       /* should only be for EK_SINGLE_STEP; should only be one */
       JdwpStepSize size = static_cast<JdwpStepSize>(pMod->step.size);
       JdwpStepDepth depth = static_cast<JdwpStepDepth>(pMod->step.depth);
-      Dbg::ConfigureStep(pMod->step.threadId, size, depth);
+      JdwpError status = Dbg::ConfigureStep(pMod->step.threadId, size, depth);
+      if (status != ERR_NONE) {
+        return status;
+      }
     } else if (pMod->modKind == MK_FIELD_ONLY) {
       /* should be for EK_FIELD_ACCESS or EK_FIELD_MODIFICATION */
       dumpEvent(pEvent);  /* TODO - need for field watches */
diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index f17a4d8..3f79601 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -122,7 +122,7 @@
     size_t width = Dbg::GetTagWidth(typeTag);
     uint64_t value = jdwpReadValue(&buf, width);
 
-    VLOG(jdwp) << StringPrintf("          '%c'(%zd): 0x%llx", typeTag, width, value);
+    VLOG(jdwp) << "          " << typeTag << StringPrintf("(%zd): 0x%llx", width, value);
     argArray[i] = value;
   }
 
@@ -152,7 +152,7 @@
     expandBufAdd1(pReply, JT_OBJECT);
     expandBufAddObjectId(pReply, exceptObjId);
 
-    VLOG(jdwp) << StringPrintf("  --> returned '%c' 0x%llx (except=%08llx)", resultTag, resultValue, exceptObjId);
+    VLOG(jdwp) << "  --> returned " << resultTag << StringPrintf(" 0x%llx (except=%08llx)", resultValue, exceptObjId);
 
     /* show detailed debug output */
     if (resultTag == JT_STRING && exceptObjId == 0) {
@@ -661,7 +661,7 @@
     size_t width = Dbg::GetTagWidth(fieldTag);
     uint64_t value = jdwpReadValue(&buf, width);
 
-    VLOG(jdwp) << StringPrintf("    --> field=%x tag=%c -> %lld", fieldId, fieldTag, value);
+    VLOG(jdwp) << "    --> field=" << fieldId << " tag=" << fieldTag << " -> " << value;
     JdwpError status = Dbg::SetStaticFieldValue(fieldId, value, width);
     if (status != ERR_NONE) {
       return status;
@@ -715,7 +715,7 @@
   RefTypeId arrayTypeId = ReadRefTypeId(&buf);
   uint32_t length = Read4BE(&buf);
 
-  VLOG(jdwp) << StringPrintf("Creating array %s[%u]", Dbg::GetClassName(arrayTypeId).c_str(), length);
+  VLOG(jdwp) << "Creating array " << Dbg::GetClassName(arrayTypeId) << "[" << length << "]";
   ObjectId objectId;
   if (!Dbg::CreateArrayObject(arrayTypeId, length, objectId)) {
     return ERR_INVALID_CLASS;
@@ -735,7 +735,7 @@
   RefTypeId refTypeId = ReadRefTypeId(&buf);
   MethodId methodId = ReadMethodId(&buf);
 
-  VLOG(jdwp) << StringPrintf("  Req for line table in %s.%s", Dbg::GetClassName(refTypeId).c_str(), Dbg::GetMethodName(refTypeId,methodId).c_str());
+  VLOG(jdwp) << "  Req for line table in " << Dbg::GetClassName(refTypeId) << "." << Dbg::GetMethodName(refTypeId,methodId);
 
   Dbg::OutputLineTable(refTypeId, methodId, pReply);
 
@@ -774,15 +774,7 @@
 static JdwpError handleOR_ReferenceType(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
   ObjectId objectId = ReadObjectId(&buf);
   VLOG(jdwp) << StringPrintf("  Req for type of objectId=0x%llx", objectId);
-
-  JDWP::JdwpTypeTag refTypeTag;
-  RefTypeId typeId;
-  Dbg::GetObjectType(objectId, &refTypeTag, &typeId);
-
-  expandBufAdd1(pReply, refTypeTag);
-  expandBufAddRefTypeId(pReply, typeId);
-
-  return ERR_NONE;
+  return Dbg::GetReferenceType(objectId, pReply);
 }
 
 /*
@@ -820,7 +812,7 @@
     size_t width = Dbg::GetTagWidth(fieldTag);
     uint64_t value = jdwpReadValue(&buf, width);
 
-    VLOG(jdwp) << StringPrintf("    --> fieldId=%x tag='%c'(%zd) value=%lld", fieldId, fieldTag, width, value);
+    VLOG(jdwp) << "    --> fieldId=" << fieldId << " tag=" << fieldTag << "(" << width << ") value=" << value;
 
     Dbg::SetFieldValue(objectId, fieldId, value, width);
   }
@@ -970,12 +962,7 @@
  */
 static JdwpError handleTR_ThreadGroup(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
   ObjectId threadId = ReadObjectId(&buf);
-
-  /* currently not handling these */
-  ObjectId threadGroupId = Dbg::GetThreadGroup(threadId);
-  expandBufAddObjectId(pReply, threadGroupId);
-
-  return ERR_NONE;
+  return Dbg::GetThreadGroup(threadId, pReply);
 }
 
 /*
@@ -1021,7 +1008,7 @@
     expandBufAdd8BE(pReply, frameId);
     AddLocation(pReply, &loc);
 
-    VLOG(jdwp) << StringPrintf("    Frame %d: id=%llx loc={type=%d cls=%llx mth=%x loc=%llx}", i, frameId, loc.typeTag, loc.classId, loc.methodId, loc.idx);
+    VLOG(jdwp) << StringPrintf("    Frame %d: id=%llx ", i, frameId) << loc;
   }
 
   return ERR_NONE;
@@ -1072,11 +1059,7 @@
  */
 static JdwpError handleTR_SuspendCount(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
   ObjectId threadId = ReadObjectId(&buf);
-
-  uint32_t suspendCount = Dbg::GetThreadSuspendCount(threadId);
-  expandBufAdd4BE(pReply, suspendCount);
-
-  return ERR_NONE;
+  return Dbg::GetThreadSuspendCount(threadId, pReply);
 }
 
 /*
@@ -1152,7 +1135,7 @@
   if (status != ERR_NONE) {
     return status;
   }
-  VLOG(jdwp) << StringPrintf("    --> %d", length);
+  VLOG(jdwp) << "    --> " << length;
 
   expandBufAdd4BE(pReply, length);
 
@@ -1262,7 +1245,7 @@
         // 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());
+        VLOG(jdwp) << "    ClassMatch: '" << pattern << "'";
         pEvent->mods[idx].classMatch.classPattern = strdup(pattern.c_str());
       }
       break;
@@ -1271,17 +1254,15 @@
         // 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());
+        VLOG(jdwp) << "    ClassExclude: '" << pattern << "'";
         pEvent->mods[idx].classExclude.classPattern = strdup(pattern.c_str());
       }
       break;
     case MK_LOCATION_ONLY:  /* restrict certain events based on loc */
       {
         JdwpLocation loc;
-
         jdwpReadLocation(&buf, &loc);
-        VLOG(jdwp) << StringPrintf("    LocationOnly: typeTag=%d classId=%llx methodId=%x idx=%llx",
-        loc.typeTag, loc.classId, loc.methodId, loc.idx);
+        VLOG(jdwp) << "    LocationOnly: " << loc;
         pEvent->mods[idx].locationOnly.loc = loc;
       }
       break;
@@ -1398,7 +1379,7 @@
     uint32_t slot = Read4BE(&buf);
     JDWP::JdwpTag reqSigByte = ReadTag(&buf);
 
-    VLOG(jdwp) << StringPrintf("    --> slot %d '%c'", slot, reqSigByte);
+    VLOG(jdwp) << "    --> slot " << slot << " " << reqSigByte;
 
     size_t width = Dbg::GetTagWidth(reqSigByte);
     uint8_t* ptr = expandBufAddSpace(pReply, width+1);
@@ -1424,7 +1405,7 @@
     size_t width = Dbg::GetTagWidth(sigByte);
     uint64_t value = jdwpReadValue(&buf, width);
 
-    VLOG(jdwp) << StringPrintf("    --> slot %d '%c' %llx", slot, sigByte, value);
+    VLOG(jdwp) << "    --> slot " << slot << " " << sigByte << " " << value;
     Dbg::SetLocalValue(threadId, frameId, slot, sigByte, value, width);
   }
 
diff --git a/src/utils.cc b/src/utils.cc
index 5348558..ce3af32 100644
--- a/src/utils.cc
+++ b/src/utils.cc
@@ -375,18 +375,22 @@
 }
 
 std::string DescriptorToDot(const char* descriptor) {
-  std::string result(DescriptorToName(descriptor));
-  std::replace(result.begin(), result.end(), '/', '.');
-  return result;
+  size_t length = strlen(descriptor);
+  if (descriptor[0] == 'L' && descriptor[length - 1] == ';') {
+    std::string result(descriptor + 1, length - 2);
+    std::replace(result.begin(), result.end(), '/', '.');
+    return result;
+  }
+  return descriptor;
 }
 
 std::string DescriptorToName(const char* descriptor) {
   size_t length = strlen(descriptor);
-  CHECK_GT(length, 0U) << descriptor;
-  CHECK_EQ(descriptor[0], 'L') << descriptor;
-  CHECK_EQ(descriptor[length - 1], ';') << descriptor;
-  std::string result(descriptor + 1, length - 2);
-  return result;
+  if (descriptor[0] == 'L' && descriptor[length - 1] == ';') {
+    std::string result(descriptor + 1, length - 2);
+    return result;
+  }
+  return descriptor;
 }
 
 std::string JniShortName(const Method* m) {