Barrier after construction of finalizable object

If thread 1 creates an object, and thread 2 executes the finalizer,
then thread 2 must be guaranteed to see a fully-constructed object.
To this end, we need to emit a store/store barrier before returning
from the object's constructor.

We already do this for objects with final fields, so this is a
minor tweak.

While working on this I noticed that Enum overrides finalize()
(for the explicit purpose of making sure that no subclass can do
so), which the VM was taking as a signal that all enumerated types
are finalizable.  In practice this doesn't matter, since the only
instances are the enum elements themselves, and we'd only GC them
if the enum class itself went away.  However, setting it correctly
means less work for dexopt and has no measurable impact on class
loading time.

Bug 3403518

Change-Id: Ifa890b5e7ef1cda2a554ba54f25c4148272c3537
diff --git a/vm/analysis/Optimize.c b/vm/analysis/Optimize.c
index 68e4d2f..d7704a4 100644
--- a/vm/analysis/Optimize.c
+++ b/vm/analysis/Optimize.c
@@ -1125,29 +1125,32 @@
         return false;
 
     /*
-     * Check to see if the class has any final fields.  If not, we don't
-     * need to generate a barrier instruction.
+     * Check to see if the class is finalizable.  The loader sets a flag
+     * if the class or one of its superclasses overrides finalize().
      */
     const ClassObject* clazz = method->clazz;
-    int idx = clazz->ifieldCount;
-    while (--idx >= 0) {
-        if (dvmIsFinalField(&clazz->ifields[idx].field))
-            break;
-    }
-    if (idx < 0)
-        return false;
+    if (IS_CLASS_FLAG_SET(clazz, CLASS_ISFINALIZABLE))
+        return true;
 
     /*
+     * Check to see if the class has any final fields.  If not, we don't
+     * need to generate a barrier instruction.
+     *
      * In theory, we only need to do this if the method actually modifies
      * a final field.  In practice, non-constructor methods are allowed
-     * by the VM to modify final fields, and there are tools that rely on
-     * this behavior.  (The compiler does not allow it.)
+     * to modify final fields, and there are 3rd-party tools that rely on
+     * this behavior.  (The compiler does not allow it, but the VM does.)
      *
      * If we alter the verifier to restrict final-field updates to
      * constructors, we can tighten this up as well.
      */
+    int idx = clazz->ifieldCount;
+    while (--idx >= 0) {
+        if (dvmIsFinalField(&clazz->ifields[idx].field))
+            return true;
+    }
 
-    return true;
+    return false;
 }
 
 /*
diff --git a/vm/oo/Class.c b/vm/oo/Class.c
index 27907b8..2455e41 100644
--- a/vm/oo/Class.c
+++ b/vm/oo/Class.c
@@ -2077,7 +2077,20 @@
     meth->jniArgInfo = 0;
 
     if (dvmCompareNameDescriptorAndMethod("finalize", "()V", meth) == 0) {
-        SET_CLASS_FLAG(clazz, CLASS_ISFINALIZABLE);
+        /*
+         * The Enum class declares a "final" finalize() method to
+         * prevent subclasses from introducing a finalizer.  We don't
+         * want to set the finalizable flag for Enum or its subclasses,
+         * so we check for it here.
+         *
+         * We also want to avoid setting it on Object, but it's easier
+         * to just strip that out later.
+         */
+        if (clazz->classLoader != NULL ||
+            strcmp(clazz->descriptor, "Ljava/lang/Enum;") != 0)
+        {
+            SET_CLASS_FLAG(clazz, CLASS_ISFINALIZABLE);
+        }
     }
 
     pDexCode = dexGetCode(pDexFile, pDexMethod);