Merge "Fix index checks for error strings in DexFileVerifier." into nyc-dev
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index a9ceebb..93081b9 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -721,6 +721,11 @@
       DCHECK(Runtime::Current()->IsAotCompiler());
       return nullptr;
     }
+    if (!methods_class->IsAssignableFrom(compiling_class.Get())) {
+      // We cannot statically determine the target method. The runtime will throw a
+      // NoSuchMethodError on this one.
+      return nullptr;
+    }
     ArtMethod* actual_method;
     if (methods_class->IsInterface()) {
       actual_method = methods_class->FindVirtualMethodForInterfaceSuper(
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 1b23622..b4d93ad 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -732,19 +732,14 @@
       if (Primitive::PrimitiveKind(heap_value->GetType())
               != Primitive::PrimitiveKind(instruction->GetType())) {
         // The only situation where the same heap location has different type is when
-        // we do an array get from a null constant. In order to stay properly typed
-        // we do not merge the array gets.
+        // we do an array get on an instruction that originates from the null constant
+        // (the null could be behind a field access, an array access, a null check or
+        // a bound type).
+        // In order to stay properly typed on primitive types, we do not eliminate
+        // the array gets.
         if (kIsDebugBuild) {
           DCHECK(heap_value->IsArrayGet()) << heap_value->DebugName();
           DCHECK(instruction->IsArrayGet()) << instruction->DebugName();
-          HInstruction* array = instruction->AsArrayGet()->GetArray();
-          DCHECK(array->IsNullCheck()) << array->DebugName();
-          HInstruction* input = HuntForOriginalReference(array->InputAt(0));
-          DCHECK(input->IsNullConstant()) << input->DebugName();
-          array = heap_value->AsArrayGet()->GetArray();
-          DCHECK(array->IsNullCheck()) << array->DebugName();
-          input = HuntForOriginalReference(array->InputAt(0));
-          DCHECK(input->IsNullConstant()) << input->DebugName();
         }
         return;
       }
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 679c274..5f3cdcf 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -2399,6 +2399,7 @@
   }
   if (!NeedsEnvironment()) {
     RemoveEnvironment();
+    SetSideEffects(SideEffects::None());
   }
 }
 
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 10a51d6..56d6a98 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -5549,6 +5549,7 @@
     SetPackedFlag<kFlagIsInDexCache>(true);
     DCHECK(!NeedsEnvironment());
     RemoveEnvironment();
