Fix various debugger method invocation bugs.
(Also fix DexFile to admit that string lengths are always non-negative.)
Change-Id: I2d01ef2411b5c7e594527790daf3e0aaa3a1b67f
diff --git a/src/debugger.cc b/src/debugger.cc
index a456f23..494ee73 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -1991,7 +1991,35 @@
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) {
+static char JdwpTagToShortyChar(JDWP::JdwpTag tag) {
+ switch (tag) {
+ default:
+ LOG(FATAL) << "unknown JDWP tag: " << PrintableChar(tag);
+
+ // Primitives.
+ case JDWP::JT_BYTE: return 'B';
+ case JDWP::JT_CHAR: return 'C';
+ case JDWP::JT_FLOAT: return 'F';
+ case JDWP::JT_DOUBLE: return 'D';
+ case JDWP::JT_INT: return 'I';
+ case JDWP::JT_LONG: return 'J';
+ case JDWP::JT_SHORT: return 'S';
+ case JDWP::JT_VOID: return 'V';
+ case JDWP::JT_BOOLEAN: return 'Z';
+
+ // Reference types.
+ case JDWP::JT_ARRAY:
+ case JDWP::JT_OBJECT:
+ case JDWP::JT_STRING:
+ case JDWP::JT_THREAD:
+ case JDWP::JT_THREAD_GROUP:
+ case JDWP::JT_CLASS_LOADER:
+ case JDWP::JT_CLASS_OBJECT:
+ return 'L';
+ }
+}
+
+JDWP::JdwpError Dbg::InvokeMethod(JDWP::ObjectId threadId, JDWP::ObjectId objectId, JDWP::RefTypeId classId, JDWP::MethodId methodId, uint32_t arg_count, uint64_t* arg_values, JDWP::JdwpTag* arg_types, uint32_t options, JDWP::JdwpTag* pResultTag, uint64_t* pResultValue, JDWP::ObjectId* pExceptionId) {
ThreadList* thread_list = Runtime::Current()->GetThreadList();
Thread* targetThread = NULL;
@@ -2030,22 +2058,54 @@
}
JDWP::JdwpError status;
- req->receiver_ = gRegistry->Get<Object*>(objectId);
- if (req->receiver_ == kInvalidObject) {
+ Object* receiver = gRegistry->Get<Object*>(objectId);
+ if (receiver == kInvalidObject) {
return JDWP::ERR_INVALID_OBJECT;
}
- req->thread_ = gRegistry->Get<Object*>(threadId); // TODO: check that this is actually a thread!
- if (req->thread_ == kInvalidObject) {
+
+ Object* thread = gRegistry->Get<Object*>(threadId);
+ if (thread == kInvalidObject) {
return JDWP::ERR_INVALID_OBJECT;
}
- req->class_ = DecodeClass(classId, status);
- if (req->class_ == NULL) {
+ // TODO: check that 'thread' is actually a java.lang.Thread!
+
+ Class* c = DecodeClass(classId, status);
+ if (c == NULL) {
return status;
}
- req->method_ = FromMethodId(methodId);
- req->num_args_ = numArgs;
- // TODO: check that the argument list is valid.
- req->arg_array_ = argArray;
+
+ Method* m = FromMethodId(methodId);
+ if (m->IsStatic() != (receiver == NULL)) {
+ return JDWP::ERR_INVALID_METHODID;
+ }
+ if (m->IsStatic()) {
+ if (m->GetDeclaringClass() != c) {
+ return JDWP::ERR_INVALID_METHODID;
+ }
+ } else {
+ if (!m->GetDeclaringClass()->IsAssignableFrom(c)) {
+ return JDWP::ERR_INVALID_METHODID;
+ }
+ }
+
+ // Check the argument list matches the method.
+ MethodHelper mh(m);
+ if (mh.GetShortyLength() - 1 != arg_count) {
+ return JDWP::ERR_ILLEGAL_ARGUMENT;
+ }
+ const char* shorty = mh.GetShorty();
+ for (size_t i = 0; i < arg_count; ++i) {
+ if (shorty[i + 1] != JdwpTagToShortyChar(arg_types[i])) {
+ return JDWP::ERR_ILLEGAL_ARGUMENT;
+ }
+ }
+
+ req->receiver_ = receiver;
+ req->thread_ = thread;
+ req->class_ = c;
+ req->method_ = m;
+ req->arg_count_ = arg_count;
+ req->arg_values_ = arg_values;
req->options_ = options;
req->invoke_needed_ = true;
}
@@ -2124,16 +2184,20 @@
// Translate the method through the vtable, unless the debugger wants to suppress it.
Method* m = pReq->method_;
- VLOG(jdwp) << "ExecuteMethod " << PrettyMethod(m);
if ((pReq->options_ & JDWP::INVOKE_NONVIRTUAL) == 0 && pReq->receiver_ != NULL) {
- m = pReq->class_->FindVirtualMethodForVirtualOrInterface(pReq->method_);
- VLOG(jdwp) << "ExecuteMethod " << PrettyMethod(m);
+ Method* actual_method = pReq->class_->FindVirtualMethodForVirtualOrInterface(pReq->method_);
+ if (actual_method != m) {
+ VLOG(jdwp) << "ExecuteMethod translated " << PrettyMethod(m) << " to " << PrettyMethod(actual_method);
+ m = actual_method;
+ }
}
+ VLOG(jdwp) << "ExecuteMethod " << PrettyMethod(m);
CHECK(m != NULL);
CHECK_EQ(sizeof(jvalue), sizeof(uint64_t));
- pReq->result_value = InvokeWithJValues(self, pReq->receiver_, m, reinterpret_cast<JValue*>(pReq->arg_array_));
+ LOG(INFO) << "self=" << self << " pReq->receiver_=" << pReq->receiver_ << " m=" << m << " #" << pReq->arg_count_ << " " << pReq->arg_values_;
+ pReq->result_value = InvokeWithJValues(self, pReq->receiver_, m, reinterpret_cast<JValue*>(pReq->arg_values_));
pReq->exception = gRegistry->Add(self->GetException());
pReq->result_tag = BasicTagFromDescriptor(MethodHelper(m).GetShorty());
diff --git a/src/debugger.h b/src/debugger.h
index 1e5ca8f..b88edbb 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -54,8 +54,8 @@
Object* thread_;
Class* class_;
Method* method_;
- uint32_t num_args_;
- uint64_t* arg_array_; /* will be NULL if numArgs==0 */
+ uint32_t arg_count_;
+ uint64_t* arg_values_; /* will be NULL if arg_count_ == 0 */
uint32_t options_;
/* result */
@@ -228,7 +228,7 @@
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);
+ static JDWP::JdwpError InvokeMethod(JDWP::ObjectId threadId, JDWP::ObjectId objectId, JDWP::RefTypeId classId, JDWP::MethodId methodId, uint32_t arg_count, uint64_t* arg_values, JDWP::JdwpTag* arg_types, uint32_t options, JDWP::JdwpTag* pResultTag, uint64_t* pResultValue, JDWP::ObjectId* pExceptObj);
static void ExecuteMethod(DebugInvokeReq* pReq);
/* perform "late registration" of an object ID */
diff --git a/src/dex_file.cc b/src/dex_file.cc
index 159065e..7b031f2 100644
--- a/src/dex_file.cc
+++ b/src/dex_file.cc
@@ -319,7 +319,7 @@
}
// Returns a pointer to the UTF-8 string data referred to by the given string_id.
-const char* DexFile::GetStringDataAndLength(const StringId& string_id, int32_t* length) const {
+const char* DexFile::GetStringDataAndLength(const StringId& string_id, uint32_t* length) const {
DCHECK(length != NULL) << GetLocation();
const byte* ptr = begin_ + string_id.string_data_off_;
*length = DecodeUnsignedLeb128(&ptr);
@@ -427,7 +427,7 @@
uint32_t hi = NumStringIds() - 1;
while (hi >= lo) {
uint32_t mid = (hi + lo) / 2;
- int32_t length;
+ uint32_t length;
const DexFile::StringId& str_id = GetStringId(mid);
const char* str = GetStringDataAndLength(str_id, &length);
int compare = CompareModifiedUtf8ToModifiedUtf8AsUtf16CodePointValues(string.c_str(), str);
@@ -563,7 +563,7 @@
for (size_t i = 0; i < type_list->Size(); ++i) {
const TypeItem& type_item = type_list->GetTypeItem(i);
uint32_t type_idx = type_item.type_idx_;
- int32_t type_length;
+ uint32_t type_length;
const char* name = StringByTypeIdx(type_idx, &type_length);
parameter_length += type_length;
descriptor.append(name);
@@ -571,7 +571,7 @@
}
descriptor.push_back(')');
uint32_t return_type_idx = proto_id.return_type_idx_;
- int32_t return_type_length;
+ uint32_t return_type_length;
const char* name = StringByTypeIdx(return_type_idx, &return_type_length);
descriptor.append(name);
if (unicode_length != NULL) {
diff --git a/src/dex_file.h b/src/dex_file.h
index d66677e..429c7bc 100644
--- a/src/dex_file.h
+++ b/src/dex_file.h
@@ -385,15 +385,15 @@
int32_t GetStringLength(const StringId& string_id) const;
// Returns a pointer to the UTF-8 string data referred to by the given string_id.
- const char* GetStringDataAndLength(const StringId& string_id, int32_t* length) const;
+ const char* GetStringDataAndLength(const StringId& string_id, uint32_t* length) const;
const char* GetStringData(const StringId& string_id) const {
- int32_t length;
+ uint32_t length;
return GetStringDataAndLength(string_id, &length);
}
// return the UTF-8 encoded string with the specified string_id index
- const char* StringDataAndLengthByIdx(uint32_t idx, int32_t* unicode_length) const {
+ const char* StringDataAndLengthByIdx(uint32_t idx, uint32_t* unicode_length) const {
if (idx == kDexNoIndex) {
*unicode_length = 0;
return NULL;
@@ -403,7 +403,7 @@
}
const char* StringDataByIdx(uint32_t idx) const {
- int32_t unicode_length;
+ uint32_t unicode_length;
return StringDataAndLengthByIdx(idx, &unicode_length);
}
@@ -431,7 +431,7 @@
}
// Get the descriptor string associated with a given type index.
- const char* StringByTypeIdx(uint32_t idx, int32_t* unicode_length) const {
+ const char* StringByTypeIdx(uint32_t idx, uint32_t* unicode_length) const {
const TypeId& type_id = GetTypeId(idx);
return StringDataAndLengthByIdx(type_id.descriptor_idx_, unicode_length);
}
@@ -537,7 +537,7 @@
const char* GetMethodShorty(const MethodId& method_id) const {
return StringDataByIdx(GetProtoId(method_id.proto_idx_).shorty_idx_);
}
- const char* GetMethodShorty(const MethodId& method_id, int32_t* length) const {
+ const char* GetMethodShorty(const MethodId& method_id, uint32_t* length) const {
return StringDataAndLengthByIdx(GetProtoId(method_id.proto_idx_).shorty_idx_, length);
}
// Returns the number of class definitions in the .dex file.
diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index 4640d98..d6b9bb3 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -96,58 +96,54 @@
/*
* Common code for *_InvokeMethod requests.
*
- * If "isConstructor" is set, this returns "objectId" rather than the
+ * If "is_constructor" is set, this returns "objectId" rather than the
* expected-to-be-void return value of the called function.
*/
static JdwpError finishInvoke(JdwpState* state,
const uint8_t* buf, int dataLen, ExpandBuf* pReply,
ObjectId threadId, ObjectId objectId, RefTypeId classId, MethodId methodId,
- bool isConstructor)
+ bool is_constructor)
{
- CHECK(!isConstructor || objectId != 0);
+ CHECK(!is_constructor || objectId != 0);
- uint32_t numArgs = Read4BE(&buf);
+ uint32_t arg_count = Read4BE(&buf);
VLOG(jdwp) << StringPrintf(" --> threadId=%llx objectId=%llx", threadId, objectId);
VLOG(jdwp) << StringPrintf(" classId=%llx methodId=%x %s.%s", classId, methodId, Dbg::GetClassName(classId).c_str(), Dbg::GetMethodName(classId, methodId).c_str());
- VLOG(jdwp) << StringPrintf(" %d args:", numArgs);
+ VLOG(jdwp) << StringPrintf(" %d args:", arg_count);
- uint64_t* argArray = NULL;
- if (numArgs > 0) {
- argArray = (ObjectId*) malloc(sizeof(ObjectId) * numArgs);
- }
-
- for (uint32_t i = 0; i < numArgs; i++) {
- JDWP::JdwpTag typeTag = ReadTag(&buf);
- size_t width = Dbg::GetTagWidth(typeTag);
- uint64_t value = jdwpReadValue(&buf, width);
-
- VLOG(jdwp) << " " << typeTag << StringPrintf("(%zd): 0x%llx", width, value);
- argArray[i] = value;
+ UniquePtr<JdwpTag[]> argTypes(arg_count > 0 ? new JdwpTag[arg_count] : NULL);
+ UniquePtr<uint64_t[]> argValues(arg_count > 0 ? new uint64_t[arg_count] : NULL);
+ for (uint32_t i = 0; i < arg_count; ++i) {
+ argTypes[i] = ReadTag(&buf);
+ size_t width = Dbg::GetTagWidth(argTypes[i]);
+ argValues[i] = jdwpReadValue(&buf, width);
+ VLOG(jdwp) << " " << argTypes[i] << StringPrintf("(%zd): 0x%llx", width, argValues[i]);
}
uint32_t options = Read4BE(&buf); /* enum InvokeOptions bit flags */
VLOG(jdwp) << StringPrintf(" options=0x%04x%s%s", options, (options & INVOKE_SINGLE_THREADED) ? " (SINGLE_THREADED)" : "", (options & INVOKE_NONVIRTUAL) ? " (NONVIRTUAL)" : "");
- JDWP::JdwpTag resultTag;
+ JdwpTag resultTag;
uint64_t resultValue;
ObjectId exceptObjId;
- JdwpError err = Dbg::InvokeMethod(threadId, objectId, classId, methodId, numArgs, argArray, options, &resultTag, &resultValue, &exceptObjId);
+ JdwpError err = Dbg::InvokeMethod(threadId, objectId, classId, methodId, arg_count, argValues.get(), argTypes.get(), options, &resultTag, &resultValue, &exceptObjId);
if (err != ERR_NONE) {
- goto bail;
+ return err;
}
if (err == ERR_NONE) {
- if (isConstructor) {
- expandBufAdd1(pReply, JT_OBJECT);
- expandBufAddObjectId(pReply, objectId);
- } else {
- size_t width = Dbg::GetTagWidth(resultTag);
+ if (is_constructor) {
+ // If we invoked a constructor (which actually returns void), return the receiver,
+ // unless we threw, in which case we return NULL.
+ resultTag = JT_OBJECT;
+ resultValue = (exceptObjId == 0) ? objectId : 0;
+ }
- expandBufAdd1(pReply, resultTag);
- if (width != 0) {
- jdwpWriteValue(pReply, width, resultValue);
- }
+ size_t width = Dbg::GetTagWidth(resultTag);
+ expandBufAdd1(pReply, resultTag);
+ if (width != 0) {
+ jdwpWriteValue(pReply, width, resultValue);
}
expandBufAdd1(pReply, JT_OBJECT);
expandBufAddObjectId(pReply, exceptObjId);
@@ -164,8 +160,6 @@
}
}
-bail:
- free(argArray);
return err;
}
diff --git a/src/object_utils.h b/src/object_utils.h
index 498b0ba..0e0d7de 100644
--- a/src/object_utils.h
+++ b/src/object_utils.h
@@ -416,7 +416,7 @@
}
return result;
}
- int32_t GetShortyLength() {
+ uint32_t GetShortyLength() {
if (shorty_ == NULL) {
GetShorty();
}
@@ -587,7 +587,7 @@
const DexFile* dex_file_;
const Method* method_;
const char* shorty_;
- int32_t shorty_len_;
+ uint32_t shorty_len_;
DISALLOW_COPY_AND_ASSIGN(MethodHelper);
};
diff --git a/src/runtime_support.cc b/src/runtime_support.cc
index d9d7e90..38d2699 100644
--- a/src/runtime_support.cc
+++ b/src/runtime_support.cc
@@ -540,7 +540,7 @@
// Fix up jobject arguments
MethodHelper mh(jni_method);
int reg_num = 2; // Current register being processed, -1 for stack arguments.
- for (int32_t i = 1; i < mh.GetShortyLength(); i++) {
+ for (uint32_t i = 1; i < mh.GetShortyLength(); i++) {
char shorty_char = mh.GetShorty()[i];
if (shorty_char == 'L') {
WorkAroundJniBugsForJobject(arg_ptr);
