ART: Refactor verifier callbacks
Change the return type of MethodVerified to void. It was never
used anyways.
Remove the callbacks calls from the core of the verifier (Verify()).
Instead, make the convenience functions do the work, and add a
parameter to supply the callback so that the verifier becomes
independent of the Runtime-stored one.
Fix up calls that now need to provide a callback, but leave places
that only run the verifier to get metadata (e.g., register type data,
lock state) without callback. This avoids callback calls when in JIT
mode.
Bug: 26075442
Change-Id: I2c270f01e4de088771d4d4b19dae4f07d77640f0
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 0a37f26..f5085ed 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3056,10 +3056,12 @@
verifier::MethodVerifier::FailureKind verifier_failure = verifier::MethodVerifier::kNoFailure;
std::string error_msg;
if (!preverified) {
+ Runtime* runtime = Runtime::Current();
verifier_failure = verifier::MethodVerifier::VerifyClass(self,
klass.Get(),
- Runtime::Current()->IsAotCompiler(),
- Runtime::Current()->IsAotCompiler(),
+ runtime->GetCompilerCallbacks(),
+ runtime->IsAotCompiler(),
+ runtime->IsAotCompiler(),
&error_msg);
}
if (preverified || verifier_failure != verifier::MethodVerifier::kHardFailure) {
diff --git a/runtime/compiler_callbacks.h b/runtime/compiler_callbacks.h
index af7b04f..a39d682 100644
--- a/runtime/compiler_callbacks.h
+++ b/runtime/compiler_callbacks.h
@@ -37,8 +37,8 @@
virtual ~CompilerCallbacks() { }
- virtual bool MethodVerified(verifier::MethodVerifier* verifier)
- SHARED_REQUIRES(Locks::mutator_lock_) = 0;
+ virtual void MethodVerified(verifier::MethodVerifier* verifier)
+ SHARED_REQUIRES(Locks::mutator_lock_) = 0;
virtual void ClassRejected(ClassReference ref) = 0;
// Return true if we should attempt to relocate to a random base address if we have not already
diff --git a/runtime/noop_compiler_callbacks.h b/runtime/noop_compiler_callbacks.h
index 1cbf2bb..02081cb 100644
--- a/runtime/noop_compiler_callbacks.h
+++ b/runtime/noop_compiler_callbacks.h
@@ -26,8 +26,7 @@
NoopCompilerCallbacks() : CompilerCallbacks(CompilerCallbacks::CallbackMode::kCompileApp) {}
~NoopCompilerCallbacks() {}
- bool MethodVerified(verifier::MethodVerifier* verifier ATTRIBUTE_UNUSED) OVERRIDE {
- return true;
+ void MethodVerified(verifier::MethodVerifier* verifier ATTRIBUTE_UNUSED) OVERRIDE {
}
void ClassRejected(ClassReference ref ATTRIBUTE_UNUSED) OVERRIDE {}
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index cf27ff2..3b0d7c4 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -116,6 +116,7 @@
MethodVerifier::FailureKind MethodVerifier::VerifyClass(Thread* self,
mirror::Class* klass,
+ CompilerCallbacks* callbacks,
bool allow_soft_failures,
bool log_hard_failures,
std::string* error) {
@@ -140,9 +141,9 @@
}
if (early_failure) {
*error = "Verifier rejected class " + PrettyDescriptor(klass) + failure_message;
- if (Runtime::Current()->IsAotCompiler()) {
+ if (callbacks != nullptr) {
ClassReference ref(&dex_file, klass->GetDexClassDefIndex());
- Runtime::Current()->GetCompilerCallbacks()->ClassRejected(ref);
+ callbacks->ClassRejected(ref);
}
return kHardFailure;
}
@@ -154,6 +155,7 @@
dex_cache,
class_loader,
class_def,
+ callbacks,
allow_soft_failures,
log_hard_failures,
error);
@@ -172,6 +174,7 @@
ClassDataItemIterator* it,
Handle<mirror::DexCache> dex_cache,
Handle<mirror::ClassLoader> class_loader,
+ CompilerCallbacks* callbacks,
bool allow_soft_failures,
bool log_hard_failures,
bool need_precise_constants,
@@ -212,6 +215,7 @@
it->GetMethodCodeItem(),
method,
it->GetMethodAccessFlags(),
+ callbacks,
allow_soft_failures,
log_hard_failures,
need_precise_constants,
@@ -241,6 +245,7 @@
Handle<mirror::DexCache> dex_cache,
Handle<mirror::ClassLoader> class_loader,
const DexFile::ClassDef* class_def,
+ CompilerCallbacks* callbacks,
bool allow_soft_failures,
bool log_hard_failures,
std::string* error) {
@@ -274,6 +279,7 @@
&it,
dex_cache,
class_loader,
+ callbacks,
allow_soft_failures,
log_hard_failures,
false /* need precise constants */,
@@ -288,6 +294,7 @@
&it,
dex_cache,
class_loader,
+ callbacks,
allow_soft_failures,
log_hard_failures,
false /* need precise constants */,
@@ -322,6 +329,7 @@
const DexFile::CodeItem* code_item,
ArtMethod* method,
uint32_t method_access_flags,
+ CompilerCallbacks* callbacks,
bool allow_soft_failures,
bool log_hard_failures,
bool need_precise_constants,
@@ -336,6 +344,12 @@
// Verification completed, however failures may be pending that didn't cause the verification
// to hard fail.
CHECK(!verifier.have_pending_hard_failure_);
+
+ if (code_item != nullptr && callbacks != nullptr) {
+ // Let the interested party know that the method was verified.
+ callbacks->MethodVerified(&verifier);
+ }
+
if (verifier.failures_.size() != 0) {
if (VLOG_IS_ON(verifier)) {
verifier.DumpFailures(VLOG_STREAM(verifier) << "Soft verification failures in "
@@ -363,8 +377,14 @@
verifier.failure_messages_[verifier.failure_messages_.size() - 1]->str();
}
result = kHardFailure;
+
+ if (callbacks != nullptr) {
+ // Let the interested party know that we failed the class.
+ ClassReference ref(dex_file, dex_file->GetIndexForClassDef(*class_def));
+ callbacks->ClassRejected(ref);
+ }
}
- if (kDebugVerify) {
+ if (VLOG_IS_ON(verifier)) {
std::cout << "\n" << verifier.info_messages_.str();
verifier.Dump(std::cout);
}
@@ -408,13 +428,18 @@
}
MethodVerifier::MethodVerifier(Thread* self,
- const DexFile* dex_file, Handle<mirror::DexCache> dex_cache,
+ const DexFile* dex_file,
+ Handle<mirror::DexCache> dex_cache,
Handle<mirror::ClassLoader> class_loader,
const DexFile::ClassDef* class_def,
- const DexFile::CodeItem* code_item, uint32_t dex_method_idx,
- ArtMethod* method, uint32_t method_access_flags,
- bool can_load_classes, bool allow_soft_failures,
- bool need_precise_constants, bool verify_to_dump,
+ const DexFile::CodeItem* code_item,
+ uint32_t dex_method_idx,
+ ArtMethod* method,
+ uint32_t method_access_flags,
+ bool can_load_classes,
+ bool allow_soft_failures,
+ bool need_precise_constants,
+ bool verify_to_dump,
bool allow_thread_suspension)
: self_(self),
arena_stack_(Runtime::Current()->GetArenaPool()),
@@ -739,10 +764,7 @@
result = result && VerifyInstructions();
// Perform code-flow analysis and return.
result = result && VerifyCodeFlow();
- // Compute information for compiler.
- if (result && runtime->IsCompiler()) {
- result = runtime->GetCompilerCallbacks()->MethodVerified(this);
- }
+
return result;
}
@@ -802,10 +824,6 @@
// Hard verification failures at compile time will still fail at runtime, so the class is
// marked as rejected to prevent it from being compiled.
case VERIFY_ERROR_BAD_CLASS_HARD: {
- if (Runtime::Current()->IsAotCompiler()) {
- ClassReference ref(dex_file_, dex_file_->GetIndexForClassDef(*class_def_));
- Runtime::Current()->GetCompilerCallbacks()->ClassRejected(ref);
- }
have_pending_hard_failure_ = true;
if (VLOG_IS_ON(verifier) && kDumpRegLinesOnHardFailureIfVLOG) {
ScopedObjectAccess soa(Thread::Current());
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index 719f0d7..79db576 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -33,6 +33,7 @@
namespace art {
+class CompilerCallbacks;
class Instruction;
struct ReferenceMap2Visitor;
class Thread;
@@ -141,6 +142,7 @@
/* Verify a class. Returns "kNoFailure" on success. */
static FailureKind VerifyClass(Thread* self,
mirror::Class* klass,
+ CompilerCallbacks* callbacks,
bool allow_soft_failures,
bool log_hard_failures,
std::string* error)
@@ -150,6 +152,7 @@
Handle<mirror::DexCache> dex_cache,
Handle<mirror::ClassLoader> class_loader,
const DexFile::ClassDef* class_def,
+ CompilerCallbacks* callbacks,
bool allow_soft_failures,
bool log_hard_failures,
std::string* error)
@@ -216,16 +219,34 @@
return can_load_classes_;
}
- MethodVerifier(Thread* self, const DexFile* dex_file, Handle<mirror::DexCache> dex_cache,
- Handle<mirror::ClassLoader> class_loader, const DexFile::ClassDef* class_def,
- const DexFile::CodeItem* code_item, uint32_t method_idx,
+ MethodVerifier(Thread* self,
+ const DexFile* dex_file,
+ Handle<mirror::DexCache> dex_cache,
+ Handle<mirror::ClassLoader> class_loader,
+ const DexFile::ClassDef* class_def,
+ const DexFile::CodeItem* code_item,
+ uint32_t method_idx,
ArtMethod* method,
- uint32_t access_flags, bool can_load_classes, bool allow_soft_failures,
- bool need_precise_constants, bool allow_thread_suspension)
+ uint32_t access_flags,
+ bool can_load_classes,
+ bool allow_soft_failures,
+ bool need_precise_constants,
+ bool allow_thread_suspension)
SHARED_REQUIRES(Locks::mutator_lock_)
- : MethodVerifier(self, dex_file, dex_cache, class_loader, class_def, code_item, method_idx,
- method, access_flags, can_load_classes, allow_soft_failures,
- need_precise_constants, false, allow_thread_suspension) {}
+ : MethodVerifier(self,
+ dex_file,
+ dex_cache,
+ class_loader,
+ class_def,
+ code_item,
+ method_idx,
+ method,
+ access_flags,
+ can_load_classes,
+ allow_soft_failures,
+ need_precise_constants,
+ false,
+ allow_thread_suspension) {}
~MethodVerifier();
@@ -299,12 +320,20 @@
}
// Private constructor for dumping.
- MethodVerifier(Thread* self, const DexFile* dex_file, Handle<mirror::DexCache> dex_cache,
- Handle<mirror::ClassLoader> class_loader, const DexFile::ClassDef* class_def,
- const DexFile::CodeItem* code_item, uint32_t method_idx,
- ArtMethod* method, uint32_t access_flags,
- bool can_load_classes, bool allow_soft_failures, bool need_precise_constants,
- bool verify_to_dump, bool allow_thread_suspension)
+ MethodVerifier(Thread* self,
+ const DexFile* dex_file,
+ Handle<mirror::DexCache> dex_cache,
+ Handle<mirror::ClassLoader> class_loader,
+ const DexFile::ClassDef* class_def,
+ const DexFile::CodeItem* code_item,
+ uint32_t method_idx,
+ ArtMethod* method,
+ uint32_t access_flags,
+ bool can_load_classes,
+ bool allow_soft_failures,
+ bool need_precise_constants,
+ bool verify_to_dump,
+ bool allow_thread_suspension)
SHARED_REQUIRES(Locks::mutator_lock_);
// Adds the given string to the beginning of the last failure message.
@@ -323,6 +352,7 @@
ClassDataItemIterator* it,
Handle<mirror::DexCache> dex_cache,
Handle<mirror::ClassLoader> class_loader,
+ CompilerCallbacks* callbacks,
bool allow_soft_failures,
bool log_hard_failures,
bool need_precise_constants,
@@ -350,6 +380,7 @@
const DexFile::CodeItem* code_item,
ArtMethod* method,
uint32_t method_access_flags,
+ CompilerCallbacks* callbacks,
bool allow_soft_failures,
bool log_hard_failures,
bool need_precise_constants,
diff --git a/runtime/verifier/method_verifier_test.cc b/runtime/verifier/method_verifier_test.cc
index c4123d5..946f842 100644
--- a/runtime/verifier/method_verifier_test.cc
+++ b/runtime/verifier/method_verifier_test.cc
@@ -37,8 +37,8 @@
// Verify the class
std::string error_msg;
- ASSERT_TRUE(MethodVerifier::VerifyClass(self, klass, true, true, &error_msg) == MethodVerifier::kNoFailure)
- << error_msg;
+ ASSERT_TRUE(MethodVerifier::VerifyClass(self, klass, nullptr, true, true, &error_msg)
+ == MethodVerifier::kNoFailure) << error_msg;
}
void VerifyDexFile(const DexFile& dex)