Fix for external bug 2711, over-eager conflicting class rejection.
The validateSuperDescriptors() test added in Cupcake (or thereabouts) was
checking things that didn't need to be checked, and rejecting classes that
didn't need to be rejected.
Expanded test 068 to include a doubled-but-okay test.
Updates FancyLoader to use a single copy of the DexFile.
diff --git a/tests/068-classloader/expected.txt b/tests/068-classloader/expected.txt
index 0ca8862..0371e07 100644
--- a/tests/068-classloader/expected.txt
+++ b/tests/068-classloader/expected.txt
@@ -4,6 +4,7 @@
Got expected CNFE/IAE #2
Got expected CNFE/IAE #3
Got expected LinkageError on DE
+Got DEO result DoubledExtendOkay 1
Ctor: doubled implement, type 1
DoubledImplement one
Got LinkageError on DI (early)
diff --git a/tests/068-classloader/src-ex/DoubledExtendOkay.java b/tests/068-classloader/src-ex/DoubledExtendOkay.java
new file mode 100644
index 0000000..766fa8e
--- /dev/null
+++ b/tests/068-classloader/src-ex/DoubledExtendOkay.java
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+/**
+ * "Okay" doubled sub-class, form #2.
+ */
+public class DoubledExtendOkay extends BaseOkay {
+ public DoubledExtendOkay() {
+ //System.out.println("Ctor: doubled extend okay, type 2");
+ }
+
+ /*
+ @Override
+ public DoubledExtendOkay getExtended() {
+ //System.out.println("getExtended 2");
+ return new DoubledExtendOkay();
+ }
+ */
+
+ public String getStr() {
+ return "DoubledExtendOkay 2";
+ }
+}
+
diff --git a/tests/068-classloader/src/BaseOkay.java b/tests/068-classloader/src/BaseOkay.java
new file mode 100644
index 0000000..6f0d508
--- /dev/null
+++ b/tests/068-classloader/src/BaseOkay.java
@@ -0,0 +1,31 @@
+/*
+ * 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.
+ */
+
+/**
+ * Common base class.
+ */
+public class BaseOkay {
+ public BaseOkay() {}
+
+ public DoubledExtendOkay getExtended() {
+ return new DoubledExtendOkay();
+ }
+
+ public static String doStuff(DoubledExtendOkay dt) {
+ return dt.getStr();
+ }
+}
+
diff --git a/tests/068-classloader/src/DoubledExtendOkay.java b/tests/068-classloader/src/DoubledExtendOkay.java
new file mode 100644
index 0000000..e8ff404
--- /dev/null
+++ b/tests/068-classloader/src/DoubledExtendOkay.java
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+/**
+ * "Okay" doubled sub-class, form #1.
+ */
+public class DoubledExtendOkay extends BaseOkay {
+ public DoubledExtendOkay() {
+ //System.out.println("Ctor: doubled extend okay, type 1");
+ }
+
+ /*
+ @Override
+ public DoubledExtendOkay getExtended() {
+ System.out.println("getExtended 1");
+ return new DoubledExtendOkay();
+ }
+ */
+
+ public String getStr() {
+ return "DoubledExtendOkay 1";
+ }
+}
+
diff --git a/tests/068-classloader/src/FancyLoader.java b/tests/068-classloader/src/FancyLoader.java
index 491fd5c..1daf155 100644
--- a/tests/068-classloader/src/FancyLoader.java
+++ b/tests/068-classloader/src/FancyLoader.java
@@ -1,4 +1,18 @@
-// Copyright 2008 The Android Open Source Project
+/*
+ * 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.
+ */
import java.io.File;
import java.io.FileNotFoundException;
@@ -29,6 +43,8 @@
/* on Dalvik, this is a DexFile; otherwise, it's null */
private Class mDexClass;
+ private Object mDexFile;
+
/**
* Construct FancyLoader, grabbing a reference to the DexFile class
* if we're running under Dalvik.
@@ -64,26 +80,29 @@
private Class<?> findClassDalvik(String name)
throws ClassNotFoundException {
- Constructor ctor;
- Object dexFile;
+ if (mDexFile == null) {
+ synchronized (FancyLoader.class) {
+ Constructor ctor;
+ /*
+ * Construct a DexFile object through reflection.
+ */
+ try {
+ ctor = mDexClass.getConstructor(new Class[] {String.class});
+ } catch (NoSuchMethodException nsme) {
+ throw new ClassNotFoundException("getConstructor failed",
+ nsme);
+ }
- /*
- * Construct a DexFile object through reflection.
- */
- try {
- ctor = mDexClass.getConstructor(new Class[] { String.class });
- } catch (NoSuchMethodException nsme) {
- throw new ClassNotFoundException("getConstructor failed", nsme);
- }
-
- try {
- dexFile = ctor.newInstance(DEX_FILE);
- } catch (InstantiationException ie) {
- throw new ClassNotFoundException("newInstance failed", ie);
- } catch (IllegalAccessException iae) {
- throw new ClassNotFoundException("newInstance failed", iae);
- } catch (InvocationTargetException ite) {
- throw new ClassNotFoundException("newInstance failed", ite);
+ try {
+ mDexFile = ctor.newInstance(DEX_FILE);
+ } catch (InstantiationException ie) {
+ throw new ClassNotFoundException("newInstance failed", ie);
+ } catch (IllegalAccessException iae) {
+ throw new ClassNotFoundException("newInstance failed", iae);
+ } catch (InvocationTargetException ite) {
+ throw new ClassNotFoundException("newInstance failed", ite);
+ }
+ }
}
/*
@@ -99,7 +118,7 @@
}
try {
- meth.invoke(dexFile, name, this);
+ meth.invoke(mDexFile, name, this);
} catch (IllegalAccessException iae) {
throw new ClassNotFoundException("loadClass failed", iae);
} catch (InvocationTargetException ite) {
diff --git a/tests/068-classloader/src/Main.java b/tests/068-classloader/src/Main.java
index f8698be..e2e6038 100644
--- a/tests/068-classloader/src/Main.java
+++ b/tests/068-classloader/src/Main.java
@@ -51,6 +51,7 @@
testAccess3(loader);
testExtend(loader);
+ testExtendOkay(loader);
testImplement(loader);
testIfaceImplement(loader);
}
@@ -175,6 +176,54 @@
}
/**
+ * Test a doubled class that extends the base class, but is okay since
+ * it doesn't override the base class method.
+ */
+ static void testExtendOkay(ClassLoader loader) {
+ Class doubledExtendOkayClass;
+ Object obj;
+
+ /* get the "alternate" version of DoubledExtendOkay */
+ try {
+ doubledExtendOkayClass = loader.loadClass("DoubledExtendOkay");
+ } catch (ClassNotFoundException cnfe) {
+ System.err.println("loadClass failed: " + cnfe);
+ return;
+ }
+
+ /* instantiate */
+ try {
+ obj = doubledExtendOkayClass.newInstance();
+ } catch (InstantiationException ie) {
+ System.err.println("newInstance failed: " + ie);
+ return;
+ } catch (IllegalAccessException iae) {
+ System.err.println("newInstance failed: " + iae);
+ return;
+ } catch (LinkageError le) {
+ System.err.println("Got unexpected LinkageError on DEO");
+ le.printStackTrace();
+ return;
+ }
+
+ /* use the base class reference to get a CL-specific instance */
+ BaseOkay baseRef = (BaseOkay) obj;
+ DoubledExtendOkay de = baseRef.getExtended();
+
+ /* try to call through it */
+ try {
+ String result;
+
+ result = BaseOkay.doStuff(de);
+ System.out.println("Got DEO result " + result);
+ } catch (LinkageError le) {
+ System.err.println("Got unexpected LinkageError on DEO");
+ le.printStackTrace();
+ return;
+ }
+ }
+
+ /**
* 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 0d120c4..0adced8 100644
--- a/vm/oo/Class.c
+++ b/vm/oo/Class.c
@@ -4001,17 +4001,32 @@
*
* What we need to do is ensure that the classes named in the method
* descriptors in our ancestors and ourselves resolve to the same class
- * objects. The only time this matters is when the classes come from
- * different class loaders, and the resolver might come up with a
- * different answer for the same class name depending on context.
+ * objects. We can get conflicts when the classes come from different
+ * class loaders, and the resolver comes up with different results for
+ * the same class name in different contexts.
*
- * We don't need to check to see if an interface's methods match with
- * its superinterface's methods, because you can't instantiate an
- * interface and do something inappropriate with it. If interface I1
- * extends I2 and is implemented by C, and I1 and I2 are in separate
- * class loaders and have conflicting views of other classes, we will
- * catch the conflict when we process C. Anything that implements I1 is
- * doomed to failure, but we don't need to catch that while processing I1.
+ * An easy way to cause the problem is to declare a base class that uses
+ * class Foo in a method signature (e.g. as the return type). Then,
+ * define a subclass and a different version of Foo, and load them from a
+ * different class loader. If the subclass overrides the method, it will
+ * have a different concept of what Foo is than its parent does, so even
+ * though the method signature strings are identical, they actually mean
+ * different things.
+ *
+ * A call to the method through a base-class reference would be treated
+ * differently than a call to the method through a subclass reference, which
+ * isn't the way polymorphism works, so we have to reject the subclass.
+ * If the subclass doesn't override the base method, then there's no
+ * problem, because calls through base-class references and subclass
+ * references end up in the same place.
+ *
+ * We don't need to check to see if an interface's methods match with its
+ * superinterface's methods, because you can't instantiate an interface
+ * and do something inappropriate with it. If interface I1 extends I2
+ * and is implemented by C, and I1 and I2 are in separate class loaders
+ * and have conflicting views of other classes, we will catch the conflict
+ * when we process C. Anything that implements I1 is doomed to failure,
+ * but we don't need to catch that while processing I1.
*
* On failure, throws an exception and returns "false".
*/
@@ -4029,11 +4044,11 @@
clazz->classLoader != clazz->super->classLoader)
{
/*
- * Walk through every method declared in the superclass, and
- * compare resolved descriptor components. We pull the Method
- * structs out of the vtable. It doesn't matter whether we get
- * the struct from the parent or child, since we just need the
- * UTF-8 descriptor, which must match.
+ * Walk through every overridden method and compare resolved
+ * descriptor components. We pull the Method structs out of
+ * the vtable. It doesn't matter whether we get the struct from
+ * the parent or child, since we just need the UTF-8 descriptor,
+ * which must match.
*
* We need to do this even for the stuff inherited from Object,
* because it's possible that the new class loader has redefined
@@ -4046,7 +4061,9 @@
// clazz->super->descriptor, clazz->super->classLoader);
for (i = clazz->super->vtableCount - 1; i >= 0; i--) {
meth = clazz->vtable[i];
- if (!checkMethodDescriptorClasses(meth, clazz->super, clazz)) {
+ if (meth != clazz->super->vtable[i] &&
+ !checkMethodDescriptorClasses(meth, clazz->super, clazz))
+ {
LOGW("Method mismatch: %s in %s (cl=%p) and super %s (cl=%p)\n",
meth->name, clazz->descriptor, clazz->classLoader,
clazz->super->descriptor, clazz->super->classLoader);
