Fixes to verifier and slowpaths for cts invoke tests.
Instructions are never rewritten, so that has been removed from the
verifier. Also, the slowpath now checks for access errors when it
can't properly resolve a method.
Change-Id: Ie38eacec8eb092ba23502471a0b27ca8ce38fe68
diff --git a/src/class_linker.cc b/src/class_linker.cc
index cd89448..e693c56 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -3404,6 +3404,7 @@
uint32_t method_idx,
DexCache* dex_cache,
ClassLoader* class_loader,
+ const Method* referrer,
InvokeType type) {
DCHECK(dex_cache != NULL);
// Check for hit in the dex cache.
@@ -3486,14 +3487,42 @@
dex_cache->SetResolvedMethod(method_idx, resolved);
return resolved;
} else {
- // We failed to find the method which means either an incompatible class change or no such
- // method.
+ // We failed to find the method which means either an access error, an incompatible class
+ // change, or no such method. First try to find the method among direct and virtual methods.
const char* name = dex_file.StringDataByIdx(method_id.name_idx_);
std::string signature(dex_file.CreateMethodSignature(method_id.proto_idx_, NULL));
switch (type) {
case kDirect:
case kStatic:
resolved = klass->FindVirtualMethod(name, signature);
+ break;
+ case kInterface:
+ case kVirtual:
+ case kSuper:
+ resolved = klass->FindDirectMethod(name, signature);
+ break;
+ }
+
+ // If we found something, check that it can be accessed by the referrer.
+ if (resolved != NULL && referrer != NULL) {
+ Class* methods_class = resolved->GetDeclaringClass();
+ Class* referring_class = referrer->GetDeclaringClass();
+ if (!referring_class->CanAccess(methods_class)) {
+ ThrowNewIllegalAccessErrorClassForMethodDispatch(Thread::Current(), referring_class, methods_class,
+ referrer, resolved, type);
+ return NULL;
+ } else if (!referring_class->CanAccessMember(methods_class,
+ resolved->GetAccessFlags())) {
+ ThrowNewIllegalAccessErrorMethod(Thread::Current(), referring_class, resolved);
+ return NULL;
+ }
+ }
+
+ // Otherwise, throw an IncompatibleClassChangeError if we found something, and check interface
+ // methods and throw if we find the method there. If we find nothing, throw a NoSuchMethodError.
+ switch (type) {
+ case kDirect:
+ case kStatic:
if (resolved != NULL) {
ThrowIncompatibleClassChangeError(type, kVirtual, resolved);
} else {
@@ -3506,7 +3535,6 @@
}
break;
case kInterface:
- resolved = klass->FindDirectMethod(name, signature);
if (resolved != NULL) {
ThrowIncompatibleClassChangeError(type, kDirect, resolved);
} else {
@@ -3522,7 +3550,6 @@
ThrowNoSuchMethodError(type, klass, name, signature);
break;
case kVirtual:
- resolved = klass->FindDirectMethod(name, signature);
if (resolved != NULL) {
ThrowIncompatibleClassChangeError(type, kDirect, resolved);
} else {
diff --git a/src/class_linker.h b/src/class_linker.h
index 8a6aaec..eed9f6a 100644
--- a/src/class_linker.h
+++ b/src/class_linker.h
@@ -174,6 +174,7 @@
uint32_t method_idx,
DexCache* dex_cache,
ClassLoader* class_loader,
+ const Method* referrer,
InvokeType type)
SHARED_LOCKS_REQUIRED(GlobalSynchronization::mutator_lock_);
@@ -185,7 +186,7 @@
DexCache* dex_cache = declaring_class->GetDexCache();
ClassLoader* class_loader = declaring_class->GetClassLoader();
const DexFile& dex_file = FindDexFile(dex_cache);
- resolved_method = ResolveMethod(dex_file, method_idx, dex_cache, class_loader, type);
+ resolved_method = ResolveMethod(dex_file, method_idx, dex_cache, class_loader, referrer, type);
}
return resolved_method;
}
diff --git a/src/common_test.h b/src/common_test.h
index 41dc76c..4424d91 100644
--- a/src/common_test.h
+++ b/src/common_test.h
@@ -346,10 +346,12 @@
class_linker_ = runtime_->GetClassLinker();
InstructionSet instruction_set = kNone;
-#if defined(__i386__)
- instruction_set = kX86;
-#elif defined(__arm__)
+#if defined(__arm__)
instruction_set = kThumb2;
+#elif defined(__mips__)
+ instruction_set = kMips;
+#elif defined(__i386__)
+ instruction_set = kX86;
#endif
runtime_->SetJniDlsymLookupStub(Compiler::CreateJniDlsymLookupStub(instruction_set));
runtime_->SetAbstractMethodErrorStubArray(Compiler::CreateAbstractMethodErrorStub(instruction_set));
diff --git a/src/compiler.cc b/src/compiler.cc
index 4dd3a09..091aad9 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -673,7 +673,7 @@
DexCache* dex_cache = mUnit->class_linker_->FindDexCache(*mUnit->dex_file_);
ClassLoader* class_loader = soa.Decode<ClassLoader*>(mUnit->class_loader_);
return mUnit->class_linker_->ResolveMethod(*mUnit->dex_file_, method_idx, dex_cache,
- class_loader, type);
+ class_loader, NULL, type);
}
bool Compiler::ComputeInstanceFieldInfo(uint32_t field_idx, OatCompilationUnit* mUnit,
@@ -1140,7 +1140,7 @@
}
while (it.HasNextDirectMethod()) {
Method* method = class_linker->ResolveMethod(dex_file, it.GetMemberIndex(), dex_cache,
- class_loader, it.GetMethodInvokeType(class_def));
+ class_loader, NULL, it.GetMethodInvokeType(class_def));
if (method == NULL) {
CHECK(self->IsExceptionPending());
self->ClearException();
@@ -1149,7 +1149,7 @@
}
while (it.HasNextVirtualMethod()) {
Method* method = class_linker->ResolveMethod(dex_file, it.GetMemberIndex(), dex_cache,
- class_loader, it.GetMethodInvokeType(class_def));
+ class_loader, NULL, it.GetMethodInvokeType(class_def));
if (method == NULL) {
CHECK(self->IsExceptionPending());
self->ClearException();
@@ -1719,13 +1719,13 @@
}
while (it.HasNextDirectMethod()) {
Method* method = class_linker->ResolveMethod(dex_file, it.GetMemberIndex(), dex_cache,
- class_loader, it.GetMethodInvokeType(class_def));
+ class_loader, NULL, it.GetMethodInvokeType(class_def));
SetGcMapsMethod(dex_file, method);
it.Next();
}
while (it.HasNextVirtualMethod()) {
Method* method = class_linker->ResolveMethod(dex_file, it.GetMemberIndex(), dex_cache,
- class_loader, it.GetMethodInvokeType(class_def));
+ class_loader, NULL, it.GetMethodInvokeType(class_def));
SetGcMapsMethod(dex_file, method);
it.Next();
}
diff --git a/src/constants_mips.h b/src/constants_mips.h
index 12f7323..32fa158 100644
--- a/src/constants_mips.h
+++ b/src/constants_mips.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011 The Android Open Source Project
+ * Copyright (C) 2012 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
diff --git a/src/image_writer.cc b/src/image_writer.cc
index e8220fc..0932c30 100644
--- a/src/image_writer.cc
+++ b/src/image_writer.cc
@@ -612,6 +612,7 @@
patch->GetReferrerMethodIdx(),
dex_cache,
NULL,
+ NULL,
patch->GetReferrerInvokeType());
CHECK(method != NULL)
<< patch->GetDexFile().GetLocation() << " " << patch->GetReferrerMethodIdx();
@@ -632,6 +633,7 @@
patch->GetTargetMethodIdx(),
dex_cache,
NULL,
+ NULL,
patch->GetTargetInvokeType());
CHECK(method != NULL)
<< patch->GetDexFile().GetLocation() << " " << patch->GetTargetMethodIdx();
diff --git a/src/oat/runtime/context.cc b/src/oat/runtime/context.cc
index 729f044..7075e42 100644
--- a/src/oat/runtime/context.cc
+++ b/src/oat/runtime/context.cc
@@ -34,7 +34,7 @@
#elif defined(__i386__)
return new x86::X86Context();
#else
- UNIMPLEMENTED(WARNING);
+ UNIMPLEMENTED(FATAL);
#endif
}
diff --git a/src/oat_writer.cc b/src/oat_writer.cc
index 4aa948c..ce15368 100644
--- a/src/oat_writer.cc
+++ b/src/oat_writer.cc
@@ -385,7 +385,7 @@
// Unchecked as we hold mutator_lock_ on entry.
ScopedObjectAccessUnchecked soa(Thread::Current());
Method* method = linker->ResolveMethod(*dex_file, method_idx, dex_cache,
- soa.Decode<ClassLoader*>(class_loader_), type);
+ soa.Decode<ClassLoader*>(class_loader_), NULL, type);
CHECK(method != NULL);
method->SetFrameSizeInBytes(frame_size_in_bytes);
method->SetCoreSpillMask(core_spill_mask);
diff --git a/src/runtime_support.cc b/src/runtime_support.cc
index 91710e7..c5eaffa 100644
--- a/src/runtime_support.cc
+++ b/src/runtime_support.cc
@@ -146,7 +146,7 @@
void ThrowIncompatibleClassChangeError(InvokeType expected_type, InvokeType found_type,
Method* method) {
std::ostringstream msg;
- msg << "The method '" << PrettyMethod(method) << "' was expected to be of type"
+ msg << "The method '" << PrettyMethod(method) << "' was expected to be of type "
<< expected_type << " but instead was found to be of type " << found_type;
ClassHelper kh(method->GetDeclaringClass());
std::string location(kh.GetLocation());
diff --git a/src/verifier/method_verifier.cc b/src/verifier/method_verifier.cc
index 220741d..0cdff83 100644
--- a/src/verifier/method_verifier.cc
+++ b/src/verifier/method_verifier.cc
@@ -222,7 +222,7 @@
while (it.HasNextDirectMethod()) {
uint32_t method_idx = it.GetMemberIndex();
InvokeType type = it.GetMethodInvokeType(class_def);
- Method* method = linker->ResolveMethod(*dex_file, method_idx, dex_cache, class_loader, type);
+ Method* method = linker->ResolveMethod(*dex_file, method_idx, dex_cache, class_loader, NULL, type);
if (method == NULL) {
DCHECK(Thread::Current()->IsExceptionPending());
// We couldn't resolve the method, but continue regardless.
@@ -248,7 +248,7 @@
while (it.HasNextVirtualMethod()) {
uint32_t method_idx = it.GetMemberIndex();
InvokeType type = it.GetMethodInvokeType(class_def);
- Method* method = linker->ResolveMethod(*dex_file, method_idx, dex_cache, class_loader, type);
+ Method* method = linker->ResolveMethod(*dex_file, method_idx, dex_cache, class_loader, NULL, type);
if (method == NULL) {
DCHECK(Thread::Current()->IsExceptionPending());
// We couldn't resolve the method, but continue regardless.
@@ -333,7 +333,6 @@
interesting_dex_pc_(-1),
monitor_enter_dex_pcs_(NULL),
have_pending_hard_failure_(false),
- have_pending_rewrite_failure_(false),
new_instance_count_(0),
monitor_enter_count_(0) {
}
@@ -401,14 +400,11 @@
// If we're optimistically running verification at compile time, turn NO_xxx, ACCESS_xxx,
// class change and instantiation errors into soft verification errors so that we re-verify
// at runtime. We may fail to find or to agree on access because of not yet available class
- // loaders, or class loaders that will differ at runtime. In some cases we will rewrite bad
- // code at runtime but don't want to allow it at compile time due its effect on the
- // soundness of the code being compiled. These cases, in the generated code, need to run as
- // "slow paths" that dynamically perform the verification and cause the behavior to be that
- // akin to an interpreter.
+ // loaders, or class loaders that will differ at runtime. In these cases, we don't want to
+ // affect the soundness of the code being compiled. Instead, the generated code runs "slow
+ // paths" that dynamically perform the verification and cause the behavior to be that akin
+ // to an interpreter.
error = VERIFY_ERROR_BAD_CLASS_SOFT;
- } else {
- have_pending_rewrite_failure_ = true;
}
break;
// Indication that verification should be retried at runtime.
@@ -2278,16 +2274,6 @@
/* immediate failure, reject class */
info_messages_ << "Rejecting opcode " << inst->DumpString(dex_file_);
return false;
- } else if (have_pending_rewrite_failure_) {
- /* replace opcode and continue on */
- std::string append("Replacing opcode ");
- append += inst->DumpString(dex_file_);
- AppendToLastFailMessage(append);
- ReplaceFailingInstruction();
- /* IMPORTANT: method->insns may have been changed */
- insns = code_item_->insns_ + work_insn_idx_;
- /* continue on as if we just handled a throw-verification-error */
- opcode_flags = Instruction::kThrow;
}
/*
* If we didn't just set the result register, clear it out. This ensures that you can only use
@@ -3061,110 +3047,6 @@
return true;
}
-void MethodVerifier::ReplaceFailingInstruction() {
- // Pop the failure and clear the need for rewriting.
- size_t failure_number = failures_.size();
- CHECK_NE(failure_number, 0U);
- DCHECK_EQ(failure_messages_.size(), failure_number);
- std::ostringstream* failure_message = failure_messages_[0];
- VerifyError failure = failures_[0];
- failures_.clear();
- failure_messages_.clear();
- have_pending_rewrite_failure_ = false;
-
- if (Runtime::Current()->IsStarted()) {
- LOG(ERROR) << "Verification attempting to replace instructions at runtime in "
- << PrettyMethod(method_idx_, *dex_file_) << " " << failure_message->str();
- return;
- }
-
- CHECK_EQ(dex_file_->GetPermissions() & PROT_WRITE, PROT_WRITE); // Dex file needs to be writable.
-
- const Instruction* inst = Instruction::At(code_item_->insns_ + work_insn_idx_);
- DCHECK(inst->IsThrow()) << "Expected instruction that will throw " << inst->Name();
- VerifyErrorRefType ref_type;
- switch (inst->Opcode()) {
- case Instruction::CONST_CLASS: // insn[1] == class ref, 2 code units (4 bytes)
- case Instruction::CHECK_CAST:
- case Instruction::INSTANCE_OF:
- case Instruction::NEW_INSTANCE:
- case Instruction::NEW_ARRAY:
- case Instruction::FILLED_NEW_ARRAY: // insn[1] == class ref, 3 code units (6 bytes)
- case Instruction::FILLED_NEW_ARRAY_RANGE:
- ref_type = VERIFY_ERROR_REF_CLASS;
- break;
- case Instruction::IGET: // insn[1] == field ref, 2 code units (4 bytes)
- case Instruction::IGET_BOOLEAN:
- case Instruction::IGET_BYTE:
- case Instruction::IGET_CHAR:
- case Instruction::IGET_SHORT:
- case Instruction::IGET_WIDE:
- case Instruction::IGET_OBJECT:
- case Instruction::IPUT:
- case Instruction::IPUT_BOOLEAN:
- case Instruction::IPUT_BYTE:
- case Instruction::IPUT_CHAR:
- case Instruction::IPUT_SHORT:
- case Instruction::IPUT_WIDE:
- case Instruction::IPUT_OBJECT:
- case Instruction::SGET:
- case Instruction::SGET_BOOLEAN:
- case Instruction::SGET_BYTE:
- case Instruction::SGET_CHAR:
- case Instruction::SGET_SHORT:
- case Instruction::SGET_WIDE:
- case Instruction::SGET_OBJECT:
- case Instruction::SPUT:
- case Instruction::SPUT_BOOLEAN:
- case Instruction::SPUT_BYTE:
- case Instruction::SPUT_CHAR:
- case Instruction::SPUT_SHORT:
- case Instruction::SPUT_WIDE:
- case Instruction::SPUT_OBJECT:
- ref_type = VERIFY_ERROR_REF_FIELD;
- break;
- case Instruction::INVOKE_VIRTUAL: // insn[1] == method ref, 3 code units (6 bytes)
- case Instruction::INVOKE_VIRTUAL_RANGE:
- case Instruction::INVOKE_SUPER:
- case Instruction::INVOKE_SUPER_RANGE:
- case Instruction::INVOKE_DIRECT:
- case Instruction::INVOKE_DIRECT_RANGE:
- case Instruction::INVOKE_STATIC:
- case Instruction::INVOKE_STATIC_RANGE:
- case Instruction::INVOKE_INTERFACE:
- case Instruction::INVOKE_INTERFACE_RANGE:
- ref_type = VERIFY_ERROR_REF_METHOD;
- break;
- default:
- LOG(FATAL) << "Error: verifier asked to replace instruction " << inst->DumpString(dex_file_);
- return;
- }
- uint16_t* insns = const_cast<uint16_t*>(code_item_->insns_);
- // THROW_VERIFICATION_ERROR is a 2 code unit instruction. We shouldn't be rewriting a 1 code unit
- // instruction, so assert it.
- size_t width = inst->SizeInCodeUnits();
- CHECK_GT(width, 1u);
- // If the instruction is larger than 2 code units, rewrite subsequent code unit sized chunks with
- // NOPs
- for (size_t i = 2; i < width; i++) {
- insns[work_insn_idx_ + i] = Instruction::NOP;
- }
- // Encode the opcode, with the failure code in the high byte
- uint16_t new_instruction = Instruction::THROW_VERIFICATION_ERROR |
- (failure << 8) | // AA - component
- (ref_type << (8 + kVerifyErrorRefTypeShift));
- insns[work_insn_idx_] = new_instruction;
- // The 2nd code unit (higher in memory) with the reference in, comes from the instruction we
- // rewrote, so nothing to do here.
- LOG(INFO) << "Verification error, replacing instructions in "
- << PrettyMethod(method_idx_, *dex_file_) << " "
- << failure_message->str();
- if (gDebugVerify) {
- std::cout << "\n" << info_messages_.str();
- Dump(std::cout);
- }
-}
-
bool MethodVerifier::UpdateRegisters(uint32_t next_insn, const RegisterLine* merge_line) {
bool changed = true;
RegisterLine* target_line = reg_table_.GetLine(next_insn);
diff --git a/src/verifier/method_verifier.h b/src/verifier/method_verifier.h
index 80f70e1..3c15c10 100644
--- a/src/verifier/method_verifier.h
+++ b/src/verifier/method_verifier.h
@@ -537,12 +537,6 @@
bool CheckNotMoveException(const uint16_t* insns, int insn_idx);
/*
- * Replace an instruction with "throw-verification-error". This allows us to
- * defer error reporting until the code path is first used.
- */
- void ReplaceFailingInstruction();
-
- /*
* Control can transfer to "next_insn". Merge the registers from merge_line into the table at
* next_insn, and set the changed flag on the target address if any of the registers were changed.
* Returns "false" if an error is encountered.
@@ -654,8 +648,6 @@
std::vector<std::ostringstream*> failure_messages_;
// Is there a pending hard failure?
bool have_pending_hard_failure_;
- // Is there a pending failure that will cause dex opcodes to be rewritten.
- bool have_pending_rewrite_failure_;
// Info message log use primarily for verifier diagnostics.
std::ostringstream info_messages_;