ART: Fix UninitializedReference handling

The merge rules in the verifier allowed Object to be successfully
merged with uninitialized references. This is invalid and should
result in a conflict. Fix by moving UninitializedReference rules
earlier.

Also add a test that forward merging is correctly allowed, both
with a valid result as well as a conflict.

Also add tests that backwards branches have the expected behavior.

Bug: 22411633
Change-Id: If837376c15f0b3550d6ce1721a3cde5901c80c7f

diff --git a/runtime/verifier/reg_type.cc b/runtime/verifier/reg_type.cc
index 9c52819..6e23234 100644 (file)
--- a/runtime/verifier/reg_type.cc
+++ b/runtime/verifier/reg_type.cc
@@ -690,6 +690,11 @@ const RegType& RegType::Merge(const RegType& incoming_type, RegTypeCache* reg_ty
   } else if (IsReferenceTypes() && incoming_type.IsReferenceTypes()) {
     if (IsZero() || incoming_type.IsZero()) {
       return SelectNonConstant(*this, incoming_type);  // 0 MERGE ref => ref
+    } else if (IsUninitializedTypes() || incoming_type.IsUninitializedTypes()) {
+      // Something that is uninitialized hasn't had its constructor called. Unitialized types are
+      // special. They may only ever be merged with themselves (must be taken care of by the
+      // caller of Merge(), see the DCHECK on entry). So mark any other merge as conflicting here.
+      return conflict;
     } else if (IsJavaLangObject() || incoming_type.IsJavaLangObject()) {
       return reg_types->JavaLangObject(false);  // Object MERGE ref => Object
     } else if (IsUnresolvedTypes() || incoming_type.IsUnresolvedTypes()) {
@@ -698,11 +703,6 @@ const RegType& RegType::Merge(const RegType& incoming_type, RegTypeCache* reg_ty
       // type that reflects our lack of knowledge and that allows the rest of the unresolved
       // mechanics to continue.
       return reg_types->FromUnresolvedMerge(*this, incoming_type);
-    } else if (IsUninitializedTypes() || incoming_type.IsUninitializedTypes()) {
-      // Something that is uninitialized hasn't had its constructor called. Mark any merge
-      // of this type with something that is initialized as conflicting. The cases of a merge
-      // with itself, 0 or Object are handled above.
-      return conflict;
     } else {  // Two reference types, compute Join
       mirror::Class* c1 = GetClass();
       mirror::Class* c2 = incoming_type.GetClass();

diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 4c17240..fd9fcaf 100644 (file)
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -31,4 +31,9 @@ b/22244733
 b/22331663
 b/22331663 (pass)
 b/22331663 (fail)
+b/22411633 (1)
+b/22411633 (2)
+b/22411633 (3)
+b/22411633 (4)
+b/22411633 (5)
 Done!

diff --git a/test/800-smali/smali/b_22411633_1.smali b/test/800-smali/smali/b_22411633_1.smali
new file mode 100644 (file)
index 0000000..ffc82a8
--- /dev/null
+++ b/test/800-smali/smali/b_22411633_1.smali
@@ -0,0 +1,35 @@
+.class public LB22411633_1;
+.super Ljava/lang/Object;
+
+
+.method public static run(Z)V
+.registers 6
+       # Make v3 & v4 defined, just use null.
+       const v3, 0
+       const v4, 0
+
+       # Allocate a java.lang.Object (do not initialize).
+       new-instance v4, Ljava/lang/Object;
+
+       # Branch forward.
+       if-eqz v5, :LabelMerge
+
+       # Just some random work.
+       add-int/lit16 v3, v3, 1
+
+       # Another branch forward.
+       if-nez v5, :LabelMerge
+
+       # Some more random work, technically dead, but reachable.
+       add-int/lit16 v3, v3, 1
+
+:LabelMerge
+       # v4 is still an uninitialized reference here. Initialize it.
+       invoke-direct {v4}, Ljava/lang/Object;-><init>()V
+
+       # And test whether it's initialized by calling hashCode.
+       invoke-virtual {v4}, Ljava/lang/Object;->hashCode()I
+
+       return-void
+
+.end method

diff --git a/test/800-smali/smali/b_22411633_2.smali b/test/800-smali/smali/b_22411633_2.smali
new file mode 100644 (file)
index 0000000..9f27c4c
--- /dev/null
+++ b/test/800-smali/smali/b_22411633_2.smali
@@ -0,0 +1,45 @@
+.class public LB22411633_2;
+.super Ljava/lang/Object;
+
+
+.method public static run(Z)V
+.registers 6
+       # Make v3 & v4 defined, just use null.
+       const v3, 0
+       const v4, 0
+
+       # Allocate a java.lang.Object (do not initialize).
+       new-instance v4, Ljava/lang/Object;
+
+       # Branch forward.
+       if-eqz v5, :LabelMerge
+
+       # Create a non-precise object reference. We can do this by merging to objects together
+       # that only have Object as a common ancestor.
+
+       # Allocate a java.lang.Object and initialize it.
+       new-instance v4, Ljava/lang/Object;
+       invoke-direct {v4}, Ljava/lang/Object;-><init>()V
+
+       if-nez v5, :LabelMergeObject
+
+       new-instance v4, Ljava/lang/Integer;
+       invoke-direct {v4}, Ljava/lang/Integer;-><init>()V
+
+:LabelMergeObject
+
+       # Dummy work to separate blocks. At this point, v4 is of type Reference<Object>.
+       add-int/lit16 v3, v3, 1
+
+:LabelMerge
+       # Merge the uninitialized Object from line 12 with the reference to Object from 31. Older
+       # rules set any reference merged with Object to Object. This is wrong in the case of the
+       # other reference being an uninitialized reference, as we'd suddenly allow calling on it.
+
+       # Test whether it's some initialized reference by calling hashCode. This should fail, as we
+       # merged initialized and uninitialized.
+       invoke-virtual {v4}, Ljava/lang/Object;->hashCode()I
+
+       return-void
+
+.end method

diff --git a/test/800-smali/smali/b_22411633_3.smali b/test/800-smali/smali/b_22411633_3.smali
new file mode 100644 (file)
index 0000000..d1212f1
--- /dev/null
+++ b/test/800-smali/smali/b_22411633_3.smali
@@ -0,0 +1,31 @@
+.class public LB22411633_3;
+.super Ljava/lang/Object;
+
+
+.method public static run(Z)V
+.registers 6
+       # Make v3 & v4 defined, just use null.
+       const v3, 0
+       const v4, 0
+
+       # Allocate a java.lang.Object (do not initialize).
+       new-instance v4, Ljava/lang/Object;
+
+       # Branch forward.
+       if-eqz v5, :LabelMerge
+
+       # Create an initialized Object.
+       new-instance v4, Ljava/lang/Object;
+       invoke-direct {v4}, Ljava/lang/Object;-><init>()V
+
+       # Just some random work.
+       add-int/lit16 v3, v3, 1
+
+:LabelMerge
+       # At this point, an initialized and an uninitialized reference are merged. However, the
+       # merge is only from forward branches. If the conflict isn't used (as here), this should
+       # pass the verifier.
+
+       return-void
+
+.end method

diff --git a/test/800-smali/smali/b_22411633_4.smali b/test/800-smali/smali/b_22411633_4.smali
new file mode 100644 (file)
index 0000000..503ca99
--- /dev/null
+++ b/test/800-smali/smali/b_22411633_4.smali
@@ -0,0 +1,25 @@
+.class public LB22411633_4;
+.super Ljava/lang/Object;
+
+
+.method public static run(Z)V
+.registers 6
+       # Do not merge into the backward branch target.
+       goto :LabelEntry
+
+:LabelBwd
+       # At this point v4 is an uninitialized reference. This should fail to verify.
+       # Note: we make sure that it is an uninitialized reference and not a conflict in sister
+       #       file b_22411633_bwdok.smali.
+       invoke-virtual {v4}, Ljava/lang/Object;->hashCode()I
+
+:LabelEntry
+       # Allocate a java.lang.Object (do not initialize).
+       new-instance v4, Ljava/lang/Object;
+
+       # Branch backward.
+       if-eqz v5, :LabelBwd
+
+       return-void
+
+.end method

diff --git a/test/800-smali/smali/b_22411633_5.smali b/test/800-smali/smali/b_22411633_5.smali
new file mode 100644 (file)
index 0000000..b7964f6
--- /dev/null
+++ b/test/800-smali/smali/b_22411633_5.smali
@@ -0,0 +1,28 @@
+.class public LB22411633_5;
+.super Ljava/lang/Object;
+
+
+.method public static run(Z)V
+.registers 6
+       # Do not merge into the backward branch target.
+       goto :LabelEntry
+
+:LabelBwd
+       # At this point v4 is an uninitialized reference. We should be able to initialize here
+       # and call a method afterwards.
+       invoke-direct {v4}, Ljava/lang/Object;-><init>()V
+       invoke-virtual {v4}, Ljava/lang/Object;->hashCode()I
+
+       # Make sure this is not an infinite loop.
+       const v5, 1
+
+:LabelEntry
+       # Allocate a java.lang.Object (do not initialize).
+       new-instance v4, Ljava/lang/Object;
+
+       # Branch backward.
+       if-eqz v5, :LabelBwd
+
+       return-void
+
+.end method

diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 8be6418..8da2af4 100644 (file)
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -109,6 +109,16 @@ public class Main {
                 new Object[] { false }, null, null));
         testCases.add(new TestCase("b/22331663 (fail)", "B22331663Fail", "run",
                 new Object[] { false }, new VerifyError(), null));
+        testCases.add(new TestCase("b/22411633 (1)", "B22411633_1", "run", new Object[] { false },
+                null, null));
+        testCases.add(new TestCase("b/22411633 (2)", "B22411633_2", "run", new Object[] { false },
+                new VerifyError(), null));
+        testCases.add(new TestCase("b/22411633 (3)", "B22411633_3", "run", new Object[] { false },
+                null, null));
+        testCases.add(new TestCase("b/22411633 (4)", "B22411633_4", "run", new Object[] { false },
+                new VerifyError(), null));
+        testCases.add(new TestCase("b/22411633 (5)", "B22411633_5", "run", new Object[] { false },
+                null, null));
     }
 
     public void runTests() {