Clean up ClassLinker::VerifyClass.
And drop kAccWasVerificationAttempted. It's not needed and duplicates
logic already present in the compiler and the class status.
This CL also enables nterp running methods with soft failures.
Test: test.py
Bug: 28313047
Change-Id: I853a6f00b9e0c38091d86fcd77167c92ff5b383c
diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc
index dabf82c..35c8dc8 100644
--- a/dex2oat/driver/compiler_driver.cc
+++ b/dex2oat/driver/compiler_driver.cc
@@ -1659,9 +1659,6 @@
if (&cls->GetDexFile() == &accessor.GetDexFile()) {
ObjectLock<mirror::Class> lock(self, cls);
mirror::Class::SetStatus(cls, status, self);
- if (status >= ClassStatus::kVerified) {
- cls->SetVerificationAttempted();
- }
}
} else {
DCHECK(self->IsExceptionPending());
@@ -1992,7 +1989,6 @@
InstructionSet instruction_set =
manager_->GetCompiler()->GetCompilerOptions().GetInstructionSet();
klass->SetSkipAccessChecksFlagOnAllMethods(GetInstructionSetPointerSize(instruction_set));
- klass->SetVerificationAttempted();
}
// Record the final class status if necessary.
ClassReference ref(manager_->GetDexFile(), class_def_index);
diff --git a/libdexfile/dex/modifiers.h b/libdexfile/dex/modifiers.h
index 3c745b2..f9cd8c8 100644
--- a/libdexfile/dex/modifiers.h
+++ b/libdexfile/dex/modifiers.h
@@ -52,8 +52,6 @@
static constexpr uint32_t kAccObsoleteMethod = 0x00040000; // method (runtime)
// Used by a method to denote that its execution does not need to go through slow path interpreter.
static constexpr uint32_t kAccSkipAccessChecks = 0x00080000; // method (runtime, not native)
-// Used by a class to denote that the verifier has attempted to check it at least once.
-static constexpr uint32_t kAccVerificationAttempted = 0x00080000; // class (runtime)
static constexpr uint32_t kAccSkipHiddenapiChecks = 0x00100000; // class (runtime)
// Used by a class to denote that this class and any objects with this as a
// declaring-class/super-class are to be considered obsolete, meaning they should not be used by.
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index 0289a7b..b48debc 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -2084,10 +2084,6 @@
<< "Attempting to redefine an unresolved class " << old_class->PrettyClass()
<< " status=" << old_class->GetStatus();
CHECK(linked_class->IsResolved());
- if (old_class->WasVerificationAttempted()) {
- // Match verification-attempted flag
- linked_class->SetVerificationAttempted();
- }
if (old_class->ShouldSkipHiddenApiChecks()) {
// Match skip hiddenapi flag
linked_class->SetSkipHiddenApiChecks();
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 719f0ca..9d9b94a 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -217,22 +217,21 @@
}
}
-// Ensures that methods have the kAccSkipAccessChecks bit set. We use the
-// kAccVerificationAttempted bit on the class access flags to determine whether this has been done
-// before.
-static void EnsureSkipAccessChecksMethods(Handle<mirror::Class> klass, PointerSize pointer_size)
+static void UpdateClassAfterVerification(Handle<mirror::Class> klass,
+ PointerSize pointer_size,
+ verifier::FailureKind failure_kind)
REQUIRES_SHARED(Locks::mutator_lock_) {
Runtime* runtime = Runtime::Current();
ClassLinker* class_linker = runtime->GetClassLinker();
- if (!klass->WasVerificationAttempted()) {
+ if (failure_kind == verifier::FailureKind::kNoFailure) {
klass->SetSkipAccessChecksFlagOnAllMethods(pointer_size);
- klass->SetVerificationAttempted();
- // Now that the class has passed verification, try to set nterp entrypoints
- // to methods that currently use the switch interpreter.
- if (interpreter::CanRuntimeUseNterp()) {
- for (ArtMethod& m : klass->GetMethods(pointer_size)) {
- ChangeInterpreterBridgeToNterp(&m, class_linker);
- }
+ }
+
+ // Now that the class has passed verification, try to set nterp entrypoints
+ // to methods that currently use the switch interpreter.
+ if (interpreter::CanRuntimeUseNterp()) {
+ for (ArtMethod& m : klass->GetMethods(pointer_size)) {
+ ChangeInterpreterBridgeToNterp(&m, class_linker);
}
}
}
@@ -2439,9 +2438,6 @@
array_class->PopulateEmbeddedVTable(image_pointer_size_);
ImTable* object_imt = java_lang_Object->GetImt(image_pointer_size_);
array_class->SetImt(object_imt, image_pointer_size_);
- // Skip EnsureSkipAccessChecksMethods(). We can skip the verified status,
- // the kAccVerificationAttempted flag is added below, and there are no
- // methods that need the kAccSkipAccessChecks flag.
DCHECK_EQ(array_class->NumMethods(), 0u);
// don't need to set new_class->SetObjectSize(..)
@@ -2467,8 +2463,6 @@
// and remove "interface".
access_flags |= kAccAbstract | kAccFinal;
access_flags &= ~kAccInterface;
- // Arrays are access-checks-clean and preverified.
- access_flags |= kAccVerificationAttempted;
array_class->SetAccessFlagsDuringLinking(access_flags);
@@ -4131,13 +4125,9 @@
CHECK(primitive_class != nullptr) << "OOM for primitive class " << type;
// Do not hold lock on the primitive class object, the initialization of
// primitive classes is done while the process is still single threaded.
- primitive_class->SetAccessFlagsDuringLinking(
- kAccPublic | kAccFinal | kAccAbstract | kAccVerificationAttempted);
+ primitive_class->SetAccessFlagsDuringLinking(kAccPublic | kAccFinal | kAccAbstract);
primitive_class->SetPrimitiveType(type);
primitive_class->SetIfTable(GetClassRoot<mirror::Object>(this)->GetIfTable());
- // Skip EnsureSkipAccessChecksMethods(). We can skip the verified status,
- // the kAccVerificationAttempted flag was added above, and there are no
- // methods that need the kAccSkipAccessChecks flag.
DCHECK_EQ(primitive_class->NumMethods(), 0u);
// Primitive classes are initialized during single threaded startup, so visibly initialized.
primitive_class->SetStatusForPrimitiveOrArray(ClassStatus::kVisiblyInitialized);
@@ -4531,7 +4521,6 @@
// Don't attempt to re-verify if already verified.
if (klass->IsVerified()) {
- EnsureSkipAccessChecksMethods(klass, image_pointer_size_);
if (verifier_deps != nullptr &&
verifier_deps->ContainsDexFile(klass->GetDexFile()) &&
!verifier_deps->HasRecordedVerifiedStatus(klass->GetDexFile(), *klass->GetClassDef()) &&
@@ -4554,8 +4543,7 @@
if (klass->IsVerifiedNeedsAccessChecks()) {
if (!Runtime::Current()->IsAotCompiler()) {
// Mark the class as having a verification attempt to avoid re-running
- // the verifier and avoid calling EnsureSkipAccessChecksMethods.
- klass->SetVerificationAttempted();
+ // the verifier.
mirror::Class::SetStatus(klass, ClassStatus::kVerified, self);
}
return verifier::FailureKind::kAccessChecksFailure;
@@ -4574,7 +4562,7 @@
// Skip verification if disabled.
if (!Runtime::Current()->IsVerificationEnabled()) {
mirror::Class::SetStatus(klass, ClassStatus::kVerified, self);
- EnsureSkipAccessChecksMethods(klass, image_pointer_size_);
+ UpdateClassAfterVerification(klass, image_pointer_size_, verifier::FailureKind::kNoFailure);
return verifier::FailureKind::kNoFailure;
}
}
@@ -4655,86 +4643,52 @@
verifier::FailureKind verifier_failure = verifier::FailureKind::kNoFailure;
if (!preverified) {
verifier_failure = PerformClassVerification(self, verifier_deps, klass, log_level, &error_msg);
+ } else if (oat_file_class_status == ClassStatus::kVerifiedNeedsAccessChecks) {
+ verifier_failure = verifier::FailureKind::kAccessChecksFailure;
}
// Verification is done, grab the lock again.
ObjectLock<mirror::Class> lock(self, klass);
+ self->AssertNoPendingException();
- if (preverified || verifier_failure != verifier::FailureKind::kHardFailure) {
- if (!preverified && verifier_failure != verifier::FailureKind::kNoFailure) {
- VLOG(class_linker) << "Soft verification failure in class "
- << klass->PrettyDescriptor()
- << " in " << klass->GetDexCache()->GetLocation()->ToModifiedUtf8()
- << " because: " << error_msg;
- }
- self->AssertNoPendingException();
- // Make sure all classes referenced by catch blocks are resolved.
- ResolveClassExceptionHandlerTypes(klass);
- if (verifier_failure == verifier::FailureKind::kNoFailure) {
- // Even though there were no verifier failures we need to respect whether the super-class and
- // super-default-interfaces were verified or requiring runtime reverification.
- if (supertype == nullptr
- || supertype->IsVerified()
- || supertype->IsVerifiedNeedsAccessChecks()) {
- mirror::Class::SetStatus(klass, ClassStatus::kVerified, self);
- } else {
- CHECK(Runtime::Current()->IsAotCompiler());
- CHECK_EQ(supertype->GetStatus(), ClassStatus::kRetryVerificationAtRuntime);
- mirror::Class::SetStatus(klass, ClassStatus::kRetryVerificationAtRuntime, self);
- // Pretend a soft failure occurred so that we don't consider the class verified below.
- verifier_failure = verifier::FailureKind::kSoftFailure;
- }
- } else {
- CHECK(verifier_failure == verifier::FailureKind::kSoftFailure ||
- verifier_failure == verifier::FailureKind::kTypeChecksFailure ||
- verifier_failure == verifier::FailureKind::kAccessChecksFailure);
- // Soft failures at compile time should be retried at runtime. Soft
- // failures at runtime will be handled by slow paths in the generated
- // code. Set status accordingly.
- if (Runtime::Current()->IsAotCompiler()) {
- if (verifier_failure == verifier::FailureKind::kSoftFailure ||
- verifier_failure == verifier::FailureKind::kTypeChecksFailure) {
- mirror::Class::SetStatus(klass, ClassStatus::kRetryVerificationAtRuntime, self);
- } else {
- mirror::Class::SetStatus(klass, ClassStatus::kVerifiedNeedsAccessChecks, self);
- }
- } else {
- mirror::Class::SetStatus(klass, ClassStatus::kVerified, self);
- // As this is a fake verified status, make sure the methods are _not_ marked
- // kAccSkipAccessChecks later.
- klass->SetVerificationAttempted();
- }
- }
- } else {
+ if (verifier_failure == verifier::FailureKind::kHardFailure) {
VLOG(verifier) << "Verification failed on class " << klass->PrettyDescriptor()
<< " in " << klass->GetDexCache()->GetLocation()->ToModifiedUtf8()
<< " because: " << error_msg;
- self->AssertNoPendingException();
ThrowVerifyError(klass.Get(), "%s", error_msg.c_str());
mirror::Class::SetStatus(klass, ClassStatus::kErrorResolved, self);
+ return verifier_failure;
}
- if (preverified || verifier_failure == verifier::FailureKind::kNoFailure) {
- if (oat_file_class_status == ClassStatus::kVerifiedNeedsAccessChecks ||
- UNLIKELY(Runtime::Current()->IsVerificationSoftFail())) {
- // Never skip access checks if the verification soft fail is forced.
- // Mark the class as having a verification attempt to avoid re-running the verifier.
- klass->SetVerificationAttempted();
- } else {
- // Class is verified so we don't need to do any access check on its methods.
- // Let the interpreter know it by setting the kAccSkipAccessChecks flag onto each
- // method.
- // Note: we're going here during compilation and at runtime. When we set the
- // kAccSkipAccessChecks flag when compiling image classes, the flag is recorded
- // in the image and is set when loading the image.
- EnsureSkipAccessChecksMethods(klass, image_pointer_size_);
- }
- }
- // Done verifying. Notify the compiler about the verification status, in case the class
- // was verified implicitly (eg super class of a compiled class).
+
+ // Make sure all classes referenced by catch blocks are resolved.
+ ResolveClassExceptionHandlerTypes(klass);
+
if (Runtime::Current()->IsAotCompiler()) {
+ if (supertype != nullptr && supertype->ShouldVerifyAtRuntime()) {
+ // Regardless of our own verification result, we need to verify the class
+ // at runtime if the super class is not verified. This is required in case
+ // we generate an app/boot image.
+ verifier_failure = verifier::FailureKind::kSoftFailure;
+ mirror::Class::SetStatus(klass, ClassStatus::kRetryVerificationAtRuntime, self);
+ } else if (verifier_failure == verifier::FailureKind::kNoFailure) {
+ mirror::Class::SetStatus(klass, ClassStatus::kVerified, self);
+ } else if (verifier_failure == verifier::FailureKind::kSoftFailure ||
+ verifier_failure == verifier::FailureKind::kTypeChecksFailure) {
+ mirror::Class::SetStatus(klass, ClassStatus::kRetryVerificationAtRuntime, self);
+ } else {
+ mirror::Class::SetStatus(klass, ClassStatus::kVerifiedNeedsAccessChecks, self);
+ }
+ // Notify the compiler about the verification status, in case the class
+ // was verified implicitly (eg super class of a compiled class). When the
+ // compiler unloads dex file after compilation, we still want to keep
+ // verification states.
Runtime::Current()->GetCompilerCallbacks()->UpdateClassState(
ClassReference(&klass->GetDexFile(), klass->GetDexClassDefIndex()), klass->GetStatus());
+ } else {
+ mirror::Class::SetStatus(klass, ClassStatus::kVerified, self);
}
+
+ UpdateClassAfterVerification(klass, image_pointer_size_, verifier_failure);
return verifier_failure;
}
@@ -4896,8 +4850,7 @@
temp_klass->SetObjectSize(sizeof(mirror::Proxy));
// Set the class access flags incl. VerificationAttempted, so we do not try to set the flag on
// the methods.
- temp_klass->SetAccessFlagsDuringLinking(
- kAccClassIsProxy | kAccPublic | kAccFinal | kAccVerificationAttempted);
+ temp_klass->SetAccessFlagsDuringLinking(kAccClassIsProxy | kAccPublic | kAccFinal);
temp_klass->SetClassLoader(soa.Decode<mirror::ClassLoader>(loader));
DCHECK_EQ(temp_klass->GetPrimitiveType(), Primitive::kPrimNot);
temp_klass->SetName(soa.Decode<mirror::String>(name));
@@ -5063,7 +5016,6 @@
{
// Lock on klass is released. Lock new class object.
ObjectLock<mirror::Class> initialization_lock(self, klass);
- EnsureSkipAccessChecksMethods(klass, image_pointer_size_);
// Conservatively go through the ClassStatus::kInitialized state.
callback = MarkClassInitialized(self, klass);
}
@@ -5832,7 +5784,6 @@
MakeInitializedClassesVisiblyInitialized(self, /*wait=*/ false);
}
}
- DCHECK(c->WasVerificationAttempted()) << c->PrettyClassAndClassLoader();
return true;
}
// SubtypeCheckInfo::Initialized must happen-before any new-instance for that type.
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index d1f3a17..e0f498d 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -116,8 +116,7 @@
EXPECT_EQ(0, primitive->GetIfTableCount());
EXPECT_TRUE(primitive->GetIfTable() != nullptr);
EXPECT_EQ(primitive->GetIfTable()->Count(), 0u);
- EXPECT_EQ(kAccPublic | kAccFinal | kAccAbstract | kAccVerificationAttempted,
- primitive->GetAccessFlags());
+ EXPECT_EQ(kAccPublic | kAccFinal | kAccAbstract, primitive->GetAccessFlags());
}
void AssertObjectClass(ObjPtr<mirror::Class> JavaLangObject)
@@ -1446,8 +1445,6 @@
static void CheckVerificationAttempted(ObjPtr<mirror::Class> c, bool preverified)
REQUIRES_SHARED(Locks::mutator_lock_) {
- EXPECT_EQ((c->GetAccessFlags() & kAccVerificationAttempted) != 0U, preverified)
- << "Class " << mirror::Class::PrettyClass(c) << " not as expected";
for (auto& m : c->GetMethods(kRuntimePointerSize)) {
CheckMethod(&m, preverified);
}
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 6a5317c..d7dfe0d 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -937,9 +937,6 @@
}
inline void Class::SetAccessFlags(uint32_t new_access_flags) {
- if (kIsDebugBuild) {
- SetAccessFlagsDCheck(new_access_flags);
- }
// Called inside a transaction when setting pre-verified flag during boot image compilation.
if (Runtime::Current()->IsActiveTransaction()) {
SetField32<true>(AccessFlagsOffset(), new_access_flags);
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index 4273209..3199db3 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -252,10 +252,6 @@
}
}
- if (kIsDebugBuild && new_status >= ClassStatus::kInitialized) {
- CHECK(h_this->WasVerificationAttempted()) << h_this->PrettyClassAndClassLoader();
- }
-
if (!class_linker_initialized) {
// When the class linker is being initialized its single threaded and by definition there can be
// no waiters. During initialization classes may appear temporary but won't be retired as their
@@ -307,10 +303,6 @@
// Do not update `object_alloc_fast_path_`. Arrays are variable size and
// instances of primitive classes cannot be created at all.
- if (kIsDebugBuild && new_status >= ClassStatus::kInitialized) {
- CHECK(WasVerificationAttempted()) << PrettyClassAndClassLoader();
- }
-
// There can be no waiters to notify as these classes are initialized
// before another thread can see them.
}
@@ -1932,13 +1924,6 @@
template void Class::GetAccessFlagsDCheck<kVerifyWrites>();
template void Class::GetAccessFlagsDCheck<kVerifyAll>();
-void Class::SetAccessFlagsDCheck(uint32_t new_access_flags) {
- uint32_t old_access_flags = GetField32<kVerifyNone>(AccessFlagsOffset());
- // kAccVerificationAttempted is retained.
- CHECK((old_access_flags & kAccVerificationAttempted) == 0 ||
- (new_access_flags & kAccVerificationAttempted) != 0);
-}
-
ObjPtr<Object> Class::GetMethodIds() {
ObjPtr<ClassExt> ext(GetExtData());
if (ext.IsNull()) {
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 131dc77..9a97fcf 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -321,22 +321,6 @@
return (GetAccessFlags() & kAccSynthetic) != 0;
}
- // Return whether the class had run the verifier at least once.
- // This does not necessarily mean that access checks are avoidable,
- // since the class methods might still need to be run with access checks.
- bool WasVerificationAttempted() REQUIRES_SHARED(Locks::mutator_lock_) {
- return (GetAccessFlags() & kAccVerificationAttempted) != 0;
- }
-
- // Mark the class as having gone through a verification attempt.
- // Mutually exclusive from whether or not each method is allowed to skip access checks.
- void SetVerificationAttempted() REQUIRES_SHARED(Locks::mutator_lock_) {
- uint32_t flags = GetField32(OFFSET_OF_OBJECT_MEMBER(Class, access_flags_));
- if ((flags & kAccVerificationAttempted) == 0) {
- SetAccessFlags(flags | kAccVerificationAttempted);
- }
- }
-
bool IsObsoleteObject() REQUIRES_SHARED(Locks::mutator_lock_) {
return (GetAccessFlags() & kAccObsoleteObject) != 0;
}
@@ -1394,8 +1378,6 @@
template<VerifyObjectFlags kVerifyFlags>
void GetAccessFlagsDCheck() REQUIRES_SHARED(Locks::mutator_lock_);
- void SetAccessFlagsDCheck(uint32_t new_access_flags) REQUIRES_SHARED(Locks::mutator_lock_);
-
// Check that the pointer size matches the one in the class linker.
ALWAYS_INLINE static void CheckPointerSize(PointerSize pointer_size);
diff --git a/runtime/nterp_helpers.cc b/runtime/nterp_helpers.cc
index a476412..12afa3a 100644
--- a/runtime/nterp_helpers.cc
+++ b/runtime/nterp_helpers.cc
@@ -208,8 +208,6 @@
bool CanMethodUseNterp(ArtMethod* method, InstructionSet isa) {
return !method->IsNative() &&
method->IsInvokable() &&
- // Nterp supports the same methods the compiler supports.
- method->IsCompilable() &&
!method->MustCountLocks() &&
// Proxy methods do not go through the JIT like other methods, so we don't
// run them with nterp.
diff --git a/test/1980-obsolete-object-cleared/expected-stdout.txt b/test/1980-obsolete-object-cleared/expected-stdout.txt
index 7f0c981..9d18b2c 100644
--- a/test/1980-obsolete-object-cleared/expected-stdout.txt
+++ b/test/1980-obsolete-object-cleared/expected-stdout.txt
@@ -32,7 +32,7 @@
Calling public boolean java.lang.Class.desiredAssertionStatus() with params: []
public boolean java.lang.Class.desiredAssertionStatus() on (obsolete)class Main$Transform with [] = false
Calling public int java.lang.Class.getAccessFlags() with params: []
-public int java.lang.Class.getAccessFlags() on (obsolete)class Main$Transform with [] = 2621441
+public int java.lang.Class.getAccessFlags() on (obsolete)class Main$Transform with [] = 2097153
Calling public java.lang.annotation.Annotation java.lang.Class.getAnnotation(java.lang.Class) with params: [[null, class java.lang.Object, (obsolete)class Main$Transform, class Main$Transform, long, class java.lang.Class]]
public java.lang.annotation.Annotation java.lang.Class.getAnnotation(java.lang.Class) with [null] throws java.lang.reflect.InvocationTargetException: java.lang.NullPointerException
public java.lang.annotation.Annotation java.lang.Class.getAnnotation(java.lang.Class) with [class java.lang.Object] throws java.lang.reflect.InvocationTargetException: java.lang.RuntimeException: Obsolete Object!
@@ -245,7 +245,7 @@
Calling public boolean java.lang.Class.desiredAssertionStatus() with params: []
public boolean java.lang.Class.desiredAssertionStatus() on class Main$Transform with [] = false
Calling public int java.lang.Class.getAccessFlags() with params: []
-public int java.lang.Class.getAccessFlags() on class Main$Transform with [] = 524289
+public int java.lang.Class.getAccessFlags() on class Main$Transform with [] = 1
Calling public java.lang.annotation.Annotation java.lang.Class.getAnnotation(java.lang.Class) with params: [[null, class java.lang.Object, (obsolete)class Main$Transform, class Main$Transform, long, class java.lang.Class]]
public java.lang.annotation.Annotation java.lang.Class.getAnnotation(java.lang.Class) with [null] throws java.lang.reflect.InvocationTargetException: java.lang.NullPointerException
public java.lang.annotation.Annotation java.lang.Class.getAnnotation(java.lang.Class) on class Main$Transform with [class java.lang.Object] = null
diff --git a/test/626-const-class-linking/clear_dex_cache_types.cc b/test/626-const-class-linking/clear_dex_cache_types.cc
index 2921d89..1aa3cce 100644
--- a/test/626-const-class-linking/clear_dex_cache_types.cc
+++ b/test/626-const-class-linking/clear_dex_cache_types.cc
@@ -41,7 +41,6 @@
if (status == ClassStatus::kResolved) {
ObjectLock<mirror::Class> lock(soa.Self(), klass);
klass->SetStatus(klass, ClassStatus::kVerified, soa.Self());
- klass->SetVerificationAttempted();
} else {
LOG(ERROR) << klass->PrettyClass() << " has unexpected status: " << status;
}