+    SetSideEffects(SideEffects::None());
   }
 
   size_t InputCount() const OVERRIDE {
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index 16fbfaa..fc62573 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -514,12 +514,18 @@
         CHECK(self->IsExceptionPending());
         return nullptr;
       } else if (!method_reference_class->IsInterface()) {
-        // It is not an interface.
-        mirror::Class* super_class = referring_class->GetSuperClass();
+        // It is not an interface. If the referring class is in the class hierarchy of the
+        // referenced class in the bytecode, we use its super class. Otherwise, we throw
+        // a NoSuchMethodError.
+        mirror::Class* super_class = nullptr;
+        if (method_reference_class->IsAssignableFrom(referring_class)) {
+          super_class = referring_class->GetSuperClass();
+        }
         uint16_t vtable_index = resolved_method->GetMethodIndex();
         if (access_check) {
           // Check existence of super class.
-          if (super_class == nullptr || !super_class->HasVTable() ||
+          if (super_class == nullptr ||
+              !super_class->HasVTable() ||
               vtable_index >= static_cast<uint32_t>(super_class->GetVTableLength())) {
             // Behavior to agree with that of the verifier.
             ThrowNoSuchMethodError(type, resolved_method->GetDeclaringClass(),
@@ -693,8 +699,13 @@
       // Need to do full type resolution...
       return nullptr;
     } else if (!method_reference_class->IsInterface()) {
-      // It is not an interface.
-      mirror::Class* super_class = referrer->GetDeclaringClass()->GetSuperClass();
+      // It is not an interface. If the referring class is in the class hierarchy of the
+      // referenced class in the bytecode, we use its super class. Otherwise, we cannot
+      // resolve the method.
+      if (!method_reference_class->IsAssignableFrom(referring_class)) {
+        return nullptr;
+      }
+      mirror::Class* super_class = referring_class->GetSuperClass();
       if (resolved_method->GetMethodIndex() >= super_class->GetVTableLength()) {
         // The super class does not have the method.
         return nullptr;
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index d05ae42..2b96328 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -4101,8 +4101,8 @@
                                     << " to super " << PrettyMethod(res_method);
         return nullptr;
       }
-      mirror::Class* super_klass = super.GetClass();
-      if (res_method->GetMethodIndex() >= super_klass->GetVTableLength()) {
+      if (!reference_class->IsAssignableFrom(GetDeclaringClass().GetClass()) ||
+          (res_method->GetMethodIndex() >= super.GetClass()->GetVTableLength())) {
         Fail(VERIFY_ERROR_NO_METHOD) << "invalid invoke-super from "
                                     << PrettyMethod(dex_method_idx_, *dex_file_)
                                     << " to super " << super
diff --git a/test/586-checker-null-array-get/src/Main.java b/test/586-checker-null-array-get/src/Main.java
index 332cfb0..e0782bc 100644
--- a/test/586-checker-null-array-get/src/Main.java
+++ b/test/586-checker-null-array-get/src/Main.java
@@ -14,10 +14,20 @@
  * limitations under the License.
  */
 
+class Test1 {
+  int[] iarr;
+}
+
+class Test2 {
+  float[] farr;
+}
+
 public class Main {
   public static Object[] getObjectArray() { return null; }
   public static long[] getLongArray() { return null; }
   public static Object getNull() { return null; }
+  public static Test1 getNullTest1() { return null; }
+  public static Test2 getNullTest2() { return null; }
 
   public static void main(String[] args) {
     try {
@@ -26,13 +36,25 @@
     } catch (NullPointerException e) {
       // Expected.
     }
+    try {
+      bar();
+      throw new Error("Expected NullPointerException");
+    } catch (NullPointerException e) {
+      // Expected.
+    }
+    try {
+      test1();
+      throw new Error("Expected NullPointerException");
+    } catch (NullPointerException e) {
+      // Expected.
+    }
   }
 
   /// CHECK-START: void Main.foo() load_store_elimination (after)
-  /// CHECK-DAG: <<Null:l\d+>>  NullConstant
-  /// CHECK-DAG: <<Check:l\d+>> NullCheck [<<Null>>]
-  /// CHECK-DAG: <<Get1:j\d+>>  ArrayGet [<<Check>>,{{i\d+}}]
-  /// CHECK-DAG: <<Get2:l\d+>>  ArrayGet [<<Check>>,{{i\d+}}]
+  /// CHECK-DAG: <<Null:l\d+>>   NullConstant
+  /// CHECK-DAG: <<Check:l\d+>>  NullCheck [<<Null>>]
+  /// CHECK-DAG: <<Get1:j\d+>>   ArrayGet [<<Check>>,{{i\d+}}]
+  /// CHECK-DAG: <<Get2:l\d+>>   ArrayGet [<<Check>>,{{i\d+}}]
   public static void foo() {
     longField = getLongArray()[0];
     objectField = getObjectArray()[0];
@@ -56,7 +78,7 @@
     // elimination pass to add a HDeoptimize. Not having the bounds check helped
     // the load store elimination think it could merge two ArrayGet with different
     // types.
-    String[] array = ((String[])getNull());
+    String[] array = (String[])getNull();
     objectField = array[0];
     objectField = array[1];
     objectField = array[2];
@@ -68,6 +90,23 @@
     longField = longArray[3];
   }
 
+  /// CHECK-START: float Main.test1() load_store_elimination (after)
+  /// CHECK-DAG: <<Null:l\d+>>       NullConstant
+  /// CHECK-DAG: <<Check1:l\d+>>     NullCheck [<<Null>>]
+  /// CHECK-DAG: <<FieldGet1:l\d+>>  InstanceFieldGet [<<Check1>>] field_name:Test1.iarr
+  /// CHECK-DAG: <<Check2:l\d+>>     NullCheck [<<FieldGet1>>]
+  /// CHECK-DAG: <<ArrayGet1:i\d+>>  ArrayGet [<<Check2>>,{{i\d+}}]
+  /// CHECK-DAG: <<ArrayGet2:f\d+>>  ArrayGet [<<Check2>>,{{i\d+}}]
+  /// CHECK-DAG:                     Return [<<ArrayGet2>>]
+  public static float test1() {
+    Test1 test1 = getNullTest1();
+    Test2 test2 = getNullTest2();;
+    int[] iarr = test1.iarr;
+    float[] farr = test2.farr;
+    iarr[0] = iarr[1];
+    return farr[0];
+  }
+
   public static long longField;
   public static Object objectField;
 }
diff --git a/test/594-invoke-super/expected.txt b/test/594-invoke-super/expected.txt
new file mode 100644
index 0000000..de26026
--- /dev/null
+++ b/test/594-invoke-super/expected.txt
@@ -0,0 +1,7 @@
+new A
+I am A's foo
+new B
+I am B's foo
+new A
+new B
+passed
diff --git a/test/594-invoke-super/info.txt b/test/594-invoke-super/info.txt
new file mode 100644
index 0000000..440d8b8
--- /dev/null
+++ b/test/594-invoke-super/info.txt
@@ -0,0 +1 @@
+Invoke-super on various references.
diff --git a/test/594-invoke-super/smali/invoke-super.smali b/test/594-invoke-super/smali/invoke-super.smali
new file mode 100644
index 0000000..6f787dd
--- /dev/null
+++ b/test/594-invoke-super/smali/invoke-super.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 LZ;
+.super LA;
+
+.method public constructor <init>()V
+.registers 1
+    invoke-direct {v0}, LA;-><init>()V
+    return-void
+.end method
+
+.method public foo()V
+.registers 3
+    new-instance v0, LY;
+    invoke-direct {v0}, LY;-><init>()V
+    invoke-super {v0}, LY;->foo()V
+    return-void
+.end method
diff --git a/test/594-invoke-super/src/Main.java b/test/594-invoke-super/src/Main.java
new file mode 100644
index 0000000..53f2bbf
--- /dev/null
+++ b/test/594-invoke-super/src/Main.java
@@ -0,0 +1,80 @@
+/*
+ * 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.InvocationTargetException;
+import java.lang.reflect.Method;
+
+//
+// Two classes A and B with method foo().
+//
+
+class A {
+  A() { System.out.println("new A"); }
+
+  public void foo() { System.out.println("I am A's foo"); }
+
+  // We previously used to invoke this method with a Y instance, due
+  // to invoke-super underspecified behavior.
+  public void bar() { System.out.println("I am A's bar"); }
+}
+
+class B {
+  B() { System.out.println("new B"); }
+
+  public void foo() { System.out.println("I am B's foo"); }
+}
+
+//
+// Two subclasses X and Y that call foo() on super.
+//
+
+class X extends A {
+  public void foo() { super.foo(); }
+}
+
+class Y extends B {
+  public void foo() { super.foo(); }
+}
+
+//
+// Driver class.
+//
+
+public class Main {
+
+  public static void main(String[] args) throws Exception {
+    // The normal stuff, X's super goes to A, Y's super goes to B.
+    new X().foo();
+    new Y().foo();
+
+    // And now it gets interesting.
+
+    // In bytecode, we define a class Z that is a subclass of A, and we call
+    // invoke-super on an instance of Y.
+    Class<?> z = Class.forName("Z");
+    Method m = z.getMethod("foo");
+    try {
+      m.invoke(z.newInstance());
+      throw new Error("Expected InvocationTargetException");
+    } catch (InvocationTargetException e) {
+      if (!(e.getCause() instanceof NoSuchMethodError)) {
+        throw new Error("Expected NoSuchMethodError");
+      }
+    }
+
+    System.out.println("passed");
+  }
+}
diff --git a/test/594-load-string-regression/expected.txt b/test/594-load-string-regression/expected.txt
new file mode 100644
index 0000000..365b0e1
--- /dev/null
+++ b/test/594-load-string-regression/expected.txt
@@ -0,0 +1 @@
+String: ""
diff --git a/test/594-load-string-regression/info.txt b/test/594-load-string-regression/info.txt
new file mode 100644
index 0000000..6a07ace
--- /dev/null
+++ b/test/594-load-string-regression/info.txt
@@ -0,0 +1,2 @@
+Regression test for LoadString listing side effects when it doesn't have any
+and triggering a DCHECK() failure when merging ClinitCheck into NewInstance.
diff --git a/test/594-load-string-regression/src/Main.java b/test/594-load-string-regression/src/Main.java
new file mode 100644
index 0000000..0b9f7b5
--- /dev/null
+++ b/test/594-load-string-regression/src/Main.java
@@ -0,0 +1,77 @@
+/*
+ * 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.
+ */
+
+public class Main {
+  static boolean doThrow = false;
+
+  // Note: We're not doing checker tests as we cannot do them specifically for a non-PIC
+  // configuration. The check here would be "prepare_for_register_allocation (before)"
+  //     CHECK:         LoadClass
+  //     CHECK-NEXT:    ClinitCheck
+  //     CHECK-NEXT:    LoadString load_kind:BootImageAddress
+  //     CHECK-NEXT:    NewInstance
+  // and "prepare_for_register_allocation (after)"
+  //     CHECK:         LoadString
+  //     CHECK-NEXT:    NewInstance
+  // but the order of instructions for non-PIC mode is different.
+  public static int $noinline$test() {
+    if (doThrow) { throw new Error(); }
+
+    int r = 0x12345678;
+    do {
+      // LICM pulls the LoadClass and ClinitCheck out of the loop, leaves NewInstance in the loop.
+      Helper h = new Helper();
+      // For non-PIC mode, LICM pulls the boot image LoadString out of the loop.
+      // (For PIC mode, the LoadString can throw and will not be moved out of the loop.)
+      String s = "";  // Empty string is known to be in the boot image.
+      r = r ^ (r >> 5);
+      h.$noinline$printString(s);
+      // During DCE after inlining, the loop back-edge disappears and the pre-header is
+      // merged with the body, leaving consecutive LoadClass, ClinitCheck, LoadString
+      // and NewInstance in non-PIC mode. The prepare_for_register_allocation pass
+      // merges the LoadClass and ClinitCheck with the NewInstance and checks that
+      // there are no instructions with side effects in between. This check used to
+      // fail because LoadString was always listing SideEffects::CanTriggerGC() even
+      // when it doesn't really have any side effects, i.e. for direct references to
+      // boot image Strings or for Strings known to be in the dex cache.
+    } while ($inline$shouldContinue());
+    return r;
+  }
+
+  static boolean $inline$shouldContinue() {
+    return false;
+  }
+
+  public static void main(String[] args) {
+    assertIntEquals(0x12345678 ^ (0x12345678 >> 5), $noinline$test());
+  }
+
+  public static void assertIntEquals(int expected, int result) {
+    if (expected != result) {
+      throw new Error("Expected: " + expected + ", found: " + result);
+    }
+  }
+}
+
+class Helper {
+  static boolean doThrow = false;
+
+  public void $noinline$printString(String s) {
+    if (doThrow) { throw new Error(); }
+
+    System.out.println("String: \"" + s + "\"");
+  }
+}