[GR-61534] Ensure static word fields are not unsafe accessed.

teshull · teshull · commit 01e61c31ef8b · 2025-02-13T08:21:54.000Z
PullRequest: graal/19860
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java
@@ -562,6 +562,7 @@ public boolean registerAsInstantiated(Object reason) {
     }
 
     protected void onInstantiated() {
+        assert !isWordType() : Assertions.errorMessage("Word types cannot be instantiated", this);
         forAllSuperTypes(superType -> AtomicUtils.atomicMark(superType, isAnySubtypeInstantiatedUpdater));
 
         universe.onTypeInstantiated(this);
@@ -904,7 +905,9 @@ public final JavaKind getStorageKind() {
      */
     public boolean isWordType() {
         /* Word types are currently the only types where kind and storageKind differ. */
-        return getJavaKind() != getStorageKind();
+        boolean wordType = getJavaKind() != getStorageKind();
+        assert !wordType || getJavaKind().isObject() : Assertions.errorMessage("Only words are expected to have a discrepancy between java kind and storage kind", this);
+        return wordType;
     }
 
     @Override
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java
@@ -598,6 +598,7 @@ public void registerEmbeddedRoot(JavaConstant root, BytecodePosition position) {
     }
 
     public void registerUnsafeAccessedStaticField(AnalysisField field) {
+        AnalysisError.guarantee(!field.getType().isWordType(), "static Word fields cannot be unsafe accessed %s", this);
         unsafeAccessedStaticFields.put(field, true);
     }
 
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/StaticFieldsSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/StaticFieldsSupport.java
@@ -38,6 +38,7 @@
 import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
 import com.oracle.svm.core.feature.InternalFeature;
 import com.oracle.svm.core.meta.SharedField;
+import com.oracle.svm.core.meta.SharedType;
 import com.oracle.svm.core.util.VMError;
 
 import jdk.graal.compiler.core.common.type.StampFactory;
@@ -189,6 +190,26 @@ public void lower(LoweringTool tool) {
             replaceAndDelete(ConstantNode.forConstant(constant, tool.getMetaAccess(), graph()));
         }
     }
+
+    /**
+     * We must ensure we are not querying the offset of a static field of a type assignable from
+     * {@link org.graalvm.word.WordBase}.
+     */
+    public interface StaticFieldValidator {
+        static void checkFieldOffsetAllowed(ResolvedJavaField field) {
+            if (field.isStatic()) {
+                if (SubstrateUtil.HOSTED) {
+                    ImageSingletons.lookup(StaticFieldValidator.class).hostedCheckFieldOffsetAllowed(field);
+                } else {
+                    SharedField sField = (SharedField) field;
+                    boolean wordType = ((SharedType) sField.getType()).isWordType();
+                    VMError.guarantee(!wordType, "static Word field offsets cannot be queried");
+                }
+            }
+        }
+
+        void hostedCheckFieldOffsetAllowed(ResolvedJavaField field);
+    }
 }
 
 @AutomaticallyRegisteredFeature
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/FieldOffsetNode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/FieldOffsetNode.java
@@ -27,6 +27,7 @@
 import static jdk.graal.compiler.nodeinfo.NodeCycles.CYCLES_0;
 import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_1;
 
+import com.oracle.svm.core.StaticFieldsSupport;
 import com.oracle.svm.core.meta.SharedField;
 import com.oracle.svm.core.util.VMError;
 
