Clean up verifier interface.
Remove verifier_callbacks and ArtMethod as argument. The verifier can
operate without them.
This allows removing the bogus DexCache::SetResolvedType in ti_redefine.
Also turn runtime throw failures into VerifyError, for cleaner interface
with users of the verifier.
Test: test.py
Bug: 28313047
Change-Id: I9ba1300f198aaf482ed43061465daea789ea732b
diff --git a/dex2oat/verifier_deps_test.cc b/dex2oat/verifier_deps_test.cc
index e833180..e480478 100644
--- a/dex2oat/verifier_deps_test.cc
+++ b/dex2oat/verifier_deps_test.cc
@@ -172,7 +172,6 @@
*class_def,
method.GetCodeItem(),
method.GetIndex(),
- resolved_method,
method.GetAccessFlags(),
/* can_load_classes= */ true,
/* allow_soft_failures= */ true,
diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc
index 0f94b5e..365d1e8 100644
--- a/oatdump/oatdump.cc
+++ b/oatdump/oatdump.cc
@@ -1494,8 +1494,16 @@
return nullptr;
}
return verifier::MethodVerifier::VerifyMethodAndDump(
- soa.Self(), vios, dex_method_idx, dex_file, dex_cache, *options_.class_loader_,
- class_def, code_item, method, method_access_flags, /* api_level= */ 0);
+ soa.Self(),
+ vios,
+ dex_method_idx,
+ dex_file,
+ dex_cache,
+ *options_.class_loader_,
+ class_def,
+ code_item,
+ method_access_flags,
+ /* api_level= */ 0);
}
return nullptr;
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index b2b839b..65bfc15 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -1873,11 +1873,6 @@
}
data->SetNewClassObject(nc.Get());
- // We really want to be able to resolve to the new class-object using this dex-cache for
- // verification work. Since we haven't put it in the class-table yet we wll just manually add it
- // to the dex-cache.
- // TODO: We should maybe do this in a better spot.
- data->GetNewDexCache()->SetResolvedType(nc->GetDexTypeIndex(), nc.Get());
data->SetInitialized();
return nc.Get();
};
diff --git a/runtime/verifier/class_verifier.cc b/runtime/verifier/class_verifier.cc
index 7559f98..7e0ed1e 100644
--- a/runtime/verifier/class_verifier.cc
+++ b/runtime/verifier/class_verifier.cc
@@ -38,6 +38,7 @@
#include "mirror/dex_cache.h"
#include "runtime.h"
#include "thread.h"
+#include "verifier_compiler_binding.h"
#include "verifier/method_verifier.h"
#include "verifier/reg_type_cache.h"
@@ -50,6 +51,16 @@
// sure we only print this once.
static bool gPrintedDxMonitorText = false;
+// A class used by the verifier to tell users about what options need to be set for given methods.
+class VerifierCallback {
+ public:
+ virtual ~VerifierCallback() {}
+ virtual void SetDontCompile(ArtMethod* method, bool value)
+ REQUIRES_SHARED(Locks::mutator_lock_) = 0;
+ virtual void SetMustCountLocks(ArtMethod* method, bool value)
+ REQUIRES_SHARED(Locks::mutator_lock_) = 0;
+};
+
class StandardVerifyCallback : public VerifierCallback {
public:
void SetDontCompile(ArtMethod* m, bool value) override REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -266,16 +277,6 @@
continue;
}
*previous_idx = method_idx;
- const InvokeType type = method.GetInvokeType(class_def.access_flags_);
- ArtMethod* resolved_method = linker->ResolveMethod<ClassLinker::ResolveMode::kNoChecks>(
- method_idx, dex_cache, class_loader, /* referrer= */ nullptr, type);
- if (resolved_method == nullptr) {
- DCHECK(self->IsExceptionPending());
- // We couldn't resolve the method, but continue regardless.
- self->ClearException();
- } else {
- DCHECK(resolved_method->GetDeclaringClassUnchecked() != nullptr) << type;
- }
std::string hard_failure_msg;
MethodVerifier::FailureData result =
MethodVerifier::VerifyMethod(self,
@@ -288,10 +289,8 @@
class_loader,
class_def,
method.GetCodeItem(),
- resolved_method,
method.GetAccessFlags(),
callbacks,
- verifier_callback,
allow_soft_failures,
log_level,
/*need_precise_constants=*/ false,
@@ -310,7 +309,38 @@
}
*error += " ";
*error += hard_failure_msg;
+ } else if (result.kind != FailureKind::kNoFailure) {
+ // Mark methods with DontCompile/MustCountLocks flags.
+ const InvokeType type = method.GetInvokeType(class_def.access_flags_);
+ ArtMethod* resolved_method = linker->ResolveMethod<ClassLinker::ResolveMode::kNoChecks>(
+ method_idx, dex_cache, class_loader, /* referrer= */ nullptr, type);
+ if (resolved_method == nullptr) {
+ DCHECK(self->IsExceptionPending());
+ // We couldn't resolve the method, but continue regardless.
+ self->ClearException();
+ } else {
+ DCHECK(resolved_method->GetDeclaringClassUnchecked() != nullptr) << type;
+ if (!CanCompilerHandleVerificationFailure(result.types)) {
+ verifier_callback->SetDontCompile(resolved_method, true);
+ }
+ if ((result.types & VerifyError::VERIFY_ERROR_LOCKING) != 0) {
+ verifier_callback->SetMustCountLocks(resolved_method, true);
+ // Print a warning about expected slow-down.
+ // Use a string temporary to print one contiguous warning.
+ std::string tmp =
+ StringPrintf("Method %s failed lock verification and will run slower.",
+ resolved_method->PrettyMethod().c_str());
+ if (!gPrintedDxMonitorText) {
+ tmp = tmp + "\nCommon causes for lock verification issues are non-optimized dex code\n"
+ "and incorrect proguard optimizations.";
+ gPrintedDxMonitorText = true;
+ }
+ LOG(WARNING) << tmp;
+ }
+ }
}
+
+ // Merge the result for the method into the global state for the class.
failure_data.Merge(result);
}
uint64_t elapsed_time_microseconds = timer.Stop();
@@ -318,25 +348,7 @@
<< ", class: " << PrettyDescriptor(dex_file->GetClassDescriptor(class_def));
GetMetrics()->ClassVerificationCount()->AddOne();
-
- if (failure_data.kind == FailureKind::kNoFailure) {
- return FailureKind::kNoFailure;
- } else {
- if ((failure_data.types & VERIFY_ERROR_LOCKING) != 0) {
- // Print a warning about expected slow-down. Use a string temporary to print one contiguous
- // warning.
- std::string tmp =
- StringPrintf("Class %s failed lock verification and will run slower.",
- PrettyDescriptor(accessor.GetDescriptor()).c_str());
- if (!gPrintedDxMonitorText) {
- tmp = tmp + "\nCommon causes for lock verification issues are non-optimized dex code\n"
- "and incorrect proguard optimizations.";
- gPrintedDxMonitorText = true;
- }
- LOG(WARNING) << tmp;
- }
- return failure_data.kind;
- }
+ return failure_data.kind;
}
void ClassVerifier::Init(ClassLinker* class_linker) {
diff --git a/runtime/verifier/class_verifier.h b/runtime/verifier/class_verifier.h
index fa9dd97..a81b0b8 100644
--- a/runtime/verifier/class_verifier.h
+++ b/runtime/verifier/class_verifier.h
@@ -49,6 +49,7 @@
namespace verifier {
+class VerifierCallback;
class VerifierDeps;
// Verifier that ensures the complete class is OK.
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 41ca523..ced62be 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -64,7 +64,6 @@
#include "stack.h"
#include "vdex_file.h"
#include "verifier/method_verifier.h"
-#include "verifier_compiler_binding.h"
#include "verifier_deps.h"
namespace art {
@@ -164,7 +163,6 @@
Handle<mirror::DexCache> dex_cache,
Handle<mirror::ClassLoader> class_loader,
const dex::ClassDef& class_def,
- ArtMethod* method,
uint32_t access_flags,
bool need_precise_constants,
bool verify_to_dump,
@@ -182,7 +180,6 @@
allow_thread_suspension,
allow_soft_failures,
aot_mode),
- method_being_verified_(method),
method_access_flags_(access_flags),
return_type_(nullptr),
dex_cache_(dex_cache),
@@ -692,12 +689,7 @@
const dex::MethodId& method_id = dex_file_->GetMethodId(dex_method_idx_);
const char* descriptor
= dex_file_->GetTypeDescriptor(dex_file_->GetTypeId(method_id.class_idx_));
- if (method_being_verified_ != nullptr) {
- ObjPtr<mirror::Class> klass = method_being_verified_->GetDeclaringClass();
- declaring_class_ = &FromClass(descriptor, klass, klass->CannotBeAssignedFromOtherTypes());
- } else {
- declaring_class_ = ®_types_.FromDescriptor(class_loader_.Get(), descriptor, false);
- }
+ declaring_class_ = ®_types_.FromDescriptor(class_loader_.Get(), descriptor, false);
}
return *declaring_class_;
}
@@ -774,7 +766,6 @@
bool HandleMoveException(const Instruction* inst) REQUIRES_SHARED(Locks::mutator_lock_);
- ArtMethod* method_being_verified_; // Its ArtMethod representation if known.
const uint32_t method_access_flags_; // Method's access flags.
const RegType* return_type_; // Lazily computed return type of the method.
// The dex_cache for the declaring class of the method.
@@ -3650,7 +3641,7 @@
DCHECK(GetInstructionFlags(*start_guess).IsOpcode());
if (flags_.have_pending_runtime_throw_failure_) {
- flags_.have_any_pending_runtime_throw_failure_ = true;
+ Fail(VERIFY_ERROR_RUNTIME_THROW, /* pending_exc= */ false);
// Reset the pending_runtime_throw flag now.
flags_.have_pending_runtime_throw_failure_ = false;
}
@@ -4922,26 +4913,11 @@
template <bool kVerifierDebug>
const RegType& MethodVerifier<kVerifierDebug>::GetMethodReturnType() {
if (return_type_ == nullptr) {
- if (method_being_verified_ != nullptr) {
- ObjPtr<mirror::Class> return_type_class = can_load_classes_
- ? method_being_verified_->ResolveReturnType()
- : method_being_verified_->LookupResolvedReturnType();
- if (return_type_class != nullptr) {
- return_type_ = &FromClass(method_being_verified_->GetReturnTypeDescriptor(),
- return_type_class,
- return_type_class->CannotBeAssignedFromOtherTypes());
- } else {
- DCHECK(!can_load_classes_ || self_->IsExceptionPending());
- self_->ClearException();
- }
- }
- if (return_type_ == nullptr) {
- const dex::MethodId& method_id = dex_file_->GetMethodId(dex_method_idx_);
- const dex::ProtoId& proto_id = dex_file_->GetMethodPrototype(method_id);
- dex::TypeIndex return_type_idx = proto_id.return_type_idx_;
- const char* descriptor = dex_file_->GetTypeDescriptor(dex_file_->GetTypeId(return_type_idx));
- return_type_ = ®_types_.FromDescriptor(class_loader_.Get(), descriptor, false);
- }
+ const dex::MethodId& method_id = dex_file_->GetMethodId(dex_method_idx_);
+ const dex::ProtoId& proto_id = dex_file_->GetMethodPrototype(method_id);
+ dex::TypeIndex return_type_idx = proto_id.return_type_idx_;
+ const char* descriptor = dex_file_->GetTypeDescriptor(dex_file_->GetTypeId(return_type_idx));
+ return_type_ = ®_types_.FromDescriptor(class_loader_.Get(), descriptor, false);
}
return *return_type_;
}
@@ -4991,7 +4967,7 @@
// This is an unreachable handler. The instruction doesn't throw, but we
// mark the method as having a pending runtime throw failure so that
// the compiler does not try to compile it.
- flags_.have_any_pending_runtime_throw_failure_ = true;
+ Fail(VERIFY_ERROR_RUNTIME_THROW, /* pending_exc= */ false);
return true;
}
// How to handle runtime failures for instructions that are not flagged kThrow.
@@ -5038,7 +5014,7 @@
class_def_(class_def),
code_item_accessor_(*dex_file, code_item),
// TODO: make it designated initialization when we compile as C++20.
- flags_({false, false, false, aot_mode}),
+ flags_({false, false, aot_mode}),
encountered_failure_types_(0),
can_load_classes_(can_load_classes),
allow_soft_failures_(allow_soft_failures),
@@ -5063,10 +5039,8 @@
Handle<mirror::ClassLoader> class_loader,
const dex::ClassDef& class_def,
const dex::CodeItem* code_item,
- ArtMethod* method,
uint32_t method_access_flags,
CompilerCallbacks* callbacks,
- VerifierCallback* verifier_callback,
bool allow_soft_failures,
HardFailLogMode log_level,
bool need_precise_constants,
@@ -5084,10 +5058,8 @@
class_loader,
class_def,
code_item,
- method,
method_access_flags,
callbacks,
- verifier_callback,
allow_soft_failures,
log_level,
need_precise_constants,
@@ -5105,10 +5077,8 @@
class_loader,
class_def,
code_item,
- method,
method_access_flags,
callbacks,
- verifier_callback,
allow_soft_failures,
log_level,
need_precise_constants,
@@ -5130,7 +5100,8 @@
verifier::VerifyError::VERIFY_ERROR_ACCESS_CLASS |
verifier::VerifyError::VERIFY_ERROR_ACCESS_FIELD |
verifier::VerifyError::VERIFY_ERROR_NO_METHOD |
- verifier::VerifyError::VERIFY_ERROR_ACCESS_METHOD;
+ verifier::VerifyError::VERIFY_ERROR_ACCESS_METHOD |
+ verifier::VerifyError::VERIFY_ERROR_RUNTIME_THROW;
return (encountered_failure_types & (~unresolved_mask)) == 0;
}
@@ -5145,10 +5116,8 @@
Handle<mirror::ClassLoader> class_loader,
const dex::ClassDef& class_def,
const dex::CodeItem* code_item,
- ArtMethod* method,
uint32_t method_access_flags,
CompilerCallbacks* callbacks,
- VerifierCallback* verifier_callback,
bool allow_soft_failures,
HardFailLogMode log_level,
bool need_precise_constants,
@@ -5172,7 +5141,6 @@
dex_cache,
class_loader,
class_def,
- method,
method_access_flags,
need_precise_constants,
/* verify to dump */ false,
@@ -5188,8 +5156,6 @@
callbacks->MethodVerified(&verifier);
}
- bool set_dont_compile = false;
- bool must_count_locks = false;
if (verifier.failures_.size() != 0) {
if (VLOG_IS_ON(verifier)) {
verifier.DumpFailures(VLOG_STREAM(verifier) << "Soft verification failures in "
@@ -5208,18 +5174,6 @@
} else {
result.kind = FailureKind::kSoftFailure;
}
- if (!CanCompilerHandleVerificationFailure(verifier.encountered_failure_types_) ||
- verifier.HasInstructionThatWillThrow()) {
- set_dont_compile = true;
- }
- if ((verifier.encountered_failure_types_ & VerifyError::VERIFY_ERROR_LOCKING) != 0) {
- must_count_locks = true;
- }
- }
-
- if (method != nullptr) {
- verifier_callback->SetDontCompile(method, set_dont_compile);
- verifier_callback->SetMustCountLocks(method, must_count_locks);
}
} else {
// Bad method data.
@@ -5311,7 +5265,6 @@
dex_cache,
class_loader,
*method->GetDeclaringClass()->GetClassDef(),
- method,
method->GetAccessFlags(),
/* need_precise_constants= */ true,
/* verify_to_dump= */ false,
@@ -5341,7 +5294,6 @@
Handle<mirror::ClassLoader> class_loader,
const dex::ClassDef& class_def,
const dex::CodeItem* code_item,
- ArtMethod* method,
uint32_t method_access_flags,
uint32_t api_level) {
impl::MethodVerifier<false>* verifier = new impl::MethodVerifier<false>(
@@ -5359,7 +5311,6 @@
dex_cache,
class_loader,
class_def,
- method,
method_access_flags,
/* need_precise_constants= */ true,
/* verify_to_dump= */ true,
@@ -5401,7 +5352,6 @@
dex_cache,
class_loader,
m->GetClassDef(),
- m,
m->GetAccessFlags(),
/* need_precise_constants= */ false,
/* verify_to_dump= */ false,
@@ -5420,7 +5370,6 @@
const dex::ClassDef& class_def,
const dex::CodeItem* code_item,
uint32_t method_idx,
- ArtMethod* method,
uint32_t access_flags,
bool can_load_classes,
bool allow_soft_failures,
@@ -5442,7 +5391,6 @@
dex_cache,
class_loader,
class_def,
- method,
access_flags,
need_precise_constants,
verify_to_dump,
@@ -5502,6 +5450,10 @@
flags_.have_pending_hard_failure_ = true;
break;
}
+
+ case VERIFY_ERROR_RUNTIME_THROW: {
+ LOG(FATAL) << "UNREACHABLE";
+ }
}
} else if (kIsDebugBuild) {
CHECK_NE(error, VERIFY_ERROR_BAD_CLASS_SOFT);
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index 1849f6b..0e59251 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -73,16 +73,6 @@
kTrackRegsAll,
};
-// A class used by the verifier to tell users about what options need to be set for given methods.
-class VerifierCallback {
- public:
- virtual ~VerifierCallback() {}
- virtual void SetDontCompile(ArtMethod* method, bool value)
- REQUIRES_SHARED(Locks::mutator_lock_) = 0;
- virtual void SetMustCountLocks(ArtMethod* method, bool value)
- REQUIRES_SHARED(Locks::mutator_lock_) = 0;
-};
-
// A mapping from a dex pc to the register line statuses as they are immediately prior to the
// execution of that instruction.
class PcToRegisterLineTable {
@@ -124,7 +114,7 @@
Handle<mirror::DexCache> dex_cache,
Handle<mirror::ClassLoader> class_loader,
const dex::ClassDef& class_def,
- const dex::CodeItem* code_item, ArtMethod* method,
+ const dex::CodeItem* code_item,
uint32_t method_access_flags,
uint32_t api_level)
REQUIRES_SHARED(Locks::mutator_lock_);
@@ -198,7 +188,7 @@
MethodReference GetMethodReference() const;
bool HasFailures() const;
bool HasInstructionThatWillThrow() const {
- return flags_.have_any_pending_runtime_throw_failure_;
+ return (encountered_failure_types_ & VERIFY_ERROR_RUNTIME_THROW) != 0;
}
virtual const RegType& ResolveCheckedClass(dex::TypeIndex class_idx)
@@ -266,10 +256,8 @@
Handle<mirror::ClassLoader> class_loader,
const dex::ClassDef& class_def_idx,
const dex::CodeItem* code_item,
- ArtMethod* method,
uint32_t method_access_flags,
CompilerCallbacks* callbacks,
- VerifierCallback* verifier_callback,
bool allow_soft_failures,
HardFailLogMode log_level,
bool need_precise_constants,
@@ -289,10 +277,8 @@
Handle<mirror::ClassLoader> class_loader,
const dex::ClassDef& class_def_idx,
const dex::CodeItem* code_item,
- ArtMethod* method,
uint32_t method_access_flags,
CompilerCallbacks* callbacks,
- VerifierCallback* verifier_callback,
bool allow_soft_failures,
HardFailLogMode log_level,
bool need_precise_constants,
@@ -314,7 +300,6 @@
const dex::ClassDef& class_def,
const dex::CodeItem* code_item,
uint32_t method_idx,
- ArtMethod* method,
uint32_t access_flags,
bool can_load_classes,
bool allow_soft_failures,
@@ -371,10 +356,6 @@
// Note: this flag is reset after processing each instruction.
bool have_pending_runtime_throw_failure_ : 1;
- // A version of the above that is not reset and thus captures if there were *any* throw
- // failures.
- bool have_any_pending_runtime_throw_failure_ : 1;
-
// Verify in AoT mode?
bool aot_mode_ : 1;
} flags_;
diff --git a/runtime/verifier/verifier_enums.h b/runtime/verifier/verifier_enums.h
index 6dc8e29..bcb5f45 100644
--- a/runtime/verifier/verifier_enums.h
+++ b/runtime/verifier/verifier_enums.h
@@ -89,6 +89,8 @@
VERIFY_ERROR_INSTANTIATION = 1 << 9, // InstantiationError.
VERIFY_ERROR_LOCKING = 1 << 10, // Could not guarantee balanced locking. This should be
// punted to the interpreter with access checks.
+ VERIFY_ERROR_RUNTIME_THROW = 1 << 11, // The interpreter found an instruction that will
+ // throw. Used for app compatibility for apps < T.
};
std::ostream& operator<<(std::ostream& os, VerifyError rhs);