From e7b30940e1459c50b003edb3909eeeb919cab3d5 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Wed, 27 May 2009 12:42:38 -0700 Subject: [PATCH] Another fix for external bug 2711 (over-eager conflicting class rejection). The validateSuperDescriptors() test also checks for conflicts with interface classes, and wasn't doing that quite right. We need to compare every method declared by an interface against the class' implementation of it. Methods implemented by superclasses are now tested in the context of the superclass, not the current class. This is a one-word fix + comments and new/updated tests. --- tests/068-classloader/expected.txt | 1 + tests/068-classloader/src-ex/GetDoubled.java | 27 ++++++++++++++++ tests/068-classloader/src/BaseOkay.java | 10 +++++- tests/068-classloader/src/IGetDoubled.java | 23 +++++++++++++ tests/068-classloader/src/Main.java | 48 ++++++++++++++++++++++++++++ vm/oo/Class.c | 12 +++++-- 6 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 tests/068-classloader/src-ex/GetDoubled.java create mode 100644 tests/068-classloader/src/IGetDoubled.java diff --git a/tests/068-classloader/expected.txt b/tests/068-classloader/expected.txt index 0371e0770..205623457 100644 --- a/tests/068-classloader/expected.txt +++ b/tests/068-classloader/expected.txt @@ -5,6 +5,7 @@ Got expected CNFE/IAE #2 Got expected CNFE/IAE #3 Got expected LinkageError on DE Got DEO result DoubledExtendOkay 1 +Got LinkageError on GD Ctor: doubled implement, type 1 DoubledImplement one Got LinkageError on DI (early) diff --git a/tests/068-classloader/src-ex/GetDoubled.java b/tests/068-classloader/src-ex/GetDoubled.java new file mode 100644 index 000000000..fed70ed05 --- /dev/null +++ b/tests/068-classloader/src-ex/GetDoubled.java @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2008 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. + */ + +/* + * The interface we implement was declared in a different class loader, + * which means the DoubledExtend we return is not the one it was declared + * to return. + */ +public class GetDoubled implements IGetDoubled { + public DoubledExtend getDoubled() { + return new DoubledExtend(); + } +} + diff --git a/tests/068-classloader/src/BaseOkay.java b/tests/068-classloader/src/BaseOkay.java index 6f0d5080d..c5c3f7aa7 100644 --- a/tests/068-classloader/src/BaseOkay.java +++ b/tests/068-classloader/src/BaseOkay.java @@ -17,7 +17,7 @@ /** * Common base class. */ -public class BaseOkay { +public class BaseOkay implements IDoubledExtendOkay { public BaseOkay() {} public DoubledExtendOkay getExtended() { @@ -29,3 +29,11 @@ public class BaseOkay { } } +/** + * Interface that declares the not-overridden method. This exists to ensure + * that the existence of an interface doesn't trip the check. + */ +interface IDoubledExtendOkay { + public DoubledExtendOkay getExtended(); +} + diff --git a/tests/068-classloader/src/IGetDoubled.java b/tests/068-classloader/src/IGetDoubled.java new file mode 100644 index 000000000..da7f806a8 --- /dev/null +++ b/tests/068-classloader/src/IGetDoubled.java @@ -0,0 +1,23 @@ +/* + * 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. + */ + +/** + * Interface, loaded from one loader, used from another. + */ +public interface IGetDoubled { + public DoubledExtend getDoubled(); +} + diff --git a/tests/068-classloader/src/Main.java b/tests/068-classloader/src/Main.java index e2e6038c9..24abfeeef 100644 --- a/tests/068-classloader/src/Main.java +++ b/tests/068-classloader/src/Main.java @@ -52,6 +52,7 @@ public class Main { testExtend(loader); testExtendOkay(loader); + testInterface(loader); testImplement(loader); testIfaceImplement(loader); } @@ -224,6 +225,53 @@ public class Main { } /** + * Try to access a doubled class through a class that implements + * an interface declared in a different class. + */ + static void testInterface(ClassLoader loader) { + Class getDoubledClass; + Object obj; + + /* get the "alternate" version of GetDoubled */ + try { + getDoubledClass = loader.loadClass("GetDoubled"); + } catch (ClassNotFoundException cnfe) { + System.err.println("loadClass failed: " + cnfe); + return; + } + + /* instantiate */ + try { + obj = getDoubledClass.newInstance(); + } catch (InstantiationException ie) { + System.err.println("newInstance failed: " + ie); + return; + } catch (IllegalAccessException iae) { + System.err.println("newInstance failed: " + iae); + return; + } catch (LinkageError le) { + // Dalvik bails here + System.err.println("Got LinkageError on GD"); + return; + } + + /* + * Cast the object to the interface, and try to use it. + */ + IGetDoubled iface = (IGetDoubled) obj; + try { + /* "de" will be the wrong variety of DoubledExtend */ + DoubledExtend de = iface.getDoubled(); + // reference impl bails here + String str = de.getStr(); + } catch (LinkageError le) { + System.err.println("Got LinkageError on GD"); + return; + } + System.err.println("Should have failed by now on GetDoubled"); + } + + /** * Test a doubled class that implements a common interface. */ static void testImplement(ClassLoader loader) { diff --git a/vm/oo/Class.c b/vm/oo/Class.c index 0adced8b2..999a1a134 100644 --- a/vm/oo/Class.c +++ b/vm/oo/Class.c @@ -4053,6 +4053,9 @@ static bool validateSuperDescriptors(const ClassObject* clazz) * We need to do this even for the stuff inherited from Object, * because it's possible that the new class loader has redefined * a basic class like String. + * + * We don't need to check stuff defined in a superclass because + * it was checked when the superclass was loaded. */ const Method* meth; @@ -4075,7 +4078,12 @@ static bool validateSuperDescriptors(const ClassObject* clazz) } /* - * Check all interfaces we implement. + * Check the methods defined by this class against the interfaces it + * implements. If we inherited the implementation from a superclass, + * we have to check it against the superclass (which might be in a + * different class loader). If the superclass also implements the + * interface, we could skip the check since by definition it was + * performed when the class was loaded. */ for (i = 0; i < clazz->iftableCount; i++) { const InterfaceEntry* iftable = &clazz->iftable[i]; @@ -4091,7 +4099,7 @@ static bool validateSuperDescriptors(const ClassObject* clazz) vtableIndex = iftable->methodIndexArray[j]; meth = clazz->vtable[vtableIndex]; - if (!checkMethodDescriptorClasses(meth, iface, clazz)) { + if (!checkMethodDescriptorClasses(meth, iface, meth->clazz)) { LOGW("Method mismatch: %s in %s (cl=%p) and " "iface %s (cl=%p)\n", meth->name, clazz->descriptor, clazz->classLoader, -- 2.11.0