Fix posting exceptions when a debugger is attached, fix UpdateDebugger, fully implement ThreadGroupReference.Children.
Posting exceptions and UpdateDebugger were broken by the recent stack-walking
rewrite.
dalvikvm never did ThreadGroupReference.Children correctly; it only admitted
that "system" is the parent of "main".
Change-Id: I386f2fa5e01fba56cb2a1af4f136dbf31da0ff07
diff --git a/src/debugger.cc b/src/debugger.cc
index 9dcdda5..e5d3780 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -1009,7 +1009,7 @@
return 0;
} else if (slot == 0) {
const DexFile::CodeItem* code_item = MethodHelper(m).GetCodeItem();
- CHECK(code_item != NULL);
+ CHECK(code_item != NULL) << PrettyMethod(m);
return code_item->registers_size_ - code_item->ins_size_;
}
return slot;
@@ -1429,11 +1429,11 @@
return DecodeThread(threadId)->IsSuspended();
}
-void Dbg::GetThreadGroupThreadsImpl(Object* thread_group, JDWP::ObjectId** ppThreadIds, uint32_t* pThreadCount) {
+void Dbg::GetThreads(JDWP::ObjectId thread_group_id, std::vector<JDWP::ObjectId>& thread_ids) {
class ThreadListVisitor {
public:
- ThreadListVisitor(const ScopedJniThreadState& ts, Object* thread_group)
- : ts_(ts), thread_group_(thread_group) {}
+ ThreadListVisitor(const ScopedJniThreadState& ts, Object* thread_group, std::vector<JDWP::ObjectId>& thread_ids)
+ : ts_(ts), thread_group_(thread_group), thread_ids_(thread_ids) {}
static void Visit(Thread* t, void* arg) {
reinterpret_cast<ThreadListVisitor*>(arg)->Visit(t);
@@ -1446,45 +1446,42 @@
return;
}
if (thread_group_ == NULL || t->GetThreadGroup(ts_) == thread_group_) {
- threads_.push_back(gRegistry->Add(t->GetPeer()));
+ thread_ids_.push_back(gRegistry->Add(t->GetPeer()));
}
}
- const std::vector<JDWP::ObjectId>& GetThreads() {
- return threads_;
- }
-
private:
const ScopedJniThreadState& ts_;
Object* const thread_group_;
- std::vector<JDWP::ObjectId> threads_;
+ std::vector<JDWP::ObjectId>& thread_ids_;
};
ScopedJniThreadState ts(Thread::Current());
- ThreadListVisitor tlv(ts, thread_group);
-
+ Object* thread_group = gRegistry->Get<Object*>(thread_group_id);
+ ThreadListVisitor tlv(ts, thread_group, thread_ids);
Runtime::Current()->GetThreadList()->ForEach(ThreadListVisitor::Visit, &tlv);
+}
- *pThreadCount = tlv.GetThreads().size();
- if (*pThreadCount == 0) {
- *ppThreadIds = NULL;
- } else {
- // TODO: pass in std::vector rather than passing around pointers.
- *ppThreadIds = new JDWP::ObjectId[*pThreadCount];
- for (size_t i = 0; i < *pThreadCount; ++i) {
- (*ppThreadIds)[i] = tlv.GetThreads()[i];
- }
+void Dbg::GetChildThreadGroups(JDWP::ObjectId thread_group_id, std::vector<JDWP::ObjectId>& child_thread_group_ids) {
+ ScopedJniThreadState ts(Thread::Current());
+ Object* thread_group = gRegistry->Get<Object*>(thread_group_id);
+
+ // Get the ArrayList<ThreadGroup> "groups" out of this thread group...
+ Field* groups_field = thread_group->GetClass()->FindInstanceField("groups", "Ljava/util/List;");
+ Object* groups_array_list = groups_field->GetObject(thread_group);
+
+ // Get the array and size out of the ArrayList<ThreadGroup>...
+ Field* array_field = groups_array_list->GetClass()->FindInstanceField("array", "[Ljava/lang/Object;");
+ Field* size_field = groups_array_list->GetClass()->FindInstanceField("size", "I");
+ ObjectArray<Object>* groups_array = array_field->GetObject(groups_array_list)->AsObjectArray<Object>();
+ const int32_t size = size_field->GetInt(groups_array_list);
+
+ // Copy the first 'size' elements out of the array into the result.
+ for (int32_t i = 0; i < size; ++i) {
+ child_thread_group_ids.push_back(gRegistry->Add(groups_array->Get(i)));
}
}
-void Dbg::GetThreadGroupThreads(JDWP::ObjectId threadGroupId, JDWP::ObjectId** ppThreadIds, uint32_t* pThreadCount) {
- GetThreadGroupThreadsImpl(gRegistry->Get<Object*>(threadGroupId), ppThreadIds, pThreadCount);
-}
-
-void Dbg::GetAllThreads(JDWP::ObjectId** ppThreadIds, uint32_t* pThreadCount) {
- GetThreadGroupThreadsImpl(NULL, ppThreadIds, pThreadCount);
-}
-
static int GetStackDepth(Thread* thread) {
struct CountStackDepthVisitor : public StackVisitor {
CountStackDepthVisitor(const ManagedStack* stack,
@@ -1618,46 +1615,25 @@
JDWP::FrameId frame_id;
};
-static Object* GetThis(Method** quickFrame) {
- struct FrameIdVisitor : public StackVisitor {
- FrameIdVisitor(const ManagedStack* stack, const std::vector<TraceStackFrame>* trace_stack,
- Method** m)
- : StackVisitor(stack, trace_stack, NULL), quick_frame_to_find(m) , frame_id(0) {}
-
- virtual bool VisitFrame() {
- if (quick_frame_to_find != GetCurrentQuickFrame()) {
- return true; // Continue.
- }
- frame_id = GetFrameId();
- return false; // Stop.
- }
-
- Method** const quick_frame_to_find;
- JDWP::FrameId frame_id;
- };
-
- Method* m = *quickFrame;
+static Object* GetThis(Thread* self, Method* m, size_t frame_id) {
+ // TODO: should we return the 'this' we passed through to non-static native methods?
if (m->IsNative() || m->IsStatic()) {
return NULL;
}
- Thread* self = Thread::Current();
- const ManagedStack* stack = self->GetManagedStack();
- const std::vector<TraceStackFrame>* trace_stack = self->GetTraceStack();
- FrameIdVisitor frameIdVisitor(stack, trace_stack, quickFrame);
- frameIdVisitor.WalkStack();
+
UniquePtr<Context> context(Context::Create());
- GetThisVisitor getThisVisitor(stack, trace_stack, context.get(), frameIdVisitor.frame_id);
- getThisVisitor.WalkStack();
- return getThisVisitor.this_object;
+ GetThisVisitor visitor(self->GetManagedStack(), self->GetTraceStack(), context.get(), frame_id);
+ visitor.WalkStack();
+ return visitor.this_object;
}
JDWP::JdwpError Dbg::GetThisObject(JDWP::ObjectId thread_id, JDWP::FrameId frame_id, JDWP::ObjectId* result) {
- UniquePtr<Context> context(Context::Create());
Thread* thread = DecodeThread(thread_id);
if (thread == NULL) {
- *result = 0;
return JDWP::ERR_INVALID_THREAD;
}
+
+ UniquePtr<Context> context(Context::Create());
GetThisVisitor visitor(thread->GetManagedStack(), thread->GetTraceStack(), context.get(), frame_id);
visitor.WalkStack();
*result = gRegistry->Add(visitor.this_object);
@@ -1869,22 +1845,23 @@
}
}
-void Dbg::PostException(Thread* thread, JDWP::FrameId throwFrameId, Method* throwMethod, uint32_t throwDexPc,
- Method* catchMethod, uint32_t catchDexPc, Throwable* exception) {
+void Dbg::PostException(Thread* thread,
+ JDWP::FrameId throw_frame_id, Method* throw_method, uint32_t throw_dex_pc,
+ Method* catch_method, uint32_t catch_dex_pc, Throwable* exception) {
if (!IsDebuggerActive()) {
return;
}
JDWP::JdwpLocation throw_location;
- SetLocation(throw_location, throwMethod, throwDexPc);
+ SetLocation(throw_location, throw_method, throw_dex_pc);
JDWP::JdwpLocation catch_location;
- SetLocation(catch_location, catchMethod, catchDexPc);
+ SetLocation(catch_location, catch_method, catch_dex_pc);
// We need 'this' for InstanceOnly filters.
- JDWP::ObjectId thread_id = gRegistry->Add(thread->GetPeer());
- JDWP::ObjectId this_id;
- JDWP::JdwpError get_this_error = GetThisObject(thread_id, throwFrameId, &this_id);
- CHECK_EQ(get_this_error, JDWP::ERR_NONE);
+ UniquePtr<Context> context(Context::Create());
+ GetThisVisitor visitor(thread->GetManagedStack(), thread->GetTraceStack(), context.get(), throw_frame_id);
+ visitor.WalkStack();
+ JDWP::ObjectId this_id = gRegistry->Add(visitor.this_object);
/*
* Hand the event to the JDWP exception handler. Note we're using the
@@ -1914,18 +1891,20 @@
gJdwpState->PostClassPrepare(tag, gRegistry->Add(c), ClassHelper(c).GetDescriptor(), state);
}
-void Dbg::UpdateDebugger(int32_t dex_pc, Thread* self, Method** sp) {
+void Dbg::UpdateDebugger(int32_t dex_pc, Thread* self) {
if (!IsDebuggerActive() || dex_pc == -2 /* fake method exit */) {
return;
}
- Method* m = self->GetCurrentMethod();
+ size_t frame_id;
+ Method* m = self->GetCurrentMethod(NULL, &frame_id);
+ //LOG(INFO) << "UpdateDebugger " << PrettyMethod(m) << "@" << dex_pc << " frame " << frame_id;
if (dex_pc == -1) {
// 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(sp), kMethodEntry);
+ Dbg::PostLocationEvent(m, 0, GetThis(self, m, frame_id), kMethodEntry);
return;
}
@@ -2006,7 +1985,7 @@
// terminates "with a thrown exception".
if (dex_pc >= 0) {
const DexFile::CodeItem* code_item = MethodHelper(m).GetCodeItem();
- CHECK(code_item != NULL);
+ CHECK(code_item != NULL) << PrettyMethod(m) << " @" << dex_pc;
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;
@@ -2016,7 +1995,7 @@
// If there's something interesting going on, see if it matches one
// of the debugger filters.
if (event_flags != 0) {
- Dbg::PostLocationEvent(m, dex_pc, GetThis(sp), event_flags);
+ Dbg::PostLocationEvent(m, dex_pc, GetThis(self, m, frame_id), event_flags);
}
}
diff --git a/src/debugger.h b/src/debugger.h
index 7067c28..542a006 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -188,8 +188,12 @@
static bool ThreadExists(JDWP::ObjectId threadId);
static bool IsSuspended(JDWP::ObjectId threadId);
//static void WaitForSuspend(JDWP::ObjectId threadId);
- static void GetThreadGroupThreads(JDWP::ObjectId threadGroupId, JDWP::ObjectId** ppThreadIds, uint32_t* pThreadCount);
- static void GetAllThreads(JDWP::ObjectId** ppThreadIds, uint32_t* pThreadCount);
+
+ // Fills 'thread_ids' with the threads in the given thread group. If thread_group_id == NULL,
+ // returns all threads.
+ static void GetThreads(JDWP::ObjectId thread_group_id, std::vector<JDWP::ObjectId>& thread_ids);
+ static void GetChildThreadGroups(JDWP::ObjectId thread_group_id, std::vector<JDWP::ObjectId>& child_thread_group_ids);
+
static int GetThreadFrameCount(JDWP::ObjectId threadId);
static JDWP::JdwpError GetThreadFrames(JDWP::ObjectId thread_id, size_t start_frame, size_t frame_count, JDWP::ExpandBuf* buf);
@@ -214,12 +218,12 @@
kMethodExit = 0x08,
};
static void PostLocationEvent(const Method* method, int pcOffset, Object* thisPtr, int eventFlags);
- static void PostException(Thread* thread, JDWP::FrameId frameId, Method* throwMethod, uint32_t throwDexPc, Method* catchMethod, uint32_t catchDexPc, Throwable* exception);
+ static void PostException(Thread* thread, JDWP::FrameId throw_frame_id, Method* throw_method, uint32_t throw_dex_pc, Method* catch_method, uint32_t catch_dex_pc, Throwable* exception);
static void PostThreadStart(Thread* t);
static void PostThreadDeath(Thread* t);
static void PostClassPrepare(Class* c);
- static void UpdateDebugger(int32_t dex_pc, Thread* self, Method** sp);
+ static void UpdateDebugger(int32_t dex_pc, Thread* self);
static void WatchLocation(const JDWP::JdwpLocation* pLoc);
static void UnwatchLocation(const JDWP::JdwpLocation* pLoc);
@@ -276,7 +280,6 @@
private:
static void DdmBroadcast(bool);
- static void GetThreadGroupThreadsImpl(Object*, JDWP::ObjectId**, uint32_t*);
static void PostThreadStartOrStop(Thread*, uint32_t);
static AllocRecord* recent_allocation_records_;
diff --git a/src/dex_file.cc b/src/dex_file.cc
index b4eb524..2b81e72 100644
--- a/src/dex_file.cc
+++ b/src/dex_file.cc
@@ -577,7 +577,7 @@
}
const CodeItem* code_item = GetCodeItem(method->GetCodeItemOffset());
- DCHECK(code_item != NULL) << GetLocation();
+ DCHECK(code_item != NULL) << PrettyMethod(method) << " " << GetLocation();
// A method with no line number info should return -1
LineNumFromPcContext context(rel_pc, -1);
diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index 1c0a096..7a796fe 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -208,19 +208,14 @@
* to be suspended, and that violates some JDWP expectations.
*/
static JdwpError VM_AllThreads(JdwpState*, const uint8_t*, int, ExpandBuf* pReply) {
- ObjectId* pThreadIds;
- uint32_t threadCount;
- Dbg::GetAllThreads(&pThreadIds, &threadCount);
+ std::vector<ObjectId> thread_ids;
+ Dbg::GetThreads(NULL, thread_ids);
- expandBufAdd4BE(pReply, threadCount);
-
- ObjectId* walker = pThreadIds;
- for (uint32_t i = 0; i < threadCount; i++) {
- expandBufAddObjectId(pReply, *walker++);
+ expandBufAdd4BE(pReply, thread_ids.size());
+ for (uint32_t i = 0; i < thread_ids.size(); ++i) {
+ expandBufAddObjectId(pReply, thread_ids[i]);
}
- free(pThreadIds);
-
return ERR_NONE;
}
@@ -236,10 +231,10 @@
*/
uint32_t groups = 1;
expandBufAdd4BE(pReply, groups);
- //threadGroupId = debugGetMainThreadGroup();
- //expandBufAdd8BE(pReply, threadGroupId);
- ObjectId threadGroupId = Dbg::GetSystemThreadGroupId();
- expandBufAddObjectId(pReply, threadGroupId);
+ //thread_group_id = debugGetMainThreadGroup();
+ //expandBufAdd8BE(pReply, thread_group_id);
+ ObjectId thread_group_id = Dbg::GetSystemThreadGroupId();
+ expandBufAddObjectId(pReply, thread_group_id);
return ERR_NONE;
}
@@ -1015,10 +1010,10 @@
* The Eclipse debugger recognizes "main" and "system" as special.
*/
static JdwpError TGR_Name(JdwpState*, const uint8_t* buf, int, ExpandBuf* pReply) {
- ObjectId threadGroupId = ReadObjectId(&buf);
- VLOG(jdwp) << StringPrintf(" Req for name of threadGroupId=%#llx", threadGroupId);
+ ObjectId thread_group_id = ReadObjectId(&buf);
+ VLOG(jdwp) << StringPrintf(" Req for name of thread_group_id=%#llx", thread_group_id);
- expandBufAddUtf8String(pReply, Dbg::GetThreadGroupName(threadGroupId));
+ expandBufAddUtf8String(pReply, Dbg::GetThreadGroupName(thread_group_id));
return ERR_NONE;
}
@@ -1028,9 +1023,9 @@
* thread group.
*/
static JdwpError TGR_Parent(JdwpState*, const uint8_t* buf, int, ExpandBuf* pReply) {
- ObjectId groupId = ReadObjectId(&buf);
+ ObjectId thread_group_id = ReadObjectId(&buf);
- ObjectId parentGroup = Dbg::GetThreadGroupParent(groupId);
+ ObjectId parentGroup = Dbg::GetThreadGroupParent(thread_group_id);
expandBufAddObjectId(pReply, parentGroup);
return ERR_NONE;
@@ -1041,30 +1036,21 @@
* specified thread group.
*/
static JdwpError TGR_Children(JdwpState*, const uint8_t* buf, int, ExpandBuf* pReply) {
- ObjectId threadGroupId = ReadObjectId(&buf);
- VLOG(jdwp) << StringPrintf(" Req for threads in threadGroupId=%#llx", threadGroupId);
+ ObjectId thread_group_id = ReadObjectId(&buf);
+ VLOG(jdwp) << StringPrintf(" Req for threads in thread_group_id=%#llx", thread_group_id);
- ObjectId* pThreadIds;
- uint32_t threadCount;
- Dbg::GetThreadGroupThreads(threadGroupId, &pThreadIds, &threadCount);
-
- expandBufAdd4BE(pReply, threadCount);
-
- for (uint32_t i = 0; i < threadCount; i++) {
- expandBufAddObjectId(pReply, pThreadIds[i]);
+ std::vector<ObjectId> thread_ids;
+ Dbg::GetThreads(thread_group_id, thread_ids);
+ expandBufAdd4BE(pReply, thread_ids.size());
+ for (uint32_t i = 0; i < thread_ids.size(); ++i) {
+ expandBufAddObjectId(pReply, thread_ids[i]);
}
- free(pThreadIds);
- /*
- * TODO: finish support for child groups
- *
- * For now, just show that "main" is a child of "system".
- */
- if (threadGroupId == Dbg::GetSystemThreadGroupId()) {
- expandBufAdd4BE(pReply, 1);
- expandBufAddObjectId(pReply, Dbg::GetMainThreadGroupId());
- } else {
- expandBufAdd4BE(pReply, 0);
+ std::vector<ObjectId> child_thread_groups_ids;
+ Dbg::GetChildThreadGroups(thread_group_id, child_thread_groups_ids);
+ expandBufAdd4BE(pReply, child_thread_groups_ids.size());
+ for (uint32_t i = 0; i < child_thread_groups_ids.size(); ++i) {
+ expandBufAddObjectId(pReply, child_thread_groups_ids[i]);
}
return ERR_NONE;
diff --git a/src/monitor.cc b/src/monitor.cc
index e5e867a..a7d08bc 100644
--- a/src/monitor.cc
+++ b/src/monitor.cc
@@ -910,7 +910,7 @@
// Is there any reason to believe there's any synchronization in this method?
const DexFile::CodeItem* code_item = mh.GetCodeItem();
- CHECK(code_item != NULL);
+ CHECK(code_item != NULL) << PrettyMethod(m);
if (code_item->tries_size_ == 0) {
return; // No "tries" implies no synchronization, so no held locks to report.
}
diff --git a/src/oat/runtime/support_debug.cc b/src/oat/runtime/support_debug.cc
index 2803e27..ef6e0b1 100644
--- a/src/oat/runtime/support_debug.cc
+++ b/src/oat/runtime/support_debug.cc
@@ -27,7 +27,7 @@
*/
extern "C" void artUpdateDebuggerFromCode(int32_t dex_pc, Thread* self, Method** sp) {
FinishCalleeSaveFrameSetup(self, sp, Runtime::kRefsAndArgs);
- Dbg::UpdateDebugger(dex_pc, self, sp);
+ Dbg::UpdateDebugger(dex_pc, self);
}
// Temporary debugging hook for compiler.
diff --git a/src/stack.cc b/src/stack.cc
index c419530..67b9bfa 100644
--- a/src/stack.cc
+++ b/src/stack.cc
@@ -128,7 +128,7 @@
return GetGPR(spill_shifts);
} else {
const DexFile::CodeItem* code_item = MethodHelper(m).GetCodeItem();
- DCHECK(code_item != NULL); // can't be NULL or how would we compile its instructions?
+ DCHECK(code_item != NULL) << PrettyMethod(m); // Can't be NULL or how would we compile its instructions?
uint32_t fp_spills = m->GetFpSpillMask();
size_t frame_size = m->GetFrameSizeInBytes();
return GetVReg(code_item, core_spills, fp_spills, frame_size, vreg);
@@ -145,7 +145,7 @@
UNIMPLEMENTED(FATAL);
}
const DexFile::CodeItem* code_item = MethodHelper(m).GetCodeItem();
- DCHECK(code_item != NULL); // can't be NULL or how would we compile its instructions?
+ DCHECK(code_item != NULL) << PrettyMethod(m); // Can't be NULL or how would we compile its instructions?
uint32_t core_spills = m->GetCoreSpillMask();
uint32_t fp_spills = m->GetFpSpillMask();
size_t frame_size = m->GetFrameSizeInBytes();
diff --git a/src/thread.cc b/src/thread.cc
index 104b9b9..53942d9 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -1643,7 +1643,7 @@
CHECK(reg_bitmap != NULL);
const VmapTable vmap_table(m->GetVmapTableRaw());
const DexFile::CodeItem* code_item = MethodHelper(m).GetCodeItem();
- DCHECK(code_item != NULL); // can't be NULL or how would we compile its instructions?
+ DCHECK(code_item != NULL) << PrettyMethod(m); // Can't be NULL or how would we compile its instructions?
uint32_t core_spills = m->GetCoreSpillMask();
uint32_t fp_spills = m->GetFpSpillMask();
size_t frame_size = m->GetFrameSizeInBytes();