Merge "ART: Correctly make methods preverified"

6 files changed, 89 insertions, 6 deletions

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 4342234..42e0899 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3532,6 +3532,7 @@ void ClassLinker::VerifyClass(ConstHandle<mirror::Class> klass) {
   // Don't attempt to re-verify if already sufficiently verified.
   if (klass->IsVerified() ||
       (klass->IsCompileTimeVerified() && Runtime::Current()->IsCompiler())) {
+    EnsurePreverifiedMethods(klass);
     return;
   }
 
@@ -3554,6 +3555,7 @@ void ClassLinker::VerifyClass(ConstHandle<mirror::Class> klass) {
   // Skip verification if disabled.
   if (!Runtime::Current()->IsVerificationEnabled()) {
     klass->SetStatus(mirror::Class::kStatusVerified, self);
+    EnsurePreverifiedMethods(klass);
     return;
   }
 
@@ -3656,7 +3658,14 @@ void ClassLinker::VerifyClass(ConstHandle<mirror::Class> klass) {
     // 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(ConstHandle<mirror::Class> klass) {
+  if ((klass->GetAccessFlags() & kAccPreverified) == 0) {
     klass->SetPreverifiedFlagOnAllMethods();
+    klass->SetAccessFlags(klass->GetAccessFlags() | kAccPreverified);
   }
 }
 
@@ -3790,7 +3799,8 @@ mirror::Class* ClassLinker::CreateProxyClass(ScopedObjectAccessAlreadyRunnable&
   }
   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));
@@ -4356,6 +4366,7 @@ bool ClassLinker::EnsureInitialized(ConstHandle<mirror::Class> c, bool can_init_
                                     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 158816d..7afa6a1 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -632,6 +632,11 @@ class ClassLinker {
                                        ConstHandle<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(ConstHandle<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 b250918..ee5fbb7 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -1091,4 +1091,65 @@ TEST_F(ClassLinkerTest, ValidatePredefinedClassSizes) {
   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 726e928..3f67468 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -741,6 +741,15 @@ inline void Class::InitializeClassVisitor::operator()(
   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 cf9501a..cff8236 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -219,10 +219,7 @@ class MANAGED Class FINAL : public Object {
   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 kAccJavaFlagsMask = 0xffff;  // bits set from Java sources
 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)