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
--- a/runtime/verifier/reg_type.cc
+++ b/runtime/verifier/reg_type.cc
@@ -690,6 +690,11 @@
} 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 @@
// 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
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -31,4 +31,9 @@
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
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
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
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
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
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
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -109,6 +109,16 @@
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() {