Workaround for final fields that are written outside of constructor

Author: Christian Wimmer

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java

@@ -46,8 +46,6 @@
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.BiPredicate;
 
-import com.oracle.svm.core.jdk.InternalVMMethod;
-import com.oracle.svm.core.jdk.LambdaFormHiddenMethod;
 import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
 import org.graalvm.compiler.core.common.spi.ForeignCallDescriptor;
 import org.graalvm.compiler.core.common.spi.ForeignCallsProvider;
@@ -105,6 +103,8 @@
 import com.oracle.svm.core.hub.HubType;
 import com.oracle.svm.core.hub.ReferenceType;
 import com.oracle.svm.core.jdk.ClassLoaderSupport;
+import com.oracle.svm.core.jdk.InternalVMMethod;
+import com.oracle.svm.core.jdk.LambdaFormHiddenMethod;
 import com.oracle.svm.core.jdk.RecordSupport;
 import com.oracle.svm.core.jdk.SealedClassSupport;
 import com.oracle.svm.core.option.HostedOptionKey;
@@ -116,6 +116,7 @@
 import com.oracle.svm.hosted.code.InliningUtilities;
 import com.oracle.svm.hosted.code.UninterruptibleAnnotationChecker;
 import com.oracle.svm.hosted.heap.PodSupport;
+import com.oracle.svm.hosted.meta.HostedField;
 import com.oracle.svm.hosted.meta.HostedType;
 import com.oracle.svm.hosted.meta.HostedUniverse;
 import com.oracle.svm.hosted.phases.AnalysisGraphBuilderPhase;
