ART: Relax IsInstantiable check

Relax the IsInstantiable check when converting an uninitialized
reference type. This is a double protection that leads to wrong
behavior as it may trigger an incorrect hard failure.

Bug: 22777307
Change-Id: Ie555b175f78876647fef083369d40bfe3fd7b69a

diff --git a/runtime/verifier/reg_type.cc b/runtime/verifier/reg_type.cc
index 9319cc2..7fe8bb9 100644 (file)
--- a/runtime/verifier/reg_type.cc
+++ b/runtime/verifier/reg_type.cc
@@ -302,7 +302,9 @@ void UndefinedType::Destroy() {
 PreciseReferenceType::PreciseReferenceType(mirror::Class* klass, const std::string& descriptor,
                                            uint16_t cache_id)
     : RegType(klass, descriptor, cache_id) {
-  DCHECK(klass->IsInstantiable());
+  // Note: no check for IsInstantiable() here. We may produce this in case an InstantiationError
+  //       would be thrown at runtime, but we need to continue verification and *not* create a
+  //       hard failure or abort.
 }
 
 std::string UnresolvedMergedType::Dump() const {

diff --git a/runtime/verifier/reg_type_cache.cc b/runtime/verifier/reg_type_cache.cc
index d656500..4469e64 100644 (file)
--- a/runtime/verifier/reg_type_cache.cc
+++ b/runtime/verifier/reg_type_cache.cc
@@ -427,9 +427,18 @@ const RegType& RegTypeCache::FromUninitialized(const RegType& uninit_type) {
         }
       }
       entry = new ReferenceType(klass, "", entries_.size());
-    } else if (klass->IsInstantiable()) {
+    } else if (!klass->IsPrimitive()) {
       // We're uninitialized because of allocation, look or create a precise type as allocations
       // may only create objects of that type.
+      // Note: we do not check whether the given klass is actually instantiable (besides being
+      //       primitive), that is, we allow interfaces and abstract classes here. The reasoning is
+      //       twofold:
+      //       1) The "new-instance" instruction to generate the uninitialized type will already
+      //          queue an instantiation error. This is a soft error that must be thrown at runtime,
+      //          and could potentially change if the class is resolved differently at runtime.
+      //       2) Checking whether the klass is instantiable and using conflict may produce a hard
+      //          error when the value is used, which leads to a VerifyError, which is not the
+      //          correct semantics.
       for (size_t i = primitive_count_; i < entries_.size(); i++) {
         const RegType* cur_entry = entries_[i];
         if (cur_entry->IsPreciseReference() && cur_entry->GetClass() == klass) {

diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index fd9fcaf..728ccea 100644 (file)
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -36,4 +36,5 @@ b/22411633 (2)
 b/22411633 (3)
 b/22411633 (4)
 b/22411633 (5)
+b/22777307
 Done!

diff --git a/test/800-smali/smali/b_22777307.smali b/test/800-smali/smali/b_22777307.smali
new file mode 100644 (file)
index 0000000..6de3c70
--- /dev/null
+++ b/test/800-smali/smali/b_22777307.smali
@@ -0,0 +1,18 @@
+.class public LB22777307;
+.super Ljava/lang/Object;
+
+# A static field. That way we can use the reference.
+.field private static sTest:Ljava/lang/Object;
+
+.method public static run()V
+.registers 2
+       # This is a broken new-instance. It needs to throw at runtime, though. This test is here to
+       # ensure we won't produce a VerifyError.
+       # Cloneable was chosen because it's an already existing interface.
+       new-instance v0, Ljava/lang/Cloneable;
+       invoke-direct {v0}, Ljava/lang/Cloneable;-><init>()V
+       sput-object v0, LB22777307;->sTest:Ljava/lang/Object;
+
+       return-void
+
+.end method

diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 8da2af4..438e214 100644 (file)
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -119,6 +119,8 @@ public class Main {
                 new VerifyError(), null));
         testCases.add(new TestCase("b/22411633 (5)", "B22411633_5", "run", new Object[] { false },
                 null, null));
+        testCases.add(new TestCase("b/22777307", "B22777307", "run", null, new InstantiationError(),
+                null));
     }
 
     public void runTests() {