From aba4ec08fbabbecb294cf88103baac7868c66640 Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Wed, 22 Jan 2025 10:57:28 +0100 Subject: [PATCH] Ensure static word fields are not unsafe accessed --- .../graal/pointsto/meta/AnalysisType.java | 5 +- .../graal/pointsto/meta/AnalysisUniverse.java | 1 + .../oracle/svm/core/StaticFieldsSupport.java | 21 ++++++++ .../svm/core/graal/nodes/FieldOffsetNode.java | 2 + .../oracle/svm/core/jdk/VarHandleSupport.java | 12 +++-- .../com/oracle/svm/core/meta/SharedType.java | 8 +++ .../oracle/svm/graal/meta/SubstrateType.java | 7 ++- .../svm/hosted/jdk/VarHandleFeature.java | 16 +++++- .../svm/hosted/jni/JNIAccessFeature.java | 2 + .../oracle/svm/hosted/meta/HostedType.java | 10 ++-- .../reflect/StaticFieldValidatorImpl.java | 51 +++++++++++++++++++ 11 files changed, 121 insertions(+), 14 deletions(-) create mode 100644 substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/StaticFieldValidatorImpl.java 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 index 92e1c4707811..9333b6823302 100644 --- 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 index 9fd730c7c8ce..d545381dd6de 100644 --- 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 index 54da8f92d01b..fbdbfaa9af37 100644 --- 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 index 581192e0c020..257c34a27b57 100644 --- 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 index 821a3095729a..6382b6a11f29 100644 --- 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,7 +117,8 @@ 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(); } @@ -125,7 +126,8 @@ 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); + 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 index 99a6e0f238c9..4df6354d76e4 100644 --- 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 index cc6d32e61c43..5634cff4bc51 100644 --- 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 index 27dfcfe43cfa..8fcea57c3b85 100644 --- 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 index 082309f51dd3..c66a41d1e714 100644 --- 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; @@ -511,6 +512,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 index b8162ee99532..fd263f73841b 100644 --- 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 new file mode 100644 index 000000000000..2ba58dd1bf93 --- /dev/null +++ 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); + } +}