From 386062d13ce20d036555a9e24b73a67b4156b5cb Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Wed, 7 Oct 2015 18:55:43 +0100 Subject: [PATCH] Make sure classes with different access checks are not GVN-ed Change-Id: I89f72fef3be35a4dd9585d97d03a3150386e0891 --- compiler/optimizing/graph_visualizer.cc | 2 ++ compiler/optimizing/nodes.h | 3 ++- test/536-checker-needs-access-check/expected.txt | 1 + test/536-checker-needs-access-check/src/Main.java | 25 +++++++++++++++++++--- .../src/other/InaccessibleClass.java | 2 +- .../src/other/InaccessibleClassProxy.java | 23 ++++++++++++++++++++ .../src2/other/InaccessibleClass.java | 2 +- .../src2/other/InaccessibleClassProxy.java | 23 ++++++++++++++++++++ 8 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 test/536-checker-needs-access-check/src/other/InaccessibleClassProxy.java create mode 100644 test/536-checker-needs-access-check/src2/other/InaccessibleClassProxy.java diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc index d38f4c862..922c46a11 100644 --- a/compiler/optimizing/graph_visualizer.cc +++ b/compiler/optimizing/graph_visualizer.cc @@ -362,6 +362,8 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor { void VisitLoadClass(HLoadClass* load_class) OVERRIDE { StartAttributeStream("gen_clinit_check") << std::boolalpha << load_class->MustGenerateClinitCheck() << std::noboolalpha; + StartAttributeStream("needs_access_check") << std::boolalpha + << load_class->NeedsAccessCheck() << std::noboolalpha; } void VisitCheckCast(HCheckCast* check_cast) OVERRIDE { diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 95dd03970..f009c410f 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -4534,7 +4534,8 @@ class HLoadClass : public HExpression<1> { bool CanBeMoved() const OVERRIDE { return true; } bool InstructionDataEquals(HInstruction* other) const OVERRIDE { - return other->AsLoadClass()->type_index_ == type_index_; + return other->AsLoadClass()->type_index_ == type_index_ && + other->AsLoadClass()->needs_access_check_ == needs_access_check_; } size_t ComputeHashCode() const OVERRIDE { return type_index_; } diff --git a/test/536-checker-needs-access-check/expected.txt b/test/536-checker-needs-access-check/expected.txt index 7dc7a33fe..4acae95b7 100644 --- a/test/536-checker-needs-access-check/expected.txt +++ b/test/536-checker-needs-access-check/expected.txt @@ -1,3 +1,4 @@ Got expected error instanceof Got expected error instanceof null Got expected error checkcast null +Got expected error instanceof (keep LoadClass with access check) diff --git a/test/536-checker-needs-access-check/src/Main.java b/test/536-checker-needs-access-check/src/Main.java index 8b19dafd2..7bd49c1c8 100644 --- a/test/536-checker-needs-access-check/src/Main.java +++ b/test/536-checker-needs-access-check/src/Main.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009 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. @@ -15,10 +15,9 @@ */ import other.InaccessibleClass; +import other.InaccessibleClassProxy; public class Main { - public static final boolean VERBOSE = false; - public static void main(String[] args) { try { testInstanceOf(); @@ -37,6 +36,12 @@ public class Main { } catch (IllegalAccessError e) { System.out.println("Got expected error checkcast null"); } + + try { + testDontGvnLoadClassWithAccessChecks(new Object()); + } catch (IllegalAccessError e) { + System.out.println("Got expected error instanceof (keep LoadClass with access check)"); + } } /// CHECK-START: boolean Main.testInstanceOf() register (after) @@ -59,5 +64,19 @@ public class Main { return (InaccessibleClass) null; } + /// CHECK-START: boolean Main.testDontGvnLoadClassWithAccessChecks(java.lang.Object) inliner (before) + /// CHECK: InvokeStaticOrDirect + + /// CHECK-START: boolean Main.testDontGvnLoadClassWithAccessChecks(java.lang.Object) inliner (after) + /// CHECK-NOT: InvokeStaticOrDirect + + /// CHECK-START: boolean Main.testDontGvnLoadClassWithAccessChecks(java.lang.Object) GVN (after) + /// CHECK: LoadClass needs_access_check:false + /// CHECK: LoadClass needs_access_check:true + public static boolean testDontGvnLoadClassWithAccessChecks(Object o) { + InaccessibleClassProxy.test(o); + return ic instanceof InaccessibleClass; + } + 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 index c49593da7..de2e1d783 100644 --- a/test/536-checker-needs-access-check/src/other/InaccessibleClass.java +++ b/test/536-checker-needs-access-check/src/other/InaccessibleClass.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009 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. diff --git a/test/536-checker-needs-access-check/src/other/InaccessibleClassProxy.java b/test/536-checker-needs-access-check/src/other/InaccessibleClassProxy.java new file mode 100644 index 000000000..4c005e4df --- /dev/null +++ b/test/536-checker-needs-access-check/src/other/InaccessibleClassProxy.java @@ -0,0 +1,23 @@ +/* + * 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. + */ + +package other; + +public class InaccessibleClassProxy { + public static boolean test(Object o) { + return o instanceof 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 index ac804ac00..273226375 100644 --- a/test/536-checker-needs-access-check/src2/other/InaccessibleClass.java +++ b/test/536-checker-needs-access-check/src2/other/InaccessibleClass.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009 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. diff --git a/test/536-checker-needs-access-check/src2/other/InaccessibleClassProxy.java b/test/536-checker-needs-access-check/src2/other/InaccessibleClassProxy.java new file mode 100644 index 000000000..4c005e4df --- /dev/null +++ b/test/536-checker-needs-access-check/src2/other/InaccessibleClassProxy.java @@ -0,0 +1,23 @@ +/* + * 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. + */ + +package other; + +public class InaccessibleClassProxy { + public static boolean test(Object o) { + return o instanceof InaccessibleClass; + } +} -- 2.11.0