Don't remove type checks if we need to perform an access check.

Change-Id: I9b9e07c7524e96ece8dc089c8379631c2f9e3320

6 files changed, 119 insertions, 2 deletions

diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index 8a0c8c2199..078d8e5194 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -242,6 +242,12 @@ static bool TypeCheckHasKnownOutcome(HLoadClass* klass, HInstruction* object, bo
 
 void InstructionSimplifierVisitor::VisitCheckCast(HCheckCast* check_cast) {
   HInstruction* object = check_cast->InputAt(0);
+  HLoadClass* load_class = check_cast->InputAt(1)->AsLoadClass();
+  if (load_class->NeedsAccessCheck()) {
+    // If we need to perform an access check we cannot remove the instruction.
+    return;
+  }
+
   if (CanEnsureNotNullAt(object, check_cast)) {
     check_cast->ClearMustDoNullCheck();
   }
@@ -255,7 +261,6 @@ void InstructionSimplifierVisitor::VisitCheckCast(HCheckCast* check_cast) {
   }
 
   bool outcome;
-  HLoadClass* load_class = check_cast->InputAt(1)->AsLoadClass();
   if (TypeCheckHasKnownOutcome(load_class, object, &outcome)) {
     if (outcome) {
       check_cast->GetBlock()->RemoveInstruction(check_cast);
@@ -277,6 +282,12 @@ void InstructionSimplifierVisitor::VisitCheckCast(HCheckCast* check_cast) {
 
 void InstructionSimplifierVisitor::VisitInstanceOf(HInstanceOf* instruction) {
   HInstruction* object = instruction->InputAt(0);
+  HLoadClass* load_class = instruction->InputAt(1)->AsLoadClass();
+  if (load_class->NeedsAccessCheck()) {
+    // If we need to perform an access check we cannot remove the instruction.
+    return;
+  }
+
   bool can_be_null = true;
   if (CanEnsureNotNullAt(object, instruction)) {
     can_be_null = false;
@@ -292,7 +303,6 @@ void InstructionSimplifierVisitor::VisitInstanceOf(HInstanceOf* instruction) {
   }
 
   bool outcome;
-  HLoadClass* load_class = instruction->InputAt(1)->AsLoadClass();
   if (TypeCheckHasKnownOutcome(load_class, object, &outcome)) {
     if (outcome && can_be_null) {
       // Type test will succeed, we just need a null test.
diff --git a/test/536-checker-needs-access-check/expected.txt b/test/536-checker-needs-access-check/expected.txt
new file mode 100644
index 0000000000..7dc7a33fe5
--- /dev/null
+++ b/test/536-checker-needs-access-check/expected.txt
@@ -0,0 +1,3 @@
+Got expected error instanceof
+Got expected error instanceof null
+Got expected error checkcast null
diff --git a/test/536-checker-needs-access-check/info.txt b/test/536-checker-needs-access-check/info.txt
new file mode 100644
index 0000000000..3413cf3625
--- /dev/null
+++ b/test/536-checker-needs-access-check/info.txt
@@ -0,0 +1 @@
+Verifies that we don't remove type checks when we need to check for access rights.
diff --git a/test/536-checker-needs-access-check/src/Main.java b/test/536-checker-needs-access-check/src/Main.java
new file mode 100644
index 0000000000..8b19dafd2a
--- /dev/null
+++ b/test/536-checker-needs-access-check/src/Main.java
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2009 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 other.InaccessibleClass;
+
+public class Main {
+    public static final boolean VERBOSE = false;
+
+    public static void main(String[] args) {
+        try {
+            testInstanceOf();
+        } catch (IllegalAccessError e) {
+            System.out.println("Got expected error instanceof");
+        }
+
+         try {
+            testInstanceOfNull();
+        } catch (IllegalAccessError e) {
+            System.out.println("Got expected error instanceof null");
+        }
+
+        try {
+            testCheckCastNull();
+        } catch (IllegalAccessError e) {
+            System.out.println("Got expected error checkcast null");
+        }
+    }
+
+    /// CHECK-START: boolean Main.testInstanceOf() register (after)
+    /// CHECK: InstanceOf
+    public static boolean testInstanceOf() {
+        return ic instanceof InaccessibleClass;
+    }
+
+    /// CHECK-START: boolean Main.testInstanceOfNull() register (after)
+    /// CHECK: InstanceOf
+    public static boolean testInstanceOfNull() {
+        return null instanceof InaccessibleClass;
+    }
+
+    // TODO: write a test for for CheckCast with not null constant (after RTP can parse arguments).
+
+    /// CHECK-START: other.InaccessibleClass Main.testCheckCastNull() register (after)
+    /// CHECK: CheckCast
+    public static InaccessibleClass testCheckCastNull() {
+        return (InaccessibleClass) null;
+    }
+
+    public static InaccessibleClass ic;
+}
diff --git a/test/536-checker-needs-access-check/src/other/InaccessibleClass.java b/test/536-checker-needs-access-check/src/other/InaccessibleClass.java
new file mode 100644
index 0000000000..c49593da75
--- /dev/null
+++ b/test/536-checker-needs-access-check/src/other/InaccessibleClass.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2009 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.
+ */
+
+package other;
+
+public class InaccessibleClass {
+}
diff --git a/test/536-checker-needs-access-check/src2/other/InaccessibleClass.java b/test/536-checker-needs-access-check/src2/other/InaccessibleClass.java
new file mode 100644
index 0000000000..ac804ac00a
--- /dev/null
+++ b/test/536-checker-needs-access-check/src2/other/InaccessibleClass.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2009 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.
+ */
+
+package other;
+
+/*package*/ class InaccessibleClass {
+}