Fix incompatible class change error for JIT stress mode
There was a problem with miranda methods, when we would dequicken to
one of these, it wouldn't resolve as virtual during the method
lowering resolve. The solution is to try resolving as interface if we
fail to resolve as virtual.
Fixed a bug in dequickening where unreachable register lines with
quick invokes would cause CHECK failuers. In this case we punt to the
interpreter (test 435-try-*).
Added test regression test. Example failure:
java.lang.IncompatibleClassChangeError: The method
'void Main$TheInterface.m()' was expected to be of type virtual but
instead was found to be of type interface (declaration of
'java.lang.reflect.ArtMethod' appears in
out/host/linux-x86/framework/core-libart-hostdex.jar)
at Main.DoStuff(Main.java:37)
at Main.main(Main.java:44)
Bug: 17950037
Change-Id: I39c32cc8849bf02032a4f61a7ce57462b7fcac75
diff --git a/compiler/dex/dex_to_dex_compiler.cc b/compiler/dex/dex_to_dex_compiler.cc
index 7e916be..fcefb6f 100644
--- a/compiler/dex/dex_to_dex_compiler.cc
+++ b/compiler/dex/dex_to_dex_compiler.cc
@@ -252,10 +252,8 @@
}
}
-void DexCompiler::CompileInvokeVirtual(Instruction* inst,
- uint32_t dex_pc,
- Instruction::Code new_opcode,
- bool is_range) {
+void DexCompiler::CompileInvokeVirtual(Instruction* inst, uint32_t dex_pc,
+ Instruction::Code new_opcode, bool is_range) {
if (!kEnableQuickening || !PerformOptimizations()) {
return;
}
diff --git a/compiler/dex/mir_method_info.cc b/compiler/dex/mir_method_info.cc
index 3d3d979..34fb1bf 100644
--- a/compiler/dex/mir_method_info.cc
+++ b/compiler/dex/mir_method_info.cc
@@ -58,8 +58,9 @@
auto current_dex_cache(hs.NewHandle<mirror::DexCache>(nullptr));
// Even if the referrer class is unresolved (i.e. we're compiling a method without class
// definition) we still want to resolve methods and record all available info.
+ Runtime* const runtime = Runtime::Current();
const DexFile* const dex_file = mUnit->GetDexFile();
- const bool use_jit = Runtime::Current()->UseJit();
+ const bool use_jit = runtime->UseJit();
const VerifiedMethod* const verified_method = mUnit->GetVerifiedMethod();
for (auto it = method_infos, end = method_infos + count; it != end; ++it) {
@@ -80,7 +81,7 @@
it->target_method_idx_ = it->MethodIndex();
current_dex_cache.Assign(dex_cache.Get());
resolved_method = compiler_driver->ResolveMethod(soa, dex_cache, class_loader, mUnit,
- it->MethodIndex(), invoke_type);
+ it->target_method_idx_, invoke_type, true);
} else {
// The method index is actually the dex PC in this case.
// Calculate the proper dex file and target method idx.
@@ -89,8 +90,7 @@
// Don't devirt if we are in a different dex file since we can't have direct invokes in
// another dex file unless we always put a direct / patch pointer.
devirt_target = nullptr;
- current_dex_cache.Assign(
- Runtime::Current()->GetClassLinker()->FindDexCache(*it->target_dex_file_));
+ current_dex_cache.Assign(runtime->GetClassLinker()->FindDexCache(*it->target_dex_file_));
CHECK(current_dex_cache.Get() != nullptr);
DexCompilationUnit cu(
mUnit->GetCompilationUnit(), mUnit->GetClassLoader(), mUnit->GetClassLinker(),
@@ -99,6 +99,14 @@
nullptr /* verified_method not used */);
resolved_method = compiler_driver->ResolveMethod(soa, current_dex_cache, class_loader, &cu,
it->target_method_idx_, invoke_type, false);
+ if (resolved_method == nullptr) {
+ // If the method is null then it should be a miranda method, in this case try
+ // re-loading it, this time as an interface method. The actual miranda method is in the
+ // vtable, but it will resolve to an interface method.
+ resolved_method = compiler_driver->ResolveMethod(
+ soa, current_dex_cache, class_loader, &cu, it->target_method_idx_, kInterface, false);
+ CHECK(resolved_method != nullptr);
+ }
if (resolved_method != nullptr) {
// Since this was a dequickened virtual, it is guaranteed to be resolved. However, it may be
// resolved to an interface method. If this is the case then change the invoke type to
@@ -123,10 +131,9 @@
it->vtable_idx_ =
compiler_driver->GetResolvedMethodVTableIndex(resolved_method, invoke_type);
}
-
MethodReference target_method(it->target_dex_file_, it->target_method_idx_);
int fast_path_flags = compiler_driver->IsFastInvoke(
- soa, dex_cache, class_loader, mUnit, referrer_class.Get(), resolved_method,
+ soa, current_dex_cache, class_loader, mUnit, referrer_class.Get(), resolved_method,
&invoke_type, &target_method, devirt_target, &it->direct_code_, &it->direct_method_);
const bool is_referrers_class = referrer_class.Get() == resolved_method->GetDeclaringClass();
const bool is_class_initialized =
diff --git a/compiler/dex/verified_method.cc b/compiler/dex/verified_method.cc
index 42d66be..5b90ba9 100644
--- a/compiler/dex/verified_method.cc
+++ b/compiler/dex/verified_method.cc
@@ -55,8 +55,8 @@
}
// Only need dequicken info for JIT so far.
- if (Runtime::Current()->UseJit()) {
- verified_method->GenerateDequickenMap(method_verifier);
+ if (Runtime::Current()->UseJit() && !verified_method->GenerateDequickenMap(method_verifier)) {
+ return nullptr;
}
}
@@ -194,9 +194,9 @@
*log2_max_gc_pc = i;
}
-void VerifiedMethod::GenerateDequickenMap(verifier::MethodVerifier* method_verifier) {
+bool VerifiedMethod::GenerateDequickenMap(verifier::MethodVerifier* method_verifier) {
if (method_verifier->HasFailures()) {
- return;
+ return false;
}
const DexFile::CodeItem* code_item = method_verifier->CodeItem();
const uint16_t* insns = code_item->insns_;
@@ -209,8 +209,11 @@
uint32_t dex_pc = inst->GetDexPc(insns);
verifier::RegisterLine* line = method_verifier->GetRegLine(dex_pc);
mirror::ArtMethod* method =
- method_verifier->GetQuickInvokedMethod(inst, line, is_range_quick);
- CHECK(method != nullptr);
+ method_verifier->GetQuickInvokedMethod(inst, line, is_range_quick, true);
+ if (method == nullptr) {
+ // It can be null if the line wasn't verified since it was unreachable.
+ return false;
+ }
// The verifier must know what the type of the object was or else we would have gotten a
// failure. Put the dex method index in the dequicken map since we need this to get number of
// arguments in the compiler.
@@ -220,7 +223,10 @@
uint32_t dex_pc = inst->GetDexPc(insns);
verifier::RegisterLine* line = method_verifier->GetRegLine(dex_pc);
mirror::ArtField* field = method_verifier->GetQuickFieldAccess(inst, line);
- CHECK(field != nullptr);
+ if (field == nullptr) {
+ // It can be null if the line wasn't verified since it was unreachable.
+ return false;
+ }
// The verifier must know what the type of the field was or else we would have gotten a
// failure. Put the dex field index in the dequicken map since we need this for lowering
// in the compiler.
@@ -228,6 +234,7 @@
dequicken_map_.Put(dex_pc, DexFileReference(field->GetDexFile(), field->GetDexFieldIndex()));
}
}
+ return true;
}
void VerifiedMethod::GenerateDevirtMap(verifier::MethodVerifier* method_verifier) {
diff --git a/compiler/dex/verified_method.h b/compiler/dex/verified_method.h
index 748bdcb..954cbf4 100644
--- a/compiler/dex/verified_method.h
+++ b/compiler/dex/verified_method.h
@@ -93,8 +93,8 @@
void GenerateDevirtMap(verifier::MethodVerifier* method_verifier)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
- // Generate dequickening map into dequicken_map_.
- void GenerateDequickenMap(verifier::MethodVerifier* method_verifier)
+ // Generate dequickening map into dequicken_map_. Returns false if there is an error.
+ bool GenerateDequickenMap(verifier::MethodVerifier* method_verifier)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
// Generate safe case set into safe_cast_set_.
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 87a29ed..a48735b 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -513,7 +513,7 @@
}
const Instruction* inst = Instruction::At(code_item_->insns_ + dex_pc);
const bool is_range = (inst->Opcode() == Instruction::INVOKE_VIRTUAL_RANGE_QUICK);
- return GetQuickInvokedMethod(inst, register_line, is_range);
+ return GetQuickInvokedMethod(inst, register_line, is_range, false);
}
bool MethodVerifier::Verify() {
@@ -3431,10 +3431,14 @@
}
mirror::ArtMethod* MethodVerifier::GetQuickInvokedMethod(const Instruction* inst,
- RegisterLine* reg_line, bool is_range) {
- DCHECK(inst->Opcode() == Instruction::INVOKE_VIRTUAL_QUICK ||
- inst->Opcode() == Instruction::INVOKE_VIRTUAL_RANGE_QUICK);
- const RegType& actual_arg_type = reg_line->GetInvocationThis(this, inst, is_range);
+ RegisterLine* reg_line, bool is_range,
+ bool allow_failure) {
+ if (is_range) {
+ DCHECK_EQ(inst->Opcode(), Instruction::INVOKE_VIRTUAL_RANGE_QUICK);
+ } else {
+ DCHECK_EQ(inst->Opcode(), Instruction::INVOKE_VIRTUAL_QUICK);
+ }
+ const RegType& actual_arg_type = reg_line->GetInvocationThis(this, inst, is_range, allow_failure);
if (!actual_arg_type.HasClass()) {
VLOG(verifier) << "Failed to get mirror::Class* from '" << actual_arg_type << "'";
return nullptr;
@@ -3445,29 +3449,29 @@
// Derive Object.class from Class.class.getSuperclass().
mirror::Class* object_klass = klass->GetClass()->GetSuperClass();
if (FailOrAbort(this, object_klass->IsObjectClass(),
- "Failed to find Object class in quickened invoke receiver",
- work_insn_idx_)) {
+ "Failed to find Object class in quickened invoke receiver", work_insn_idx_)) {
return nullptr;
}
dispatch_class = object_klass;
} else {
dispatch_class = klass;
}
- if (FailOrAbort(this, dispatch_class->HasVTable(),
- "Receiver class has no vtable for quickened invoke at ",
- work_insn_idx_)) {
+ if (!dispatch_class->HasVTable()) {
+ FailOrAbort(this, allow_failure, "Receiver class has no vtable for quickened invoke at ",
+ work_insn_idx_);
return nullptr;
}
uint16_t vtable_index = is_range ? inst->VRegB_3rc() : inst->VRegB_35c();
- if (FailOrAbort(this, static_cast<int32_t>(vtable_index) < dispatch_class->GetVTableLength(),
- "Receiver class has not enough vtable slots for quickened invoke at ",
- work_insn_idx_)) {
+ if (static_cast<int32_t>(vtable_index) >= dispatch_class->GetVTableLength()) {
+ FailOrAbort(this, allow_failure,
+ "Receiver class has not enough vtable slots for quickened invoke at ",
+ work_insn_idx_);
return nullptr;
}
mirror::ArtMethod* res_method = dispatch_class->GetVTableEntry(vtable_index);
- if (FailOrAbort(this, !self_->IsExceptionPending(),
- "Unexpected exception pending for quickened invoke at ",
- work_insn_idx_)) {
+ if (self_->IsExceptionPending()) {
+ FailOrAbort(this, allow_failure, "Unexpected exception pending for quickened invoke at ",
+ work_insn_idx_);
return nullptr;
}
return res_method;
@@ -3478,8 +3482,7 @@
DCHECK(Runtime::Current()->IsStarted() || verify_to_dump_)
<< PrettyMethod(dex_method_idx_, *dex_file_, true) << "@" << work_insn_idx_;
- mirror::ArtMethod* res_method = GetQuickInvokedMethod(inst, work_line_.get(),
- is_range);
+ mirror::ArtMethod* res_method = GetQuickInvokedMethod(inst, work_line_.get(), is_range, false);
if (res_method == nullptr) {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "Cannot infer method from " << inst->Name();
return nullptr;
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index bdd6259..d7c2071 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -244,7 +244,7 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
// Returns the method of a quick invoke or nullptr if it cannot be found.
mirror::ArtMethod* GetQuickInvokedMethod(const Instruction* inst, RegisterLine* reg_line,
- bool is_range)
+ bool is_range, bool allow_failure)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
// Returns the access field of a quick field access (iget/iput-quick) or nullptr
// if it cannot be found.
diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc
index 3b09871..ed588fc 100644
--- a/runtime/verifier/register_line.cc
+++ b/runtime/verifier/register_line.cc
@@ -81,18 +81,23 @@
}
const RegType& RegisterLine::GetInvocationThis(MethodVerifier* verifier, const Instruction* inst,
- bool is_range) {
+ bool is_range, bool allow_failure) {
const size_t args_count = is_range ? inst->VRegA_3rc() : inst->VRegA_35c();
if (args_count < 1) {
- verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invoke lacks 'this'";
+ if (!allow_failure) {
+ verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invoke lacks 'this'";
+ }
return verifier->GetRegTypeCache()->Conflict();
}
/* Get the element type of the array held in vsrc */
const uint32_t this_reg = (is_range) ? inst->VRegC_3rc() : inst->VRegC_35c();
const RegType& this_type = GetRegisterType(verifier, this_reg);
if (!this_type.IsReferenceTypes()) {
- verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "tried to get class from non-reference register v"
- << this_reg << " (type=" << this_type << ")";
+ if (!allow_failure) {
+ verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD)
+ << "tried to get class from non-reference register v" << this_reg
+ << " (type=" << this_type << ")";
+ }
return verifier->GetRegTypeCache()->Conflict();
}
return this_type;
diff --git a/runtime/verifier/register_line.h b/runtime/verifier/register_line.h
index ca61a0b..376dbf1 100644
--- a/runtime/verifier/register_line.h
+++ b/runtime/verifier/register_line.h
@@ -188,9 +188,11 @@
*
* The argument count is in vA, and the first argument is in vC, for both "simple" and "range"
* versions. We just need to make sure vA is >= 1 and then return vC.
+ * allow_failure will return Conflict() instead of causing a verification failure if there is an
+ * error.
*/
const RegType& GetInvocationThis(MethodVerifier* verifier, const Instruction* inst,
- bool is_range)
+ bool is_range, bool allow_failure = false)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
/*
diff --git a/test/135-MirandaDispatch/expected.txt b/test/135-MirandaDispatch/expected.txt
new file mode 100644
index 0000000..134d8d0
--- /dev/null
+++ b/test/135-MirandaDispatch/expected.txt
@@ -0,0 +1 @@
+Finishing
diff --git a/test/135-MirandaDispatch/info.txt b/test/135-MirandaDispatch/info.txt
new file mode 100644
index 0000000..22d2777
--- /dev/null
+++ b/test/135-MirandaDispatch/info.txt
@@ -0,0 +1,6 @@
+Regression test for JIT related incompatible class changes caused by miranda methods.
+E.g.
+java.lang.IncompatibleClassChangeError: The method 'void Main$TheInterface.m()' was expected to be of type virtual but instead was found to be of type interface (declaration of 'java.lang.reflect.ArtMethod' appears in out/host/linux-x86/framework/core-libart-hostdex.jar)
+ at Main.DoStuff(Main.java:37)
+ at Main.main(Main.java:44)
+
diff --git a/test/135-MirandaDispatch/src/Main.java b/test/135-MirandaDispatch/src/Main.java
new file mode 100644
index 0000000..bb005b0
--- /dev/null
+++ b/test/135-MirandaDispatch/src/Main.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2015 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.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+ // Enough to trigger JIT.
+ static final int loopIterations = 5000;
+ static int counter = 0;
+
+ static interface TheInterface {
+ public void m();
+ }
+
+ static abstract class AbstractClass implements TheInterface {
+ }
+
+ static class ConcreteClass extends AbstractClass {
+ public void m() {
+ ++counter;
+ }
+ }
+
+ static void doStuff(AbstractClass c) {
+ for (int i = 0; i < loopIterations; ++i) {
+ c.m();
+ }
+ }
+
+ public static void main(String[] args) throws Exception {
+ ConcreteClass o = new ConcreteClass();
+ for (int i = 0; i < loopIterations; ++i) {
+ doStuff(o);
+ }
+ if (counter != loopIterations * loopIterations) {
+ System.out.println("Expected " + loopIterations * loopIterations + " got " + counter);
+ }
+ System.out.println("Finishing");
+ }
+}