@@ -157,6 +158,8 @@ public class SVMHost extends HostVM {
     private final ConcurrentMap<AnalysisMethod, Set<AnalysisType>> initializedClasses = new ConcurrentHashMap<>();
     private final ConcurrentMap<AnalysisMethod, Boolean> analysisTrivialMethods = new ConcurrentHashMap<>();
 
+    private final Set<AnalysisField> finalFieldsStoredOutsideOfConstructor = ConcurrentHashMap.newKeySet();
+
     public SVMHost(OptionValues options, ClassLoader classLoader, ClassInitializationSupport classInitializationSupport,
                     UnsafeAutomaticSubstitutionProcessor automaticSubstitutions, Platform platform, SnippetReflectionProvider originalSnippetReflection) {
         super(options, classLoader);
@@ -219,7 +222,7 @@ public void checkForbidden(AnalysisType type, AnalysisType.UsageKind kind) {
 
     @Override
     public Instance createGraphBuilderPhase(HostedProviders providers, GraphBuilderConfiguration graphBuilderConfig, OptimisticOptimizations optimisticOpts, IntrinsicContext initialIntrinsicContext) {
-        return new AnalysisGraphBuilderPhase(providers, graphBuilderConfig, optimisticOpts, initialIntrinsicContext, providers.getWordTypes());
+        return new AnalysisGraphBuilderPhase(providers, graphBuilderConfig, optimisticOpts, initialIntrinsicContext, providers.getWordTypes(), this);
     }
 
     @Override
@@ -774,4 +777,29 @@ public Comparator<? super ResolvedJavaType> getTypeComparator() {
             return HostedUniverse.TYPE_COMPARATOR.compare((HostedType) o1, (HostedType) o2);
         };
     }
+
+    /**
+     * According to the Java VM specification, final instance fields are only allowed to be written
+     * in a constructor. But some compilers violate the spec, notably the Scala compiler. This means
+     * final fields of image heap objects can be modified at image run time, and constant folding
+     * such fields at image build time would fold in the wrong value. As a workaround, we record
+     * which final instance fields are written outside of a constructor, and disable constant
+     * folding for those fields.
+     *
+     * Note that there can be races: A constant folding can happen during bytecode parsing before
+     * the store was recorded. Currently, we do not encounter that problem because constant folding
+     * only happens after static analysis. But there would not be a good solution other than
+     * aborting the image build, because a constant folding cannot be reversed.
+     */
+    public void recordFieldStore(ResolvedJavaField field, ResolvedJavaMethod method) {
+        if (!field.isStatic() && field.isFinal() && (!method.isConstructor() || !field.getDeclaringClass().equals(method.getDeclaringClass()))) {
+            AnalysisField aField = field instanceof HostedField ? ((HostedField) field).getWrapped() : (AnalysisField) field;
+            finalFieldsStoredOutsideOfConstructor.add(aField);
+        }
+    }
+
+    public boolean preventConstantFolding(ResolvedJavaField field) {
+        AnalysisField aField = field instanceof HostedField ? ((HostedField) field).getWrapped() : (AnalysisField) field;
+        return finalFieldsStoredOutsideOfConstructor.contains(aField);
+    }
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ameta/AnalysisConstantFieldProvider.java

@@ -46,7 +46,7 @@ public class AnalysisConstantFieldProvider extends SharedConstantFieldProvider {
 
     public AnalysisConstantFieldProvider(AnalysisUniverse universe, AnalysisMetaAccess metaAccess, AnalysisConstantReflectionProvider constantReflection,
                     ClassInitializationSupport classInitializationSupport) {
-        super(metaAccess, classInitializationSupport);
+        super(metaAccess, classInitializationSupport, (SVMHost) universe.hostVM());
         this.universe = universe;
         this.constantReflection = constantReflection;
     }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/HostedRuntimeConfigurationBuilder.java

@@ -106,7 +106,7 @@ protected CodeCacheProvider createCodeCacheProvider(RegisterConfig registerConfi
 
     @Override
     protected ConstantFieldProvider createConstantFieldProvider(Providers p) {
-        return new HostedConstantFieldProvider(p.getMetaAccess(), classInitializationSupport);
+        return new HostedConstantFieldProvider(p.getMetaAccess(), classInitializationSupport, universe.hostVM());
     }
 
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedConstantFieldProvider.java

@@ -29,6 +29,7 @@
 
 import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.svm.core.meta.ReadableJavaField;
+import com.oracle.svm.hosted.SVMHost;
 import com.oracle.svm.hosted.ameta.AnalysisConstantFieldProvider;
 import com.oracle.svm.hosted.classinitialization.ClassInitializationSupport;
 
@@ -38,8 +39,8 @@
 @Platforms(Platform.HOSTED_ONLY.class)
 public class HostedConstantFieldProvider extends SharedConstantFieldProvider {
 
-    public HostedConstantFieldProvider(MetaAccessProvider metaAccess, ClassInitializationSupport classInitializationSupport) {
-        super(metaAccess, classInitializationSupport);
+    public HostedConstantFieldProvider(MetaAccessProvider metaAccess, ClassInitializationSupport classInitializationSupport, SVMHost hostVM) {
+        super(metaAccess, classInitializationSupport, hostVM);
     }
 
     /**

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/SharedConstantFieldProvider.java

@@ -30,6 +30,7 @@
 
 import com.oracle.graal.pointsto.infrastructure.UniverseMetaAccess;
 import com.oracle.svm.core.meta.MethodPointer;
+import com.oracle.svm.hosted.SVMHost;
 import com.oracle.svm.hosted.classinitialization.ClassInitializationSupport;
 
 import jdk.vm.ci.meta.JavaConstant;
@@ -42,18 +43,24 @@ public abstract class SharedConstantFieldProvider extends JavaConstantFieldProvi
 
     protected final ClassInitializationSupport classInitializationSupport;
     protected final UniverseMetaAccess metaAccess;
+    protected final SVMHost hostVM;
 
-    public SharedConstantFieldProvider(MetaAccessProvider metaAccess, ClassInitializationSupport classInitializationSupport) {
+    public SharedConstantFieldProvider(MetaAccessProvider metaAccess, ClassInitializationSupport classInitializationSupport, SVMHost hostVM) {
         super(metaAccess);
         this.classInitializationSupport = classInitializationSupport;
         this.metaAccess = (UniverseMetaAccess) metaAccess;
+        this.hostVM = hostVM;
     }
 
     @Override
     public boolean isFinalField(ResolvedJavaField field, ConstantFieldTool<?> tool) {
         if (classInitializationSupport.shouldInitializeAtRuntime(field.getDeclaringClass())) {
             return false;
         }
+        if (hostVM.preventConstantFolding(field)) {
+            return false;
+        }
+
         return super.isFinalField(field, tool);
     }
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/AnalysisGraphBuilderPhase.java

@@ -24,7 +24,6 @@
  */
 package com.oracle.svm.hosted.phases;
 
-import com.oracle.graal.pointsto.infrastructure.AnalysisConstantPool;
 import org.graalvm.compiler.core.common.BootstrapMethodIntrospection;
 import org.graalvm.compiler.java.BytecodeParser;
 import org.graalvm.compiler.java.GraphBuilderPhase;
@@ -40,29 +39,39 @@
 import org.graalvm.compiler.phases.OptimisticOptimizations;
 import org.graalvm.compiler.word.WordTypes;
 
+import com.oracle.graal.pointsto.infrastructure.AnalysisConstantPool;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
 import com.oracle.svm.core.SubstrateOptions;
+import com.oracle.svm.hosted.SVMHost;
 import com.oracle.svm.util.ModuleSupport;
 
 import jdk.vm.ci.meta.JavaKind;
+import jdk.vm.ci.meta.ResolvedJavaField;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 
 public class AnalysisGraphBuilderPhase extends SharedGraphBuilderPhase {
 
+    private final SVMHost hostVM;
+
     public AnalysisGraphBuilderPhase(CoreProviders providers,
-                    GraphBuilderConfiguration graphBuilderConfig, OptimisticOptimizations optimisticOpts, IntrinsicContext initialIntrinsicContext, WordTypes wordTypes) {
+                    GraphBuilderConfiguration graphBuilderConfig, OptimisticOptimizations optimisticOpts, IntrinsicContext initialIntrinsicContext, WordTypes wordTypes, SVMHost hostVM) {
         super(providers, graphBuilderConfig, optimisticOpts, initialIntrinsicContext, wordTypes);
+        this.hostVM = hostVM;
     }
 
     @Override
     protected BytecodeParser createBytecodeParser(StructuredGraph graph, BytecodeParser parent, ResolvedJavaMethod method, int entryBCI, IntrinsicContext intrinsicContext) {
-        return new AnalysisBytecodeParser(this, graph, parent, method, entryBCI, intrinsicContext);
+        return new AnalysisBytecodeParser(this, graph, parent, method, entryBCI, intrinsicContext, hostVM);
     }
 
     public static class AnalysisBytecodeParser extends SharedBytecodeParser {
+
+        private final SVMHost hostVM;
+
         protected AnalysisBytecodeParser(GraphBuilderPhase.Instance graphBuilderInstance, StructuredGraph graph, BytecodeParser parent, ResolvedJavaMethod method, int entryBCI,
-                        IntrinsicContext intrinsicContext) {
+                        IntrinsicContext intrinsicContext, SVMHost hostVM) {
             super(graphBuilderInstance, graph, parent, method, entryBCI, intrinsicContext, true);
+            this.hostVM = hostVM;
         }
 
         @Override
@@ -125,5 +134,11 @@ protected void genInvokeDynamic(int cpi, int opcode) {
             }
             super.genInvokeDynamic(cpi, opcode);
         }
+
+        @Override
+        protected void genStoreField(ValueNode receiver, ResolvedJavaField field, ValueNode value) {
+            hostVM.recordFieldStore(field, method);
+            super.genStoreField(receiver, field, value);
+        }
     }
 }