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/entrypoints/entrypoint_utils.cc b/runtime/entrypoints/entrypoint_utils.cc
index 320273d..a0e32f5 100644
--- a/runtime/entrypoints/entrypoint_utils.cc
+++ b/runtime/entrypoints/entrypoint_utils.cc
@@ -189,18 +189,21 @@
// Do nothing.
return zero;
} else {
+ StackHandleScope<1> hs(soa.Self());
+ MethodHelper mh_interface_method(
+ hs.NewHandle(soa.Decode<mirror::ArtMethod*>(interface_method_jobj)));
+ // This can cause thread suspension.
+ mirror::Class* result_type = mh_interface_method.GetReturnType();
mirror::Object* result_ref = soa.Decode<mirror::Object*>(result);
mirror::Object* rcvr = soa.Decode<mirror::Object*>(rcvr_jobj);
- mirror::ArtMethod* interface_method =
- soa.Decode<mirror::ArtMethod*>(interface_method_jobj);
- mirror::Class* result_type = MethodHelper(interface_method).GetReturnType();
mirror::ArtMethod* proxy_method;
- if (interface_method->GetDeclaringClass()->IsInterface()) {
- proxy_method = rcvr->GetClass()->FindVirtualMethodForInterface(interface_method);
+ if (mh_interface_method.GetMethod()->GetDeclaringClass()->IsInterface()) {
+ proxy_method = rcvr->GetClass()->FindVirtualMethodForInterface(
+ mh_interface_method.GetMethod());
} else {
// Proxy dispatch to a method defined in Object.
- DCHECK(interface_method->GetDeclaringClass()->IsObjectClass());
- proxy_method = interface_method;
+ DCHECK(mh_interface_method.GetMethod()->GetDeclaringClass()->IsObjectClass());
+ proxy_method = mh_interface_method.GetMethod();
}
ThrowLocation throw_location(rcvr, proxy_method, -1);
JValue result_unboxed;
diff --git a/runtime/entrypoints/entrypoint_utils.h b/runtime/entrypoints/entrypoint_utils.h
index d0ae746..09899c0 100644
--- a/runtime/entrypoints/entrypoint_utils.h
+++ b/runtime/entrypoints/entrypoint_utils.h
@@ -433,9 +433,8 @@
if (access_check &&
(vtable == nullptr || vtable_index >= static_cast<uint32_t>(vtable->GetLength()))) {
// Behavior to agree with that of the verifier.
- MethodHelper mh(resolved_method);
- ThrowNoSuchMethodError(type, resolved_method->GetDeclaringClass(), mh.GetName(),
- mh.GetSignature());
+ ThrowNoSuchMethodError(type, resolved_method->GetDeclaringClass(),
+ resolved_method->GetName(), resolved_method->GetSignature());
return nullptr; // Failure.
}
DCHECK(vtable != nullptr);
@@ -450,9 +449,8 @@
vtable = (super_class != nullptr) ? super_class->GetVTable() : nullptr;
if (vtable == nullptr || vtable_index >= static_cast<uint32_t>(vtable->GetLength())) {
// Behavior to agree with that of the verifier.
- MethodHelper mh(resolved_method);
- ThrowNoSuchMethodError(type, resolved_method->GetDeclaringClass(), mh.GetName(),
- mh.GetSignature());
+ ThrowNoSuchMethodError(type, resolved_method->GetDeclaringClass(),
+ resolved_method->GetName(), resolved_method->GetSignature());
return nullptr; // Failure.
}
} else {
@@ -682,11 +680,13 @@
JniAbortF(NULL, "invalid reference returned from %s", PrettyMethod(m).c_str());
}
// Make sure that the result is an instance of the type this method was expected to return.
- mirror::Class* return_type = MethodHelper(m).GetReturnType();
+ StackHandleScope<1> hs(self);
+ Handle<mirror::ArtMethod> h_m(hs.NewHandle(m));
+ mirror::Class* return_type = MethodHelper(h_m).GetReturnType();
if (!o->InstanceOf(return_type)) {
- JniAbortF(NULL, "attempt to return an instance of %s from %s",
- PrettyTypeOf(o).c_str(), PrettyMethod(m).c_str());
+ JniAbortF(NULL, "attempt to return an instance of %s from %s", PrettyTypeOf(o).c_str(),
+ PrettyMethod(h_m.Get()).c_str());
}
}
diff --git a/runtime/entrypoints/portable/portable_fillarray_entrypoints.cc b/runtime/entrypoints/portable/portable_fillarray_entrypoints.cc
index 1005d0e..335a617 100644
--- a/runtime/entrypoints/portable/portable_fillarray_entrypoints.cc
+++ b/runtime/entrypoints/portable/portable_fillarray_entrypoints.cc
@@ -26,7 +26,7 @@
mirror::Array* array,
uint32_t payload_offset)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
- const DexFile::CodeItem* code_item = MethodHelper(method).GetCodeItem();
+ const DexFile::CodeItem* code_item = method->GetCodeItem();
const Instruction::ArrayDataPayload* payload =
reinterpret_cast<const Instruction::ArrayDataPayload*>(code_item->insns_ + payload_offset);
DCHECK_EQ(payload->ident, static_cast<uint16_t>(Instruction::kArrayDataSignature));
diff --git a/runtime/entrypoints/portable/portable_invoke_entrypoints.cc b/runtime/entrypoints/portable/portable_invoke_entrypoints.cc
index 3a898e8..eb50ec3 100644
--- a/runtime/entrypoints/portable/portable_invoke_entrypoints.cc
+++ b/runtime/entrypoints/portable/portable_invoke_entrypoints.cc
@@ -41,9 +41,8 @@
// When we return, the caller will branch to this address, so it had better not be 0!
if (UNLIKELY(code == NULL)) {
- MethodHelper mh(method);
LOG(FATAL) << "Code was NULL in method: " << PrettyMethod(method)
- << " location: " << mh.GetDexFile().GetLocation();
+ << " location: " << method->GetDexFile()->GetLocation();
}
return method;
}
diff --git a/runtime/entrypoints/portable/portable_throw_entrypoints.cc b/runtime/entrypoints/portable/portable_throw_entrypoints.cc
index 1fdb832..189e6b5 100644
--- a/runtime/entrypoints/portable/portable_throw_entrypoints.cc
+++ b/runtime/entrypoints/portable/portable_throw_entrypoints.cc
@@ -79,8 +79,9 @@
return -1;
}
mirror::Class* exception_type = exception->GetClass();
- MethodHelper mh(current_method);
- const DexFile::CodeItem* code_item = mh.GetCodeItem();
+ StackHandleScope<1> hs(self);
+ MethodHelper mh(hs.NewHandle(current_method));
+ const DexFile::CodeItem* code_item = current_method->GetCodeItem();
DCHECK_LT(ti_offset, code_item->tries_size_);
const DexFile::TryItem* try_item = DexFile::GetTryItems(*code_item, ti_offset);
@@ -102,7 +103,7 @@
// TODO: check, the verifier (class linker?) should take care of resolving all exception
// classes early.
LOG(WARNING) << "Unresolved exception class when finding catch block: "
- << mh.GetTypeDescriptorFromTypeIdx(iter_type_idx);
+ << current_method->GetTypeDescriptorFromTypeIdx(iter_type_idx);
} else if (iter_exception_type->IsAssignableFrom(exception_type)) {
catch_dex_pc = it.GetHandlerAddress();
result = iter_index;
@@ -112,13 +113,11 @@
}
if (result != -1) {
// Handler found.
- Runtime::Current()->GetInstrumentation()->ExceptionCaughtEvent(self,
- throw_location,
- current_method,
- catch_dex_pc,
- exception);
+ Runtime::Current()->GetInstrumentation()->ExceptionCaughtEvent(
+ self, throw_location, current_method, catch_dex_pc, exception);
// If the catch block has no move-exception then clear the exception for it.
- const Instruction* first_catch_instr = Instruction::At(&mh.GetCodeItem()->insns_[catch_dex_pc]);
+ const Instruction* first_catch_instr = Instruction::At(
+ ¤t_method->GetCodeItem()->insns_[catch_dex_pc]);
if (first_catch_instr->Opcode() != Instruction::MOVE_EXCEPTION) {
self->ClearException();
}
diff --git a/runtime/entrypoints/portable/portable_trampoline_entrypoints.cc b/runtime/entrypoints/portable/portable_trampoline_entrypoints.cc
index 3756f47..6825e78 100644
--- a/runtime/entrypoints/portable/portable_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/portable/portable_trampoline_entrypoints.cc
@@ -196,8 +196,9 @@
return 0;
} else {
const char* old_cause = self->StartAssertNoThreadSuspension("Building interpreter shadow frame");
- MethodHelper mh(method);
- const DexFile::CodeItem* code_item = mh.GetCodeItem();
+ StackHandleScope<2> hs(self);
+ MethodHelper mh(hs.NewHandle(method));
+ const DexFile::CodeItem* code_item = method->GetCodeItem();
uint16_t num_regs = code_item->registers_size_;
void* memory = alloca(ShadowFrame::ComputeSize(num_regs));
ShadowFrame* shadow_frame(ShadowFrame::Create(num_regs, NULL, // No last shadow coming from quick.
@@ -214,7 +215,6 @@
if (method->IsStatic() && !method->GetDeclaringClass()->IsInitializing()) {
// Ensure static method's class is initialized.
- StackHandleScope<1> hs(self);
Handle<mirror::Class> h_class(hs.NewHandle(method->GetDeclaringClass()));
if (!Runtime::Current()->GetClassLinker()->EnsureInitialized(h_class, true, true)) {
DCHECK(Thread::Current()->IsExceptionPending());
@@ -294,7 +294,8 @@
jobject rcvr_jobj = soa.AddLocalReference<jobject>(receiver);
// Placing arguments into args vector and remove the receiver.
- MethodHelper proxy_mh(proxy_method);
+ StackHandleScope<1> hs(self);
+ MethodHelper proxy_mh(hs.NewHandle(proxy_method));
std::vector<jvalue> args;
BuildPortableArgumentVisitor local_ref_visitor(proxy_mh, sp, soa, args);
local_ref_visitor.VisitArguments();
@@ -327,7 +328,7 @@
InvokeType invoke_type;
bool is_range;
if (called->IsRuntimeMethod()) {
- const DexFile::CodeItem* code = MethodHelper(caller).GetCodeItem();
+ const DexFile::CodeItem* code = caller->GetCodeItem();
CHECK_LT(dex_pc, code->insns_size_in_code_units_);
const Instruction* instr = Instruction::At(&code->insns_[dex_pc]);
Instruction::Code instr_code = instr->Opcode();
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 5374f22..05033fc 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -474,16 +474,16 @@
} else {
DCHECK(!method->IsNative()) << PrettyMethod(method);
const char* old_cause = self->StartAssertNoThreadSuspension("Building interpreter shadow frame");
- MethodHelper mh(method);
- const DexFile::CodeItem* code_item = mh.GetCodeItem();
+ const DexFile::CodeItem* code_item = method->GetCodeItem();
DCHECK(code_item != nullptr) << PrettyMethod(method);
uint16_t num_regs = code_item->registers_size_;
void* memory = alloca(ShadowFrame::ComputeSize(num_regs));
ShadowFrame* shadow_frame(ShadowFrame::Create(num_regs, NULL, // No last shadow coming from quick.
method, 0, memory));
size_t first_arg_reg = code_item->registers_size_ - code_item->ins_size_;
- BuildQuickShadowFrameVisitor shadow_frame_builder(sp, mh.IsStatic(), mh.GetShorty(),
- mh.GetShortyLength(),
+ uint32_t shorty_len = 0;
+ const char* shorty = method->GetShorty(&shorty_len);
+ BuildQuickShadowFrameVisitor shadow_frame_builder(sp, method->IsStatic(), shorty, shorty_len,
shadow_frame, first_arg_reg);
shadow_frame_builder.VisitArguments();
// Push a transition back into managed code onto the linked list in thread.
@@ -503,6 +503,8 @@
}
}
+ StackHandleScope<1> hs(self);
+ MethodHelper mh(hs.NewHandle(method));
JValue result = interpreter::EnterInterpreterFromStub(self, mh, code_item, *shadow_frame);
// Pop transition.
self->PopManagedStackFragment(fragment);
@@ -604,11 +606,13 @@
jobject rcvr_jobj = soa.AddLocalReference<jobject>(receiver);
// Placing arguments into args vector and remove the receiver.
- MethodHelper proxy_mh(proxy_method);
- DCHECK(!proxy_mh.IsStatic()) << PrettyMethod(proxy_method);
+ mirror::ArtMethod* non_proxy_method = proxy_method->GetInterfaceMethodIfProxy();
+ CHECK(!non_proxy_method->IsStatic()) << PrettyMethod(proxy_method) << " "
+ << PrettyMethod(non_proxy_method);
std::vector<jvalue> args;
- BuildQuickArgumentVisitor local_ref_visitor(sp, proxy_mh.IsStatic(), proxy_mh.GetShorty(),
- proxy_mh.GetShortyLength(), &soa, &args);
+ uint32_t shorty_len = 0;
+ const char* shorty = proxy_method->GetShorty(&shorty_len);
+ BuildQuickArgumentVisitor local_ref_visitor(sp, false, shorty, shorty_len, &soa, &args);
local_ref_visitor.VisitArguments();
DCHECK_GT(args.size(), 0U) << PrettyMethod(proxy_method);
@@ -623,8 +627,7 @@
// All naked Object*s should now be in jobjects, so its safe to go into the main invoke code
// that performs allocations.
self->EndAssertNoThreadSuspension(old_cause);
- JValue result = InvokeProxyInvocationHandler(soa, proxy_mh.GetShorty(),
- rcvr_jobj, interface_method_jobj, args);
+ JValue result = InvokeProxyInvocationHandler(soa, shorty, rcvr_jobj, interface_method_jobj, args);
// Restore references which might have moved.
local_ref_visitor.FixupReferences();
return result.GetJ();
@@ -691,11 +694,8 @@
if (called->IsRuntimeMethod()) {
uint32_t dex_pc = caller->ToDexPc(QuickArgumentVisitor::GetCallingPc(sp));
const DexFile::CodeItem* code;
- {
- MethodHelper mh(caller);
- dex_file = &mh.GetDexFile();
- code = mh.GetCodeItem();
- }
+ dex_file = caller->GetDexFile();
+ code = caller->GetCodeItem();
CHECK_LT(dex_pc, code->insns_size_in_code_units_);
const Instruction* instr = Instruction::At(&code->insns_[dex_pc]);
Instruction::Code instr_code = instr->Opcode();
@@ -751,7 +751,7 @@
} else {
invoke_type = kStatic;
- dex_file = &MethodHelper(called).GetDexFile();
+ dex_file = called->GetDexFile();
dex_method_idx = called->GetDexMethodIndex();
}
uint32_t shorty_len;
@@ -797,9 +797,10 @@
// Calling from one dex file to another, need to compute the method index appropriate to
// the caller's dex file. Since we get here only if the original called was a runtime
// method, we've got the correct dex_file and a dex_method_idx from above.
- DCHECK(&MethodHelper(caller).GetDexFile() == dex_file);
- uint32_t method_index =
- MethodHelper(called).FindDexMethodIndexInOtherDexFile(*dex_file, dex_method_idx);
+ DCHECK_EQ(caller->GetDexFile(), dex_file);
+ StackHandleScope<1> hs(self);
+ MethodHelper mh(hs.NewHandle(called));
+ uint32_t method_index = mh.FindDexMethodIndexInOtherDexFile(*dex_file, dex_method_idx);
if (method_index != DexFile::kDexNoIndex) {
caller->GetDexCacheResolvedMethods()->Set<false>(method_index, called);
}
@@ -1511,10 +1512,9 @@
DCHECK(called->IsNative()) << PrettyMethod(called, true);
// run the visitor
- MethodHelper mh(called);
-
- BuildGenericJniFrameVisitor visitor(&sp, called->IsStatic(), mh.GetShorty(), mh.GetShortyLength(),
- self);
+ uint32_t shorty_len = 0;
+ const char* shorty = called->GetShorty(&shorty_len);
+ BuildGenericJniFrameVisitor visitor(&sp, called->IsStatic(), shorty, shorty_len, self);
visitor.VisitArguments();
visitor.FinalizeHandleScope(self);
@@ -1555,7 +1555,7 @@
// End JNI, as the assembly will move to deliver the exception.
jobject lock = called->IsSynchronized() ? visitor.GetFirstHandleScopeJObject() : nullptr;
- if (mh.GetShorty()[0] == 'L') {
+ if (shorty[0] == 'L') {
artQuickGenericJniEndJNIRef(self, cookie, nullptr, lock);
} else {
artQuickGenericJniEndJNINonRef(self, cookie, lock);
@@ -1594,8 +1594,7 @@
lock = table->GetHandle(0).ToJObject();
}
- MethodHelper mh(called);
- char return_shorty_char = mh.GetShorty()[0];
+ char return_shorty_char = called->GetShorty()[0];
if (return_shorty_char == 'L') {
return artQuickGenericJniEndJNIRef(self, cookie, result.l, lock);
@@ -1721,7 +1720,7 @@
// When we return, the caller will branch to this address, so it had better not be 0!
DCHECK(code != nullptr) << "Code was NULL in method: " << PrettyMethod(method) << " location: "
- << MethodHelper(method).GetDexFile().GetLocation();
+ << method->GetDexFile()->GetLocation();
return GetSuccessValue(code, method);
}
@@ -1870,7 +1869,7 @@
uintptr_t caller_pc = 0;
#endif
uint32_t dex_pc = caller_method->ToDexPc(caller_pc);
- const DexFile::CodeItem* code = MethodHelper(caller_method).GetCodeItem();
+ const DexFile::CodeItem* code = caller_method->GetCodeItem();
CHECK_LT(dex_pc, code->insns_size_in_code_units_);
const Instruction* instr = Instruction::At(&code->insns_[dex_pc]);
Instruction::Code instr_code = instr->Opcode();
@@ -1908,7 +1907,7 @@
// When we return, the caller will branch to this address, so it had better not be 0!
DCHECK(code != nullptr) << "Code was NULL in method: " << PrettyMethod(method) << " location: "
- << MethodHelper(method).GetDexFile().GetLocation();
+ << method->GetDexFile()->GetLocation();
return GetSuccessValue(code, method);
}