commit 9735865121118f255c4c15183b236f9f30e795f1
author Elliott Hughes <enh@google.com> Mon Oct 15 14:29:23 2012 -0700
committer Elliott Hughes <enh@google.com> Mon Oct 15 14:29:23 2012 -0700
tree 43399c6b9f6d920d11e006f19b53529c1631290b
parent ed94ff793680873d2c942641ccda9ea81c95b5ad

Revert the fix for incorrect package-private overriding.

This fixes FIFA 2012, the download portion of which relies on the
old bug, without breaking instrumentation tests. The problem is that
dexopt tries to quicken method calls, and gets confused by this case
where the apparent static method resolution differs from the actual
one at runtime, depending on the targetSdkVersion of the specific app.
dexopt can't make an ahead-of-time decision in a world where the rules
might change at runtime.

Bug: 7301030
Bug: 7343420
Change-Id: Iaa15611f099546b7e54279cfd6abc9b4cdcb9812

vm/oo/Class.cpp

diff --git a/vm/oo/Class.cpp b/vm/oo/Class.cpp
index eb816b0..85ac9a7 100644
--- a/vm/oo/Class.cpp
+++ b/vm/oo/Class.cpp
@@ -2914,25 +2914,29 @@
                 Method* superMeth = clazz->vtable[si];
 
                 if (dvmCompareMethodNamesAndProtos(localMeth, superMeth) == 0) {
-                    if (dvmCheckMethodAccess(clazz, superMeth)) {
-                        /* verify */
-                        if (dvmIsFinalMethod(superMeth)) {
-                            ALOGW("Method %s.%s overrides final %s.%s",
-                                localMeth->clazz->descriptor, localMeth->name,
-                                superMeth->clazz->descriptor, superMeth->name);
-                            goto bail;
-                        }
-                        clazz->vtable[si] = localMeth;
-                        localMeth->methodIndex = (u2) si;
-                        //ALOGV("+++   override %s.%s (slot %d)",
-                        //    clazz->descriptor, localMeth->name, si);
-                        break;
-                    } else {
-                        ALOGW("in older versions of dalvik, method %s.%s would have incorrectly "
-                              "overridden package-private method with same name in %s",
+                    // We should have an access check here, but some apps rely on us not
+                    // checking access: http://b/7301030
+                    bool isAccessible = dvmCheckMethodAccess(clazz, superMeth);
+                    if (dvmIsFinalMethod(superMeth)) {
+                        ALOGE("Method %s.%s overrides final %s.%s",
+                              localMeth->clazz->descriptor, localMeth->name,
+                              superMeth->clazz->descriptor, superMeth->name);
+                        goto bail;
+                    }
+
+                    // Warn if we just spotted code relying on this bug...
+                    if (!isAccessible) {
+                        ALOGW("method %s.%s incorrectly overrides "
+                              "package-private method with same name in %s",
                               localMeth->clazz->descriptor, localMeth->name,
                               superMeth->clazz->descriptor);
                     }
+
+                    clazz->vtable[si] = localMeth;
+                    localMeth->methodIndex = (u2) si;
+                    //ALOGV("+++   override %s.%s (slot %d)",
+                    //    clazz->descriptor, localMeth->name, si);
+                    break;
                 }
             }