diff options
-rw-r--r-- | runtime/mirror/class.cc | 15 | ||||
-rw-r--r-- | runtime/verifier/method_verifier.cc | 31 | ||||
-rw-r--r-- | test/075-verification-error/expected.txt | 1 | ||||
-rw-r--r-- | test/075-verification-error/src/BadIfaceImpl.java | 17 | ||||
-rw-r--r-- | test/075-verification-error/src/BadInterface.java | 21 | ||||
-rw-r--r-- | test/075-verification-error/src/Main.java | 14 | ||||
-rw-r--r-- | test/075-verification-error/src2/BadInterface.java | 17 | ||||
-rw-r--r-- | test/162-method-resolution/expected.txt | 3 | ||||
-rw-r--r-- | test/162-method-resolution/jasmin/Test10Base.j | 25 | ||||
-rw-r--r-- | test/162-method-resolution/jasmin/Test10User.j | 36 | ||||
-rw-r--r-- | test/162-method-resolution/multidex.jpp | 10 | ||||
-rw-r--r-- | test/162-method-resolution/src/Main.java | 26 | ||||
-rw-r--r-- | test/162-method-resolution/src/Test10Interface.java | 18 |
13 files changed, 232 insertions, 2 deletions
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc index 6642869900..c775cf4613 100644 --- a/runtime/mirror/class.cc +++ b/runtime/mirror/class.cc @@ -464,14 +464,25 @@ ArtMethod* Class::FindInterfaceMethod(ObjPtr<DexCache> dex_cache, return FindInterfaceMethod(name, signature, pointer_size); } +static inline bool IsValidInheritanceCheck(ObjPtr<mirror::Class> klass, + ObjPtr<mirror::Class> declaring_class) + REQUIRES_SHARED(Locks::mutator_lock_) { + if (klass->IsArrayClass()) { + return declaring_class->IsObjectClass(); + } else if (klass->IsInterface()) { + return declaring_class->IsObjectClass() || declaring_class == klass; + } else { + return klass->IsSubClass(declaring_class); + } +} + static inline bool IsInheritedMethod(ObjPtr<mirror::Class> klass, ObjPtr<mirror::Class> declaring_class, ArtMethod& method) REQUIRES_SHARED(Locks::mutator_lock_) { DCHECK_EQ(declaring_class, method.GetDeclaringClass()); DCHECK_NE(klass, declaring_class); - DCHECK(klass->IsArrayClass() ? declaring_class->IsObjectClass() - : klass->IsSubClass(declaring_class)); + DCHECK(IsValidInheritanceCheck(klass, declaring_class)); uint32_t access_flags = method.GetAccessFlags(); if ((access_flags & (kAccPublic | kAccProtected)) != 0) { return true; diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 0c460db7da..312d8dfc18 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -3861,10 +3861,20 @@ ArtMethod* MethodVerifier::ResolveMethodAndCheckAccess( // TODO: Maybe we should not record dependency if the invoke type does not match the lookup type. VerifierDeps::MaybeRecordMethodResolution(*dex_file_, dex_method_idx, res_method); + bool must_fail = false; + // This is traditional and helps with screwy bytecode. It will tell you that, yes, a method + // exists, but that it's called incorrectly. This significantly helps debugging, as locally it's + // hard to see the differences. + // If we don't have res_method here we must fail. Just use this bool to make sure of that with a + // DCHECK. if (res_method == nullptr) { + must_fail = true; // Try to find the method also with the other type for better error reporting below // but do not store such bogus lookup result in the DexCache or VerifierDeps. if (klass->IsInterface()) { + // NB This is normally not really allowed but we want to get any static or private object + // methods for error message purposes. This will never be returned. + // TODO We might want to change the verifier to not require this. res_method = klass->FindClassMethod(dex_cache_.Get(), dex_method_idx, pointer_size); } else { // If there was an interface method with the same signature, @@ -3922,6 +3932,20 @@ ArtMethod* MethodVerifier::ResolveMethodAndCheckAccess( } } + // Check specifically for non-public object methods being provided for interface dispatch. This + // can occur if we failed to find a method with FindInterfaceMethod but later find one with + // FindClassMethod for error message use. + if (method_type == METHOD_INTERFACE && + res_method->GetDeclaringClass()->IsObjectClass() && + !res_method->IsPublic()) { + Fail(VERIFY_ERROR_NO_METHOD) << "invoke-interface " << klass->PrettyDescriptor() << "." + << dex_file_->GetMethodName(method_id) << " " + << dex_file_->GetMethodSignature(method_id) << " resolved to " + << "non-public object method " << res_method->PrettyMethod() << " " + << "but non-public Object methods are excluded from interface " + << "method resolution."; + return nullptr; + } // Check if access is allowed. if (!referrer.CanAccessMember(res_method->GetDeclaringClass(), res_method->GetAccessFlags())) { Fail(VERIFY_ERROR_ACCESS_METHOD) << "illegal method access (call " @@ -3950,6 +3974,13 @@ ArtMethod* MethodVerifier::ResolveMethodAndCheckAccess( "type of " << res_method->PrettyMethod(); return nullptr; } + // Make sure we weren't expecting to fail. + DCHECK(!must_fail) << "invoke type (" << method_type << ")" + << klass->PrettyDescriptor() << "." + << dex_file_->GetMethodName(method_id) << " " + << dex_file_->GetMethodSignature(method_id) << " unexpectedly resolved to " + << res_method->PrettyMethod() << " without error. Initially this method was " + << "not found so we were expecting to fail for some reason."; return res_method; } diff --git a/test/075-verification-error/expected.txt b/test/075-verification-error/expected.txt index 6e4f584d3a..7ccc32cbac 100644 --- a/test/075-verification-error/expected.txt +++ b/test/075-verification-error/expected.txt @@ -10,3 +10,4 @@ Got expected IllegalAccessError (smethod) Got expected IllegalAccessError (meth-class) Got expected IllegalAccessError (field-class) Got expected IllegalAccessError (meth-meth) +Got expected IncompatibleClassChangeError (interface) diff --git a/test/075-verification-error/src/BadIfaceImpl.java b/test/075-verification-error/src/BadIfaceImpl.java new file mode 100644 index 0000000000..fa2a970127 --- /dev/null +++ b/test/075-verification-error/src/BadIfaceImpl.java @@ -0,0 +1,17 @@ +/* + * Copyright (C) 2017 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 BadIfaceImpl implements BadInterface { } diff --git a/test/075-verification-error/src/BadInterface.java b/test/075-verification-error/src/BadInterface.java new file mode 100644 index 0000000000..439aba467a --- /dev/null +++ b/test/075-verification-error/src/BadInterface.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2017 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 interface BadInterface { + public default Object internalClone() { + throw new Error("Should not be called"); + } +} diff --git a/test/075-verification-error/src/Main.java b/test/075-verification-error/src/Main.java index 3f2881eb10..13aeaee7e4 100644 --- a/test/075-verification-error/src/Main.java +++ b/test/075-verification-error/src/Main.java @@ -28,6 +28,20 @@ public class Main { testClassNewInstance(); testMissingStuff(); testBadAccess(); + testBadInterfaceMethod(); + } + /** + * Try to create and invoke a non-existant interface method. + */ + static void testBadInterfaceMethod() { + BadInterface badiface = new BadIfaceImpl(); + try { + badiface.internalClone(); + } catch (IncompatibleClassChangeError icce) { + // TODO b/64274113 This should really be an NSME + System.out.println("Got expected IncompatibleClassChangeError (interface)"); + if (VERBOSE) System.out.println("--- " + icce); + } } /** diff --git a/test/075-verification-error/src2/BadInterface.java b/test/075-verification-error/src2/BadInterface.java new file mode 100644 index 0000000000..5d939cb264 --- /dev/null +++ b/test/075-verification-error/src2/BadInterface.java @@ -0,0 +1,17 @@ +/* + * Copyright (C) 2017 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 interface BadInterface { } + diff --git a/test/162-method-resolution/expected.txt b/test/162-method-resolution/expected.txt index 1bf39c90de..9b48a4cc8f 100644 --- a/test/162-method-resolution/expected.txt +++ b/test/162-method-resolution/expected.txt @@ -41,3 +41,6 @@ Test9Derived.foo() Calling Test9User2.test(): Caught java.lang.reflect.InvocationTargetException caused by java.lang.IncompatibleClassChangeError +Calling Test10User.test(): +Caught java.lang.reflect.InvocationTargetException + caused by java.lang.IncompatibleClassChangeError diff --git a/test/162-method-resolution/jasmin/Test10Base.j b/test/162-method-resolution/jasmin/Test10Base.j new file mode 100644 index 0000000000..628f38d6d5 --- /dev/null +++ b/test/162-method-resolution/jasmin/Test10Base.j @@ -0,0 +1,25 @@ +; Copyright (C) 2017 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 Test10Base +.super java/lang/Object +.implements Test10Interface + +.method public <init>()V + .limit stack 1 + .limit locals 1 + aload_0 + invokespecial java/lang/Object/<init>()V + return +.end method diff --git a/test/162-method-resolution/jasmin/Test10User.j b/test/162-method-resolution/jasmin/Test10User.j new file mode 100644 index 0000000000..6beadaba12 --- /dev/null +++ b/test/162-method-resolution/jasmin/Test10User.j @@ -0,0 +1,36 @@ +; Copyright (C) 2017 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 Test10User +.super java/lang/Object + +.method public static test()V + .limit stack 3 + .limit locals 3 + new Test10Base + dup + invokespecial Test10Base.<init>()V + invokestatic Test10User.doInvoke(LTest10Interface;)V + return +.end method + +.method public static doInvoke(LTest10Interface;)V + .limit stack 3 + .limit locals 3 + aload_0 + invokeinterface Test10Interface.clone()Ljava.lang.Object; 1 + pop + return +.end method + diff --git a/test/162-method-resolution/multidex.jpp b/test/162-method-resolution/multidex.jpp index 22e3aeeadc..5722f7f1d8 100644 --- a/test/162-method-resolution/multidex.jpp +++ b/test/162-method-resolution/multidex.jpp @@ -112,6 +112,16 @@ Test9User2: @@com.android.jack.annotations.ForceInMainDex class Test9User2 +Test10Base: + @@com.android.jack.annotations.ForceInMainDex + class Test10Base +Test10Interface: + @@com.android.jack.annotations.ForceInMainDex + class Test10Interface +Test10User: + @@com.android.jack.annotations.ForceInMainDex + class Test10User + Main: @@com.android.jack.annotations.ForceInMainDex class Main diff --git a/test/162-method-resolution/src/Main.java b/test/162-method-resolution/src/Main.java index fa95aa755c..864c87850b 100644 --- a/test/162-method-resolution/src/Main.java +++ b/test/162-method-resolution/src/Main.java @@ -36,6 +36,7 @@ public class Main { test7(); test8(); test9(); + test10(); // TODO: How to test that interface method resolution returns the unique // maximally-specific non-abstract superinterface method if there is one? @@ -376,6 +377,31 @@ public class Main { invokeUserTest("Test9User2"); } + /* + * Test10 + * ----- + * Tested function: + * public class Test10Base implements Test10Interface { } + * public interface Test10Interface { } + * Tested invokes: + * invoke-interface Test10Interface.clone()Ljava/lang/Object; from Test10Caller in first dex + * TODO b/64274113 This should throw a NSME (JLS 13.4.12) but actually throws an ICCE. + * expected: Throws NoSuchMethodError (JLS 13.4.12) + * actual: Throws IncompatibleClassChangeError + * + * This test is simulating compiling Test10Interface with "public Object clone()" method, along + * with every other class. Then we delete "clone" from Test10Interface only, which under JLS + * 13.4.12 is expected to be binary incompatible and throw a NoSuchMethodError. + * + * Files: + * jasmin/Test10Base.j - implements Test10Interface + * jasmin/Test10Interface.java - defines empty interface + * jasmin/Test10User.j - invokeinterface Test10Interface.clone()Ljava/lang/Object; + */ + private static void test10() throws Exception { + invokeUserTest("Test10User"); + } + private static void invokeUserTest(String userName) throws Exception { System.out.println("Calling " + userName + ".test():"); try { diff --git a/test/162-method-resolution/src/Test10Interface.java b/test/162-method-resolution/src/Test10Interface.java new file mode 100644 index 0000000000..3c75ea59be --- /dev/null +++ b/test/162-method-resolution/src/Test10Interface.java @@ -0,0 +1,18 @@ +/* + * Copyright (C) 2017 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 interface Test10Interface { } + |