ART: Correctly make methods preverified
Bug: 16828525
Change-Id: I66756348b2aa50e41dacca59769b6810a91c73b0
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 9d67b8c..f2c6af5 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3415,6 +3415,7 @@
// Don't attempt to re-verify if already sufficiently verified.
if (klass->IsVerified() ||
(klass->IsCompileTimeVerified() && Runtime::Current()->IsCompiler())) {
+ EnsurePreverifiedMethods(klass);
return;
}
@@ -3437,6 +3438,7 @@
// Skip verification if disabled.
if (!Runtime::Current()->IsVerificationEnabled()) {
klass->SetStatus(mirror::Class::kStatusVerified, self);
+ EnsurePreverifiedMethods(klass);
return;
}
@@ -3539,7 +3541,14 @@
// Note: we're going here during compilation and at runtime. When we set the
// kAccPreverified flag when compiling image classes, the flag is recorded
// in the image and is set when loading the image.
+ EnsurePreverifiedMethods(klass);
+ }
+}
+
+void ClassLinker::EnsurePreverifiedMethods(Handle<mirror::Class> klass) {
+ if ((klass->GetAccessFlags() & kAccPreverified) == 0) {
klass->SetPreverifiedFlagOnAllMethods();
+ klass->SetAccessFlags(klass->GetAccessFlags() | kAccPreverified);
}
}
@@ -3677,7 +3686,8 @@
}
DCHECK(klass->GetClass() != NULL);
klass->SetObjectSize(sizeof(mirror::Proxy));
- klass->SetAccessFlags(kAccClassIsProxy | kAccPublic | kAccFinal);
+ // Set the class access flags incl. preverified, so we do not try to set the flag on the methods.
+ klass->SetAccessFlags(kAccClassIsProxy | kAccPublic | kAccFinal | kAccPreverified);
klass->SetClassLoader(soa.Decode<mirror::ClassLoader*>(loader));
DCHECK_EQ(klass->GetPrimitiveType(), Primitive::kPrimNot);
klass->SetName(soa.Decode<mirror::String*>(name));
@@ -4238,6 +4248,7 @@
bool can_init_parents) {
DCHECK(c.Get() != nullptr);
if (c->IsInitialized()) {
+ EnsurePreverifiedMethods(c);
return true;
}
const bool success = InitializeClass(c, can_init_fields, can_init_parents);
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 79cbb02..aef0866 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -626,6 +626,11 @@
Handle<mirror::ArtMethod> prototype)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ // Ensures that methods have the kAccPreverified bit set. We use the kAccPreverfied bit on the
+ // class access flags to determine whether this has been done before.
+ void EnsurePreverifiedMethods(Handle<mirror::Class> c)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+
mirror::Class* LookupClassFromTableLocked(const char* descriptor,
const mirror::ClassLoader* class_loader,
size_t hash)
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index 69c281e..883c6e2 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -1092,4 +1092,65 @@
EXPECT_EQ(c->GetClassSize(), mirror::ArtMethod::ClassSize());
}
+static void CheckMethod(mirror::ArtMethod* method, bool verified)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ if (!method->IsNative() && !method->IsAbstract()) {
+ EXPECT_EQ((method->GetAccessFlags() & kAccPreverified) != 0U, verified)
+ << PrettyMethod(method, true);
+ }
+}
+
+static void CheckPreverified(mirror::Class* c, bool preverified)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ EXPECT_EQ((c->GetAccessFlags() & kAccPreverified) != 0U, preverified)
+ << "Class " << PrettyClass(c) << " not as expected";
+ for (uint32_t i = 0; i < c->NumDirectMethods(); ++i) {
+ CheckMethod(c->GetDirectMethod(i), preverified);
+ }
+ for (uint32_t i = 0; i < c->NumVirtualMethods(); ++i) {
+ CheckMethod(c->GetVirtualMethod(i), preverified);
+ }
+}
+
+TEST_F(ClassLinkerTest, Preverified_InitializedBoot) {
+ ScopedObjectAccess soa(Thread::Current());
+
+ mirror::Class* JavaLangObject = class_linker_->FindSystemClass(soa.Self(), "Ljava/lang/Object;");
+ ASSERT_TRUE(JavaLangObject != NULL);
+ EXPECT_TRUE(JavaLangObject->IsInitialized()) << "Not testing already initialized class from the "
+ "core";
+ CheckPreverified(JavaLangObject, true);
+}
+
+TEST_F(ClassLinkerTest, Preverified_UninitializedBoot) {
+ ScopedObjectAccess soa(Thread::Current());
+
+ StackHandleScope<1> hs(soa.Self());
+
+ Handle<mirror::Class> security_manager(hs.NewHandle(class_linker_->FindSystemClass(
+ soa.Self(), "Ljava/lang/SecurityManager;")));
+ EXPECT_FALSE(security_manager->IsInitialized()) << "Not testing uninitialized class from the "
+ "core";
+
+ CheckPreverified(security_manager.Get(), false);
+
+ class_linker_->EnsureInitialized(security_manager, true, true);
+ CheckPreverified(security_manager.Get(), true);
+}
+
+TEST_F(ClassLinkerTest, Preverified_App) {
+ ScopedObjectAccess soa(Thread::Current());
+
+ StackHandleScope<2> hs(soa.Self());
+ Handle<mirror::ClassLoader> class_loader(
+ hs.NewHandle(soa.Decode<mirror::ClassLoader*>(LoadDex("Statics"))));
+ Handle<mirror::Class> statics(
+ hs.NewHandle(class_linker_->FindClass(soa.Self(), "LStatics;", class_loader)));
+
+ CheckPreverified(statics.Get(), false);
+
+ class_linker_->EnsureInitialized(statics, true, true);
+ CheckPreverified(statics.Get(), true);
+}
+
} // namespace art
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index b0ff7ea..8e44471 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -724,6 +724,15 @@
klass->SetDexTypeIndex(DexFile::kDexNoIndex16); // Default to no valid type index.
}
+inline void Class::SetAccessFlags(uint32_t new_access_flags) {
+ // Called inside a transaction when setting pre-verified flag during boot image compilation.
+ if (Runtime::Current()->IsActiveTransaction()) {
+ SetField32<true>(OFFSET_OF_OBJECT_MEMBER(Class, access_flags_), new_access_flags);
+ } else {
+ SetField32<false>(OFFSET_OF_OBJECT_MEMBER(Class, access_flags_), new_access_flags);
+ }
+}
+
} // namespace mirror
} // namespace art
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 13d0c80..43ac98d 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -217,10 +217,7 @@
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
uint32_t GetAccessFlags() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
- void SetAccessFlags(uint32_t new_access_flags) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
- // Not called within a transaction.
- SetField32<false>(OFFSET_OF_OBJECT_MEMBER(Class, access_flags_), new_access_flags);
- }
+ void SetAccessFlags(uint32_t new_access_flags) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
// Returns true if the class is an interface.
bool IsInterface() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
diff --git a/runtime/modifiers.h b/runtime/modifiers.h
index e599b03..2814ed8 100644
--- a/runtime/modifiers.h
+++ b/runtime/modifiers.h
@@ -45,7 +45,7 @@
static const uint32_t kAccConstructor = 0x00010000; // method (dex only) <init> and <clinit>
static const uint32_t kAccDeclaredSynchronized = 0x00020000; // method (dex only)
static const uint32_t kAccClassIsProxy = 0x00040000; // class (dex only)
-static const uint32_t kAccPreverified = 0x00080000; // method (dex only)
+static const uint32_t kAccPreverified = 0x00080000; // class (runtime), method (dex only)
static const uint32_t kAccFastNative = 0x0080000; // method (dex only)
static const uint32_t kAccPortableCompiled = 0x0100000; // method (dex only)
