Make verifier field/method resolution conformant.
The VM spec spells out a particular way to look for fields. It assumes
that static and virtual fields are piled together into a single pool,
so it makes sense to scan through both kinds when resolving. In Dalvik,
field definitions are separated by scope, so we can save a little time
by only searching through the appropriate list.
It turns out that you can create a situation where a field with the same
name is available in both static and virtual forms in the same class.
javac won't let you do this in a single class, but with separate
compilation and inheritance you can pull it off. In these situations,
Dalvik can do the wrong thing. For example, if you ask for a static
field, Dalvik will happily use the static field from a superclass without
realizing that there's an instance field with the same name in the current
class. It's supposed to find the instance field, realize that it's not
static, and throw an exception.
This change updates the verifier to do an "untyped" scan like the VM
spec wants. Problematic situations are identifed and result in an
"incompatible class change" exception.
This does not alter "direct" method lookups (constructors, private
methods).
I also altered the annotation "ambiguous" method lookup to use the new
function, since that's probably the desired behavior there as well.
diff --git a/vm/analysis/DexOptimize.c b/vm/analysis/DexOptimize.c
index 025e7cb..b3e2d40 100644
--- a/vm/analysis/DexOptimize.c
+++ b/vm/analysis/DexOptimize.c
@@ -1765,7 +1765,7 @@
return NULL;
}
- resField = dvmFindInstanceFieldHier(resClass,
+ resField = (InstField*)dvmFindFieldHier(resClass,
dexStringById(pDvmDex->pDexFile, pFieldId->nameIdx),
dexStringByTypeIdx(pDvmDex->pDexFile, pFieldId->typeIdx));
if (resField == NULL) {
@@ -1776,6 +1776,14 @@
*pFailure = VERIFY_ERROR_NO_FIELD;
return NULL;
}
+ if (dvmIsStaticField(&resField->field)) {
+ LOGD("DexOpt: wanted instance, got static for field %s.%s\n",
+ resClass->descriptor,
+ dexStringById(pDvmDex->pDexFile, pFieldId->nameIdx));
+ if (pFailure != NULL)
+ *pFailure = VERIFY_ERROR_CLASS_CHANGE;
+ return NULL;
+ }
/*
* Add it to the resolved table so we're faster on the next lookup.
@@ -1830,7 +1838,7 @@
return NULL;
}
- resField = dvmFindStaticFieldHier(resClass,
+ resField = (StaticField*)dvmFindFieldHier(resClass,
dexStringById(pDvmDex->pDexFile, pFieldId->nameIdx),
dexStringByTypeIdx(pDvmDex->pDexFile, pFieldId->typeIdx));
if (resField == NULL) {
@@ -1839,6 +1847,14 @@
*pFailure = VERIFY_ERROR_NO_FIELD;
return NULL;
}
+ if (!dvmIsStaticField(&resField->field)) {
+ LOGD("DexOpt: wanted static, got instance for field %s.%s\n",
+ resClass->descriptor,
+ dexStringById(pDvmDex->pDexFile, pFieldId->nameIdx));
+ if (pFailure != NULL)
+ *pFailure = VERIFY_ERROR_CLASS_CHANGE;
+ return NULL;
+ }
/*
* Add it to the resolved table so we're faster on the next lookup.
@@ -1919,7 +1935,9 @@
DvmDex* pDvmDex = referrer->pDvmDex;
Method* resMethod;
- assert(methodType != METHOD_INTERFACE);
+ assert(methodType == METHOD_DIRECT ||
+ methodType == METHOD_VIRTUAL ||
+ methodType == METHOD_STATIC);
LOGVV("--- resolving method %u (referrer=%s)\n", methodIdx,
referrer->descriptor);
@@ -1961,11 +1979,9 @@
if (methodType == METHOD_DIRECT) {
resMethod = dvmFindDirectMethod(resClass,
dexStringById(pDvmDex->pDexFile, pMethodId->nameIdx), &proto);
- } else if (methodType == METHOD_STATIC) {
- resMethod = dvmFindDirectMethodHier(resClass,
- dexStringById(pDvmDex->pDexFile, pMethodId->nameIdx), &proto);
} else {
- resMethod = dvmFindVirtualMethodHier(resClass,
+ /* METHOD_STATIC or METHOD_VIRTUAL */
+ resMethod = dvmFindMethodHier(resClass,
dexStringById(pDvmDex->pDexFile, pMethodId->nameIdx), &proto);
}
@@ -1976,6 +1992,23 @@
*pFailure = VERIFY_ERROR_NO_METHOD;
return NULL;
}
+ if (methodType == METHOD_STATIC) {
+ if (!dvmIsStaticMethod(resMethod)) {
+ LOGD("DexOpt: wanted static, got instance for method %s.%s\n",
+ resClass->descriptor, resMethod->name);
+ if (pFailure != NULL)
+ *pFailure = VERIFY_ERROR_CLASS_CHANGE;
+ return NULL;
+ }
+ } else if (methodType == METHOD_VIRTUAL) {
+ if (dvmIsStaticMethod(resMethod)) {
+ LOGD("DexOpt: wanted instance, got static for method %s.%s\n",
+ resClass->descriptor, resMethod->name);
+ if (pFailure != NULL)
+ *pFailure = VERIFY_ERROR_CLASS_CHANGE;
+ return NULL;
+ }
+ }
/* see if this is a pure-abstract method */
if (dvmIsAbstractMethod(resMethod) && !dvmIsAbstractClass(resClass)) {
diff --git a/vm/oo/Object.c b/vm/oo/Object.c
index 96493f7..eff0983 100644
--- a/vm/oo/Object.c
+++ b/vm/oo/Object.c
@@ -150,6 +150,58 @@
}
/*
+ * Find a matching field, in this class or a superclass.
+ *
+ * We scan both the static and instance field lists in the class. If it's
+ * not found there, we check the direct interfaces, and then recursively
+ * scan the superclasses. This is the order prescribed in the VM spec
+ * (v2 5.4.3.2).
+ *
+ * In most cases we know that we're looking for either a static or an
+ * instance field and there's no value in searching through both types.
+ * During verification we need to recognize and reject certain unusual
+ * situations, and we won't see them unless we walk the lists this way.
+ */
+Field* dvmFindFieldHier(const ClassObject* clazz, const char* fieldName,
+ const char* signature)
+{
+ Field* pField;
+
+ /*
+ * Search for a match in the current class. Which set we scan first
+ * doesn't really matter.
+ */
+ pField = (Field*) dvmFindStaticField(clazz, fieldName, signature);
+ if (pField != NULL)
+ return pField;
+ pField = (Field*) dvmFindInstanceField(clazz, fieldName, signature);
+ if (pField != NULL)
+ return pField;
+
+ /*
+ * See if it's in any of our interfaces. We don't check interfaces
+ * inherited from the superclass yet.
+ */
+ int i = 0;
+ if (clazz->super != NULL) {
+ assert(clazz->iftableCount >= clazz->super->iftableCount);
+ i = clazz->super->iftableCount;
+ }
+ for ( ; i < clazz->iftableCount; i++) {
+ ClassObject* iface = clazz->iftable[i].clazz;
+ pField = (Field*) dvmFindStaticField(iface, fieldName, signature);
+ if (pField != NULL)
+ return pField;
+ }
+
+ if (clazz->super != NULL)
+ return dvmFindFieldHier(clazz->super, fieldName, signature);
+ else
+ return NULL;
+}
+
+
+/*
* Compare the given name, return type, and argument types with the contents
* of the given method. This returns 0 if they are equal and non-zero if not.
*/
@@ -365,27 +417,34 @@
/*
* Look for a match in the given clazz. Returns the match if found
* or NULL if not.
+ *
+ * "wantedType" should be METHOD_VIRTUAL or METHOD_DIRECT to indicate the
+ * list to search through. If the match can come from either list, use
+ * MATCH_UNKNOWN to scan both.
*/
static Method* findMethodInListByProto(const ClassObject* clazz,
- bool findVirtual, bool isHier, const char* name, const DexProto* proto)
+ MethodType wantedType, bool isHier, const char* name, const DexProto* proto)
{
while (clazz != NULL) {
- Method* methods;
- size_t methodCount;
- size_t i;
+ int i;
- if (findVirtual) {
- methods = clazz->virtualMethods;
- methodCount = clazz->virtualMethodCount;
- } else {
- methods = clazz->directMethods;
- methodCount = clazz->directMethodCount;
+ /*
+ * Check the virtual and/or direct method lists.
+ */
+ if (wantedType == METHOD_VIRTUAL || wantedType == METHOD_UNKNOWN) {
+ for (i = 0; i < clazz->virtualMethodCount; i++) {
+ Method* method = &clazz->virtualMethods[i];
+ if (dvmCompareNameProtoAndMethod(name, proto, method) == 0) {
+ return method;
+ }
+ }
}
-
- for (i = 0; i < methodCount; i++) {
- Method* method = &methods[i];
- if (dvmCompareNameProtoAndMethod(name, proto, method) == 0) {
- return method;
+ if (wantedType == METHOD_DIRECT || wantedType == METHOD_UNKNOWN) {
+ for (i = 0; i < clazz->directMethodCount; i++) {
+ Method* method = &clazz->directMethods[i];
+ if (dvmCompareNameProtoAndMethod(name, proto, method) == 0) {
+ return method;
+ }
}
}
@@ -452,7 +511,8 @@
Method* dvmFindVirtualMethod(const ClassObject* clazz, const char* methodName,
const DexProto* proto)
{
- return findMethodInListByProto(clazz, true, false, methodName, proto);
+ return findMethodInListByProto(clazz, METHOD_VIRTUAL, false, methodName,
+ proto);
}
/*
@@ -477,7 +537,8 @@
Method* dvmFindVirtualMethodHier(const ClassObject* clazz,
const char* methodName, const DexProto* proto)
{
- return findMethodInListByProto(clazz, true, true, methodName, proto);
+ return findMethodInListByProto(clazz, METHOD_VIRTUAL, true, methodName,
+ proto);
}
/*
@@ -514,7 +575,8 @@
Method* dvmFindDirectMethod(const ClassObject* clazz, const char* methodName,
const DexProto* proto)
{
- return findMethodInListByProto(clazz, false, false, methodName, proto);
+ return findMethodInListByProto(clazz, METHOD_DIRECT, false, methodName,
+ proto);
}
/*
@@ -526,10 +588,32 @@
Method* dvmFindDirectMethodHier(const ClassObject* clazz,
const char* methodName, const DexProto* proto)
{
- return findMethodInListByProto(clazz, false, true, methodName, proto);
+ return findMethodInListByProto(clazz, METHOD_DIRECT, true, methodName,
+ proto);
}
/*
+ * Find a virtual or static method in a class. If we don't find it, try the
+ * superclass. This is compatible with the VM spec (v2 5.4.3.3) method
+ * search order, but it stops short of scanning through interfaces (which
+ * should be done after this function completes).
+ *
+ * In most cases we know that we're looking for either a static or an
+ * instance field and there's no value in searching through both types.
+ * During verification we need to recognize and reject certain unusual
+ * situations, and we won't see them unless we walk the lists this way.
+ *
+ * Returns NULL if the method can't be found. (Does not throw an exception.)
+ */
+Method* dvmFindMethodHier(const ClassObject* clazz, const char* methodName,
+ const DexProto* proto)
+{
+ return findMethodInListByProto(clazz, METHOD_UNKNOWN, true, methodName,
+ proto);
+}
+
+
+/*
* We have a method pointer for a method in "clazz", but it might be
* pointing to a method in a derived class. We want to find the actual entry
* from the class' vtable. If "clazz" is an interface, we have to do a
diff --git a/vm/oo/Object.h b/vm/oo/Object.h
index 0bf2728..b8236db 100644
--- a/vm/oo/Object.h
+++ b/vm/oo/Object.h
@@ -582,6 +582,8 @@
const char* methodName, const DexProto* proto);
Method* dvmFindVirtualMethodHier(const ClassObject* clazz,
const char* methodName, const DexProto* proto);
+Method* dvmFindMethodHier(const ClassObject* clazz, const char* methodName,
+ const DexProto* proto);
/*
* Find the implementation of "meth" in "clazz".
@@ -611,6 +613,8 @@
const char* fieldName, const char* signature);
StaticField* dvmFindStaticFieldHier(const ClassObject* clazz,
const char* fieldName, const char* signature);
+Field* dvmFindFieldHier(const ClassObject* clazz, const char* fieldName,
+ const char* signature);
/*
* Find a field and return the byte offset from the object pointer. Only
diff --git a/vm/reflect/Annotation.c b/vm/reflect/Annotation.c
index c07c602..109c7fb 100644
--- a/vm/reflect/Annotation.c
+++ b/vm/reflect/Annotation.c
@@ -330,11 +330,12 @@
resMethod = dvmFindDirectMethod(resClass, name, &proto);
} else {
/*
- * Try both lists, and scan up the tree.
+ * Do a hierarchical scan for direct and virtual methods.
+ *
+ * This uses the search order from the VM spec (v2 5.4.3.3), which
+ * seems appropriate here.
*/
- resMethod = dvmFindVirtualMethodHier(resClass, name, &proto);
- if (resMethod == NULL)
- resMethod = dvmFindDirectMethodHier(resClass, name, &proto);
+ resMethod = dvmFindMethodHier(resClass, name, &proto);
}
return resMethod;
