Test component type for errors too.

With regression test.

Rationale:
Moved erroneous check in convenience method, so we
put all the same logic in one place. When testing
for erroneous T[], check both the array type
as well at the component type T for errors
(it is possible T[] is not marked erroneous
even though T is eventually).

BUG=28358598

Change-Id: I11339a976dc83e0493a99e6bb97f3a058ca3f796

8 files changed, 122 insertions, 11 deletions

diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index fe75451ad2..6703695484 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -2218,6 +2218,7 @@ ReferenceTypeInfo ReferenceTypeInfo::Create(TypeHandle type_handle, bool is_exac
     ScopedObjectAccess soa(Thread::Current());
     DCHECK(IsValidHandle(type_handle));
     DCHECK(!type_handle->IsErroneous());
+    DCHECK(!type_handle->IsArrayClass() || !type_handle->GetComponentType()->IsErroneous());
     if (!is_exact) {
       DCHECK(!type_handle->CannotBeAssignedFromOtherTypes())
           << "Callers of ReferenceTypeInfo::Create should ensure is_exact is properly computed";
diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc
index 04c9ff9d6d..7e1fd75cdc 100644
--- a/compiler/optimizing/reference_type_propagation.cc
+++ b/compiler/optimizing/reference_type_propagation.cc
@@ -46,6 +46,13 @@ static inline ReferenceTypeInfo::TypeHandle GetRootHandle(StackHandleScopeCollec
   return *cache;
 }
 
+// Returns true if klass is admissible to the propagation: non-null and non-erroneous.
+// For an array type, we also check if the component type is admissible.
+static bool IsAdmissible(mirror::Class* klass) SHARED_REQUIRES(Locks::mutator_lock_) {
+  return klass != nullptr && !klass->IsErroneous() &&
+      (!klass->IsArrayClass() || IsAdmissible(klass->GetComponentType()));
+}
+
 ReferenceTypeInfo::TypeHandle ReferenceTypePropagation::HandleCache::GetObjectClassHandle() {
   return GetRootHandle(handles_, ClassLinker::kJavaLangObject, &object_class_handle_);
 }
@@ -453,15 +460,10 @@ void ReferenceTypePropagation::RTPVisitor::SetClassAsTypeInfo(HInstruction* inst
     }
     instr->SetReferenceTypeInfo(
         ReferenceTypeInfo::Create(handle_cache_->GetStringClassHandle(), /* is_exact */ true));
-  } else if (klass != nullptr) {
-    if (klass->IsErroneous()) {
-      // Set inexact object type for erroneous types.
-      instr->SetReferenceTypeInfo(instr->GetBlock()->GetGraph()->GetInexactObjectRti());
-    } else {
-      ReferenceTypeInfo::TypeHandle handle = handle_cache_->NewHandle(klass);
-      is_exact = is_exact || handle->CannotBeAssignedFromOtherTypes();
-      instr->SetReferenceTypeInfo(ReferenceTypeInfo::Create(handle, is_exact));
-    }
+  } else if (IsAdmissible(klass)) {
+    ReferenceTypeInfo::TypeHandle handle = handle_cache_->NewHandle(klass);
+    is_exact = is_exact || handle->CannotBeAssignedFromOtherTypes();
+    instr->SetReferenceTypeInfo(ReferenceTypeInfo::Create(handle, is_exact));
   } else {
     instr->SetReferenceTypeInfo(instr->GetBlock()->GetGraph()->GetInexactObjectRti());
   }
@@ -563,7 +565,7 @@ void ReferenceTypePropagation::RTPVisitor::VisitLoadClass(HLoadClass* instr) {
                                                        instr->GetDexFile(),
                                                        instr->GetTypeIndex(),
                                                        hint_dex_cache_);
-  if (resolved_class != nullptr && !resolved_class->IsErroneous()) {
+  if (IsAdmissible(resolved_class)) {
     instr->SetLoadedClassRTI(ReferenceTypeInfo::Create(
         handle_cache_->NewHandle(resolved_class), /* is_exact */ true));
   }
@@ -742,7 +744,7 @@ void ReferenceTypePropagation::UpdateArrayGet(HArrayGet* instr, HandleCache* han
   }
 
   Handle<mirror::Class> handle = parent_rti.GetTypeHandle();
-  if (handle->IsObjectArrayClass() && !handle->GetComponentType()->IsErroneous()) {
+  if (handle->IsObjectArrayClass() && IsAdmissible(handle->GetComponentType())) {
     ReferenceTypeInfo::TypeHandle component_handle =
         handle_cache->NewHandle(handle->GetComponentType());
     bool is_exact = component_handle->CannotBeAssignedFromOtherTypes();
diff --git a/test/595-error-class/expected.txt b/test/595-error-class/expected.txt
new file mode 100644
index 0000000000..b0aad4deb5
--- /dev/null
+++ b/test/595-error-class/expected.txt
@@ -0,0 +1 @@
+passed
diff --git a/test/595-error-class/info.txt b/test/595-error-class/info.txt
new file mode 100644
index 0000000000..a58b8b31b4
--- /dev/null
+++ b/test/595-error-class/info.txt
@@ -0,0 +1 @@
+Regression test on merging array type with error component type.
diff --git a/test/595-error-class/smali/error.smali b/test/595-error-class/smali/error.smali
new file mode 100644
index 0000000000..925c34b293
--- /dev/null
+++ b/test/595-error-class/smali/error.smali
@@ -0,0 +1,23 @@
+# Copyright (C) 2016 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 final LAnError;
+
+.super LSuperOfAnError;
+
+# Override a final method to put this class in the error state.
+.method public foo()V
+  .registers 1
+  return-void
+.end method
diff --git a/test/595-error-class/smali/merge.smali b/test/595-error-class/smali/merge.smali
new file mode 100644
index 0000000000..2f8b41504e
--- /dev/null
+++ b/test/595-error-class/smali/merge.smali
@@ -0,0 +1,31 @@
+#
+# Copyright (C) 2016 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 LMerge;
+.super Ljava/lang/Object;
+
+# Method that selects between x = new Integer[] or new AnError[],
+# Reference type propagation should correctly see error in component type.
+.method public static select(Z)Ljava/lang/Object;
+    .registers 2
+    const/16 v0, 10
+    if-eqz v1, :Skip
+    new-array v0, v0, [LAnError;
+    goto :Done
+:Skip
+    new-array v0, v0, [Ljava/lang/Integer;
+:Done
+    return-object v0
+.end method
diff --git a/test/595-error-class/smali/super.smali b/test/595-error-class/smali/super.smali
new file mode 100644
index 0000000000..da7467d164
--- /dev/null
+++ b/test/595-error-class/smali/super.smali
@@ -0,0 +1,22 @@
+# Copyright (C) 2016 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 LSuperOfAnError;
+
+.super Ljava/lang/Object;
+
+.method public final foo()V
+  .registers 1
+  return-void
+.end method
diff --git a/test/595-error-class/src/Main.java b/test/595-error-class/src/Main.java
new file mode 100644
index 0000000000..655fa4336a
--- /dev/null
+++ b/test/595-error-class/src/Main.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2016 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.*;
+
+public class Main {
+
+  public static void main(String args[]) throws Throwable {
+    Class<?> c = Class.forName("Merge");
+    Method m = c.getMethod("select", boolean.class);
+    Object x = m.invoke(null, true);
+    if (x == null) {
+      throw new Error("Did not get array");
+    }
+    System.out.println("passed");
+  }
+}