ART: Fix bug in reference type propagation
Reference type propagation assumed that type is exact if the class is
final. This does not hold for arrays which are always final and the
component type needs to be considered.
Bug: 24084144
Change-Id: Ib72e12a018437c404e82f7ad414554c66a4c6f8c
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 2966076..efd4fcf 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -506,7 +506,7 @@
ReferenceTypeInfo::TypeHandle return_handle =
handles_->NewHandle(resolved_method->GetReturnType(true /* resolve */, pointer_size));
return_replacement->SetReferenceTypeInfo(ReferenceTypeInfo::Create(
- return_handle, return_handle->IsFinal() /* is_exact */));
+ return_handle, return_handle->CannotBeAssignedFromOtherTypes() /* is_exact */));
}
}
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index b3cf0b3..3915ac7 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -20,6 +20,7 @@
#include "ssa_builder.h"
#include "base/bit_vector-inl.h"
#include "base/bit_utils.h"
+#include "mirror/class-inl.h"
#include "utils/growable_array.h"
#include "scoped_thread_state_change.h"
@@ -1773,7 +1774,7 @@
DCHECK(upper_bound_rti.IsSupertypeOf(rti))
<< " upper_bound_rti: " << upper_bound_rti
<< " rti: " << rti;
- DCHECK(!upper_bound_rti.GetTypeHandle()->IsFinal() || rti.IsExact());
+ DCHECK(!upper_bound_rti.GetTypeHandle()->CannotBeAssignedFromOtherTypes() || rti.IsExact());
}
}
reference_type_info_ = rti;
diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc
index 0384e46..a88c543 100644
--- a/compiler/optimizing/reference_type_propagation.cc
+++ b/compiler/optimizing/reference_type_propagation.cc
@@ -167,7 +167,7 @@
ReferenceTypeInfo class_rti = load_class->GetLoadedClassRTI();
HBoundType* bound_type = new (arena) HBoundType(obj, class_rti, upper_can_be_null);
// Narrow the type as much as possible.
- if (class_rti.GetTypeHandle()->IsFinal()) {
+ if (class_rti.GetTypeHandle()->CannotBeAssignedFromOtherTypes()) {
bound_type->SetReferenceTypeInfo(
ReferenceTypeInfo::Create(class_rti.GetTypeHandle(), /* is_exact */ true));
} else if (obj_rti.IsValid() && class_rti.IsSupertypeOf(obj_rti)) {
@@ -380,7 +380,7 @@
} else if (klass != nullptr) {
ScopedObjectAccess soa(Thread::Current());
ReferenceTypeInfo::TypeHandle handle = handles_->NewHandle(klass);
- is_exact = is_exact || klass->IsFinal();
+ is_exact = is_exact || klass->CannotBeAssignedFromOtherTypes();
instr->SetReferenceTypeInfo(ReferenceTypeInfo::Create(handle, is_exact));
} else {
instr->SetReferenceTypeInfo(
diff --git a/test/530-checker-regression-reftype-final/expected.txt b/test/530-checker-regression-reftype-final/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/530-checker-regression-reftype-final/expected.txt
diff --git a/test/530-checker-regression-reftype-final/info.txt b/test/530-checker-regression-reftype-final/info.txt
new file mode 100644
index 0000000..07789d6
--- /dev/null
+++ b/test/530-checker-regression-reftype-final/info.txt
@@ -0,0 +1 @@
+Regression test for optimizing that used assume that array types are always exact.
diff --git a/test/530-checker-regression-reftype-final/smali/TestCase.smali b/test/530-checker-regression-reftype-final/smali/TestCase.smali
new file mode 100644
index 0000000..8fd7bb7
--- /dev/null
+++ b/test/530-checker-regression-reftype-final/smali/TestCase.smali
@@ -0,0 +1,59 @@
+# Copyright (C) 2015 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+.class public LTestCase;
+.super Ljava/lang/Object;
+
+# Inliner used to assign exact type to the artificial multiple-return phi if the
+# class type was final which does not hold for arrays.
+
+# The type information is only used by recursive calls to the inliner and is
+# overwritten by the next pass of reference type propagation. Since we do not
+# inline any methods from array classes, this bug cannot be triggered and we
+# verify it using Checker.
+
+## CHECK-START: void TestCase.testInliner() reference_type_propagation_after_inlining (before)
+## CHECK-DAG: CheckCast [<<Phi:l\d+>>,{{l\d+}}]
+## CHECK-DAG: <<Phi>> Phi klass:java.lang.Object[] exact:false
+
+.method public static testInliner()V
+ .registers 3
+
+ invoke-static {}, Ljava/lang/System;->nanoTime()J
+ move-result-wide v0
+ long-to-int v0, v0
+
+ invoke-static {v0}, LTestCase;->$inline$getArray(I)[Ljava/lang/Object;
+ move-result-object v0
+
+ check-cast v0, [LMain$MyClassA;
+ return-void
+
+.end method
+
+.method public static $inline$getArray(I)[Ljava/lang/Object;
+ .registers 2
+ if-eqz p0, :else
+
+ :then
+ const/4 v0, 2
+ new-array v0, v0, [LMain$MyClassA;
+ return-object v0
+
+ :else
+ const/4 v0, 3
+ new-array v0, v0, [LMain$MyClassB;
+ return-object v0
+
+.end method
diff --git a/test/530-checker-regression-reftype-final/src/Main.java b/test/530-checker-regression-reftype-final/src/Main.java
new file mode 100644
index 0000000..f86b515
--- /dev/null
+++ b/test/530-checker-regression-reftype-final/src/Main.java
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.lang.reflect.Method;
+
+public class Main {
+
+ class MyClassA {}
+ class MyClassB extends MyClassA {}
+
+ public static void main(String[] args) throws Exception {
+ testReferenceTypePropagation();
+ invokeTestInliner();
+ }
+
+ // Reference type propagation (RTP) used to assume that if a class is final,
+ // then the type must be exact. This does not hold for arrays which are always
+ // final, i.e. not extendable, but may be assigned to from values of the
+ // components type subclasses.
+
+ public static void testReferenceTypePropagation() throws Exception {
+ boolean expectTrue;
+
+ // Bug #1: RTP would set the type of `array` to exact Object[]. Instruction
+ // simplifier would then simplify the instanceof to `false`.
+ Object[] array = $noinline$getArray();
+ expectTrue = array instanceof MyClassA[];
+ if (!expectTrue) {
+ throw new Exception("Incorrect type check.");
+ }
+
+ // Bug #2: This is the true-branch of the instanceof above. The bound type
+ // for `array` would be again set to exact MyClassA[] and incorrectly
+ // simplify the second instanceof to `false`.
+ expectTrue = array instanceof MyClassB[];
+ if (!expectTrue) {
+ throw new Exception("Incorrect type bound.");
+ }
+ }
+
+ public static void invokeTestInliner() throws Exception {
+ Class<?> c = Class.forName("TestCase");
+ Method m = c.getMethod("testInliner");
+ m.invoke(null);
+ }
+
+ public static Object[] $noinline$getArray() {
+ if (doThrow) throw new Error();
+ return new MyClassB[2];
+ }
+
+ static boolean doThrow = false;
+}