Change MethodHelper to use a Handle.
Added ConstHandle to help prevent errors where you modify the value
stored in the handle of the caller. Also fixed compaction bugs
related to not knowing MethodHelper::GetReturnType can resolve types.
This bug was present in interpreter RETURN_OBJECT.
Bug: 13077697
Change-Id: I71f964d4d810ab4debda1a09bc968af8f3c874a3
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 9cfba8d..6dbc6a0 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -400,8 +400,7 @@
}
const char* old_cause = self->StartAssertNoThreadSuspension("EnterInterpreterFromInvoke");
- MethodHelper mh(method);
- const DexFile::CodeItem* code_item = mh.GetCodeItem();
+ const DexFile::CodeItem* code_item = method->GetCodeItem();
uint16_t num_regs;
uint16_t num_ins;
if (code_item != NULL) {
@@ -413,7 +412,7 @@
return;
} else {
DCHECK(method->IsNative());
- num_regs = num_ins = ArtMethod::NumArgRegisters(mh.GetShorty());
+ num_regs = num_ins = ArtMethod::NumArgRegisters(method->GetShorty());
if (!method->IsStatic()) {
num_regs++;
num_ins++;
@@ -431,9 +430,10 @@
shadow_frame->SetVRegReference(cur_reg, receiver);
++cur_reg;
}
- const char* shorty = mh.GetShorty();
+ uint32_t shorty_len = 0;
+ const char* shorty = method->GetShorty(&shorty_len);
for (size_t shorty_pos = 0, arg_pos = 0; cur_reg < num_regs; ++shorty_pos, ++arg_pos, cur_reg++) {
- DCHECK_LT(shorty_pos + 1, mh.GetShortyLength());
+ DCHECK_LT(shorty_pos + 1, shorty_len);
switch (shorty[shorty_pos + 1]) {
case 'L': {
Object* o = reinterpret_cast<StackReference<Object>*>(&args[arg_pos])->AsMirrorPtr();
@@ -465,6 +465,8 @@
}
}
if (LIKELY(!method->IsNative())) {
+ StackHandleScope<1> hs(self);
+ MethodHelper mh(hs.NewHandle(method));
JValue r = Execute(self, mh, code_item, *shadow_frame, JValue());
if (result != NULL) {
*result = r;
@@ -488,11 +490,11 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
JValue value;
value.SetJ(ret_val->GetJ()); // Set value to last known result in case the shadow frame chain is empty.
- MethodHelper mh;
while (shadow_frame != NULL) {
self->SetTopOfShadowStack(shadow_frame);
- mh.ChangeMethod(shadow_frame->GetMethod());
- const DexFile::CodeItem* code_item = mh.GetCodeItem();
+ StackHandleScope<1> hs(self);
+ MethodHelper mh(hs.NewHandle(shadow_frame->GetMethod()));
+ const DexFile::CodeItem* code_item = mh.GetMethod()->GetCodeItem();
value = Execute(self, mh, code_item, *shadow_frame, value);
ShadowFrame* old_frame = shadow_frame;
shadow_frame = shadow_frame->GetLink();
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index a66bd94..c6e05ae 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -54,8 +54,7 @@
bool DoCall(ArtMethod* method, Thread* self, ShadowFrame& shadow_frame,
const Instruction* inst, uint16_t inst_data, JValue* result) {
// Compute method information.
- MethodHelper mh(method);
- const DexFile::CodeItem* code_item = mh.GetCodeItem();
+ const DexFile::CodeItem* code_item = method->GetCodeItem();
const uint16_t num_ins = (is_range) ? inst->VRegA_3rc(inst_data) : inst->VRegA_35c(inst_data);
uint16_t num_regs;
if (LIKELY(code_item != NULL)) {
@@ -73,6 +72,8 @@
// Initialize new shadow frame.
const size_t first_dest_reg = num_regs - num_ins;
+ StackHandleScope<1> hs(self);
+ MethodHelper mh(hs.NewHandle(method));
if (do_assignability_check) {
// Slow path.
// We might need to do class loading, which incurs a thread state change to kNative. So
@@ -82,8 +83,9 @@
// We need to do runtime check on reference assignment. We need to load the shorty
// to get the exact type of each reference argument.
- const DexFile::TypeList* params = mh.GetParameterTypeList();
- const char* shorty = mh.GetShorty();
+ const DexFile::TypeList* params = method->GetParameterTypeList();
+ uint32_t shorty_len = 0;
+ const char* shorty = method->GetShorty(&shorty_len);
// TODO: find a cleaner way to separate non-range and range information without duplicating code.
uint32_t arg[5]; // only used in invoke-XXX.
@@ -98,13 +100,13 @@
size_t dest_reg = first_dest_reg;
size_t arg_offset = 0;
if (!method->IsStatic()) {
- size_t receiver_reg = (is_range) ? vregC : arg[0];
+ size_t receiver_reg = is_range ? vregC : arg[0];
new_shadow_frame->SetVRegReference(dest_reg, shadow_frame.GetVRegReference(receiver_reg));
++dest_reg;
++arg_offset;
}
for (uint32_t shorty_pos = 0; dest_reg < num_regs; ++shorty_pos, ++dest_reg, ++arg_offset) {
- DCHECK_LT(shorty_pos + 1, mh.GetShortyLength());
+ DCHECK_LT(shorty_pos + 1, shorty_len);
const size_t src_reg = (is_range) ? vregC + arg_offset : arg[arg_offset];
switch (shorty[shorty_pos + 1]) {
case 'L': {
@@ -120,7 +122,7 @@
self->ThrowNewExceptionF(self->GetCurrentLocationForThrow(),
"Ljava/lang/VirtualMachineError;",
"Invoking %s with bad arg %d, type '%s' not instance of '%s'",
- mh.GetName(), shorty_pos,
+ method->GetName(), shorty_pos,
o->GetClass()->GetDescriptor().c_str(),
arg_type->GetDescriptor().c_str());
return false;
@@ -368,8 +370,9 @@
Object* obj = shadow_frame->GetVRegReference(arg_offset);
result->SetI(obj->IdentityHashCode());
} else if (name == "java.lang.String java.lang.reflect.ArtMethod.getMethodName(java.lang.reflect.ArtMethod)") {
- ArtMethod* method = shadow_frame->GetVRegReference(arg_offset)->AsArtMethod();
- result->SetL(MethodHelper(method).GetNameAsString());
+ StackHandleScope<1> hs(self);
+ MethodHelper mh(hs.NewHandle(shadow_frame->GetVRegReference(arg_offset)->AsArtMethod()));
+ result->SetL(mh.GetNameAsString(self));
} else if (name == "void java.lang.System.arraycopy(java.lang.Object, int, java.lang.Object, int, int)" ||
name == "void java.lang.System.arraycopy(char[], int, char[], int, int)") {
// Special case array copying without initializing System.
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index 6e136d6..88a8468 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -604,10 +604,15 @@
ThrowLocation throw_location;
mirror::Throwable* exception = self->GetException(&throw_location);
bool clear_exception = false;
- StackHandleScope<3> hs(self);
- Handle<mirror::Class> exception_class(hs.NewHandle(exception->GetClass()));
- uint32_t found_dex_pc = shadow_frame.GetMethod()->FindCatchBlock(exception_class, dex_pc,
- &clear_exception);
+ uint32_t found_dex_pc;
+ {
+ StackHandleScope<3> hs(self);
+ Handle<mirror::Class> exception_class(hs.NewHandle(exception->GetClass()));
+ Handle<mirror::ArtMethod> h_method(hs.NewHandle(shadow_frame.GetMethod()));
+ HandleWrapper<mirror::Object> h(hs.NewHandleWrapper(&this_object));
+ found_dex_pc = mirror::ArtMethod::FindCatchBlock(h_method, exception_class, dex_pc,
+ &clear_exception);
+ }
if (found_dex_pc == DexFile::kDexNoIndex) {
instrumentation->MethodUnwindEvent(self, this_object,
shadow_frame.GetMethod(), dex_pc);
@@ -627,7 +632,7 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
static inline void UnexpectedOpcode(const Instruction* inst, MethodHelper& mh) {
- LOG(FATAL) << "Unexpected instruction: " << inst->DumpString(&mh.GetDexFile());
+ LOG(FATAL) << "Unexpected instruction: " << inst->DumpString(mh.GetMethod()->GetDexFile());
exit(0); // Unreachable, keep GCC happy.
}
@@ -640,7 +645,7 @@
std::ostringstream oss;
oss << PrettyMethod(shadow_frame.GetMethod())
<< StringPrintf("\n0x%x: ", dex_pc)
- << inst->DumpString(&mh.GetDexFile()) << "\n";
+ << inst->DumpString(mh.GetMethod()->GetDexFile()) << "\n";
for (uint32_t i = 0; i < shadow_frame.NumberOfVRegs(); ++i) {
uint32_t raw_value = shadow_frame.GetVReg(i);
Object* ref_value = shadow_frame.GetVRegReference(i);
diff --git a/runtime/interpreter/interpreter_goto_table_impl.cc b/runtime/interpreter/interpreter_goto_table_impl.cc
index 623d9c3..19673ac 100644
--- a/runtime/interpreter/interpreter_goto_table_impl.cc
+++ b/runtime/interpreter/interpreter_goto_table_impl.cc
@@ -328,11 +328,13 @@
if (UNLIKELY(self->TestAllFlags())) {
CheckSuspend(self);
}
- Object* obj_result = shadow_frame.GetVRegReference(inst->VRegA_11x(inst_data));
- result.SetJ(0);
- result.SetL(obj_result);
+ const uint8_t vreg_index = inst->VRegA_11x(inst_data);
+ Object* obj_result = shadow_frame.GetVRegReference(vreg_index);
if (do_assignability_check && obj_result != NULL) {
- Class* return_type = MethodHelper(shadow_frame.GetMethod()).GetReturnType();
+ StackHandleScope<1> hs(self);
+ MethodHelper mh(hs.NewHandle(shadow_frame.GetMethod()));
+ Class* return_type = mh.GetReturnType();
+ obj_result = shadow_frame.GetVRegReference(vreg_index);
if (return_type == NULL) {
// Return the pending exception.
HANDLE_PENDING_EXCEPTION();
@@ -347,6 +349,7 @@
HANDLE_PENDING_EXCEPTION();
}
}
+ result.SetL(obj_result);
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
if (UNLIKELY(instrumentation->HasMethodExitListeners())) {
instrumentation->MethodExitEvent(self, shadow_frame.GetThisObject(code_item->ins_size_),
diff --git a/runtime/interpreter/interpreter_switch_impl.cc b/runtime/interpreter/interpreter_switch_impl.cc
index d592a53..a43fad3 100644
--- a/runtime/interpreter/interpreter_switch_impl.cc
+++ b/runtime/interpreter/interpreter_switch_impl.cc
@@ -244,11 +244,14 @@
if (UNLIKELY(self->TestAllFlags())) {
CheckSuspend(self);
}
- Object* obj_result = shadow_frame.GetVRegReference(inst->VRegA_11x(inst_data));
- result.SetJ(0);
- result.SetL(obj_result);
+ const size_t ref_idx = inst->VRegA_11x(inst_data);
+ Object* obj_result = shadow_frame.GetVRegReference(ref_idx);
if (do_assignability_check && obj_result != NULL) {
- Class* return_type = MethodHelper(shadow_frame.GetMethod()).GetReturnType();
+ StackHandleScope<1> hs(self);
+ MethodHelper mhs(hs.NewHandle(shadow_frame.GetMethod()));
+ Class* return_type = mhs.GetReturnType();
+ // Re-load since it might have moved.
+ obj_result = shadow_frame.GetVRegReference(ref_idx);
if (return_type == NULL) {
// Return the pending exception.
HANDLE_PENDING_EXCEPTION();
@@ -263,6 +266,7 @@
HANDLE_PENDING_EXCEPTION();
}
}
+ result.SetL(obj_result);
if (UNLIKELY(instrumentation->HasMethodExitListeners())) {
instrumentation->MethodExitEvent(self, shadow_frame.GetThisObject(code_item->ins_size_),
shadow_frame.GetMethod(), inst->GetDexPc(insns),