Merge "ART: Remove old aget on null workaround"

14 files changed, 206 insertions, 120 deletions

diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index be58a57c24..fefb4f6526 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -4691,12 +4691,19 @@ void MethodVerifier::VerifyAGet(const Instruction* inst,
   } else {
     const RegType& array_type = work_line_->GetRegisterType(this, inst->VRegB_23x());
     if (array_type.IsZeroOrNull()) {
-      have_pending_runtime_throw_failure_ = true;
       // Null array class; this code path will fail at runtime. Infer a merge-able type from the
-      // instruction type. TODO: have a proper notion of bottom here.
-      if (!is_primitive || insn_type.IsCategory1Types()) {
-        // Reference or category 1
-        work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), reg_types_.Zero());
+      // instruction type.
+      if (!is_primitive) {
+        work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), reg_types_.Null());
+      } else if (insn_type.IsInteger()) {
+        // Pick a non-zero constant (to distinguish with null) that can fit in any primitive.
+        // We cannot use 'insn_type' as it could be a float array or an int array.
+        work_line_->SetRegisterType<LockOp::kClear>(
+            this, inst->VRegA_23x(), DetermineCat1Constant(1, need_precise_constants_));
+      } else if (insn_type.IsCategory1Types()) {
+        // Category 1
+        // The 'insn_type' is exactly the type we need.
+        work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), insn_type);
       } else {
         // Category 2
         work_line_->SetRegisterTypeWide(this, inst->VRegA_23x(),
diff --git a/test/518-null-array-get/expected.txt b/test/518-null-array-get/expected.txt
index e69de29bb2..ae5318e53d 100644
--- a/test/518-null-array-get/expected.txt
+++ b/test/518-null-array-get/expected.txt
@@ -0,0 +1,6 @@
+NullArrayFailInt2Object
+NullArrayFailObject2Int
+NullArraySuccessInt
+NullArraySuccessInt2Float
+NullArraySuccessShort
+NullArraySuccessRef
diff --git a/test/518-null-array-get/info.txt b/test/518-null-array-get/info.txt
index 407f590b2b..71e0332e62 100644
--- a/test/518-null-array-get/info.txt
+++ b/test/518-null-array-get/info.txt
@@ -1,3 +1,9 @@
-Regression test for Quick and Optimizing that used
-to crash on an aget-object + int-to-byte sequence
-(accepted by the verifier in the case the array was null).
+Codifies that the verifier should reject type-unsafe
+instructions in dead code after aget on null, but pass
+type-safe dead code.
+
+Previously verification stopped after aget on null and
+punted the method to the interpreter in an effort to avoid
+compiler crashes. As broken code appears very uncommon,
+ensure verifier strictness and help the compilers see more
+code.
diff --git a/test/706-jit-skip-compilation/run b/test/518-null-array-get/smali/NullArrayFailInt2Object.smali
index 6c5720a099..ca4ed10660 100644
--- a/test/706-jit-skip-compilation/run
+++ b/test/518-null-array-get/smali/NullArrayFailInt2Object.smali
@@ -1,12 +1,10 @@
-#!/bin/bash
-#
-# Copyright (C) 2016 The Android Open Source Project
+# 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
+#      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,
@@ -14,6 +12,17 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# Run without the app image, otherwise the verification results will be cached
-# in the ArtMethod of the image and the test will be skewed.
-exec ${RUN} "${@}" --no-app-image
+# Check that the result of aget on null cannot be used as a reference.
+
+.class public LNullArrayFailInt2Object;
+
+.super Ljava/lang/Object;
+
+.method public static method()V
+   .registers 2
+   const/4 v0, 0
+   const/4 v1, 0
+   aget v0, v0, v1
+   invoke-virtual { v0 }, Ljava/lang/Object;->toString()Ljava/lang/String;
+   return-void
+.end method
diff --git a/test/518-null-array-get/smali/NullArray.smali b/test/518-null-array-get/smali/NullArrayFailObject2Int.smali
index 52abc38473..83823a24e5 100644
--- a/test/518-null-array-get/smali/NullArray.smali
+++ b/test/518-null-array-get/smali/NullArrayFailObject2Int.smali
@@ -12,7 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-.class public LNullArray;
+# Check that the result of aget-object on null cannot be used as an integral.
+
+.class public LNullArrayFailObject2Int;
 
 .super Ljava/lang/Object;
 
diff --git a/test/518-null-array-get/smali/NullArraySuccessInt.smali b/test/518-null-array-get/smali/NullArraySuccessInt.smali
new file mode 100644
index 0000000000..01cf1c92ab
--- /dev/null
+++ b/test/518-null-array-get/smali/NullArraySuccessInt.smali
@@ -0,0 +1,33 @@
+# 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.
+
+# Check that the result of aget on null can be used as an int.
+
+.class public LNullArraySuccessInt;
+
+.super Ljava/lang/Object;
+
+.method public static intMethod(I)V
+   .registers 1
+   return-void
+.end method
+
+.method public static method()V
+   .registers 2
+   const/4 v0, 0
+   const/4 v1, 0
+   aget v0, v0, v1
+   invoke-static { v0 }, LNullArraySuccessInt;->intMethod(I)V
+   return-void
+.end method
diff --git a/test/518-null-array-get/smali/NullArraySuccessInt2Float.smali b/test/518-null-array-get/smali/NullArraySuccessInt2Float.smali
new file mode 100644
index 0000000000..bd59d5f68e
--- /dev/null
+++ b/test/518-null-array-get/smali/NullArraySuccessInt2Float.smali
@@ -0,0 +1,33 @@
+# 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.
+
+# Check that the result of aget on null can be used as a float.
+
+.class public LNullArraySuccessInt2Float;
+
+.super Ljava/lang/Object;
+
+.method public static floatMethod(F)V
+   .registers 1
+   return-void
+.end method
+
+.method public static method()V
+   .registers 2
+   const/4 v0, 0
+   const/4 v1, 0
+   aget v0, v0, v1
+   invoke-static { v0 }, LNullArraySuccessInt2Float;->floatMethod(F)V
+   return-void
+.end method
diff --git a/test/518-null-array-get/smali/NullArraySuccessRef.smali b/test/518-null-array-get/smali/NullArraySuccessRef.smali
new file mode 100644
index 0000000000..2f512d4089
--- /dev/null
+++ b/test/518-null-array-get/smali/NullArraySuccessRef.smali
@@ -0,0 +1,33 @@
+# 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.
+
+# Check that the result of aget-object on null can be used as a reference.
+
+.class public LNullArraySuccessRef;
+
+.super Ljava/lang/Object;
+
+.method public voidMethod()V
+   .registers 1
+   return-void
+.end method
+
+.method public static method()V
+   .registers 2
+   const/4 v0, 0
+   const/4 v1, 0
+   aget-object v0, v0, v1
+   invoke-virtual { v0 }, LNullArraySuccessRef;->voidMethod()V
+   return-void
+.end method
diff --git a/test/518-null-array-get/smali/NullArraySuccessShort.smali b/test/518-null-array-get/smali/NullArraySuccessShort.smali
new file mode 100644
index 0000000000..d332e51f52
--- /dev/null
+++ b/test/518-null-array-get/smali/NullArraySuccessShort.smali
@@ -0,0 +1,33 @@
+# 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.
+
+# Check that the result of aget-short on null can be used as a short.
+
+.class public LNullArraySuccessShort;
+
+.super Ljava/lang/Object;
+
+.method public static shortMethod(S)V
+   .registers 1
+   return-void
+.end method
+
+.method public static method()V
+   .registers 2
+   const/4 v0, 0
+   const/4 v1, 0
+   aget-short v0, v0, v1
+   invoke-static { v0 }, LNullArraySuccessShort;->shortMethod(S)V
+   return-void
+.end method
diff --git a/test/518-null-array-get/src/Main.java b/test/518-null-array-get/src/Main.java
index 66e50aacd7..678aef1f43 100644
--- a/test/518-null-array-get/src/Main.java
+++ b/test/518-null-array-get/src/Main.java
@@ -22,16 +22,36 @@ public class Main {
   class InnerClass {}
 
   public static void main(String[] args) throws Exception {
-    Class<?> c = Class.forName("NullArray");
-    Method m = c.getMethod("method");
-    Object[] arguments = { };
+    checkLoad("NullArrayFailInt2Object", true);
+    checkLoad("NullArrayFailObject2Int", true);
+    checkLoad("NullArraySuccessInt", false);
+    checkLoad("NullArraySuccessInt2Float", false);
+    checkLoad("NullArraySuccessShort", false);
+    checkLoad("NullArraySuccessRef", false);
+  }
+
+  private static void checkLoad(String className, boolean expectError) throws Exception {
+    Class<?> c;
     try {
-      m.invoke(null, arguments);
-      throw new Error("Expected an InvocationTargetException");
-    } catch (InvocationTargetException e) {
-      if (!(e.getCause() instanceof NullPointerException)) {
-        throw new Error("Expected a NullPointerException");
+      c = Class.forName(className);
+      if (expectError) {
+        throw new RuntimeException("Expected error for " + className);
+      }
+      Method m = c.getMethod("method");
+      try {
+        m.invoke(null);
+        throw new RuntimeException("Expected an InvocationTargetException");
+      } catch (InvocationTargetException e) {
+        if (!(e.getCause() instanceof NullPointerException)) {
+          throw new RuntimeException("Expected a NullPointerException");
+        }
+        System.out.println(className);
+      }
+    } catch (VerifyError e) {
+      if (!expectError) {
+        throw new RuntimeException(e);
       }
+      System.out.println(className);
     }
   }
 }
diff --git a/test/706-jit-skip-compilation/expected.txt b/test/706-jit-skip-compilation/expected.txt
deleted file mode 100644
index 6a5618ebc6..0000000000
--- a/test/706-jit-skip-compilation/expected.txt
+++ /dev/null
@@ -1 +0,0 @@
-JNI_OnLoad called
diff --git a/test/706-jit-skip-compilation/info.txt b/test/706-jit-skip-compilation/info.txt
deleted file mode 100644
index e9ef86bfb3..0000000000
--- a/test/706-jit-skip-compilation/info.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-Regression test for the JIT crashing when compiling a method with invalid
-dead dex code. For not compilable methods we don't gather samples and we don't
-trigger JIT compilation. However kAccDontBotherCompile is not persisted in the
-oat file and so we may end up compiling a method which we shouldn't.
diff --git a/test/706-jit-skip-compilation/smali/errclass.smali b/test/706-jit-skip-compilation/smali/errclass.smali
deleted file mode 100644
index 410504cb2f..0000000000
--- a/test/706-jit-skip-compilation/smali/errclass.smali
+++ /dev/null
@@ -1,34 +0,0 @@
-# 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 LErrClass;
-
-.super Ljava/lang/Object;
-
-.method public static errMethod()J
-   .registers 8
-   const/4 v0, 0x0
-   const/4 v3, 0x0
-   aget v1, v0, v3  # v0 is null, this will alays throw and the invalid code
-                    # below will not be verified.
-   move v3, v4
-   move-wide/from16 v6, v2  # should trigger a verification error if verified as
-                            # v3 is a single register but used as a pair here.
-   return v6
-.end method
-
-# Add a field to work around demerger bug b/18051191.
-#   Failure to verify dex file '...': Offset(552) should be zero when size is zero for field-ids.
-.field private a:I
diff --git a/test/706-jit-skip-compilation/src/Main.java b/test/706-jit-skip-compilation/src/Main.java
deleted file mode 100644
index aa847248d6..0000000000
--- a/test/706-jit-skip-compilation/src/Main.java
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * 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;
-
-public class Main {
-  public static void main(String[] args) throws Exception {
-    System.loadLibrary(args[0]);
-    Class<?> c = Class.forName("ErrClass");
-    Method m = c.getMethod("errMethod");
-
-    // Print the counter before invokes. The golden file expects this to be 0.
-    int hotnessCounter = getHotnessCounter(c, "errMethod");
-    if (hotnessCounter != 0) {
-      throw new RuntimeException("Unexpected hotnessCounter: " + hotnessCounter);
-    }
-
-    // Loop enough to make sure the interpreter reports invocations count.
-    long result = 0;
-    for (int i = 0; i < 10000; i++) {
-      try {
-        result += (Long)m.invoke(null);
-        hotnessCounter = getHotnessCounter(c, "errMethod");
-        if (hotnessCounter != 0) {
-          throw new RuntimeException(
-            "Unexpected hotnessCounter: " + hotnessCounter);
-        }
-
-      } catch (InvocationTargetException e) {
-        if (!(e.getCause() instanceof NullPointerException)) {
-          throw e;
-        }
-      }
-    }
-
-    // Not compilable methods should not increase their hotness counter.
-    if (hotnessCounter != 0) {
-      throw new RuntimeException("Unexpected hotnessCounter: " + hotnessCounter);
-    }
-  }
-
-  public static native int getHotnessCounter(Class cls, String method_name);
-}