@@ -83,6 +84,7 @@ public static ValueNode create(JavaKind kind, ResolvedJavaField field) {
     protected FieldOffsetNode(ResolvedJavaField field) {
         super(TYPE, StampFactory.forInteger(JavaKind.Long, 0, Integer.MAX_VALUE));
         this.field = field;
+        StaticFieldsSupport.StaticFieldValidator.checkFieldOffsetAllowed(field);
     }
 
     @Override
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/VarHandleSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/VarHandleSupport.java
@@ -52,7 +52,7 @@ public static VarHandleSupport singleton() {
         return ImageSingletons.lookup(VarHandleSupport.class);
     }
 
-    protected abstract ResolvedJavaField findVarHandleField(Object varHandle);
+    protected abstract ResolvedJavaField findVarHandleField(Object varHandle, boolean guaranteeUnsafeAccessed);
 }
 
 abstract class VarHandleFieldOffsetComputer implements FieldValueTransformerWithAvailability {
@@ -70,7 +70,7 @@ public boolean isAvailable() {
 
     @Override
     public Object transform(Object receiver, Object originalValue) {
-        ResolvedJavaField field = VarHandleSupport.singleton().findVarHandleField(receiver);
+        ResolvedJavaField field = VarHandleSupport.singleton().findVarHandleField(receiver, true);
         int offset = field.getOffset();
         if (offset <= 0) {
             throw VMError.shouldNotReachHere("Field is not marked as unsafe accessed: " + field);
@@ -90,7 +90,7 @@ public Object transform(Object receiver, Object originalValue) {
     public ValueNode intrinsify(CoreProviders providers, JavaConstant receiver) {
         Object varHandle = providers.getSnippetReflection().asObject(Object.class, receiver);
         if (varHandle != null) {
-            ResolvedJavaField field = VarHandleSupport.singleton().findVarHandleField(varHandle);
+            ResolvedJavaField field = VarHandleSupport.singleton().findVarHandleField(varHandle, false);
             return FieldOffsetNode.create(kind, field);
         }
         return null;
@@ -117,15 +117,17 @@ public boolean isAvailable() {
 
     @Override
     public Object transform(Object receiver, Object originalValue) {
-        ResolvedJavaField field = VarHandleSupport.singleton().findVarHandleField(receiver);
+        ResolvedJavaField field = VarHandleSupport.singleton().findVarHandleField(receiver, false);
+        StaticFieldsSupport.StaticFieldValidator.checkFieldOffsetAllowed(field);
         return field.getType().getJavaKind().isPrimitive() ? StaticFieldsSupport.getStaticPrimitiveFields() : StaticFieldsSupport.getStaticObjectFields();
     }
 
     @Override
     public ValueNode intrinsify(CoreProviders providers, JavaConstant receiver) {
         Object varHandle = providers.getSnippetReflection().asObject(Object.class, receiver);
         if (varHandle != null) {
-            ResolvedJavaField field = VarHandleSupport.singleton().findVarHandleField(varHandle);
+            ResolvedJavaField field = VarHandleSupport.singleton().findVarHandleField(varHandle, false);
+            StaticFieldsSupport.StaticFieldValidator.checkFieldOffsetAllowed(field);
             return StaticFieldsSupport.createStaticFieldBaseNode(field.getType().getJavaKind().isPrimitive());
         }
         return null;
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/meta/SharedType.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/meta/SharedType.java
@@ -24,6 +24,8 @@
  */
 package com.oracle.svm.core.meta;
 
+import org.graalvm.word.WordBase;
+
 import com.oracle.svm.core.hub.DynamicHub;
 import com.oracle.svm.core.util.VMError;
 
@@ -47,6 +49,12 @@ public interface SharedType extends ResolvedJavaType {
 
     int getTypeID();
 
+    /**
+     * Returns true if this type is part of the word type hierarchy, i.e, implements
+     * {@link WordBase}.
+     */
+    boolean isWordType();
+
     @Override
     default ResolvedJavaMethod resolveMethod(ResolvedJavaMethod method, ResolvedJavaType callerType) {
         /*
diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateType.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateType.java
@@ -98,13 +98,18 @@ public void setSingleImplementor(DynamicHub uniqueConcreteImplementation) {
      */
     @Override
     public final JavaKind getStorageKind() {
-        if (WordBase.class.isAssignableFrom(DynamicHub.toClass(hub))) {
+        if (isWordType()) {
             return ConfigurationValues.getWordKind();
         } else {
             return getJavaKind();
         }
     }
 
+    @Override
+    public boolean isWordType() {
+        return WordBase.class.isAssignableFrom(DynamicHub.toClass(hub));
+    }
+
     @Override
     public DynamicHub getHub() {
         return hub;
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/VarHandleFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/VarHandleFeature.java
@@ -97,8 +97,20 @@ public class VarHandleFeature implements InternalFeature {
 
     class VarHandleSupportImpl extends VarHandleSupport {
         @Override
-        protected ResolvedJavaField findVarHandleField(Object varHandle) {
-            return hUniverse != null ? findVarHandleHostedField(varHandle) : findVarHandleAnalysisField(varHandle);
+        protected ResolvedJavaField findVarHandleField(Object varHandle, boolean guaranteeUnsafeAccessed) {
+            ResolvedJavaField result;
+            if (hUniverse != null) {
+                result = findVarHandleHostedField(varHandle);
+            } else {
+                result = findVarHandleAnalysisField(varHandle);
+            }
+
+            if (guaranteeUnsafeAccessed) {
+                AnalysisField aField = result instanceof AnalysisField ? (AnalysisField) result : ((HostedField) result).getWrapped();
+                VMError.guarantee(aField.isUnsafeAccessed(), "Field not registered as unsafe accessed %s", aField);
+            }
+
+            return result;
         }
     }
 
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java
@@ -61,6 +61,7 @@
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
 import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.graal.pointsto.meta.AnalysisUniverse;
+import com.oracle.svm.core.StaticFieldsSupport;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.config.ObjectLayout;
 import com.oracle.svm.core.configure.ConfigurationConditionResolver;
@@ -512,6 +513,7 @@ private static void addField(Field reflField, boolean writable, DuringAnalysisAc
         } else if (field.isStatic() && field.isFinal()) {
             MaterializedConstantFields.singleton().register(field);
         }
+        StaticFieldsSupport.StaticFieldValidator.checkFieldOffsetAllowed(field);
 
         BigBang bb = access.getBigBang();
         bb.registerAsJNIAccessed(field, writable);
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedType.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedType.java
@@ -35,6 +35,7 @@
 import com.oracle.svm.core.meta.SharedType;
 import com.oracle.svm.core.util.VMError;
 
+import jdk.graal.compiler.debug.Assertions;
 import jdk.vm.ci.meta.Assumptions.AssumptionResult;
 import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.JavaKind;
@@ -261,13 +262,12 @@ public int[] getOpenTypeWorldTypeCheckSlots() {
         return openTypeWorldTypeCheckSlots;
     }
 
-    /**
-     * Returns true if this type is part of the word type hierarchy, i.e, implements
-     * {@link WordBase}.
-     */
+    @Override
     public boolean isWordType() {
         /* Word types have the kind Object, but a primitive storageKind. */
-        return kind != storageKind;
+        boolean wordType = kind != storageKind;
+        assert !wordType || kind.isObject() : Assertions.errorMessage("Only words are expected to have a discrepancy between java kind and storage kind", this);
+        return wordType;
     }
 
     /**
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/StaticFieldValidatorImpl.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/StaticFieldValidatorImpl.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.svm.hosted.reflect;
+
+import com.oracle.graal.pointsto.meta.AnalysisField;
+import com.oracle.svm.core.StaticFieldsSupport;
+import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
+import com.oracle.svm.core.meta.SharedField;
+import com.oracle.svm.core.meta.SharedType;
+import com.oracle.svm.core.util.VMError;
+
+import jdk.vm.ci.meta.ResolvedJavaField;
+
+@AutomaticallyRegisteredImageSingleton(value = StaticFieldsSupport.StaticFieldValidator.class)
+public class StaticFieldValidatorImpl implements StaticFieldsSupport.StaticFieldValidator {
+
+    @Override
+    public void hostedCheckFieldOffsetAllowed(ResolvedJavaField field) {
+        boolean wordType;
+        if (field instanceof AnalysisField aField) {
+            wordType = aField.getType().isWordType();
+        } else {
+            SharedField sField = (SharedField) field;
+            wordType = ((SharedType) sField.getType()).isWordType();
+        }
+
+        VMError.guarantee(!wordType, "static Word field offsets cannot be queried %s", field);
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -598,6 +598,7 @@ public void registerEmbeddedRoot(JavaConstant root, BytecodePosition position) {`
`598`	`598`	`}`
`599`	`599`
`600`	`600`	`public void registerUnsafeAccessedStaticField(AnalysisField field) {`
	`601`	`+ AnalysisError.guarantee(!field.getType().isWordType(), "static Word fields cannot be unsafe accessed %s", this);`
`601`	`602`	`unsafeAccessedStaticFields.put(field, true);`
`602`	`603`	`}`
`603`	`604`