From 8edeed6ce63d8655ffac4766a52e059830fb935e Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Mon, 10 Jun 2024 16:45:06 -0700 Subject: [PATCH] Remove concept of frozen unsafe accessed fields --- .../StandaloneAnalysisFeatureImpl.java | 9 --- .../graal/pointsto/flow/FieldTypeFlow.java | 6 +- .../flow/FrozenFieldFilterTypeFlow.java | 78 ------------------- .../pointsto/flow/OffsetStoreTypeFlow.java | 10 +-- .../flow/context/object/AnalysisObject.java | 11 +-- .../graal/pointsto/meta/AnalysisField.java | 28 ------- .../pointsto/typestate/PointsToStats.java | 4 - .../pointsto/typestore/FieldTypeStore.java | 31 -------- .../com/oracle/svm/hosted/FeatureImpl.java | 9 --- .../svm/hosted/jni/JNIAccessFeature.java | 1 - 10 files changed, 7 insertions(+), 180 deletions(-) delete mode 100644 substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FrozenFieldFilterTypeFlow.java diff --git a/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/features/StandaloneAnalysisFeatureImpl.java b/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/features/StandaloneAnalysisFeatureImpl.java index 3758d1c7e7ad..310d5e62a9a7 100644 --- a/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/features/StandaloneAnalysisFeatureImpl.java +++ b/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/features/StandaloneAnalysisFeatureImpl.java @@ -221,15 +221,6 @@ public boolean registerAsUnsafeAccessed(AnalysisField aField, Object reason) { return false; } - public void registerAsFrozenUnsafeAccessed(Field field) { - registerAsFrozenUnsafeAccessed(getMetaAccess().lookupJavaField(field)); - } - - public void registerAsFrozenUnsafeAccessed(AnalysisField aField) { - aField.registerAsFrozenUnsafeAccessed(); - registerAsUnsafeAccessed(aField, "registered from standalone feature"); - } - public void registerAsUnsafeAccessed(Field field, Object reason) { registerAsUnsafeAccessed(getMetaAccess().lookupJavaField(field), reason); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldTypeFlow.java index 76bfb6223a48..402543b61c8e 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldTypeFlow.java @@ -55,17 +55,17 @@ private static TypeState initialFieldState(AnalysisField field) { } /** The holder of the field flow (null for static fields). */ - private AnalysisObject object; + private final AnalysisObject object; /** A filter flow used for unsafe writes. */ private volatile FieldFilterTypeFlow filterFlow; public FieldTypeFlow(AnalysisField field, AnalysisType type) { - super(field, filterUncheckedInterface(type), initialFieldState(field)); + this(field, type, null); } public FieldTypeFlow(AnalysisField field, AnalysisType type, AnalysisObject object) { - this(field, type); + super(field, filterUncheckedInterface(type), initialFieldState(field)); this.object = object; } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FrozenFieldFilterTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FrozenFieldFilterTypeFlow.java deleted file mode 100644 index 959a3cb19988..000000000000 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FrozenFieldFilterTypeFlow.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright (c) 2017, 2017, 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.graal.pointsto.flow; - -import com.oracle.graal.pointsto.PointsToAnalysis; -import com.oracle.graal.pointsto.meta.AnalysisField; -import com.oracle.graal.pointsto.typestate.TypeState; - -/** - * The frozen field filter flow is used for unsafe writes to frozen type state fields. An unsafe - * write to an object can write to any of the unsafe accessed fields of that object, however the - * written values may not be compatible with all the fields. Thus the written values need to be - * filtered using the field FrozenFieldFilterTypeFlow type. The FrozenFieldFilterTypeFlow is a - * simplified version of FilterTypeFlow in that it is 'not-exact', i.e., it allows types of that are - * sub-types of the field declared type, it is always 'assignable', i.e., it allows types that are - * assignable to the field declared type, and it 'includes-null', i.e., it allows null values to - * pass through. However it's 'source' is an AnalysisField and not a ValueNode, thus it's a - * completely different class. The difference between this and The FieldFilterTypeFlow is that this - * one filters on the types reachable through normal writes only and not by the declared type. - */ -public class FrozenFieldFilterTypeFlow extends TypeFlow { - - private final UnsafeWriteSinkTypeFlow unsafeSink; - - @SuppressWarnings("this-escape") - public FrozenFieldFilterTypeFlow(PointsToAnalysis bb, AnalysisField field, UnsafeWriteSinkTypeFlow unsafeSink) { - super(field, field.getType()); - this.unsafeSink = unsafeSink; - this.source.getInstanceFieldFlow().addObserver(bb, this); - } - - @Override - public TypeState filter(PointsToAnalysis bb, TypeState update) { - /* - * Filter using the current type state of the field, i.e., the type state constructed - * through normal writes and non frozen unsafe writes, if any. - */ - TypeState fieldState = this.source.getInstanceFieldFlow().getState(); - return TypeState.forIntersection(bb, update, fieldState); - } - - @Override - public void onObservedUpdate(PointsToAnalysis bb) { - /* - * The field type state has changed. Since the type states can only increase, the new filter - * will be more coarse, i.e., will let more objects go through. Push the unsafe writes, - * collected in the sink, through the new filter. - */ - addState(bb, unsafeSink.getState()); - } - - @Override - public String toString() { - return "FrozenFieldFilterTypeFlow<" + source + ">"; - } -} diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetStoreTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetStoreTypeFlow.java index 00734bc8a3c0..b01f8da156ec 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetStoreTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetStoreTypeFlow.java @@ -213,20 +213,14 @@ public void forceUpdate(PointsToAnalysis bb) { * can write to any of the static fields marked for unsafe access. */ for (AnalysisField field : bb.getUniverse().getUnsafeAccessedStaticFields()) { - this.addUse(bb, field.getStaticFieldFlow().filterFlow(bb)); + addUse(bb, field.getStaticFieldFlow().filterFlow(bb)); } } void handleUnsafeAccessedFields(PointsToAnalysis bb, Collection unsafeAccessedFields, AnalysisObject object) { for (AnalysisField field : unsafeAccessedFields) { /* Write through the field filter flow. */ - if (field.hasUnsafeFrozenTypeState()) { - UnsafeWriteSinkTypeFlow unsafeWriteSink = object.getUnsafeWriteSinkFrozenFilterFlow(bb, objectFlow, source, field); - this.addUse(bb, unsafeWriteSink); - } else { - FieldFilterTypeFlow fieldFilterFlow = object.getInstanceFieldFilterFlow(bb, objectFlow, source, field); - this.addUse(bb, fieldFilterFlow); - } + addUse(bb, object.getInstanceFieldFilterFlow(bb, objectFlow, source, field)); } } } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/AnalysisObject.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/AnalysisObject.java index 4e74d58f185d..b130d4e84e5a 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/AnalysisObject.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/AnalysisObject.java @@ -35,7 +35,6 @@ import com.oracle.graal.pointsto.flow.FieldFilterTypeFlow; import com.oracle.graal.pointsto.flow.FieldTypeFlow; import com.oracle.graal.pointsto.flow.TypeFlow; -import com.oracle.graal.pointsto.flow.UnsafeWriteSinkTypeFlow; import com.oracle.graal.pointsto.meta.AnalysisField; import com.oracle.graal.pointsto.meta.AnalysisType; import com.oracle.graal.pointsto.meta.AnalysisUniverse; @@ -198,7 +197,7 @@ public ArrayElementsTypeFlow getArrayElementsFlow(PointsToAnalysis bb, boolean i return isStore ? arrayElementsTypeStore.writeFlow() : arrayElementsTypeStore.readFlow(); } - /** Returns the filter field flow corresponding to an unsafe accessed filed. */ + /** Returns the filter field flow corresponding to an unsafe accessed field. */ public FieldFilterTypeFlow getInstanceFieldFilterFlow(PointsToAnalysis bb, TypeFlow objectFlow, BytecodePosition context, AnalysisField field) { assert !Modifier.isStatic(field.getModifiers()) && field.isUnsafeAccessed() : field; @@ -206,13 +205,7 @@ public FieldFilterTypeFlow getInstanceFieldFilterFlow(PointsToAnalysis bb, TypeF return fieldTypeStore.writeFlow().filterFlow(bb); } - public UnsafeWriteSinkTypeFlow getUnsafeWriteSinkFrozenFilterFlow(PointsToAnalysis bb, TypeFlow objectFlow, BytecodePosition context, AnalysisField field) { - assert !Modifier.isStatic(field.getModifiers()) && field.hasUnsafeFrozenTypeState() : field; - FieldTypeStore fieldTypeStore = getInstanceFieldTypeStore(bb, objectFlow, context, field); - return fieldTypeStore.unsafeWriteSinkFlow(bb); - } - - /** Returns the instance field flow corresponding to a filed of the object's type. */ + /** Returns the instance field flow corresponding to a field of the object's type. */ public FieldTypeFlow getInstanceFieldFlow(PointsToAnalysis bb, AnalysisField field, boolean isStore) { return getInstanceFieldFlow(bb, null, null, field, isStore); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java index 86446ccfeb68..2335a6268696 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java @@ -29,7 +29,6 @@ import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import com.oracle.graal.pointsto.api.HostVM; @@ -68,9 +67,6 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa private static final AtomicReferenceFieldUpdater isUnsafeAccessedUpdater = AtomicReferenceFieldUpdater .newUpdater(AnalysisField.class, Object.class, "isUnsafeAccessed"); - private static final AtomicIntegerFieldUpdater unsafeFrozenTypeStateUpdater = AtomicIntegerFieldUpdater - .newUpdater(AnalysisField.class, "unsafeFrozenTypeState"); - private final int id; /** Marks a field loaded from a base layer. */ private final boolean isInBaseLayer; @@ -94,12 +90,7 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa @SuppressWarnings("unused") private volatile Object isAccessed; @SuppressWarnings("unused") private volatile Object isWritten; @SuppressWarnings("unused") private volatile Object isFolded; - - private boolean isJNIAccessed; - @SuppressWarnings("unused") private volatile Object isUnsafeAccessed; - @SuppressWarnings("unused") private volatile int unsafeFrozenTypeState; - @SuppressWarnings("unused") private volatile Object observers; /** * By default all instance fields are null before are initialized. It can be specified by @@ -110,8 +101,6 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa private ConcurrentMap readBy; private ConcurrentMap writtenBy; - protected TypeState instanceFieldTypeState; - /** Field's position in the list of declaring type's fields, including inherited fields. */ protected int position; @@ -254,7 +243,6 @@ public void cleanupAfterAnalysis() { initialInstanceFieldFlow = null; readBy = null; writtenBy = null; - instanceFieldTypeState = null; } public boolean registerAsAccessed(Object reason) { @@ -361,22 +349,6 @@ public boolean isUnsafeAccessed() { return AtomicUtils.isSet(this, isUnsafeAccessedUpdater); } - public void registerAsJNIAccessed() { - isJNIAccessed = true; - } - - public boolean isJNIAccessed() { - return isJNIAccessed; - } - - public void registerAsFrozenUnsafeAccessed() { - unsafeFrozenTypeStateUpdater.set(this, 1); - } - - public boolean hasUnsafeFrozenTypeState() { - return AtomicUtils.isSet(this, unsafeFrozenTypeStateUpdater); - } - public Object getReadBy() { return isReadUpdater.get(this); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/PointsToStats.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/PointsToStats.java index 4eacb47b864b..667ee6257ec9 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/PointsToStats.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/PointsToStats.java @@ -57,7 +57,6 @@ import com.oracle.graal.pointsto.flow.FilterTypeFlow; import com.oracle.graal.pointsto.flow.FormalParamTypeFlow; import com.oracle.graal.pointsto.flow.FormalReturnTypeFlow; -import com.oracle.graal.pointsto.flow.FrozenFieldFilterTypeFlow; import com.oracle.graal.pointsto.flow.InvokeTypeFlow; import com.oracle.graal.pointsto.flow.LoadFieldTypeFlow.LoadInstanceFieldTypeFlow; import com.oracle.graal.pointsto.flow.LoadFieldTypeFlow.LoadStaticFieldTypeFlow; @@ -485,9 +484,6 @@ private static String asString(TypeFlow flow) { } else if (flow instanceof FieldFilterTypeFlow) { FieldFilterTypeFlow filter = (FieldFilterTypeFlow) flow; return "FieldFilter(" + formatField(filter.getSource()) + ")"; - } else if (flow instanceof FrozenFieldFilterTypeFlow) { - FrozenFieldFilterTypeFlow filter = (FrozenFieldFilterTypeFlow) flow; - return "FrozenFieldFilter(" + formatField(filter.getSource()) + ")"; } else if (flow instanceof NewInstanceTypeFlow) { return "NewInstance(" + flow.getDeclaredType().toJavaName(false) + ")@" + formatSource(flow); } else if (flow instanceof DynamicNewInstanceTypeFlow) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestore/FieldTypeStore.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestore/FieldTypeStore.java index 55d9cc191e65..84591cb3f5b8 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestore/FieldTypeStore.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestore/FieldTypeStore.java @@ -24,12 +24,8 @@ */ package com.oracle.graal.pointsto.typestore; -import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; - import com.oracle.graal.pointsto.PointsToAnalysis; import com.oracle.graal.pointsto.flow.FieldTypeFlow; -import com.oracle.graal.pointsto.flow.FrozenFieldFilterTypeFlow; -import com.oracle.graal.pointsto.flow.UnsafeWriteSinkTypeFlow; import com.oracle.graal.pointsto.flow.context.object.AnalysisObject; import com.oracle.graal.pointsto.meta.AnalysisField; @@ -38,18 +34,9 @@ */ public abstract class FieldTypeStore { - private static final AtomicReferenceFieldUpdater FROZEN_FILTER_FLOW_UPDATER = AtomicReferenceFieldUpdater.newUpdater(FieldTypeStore.class, - FrozenFieldFilterTypeFlow.class, "frozenFilterFlow"); - private static final AtomicReferenceFieldUpdater UNSAFE_WRITE_SINK_FLOW_UPDATER = AtomicReferenceFieldUpdater.newUpdater(FieldTypeStore.class, - UnsafeWriteSinkTypeFlow.class, "unsafeWriteSinkFlow"); - /** The holder of the field flow. */ protected final AnalysisObject object; protected final AnalysisField field; - /** A filter flow used for unsafe writes on frozen type state. */ - private volatile FrozenFieldFilterTypeFlow frozenFilterFlow; - /** A sink used for unsafe writes on frozen type state. */ - private volatile UnsafeWriteSinkTypeFlow unsafeWriteSinkFlow; protected FieldTypeStore(AnalysisField field, AnalysisObject object) { this.field = field; @@ -71,22 +58,4 @@ public AnalysisField field() { /** Overridden for field type stores that need lazy initialization. */ public void init(@SuppressWarnings("unused") PointsToAnalysis bb) { } - - public UnsafeWriteSinkTypeFlow unsafeWriteSinkFlow(PointsToAnalysis bb) { - assert field.hasUnsafeFrozenTypeState() : "Unsafe write sink flow requested for non unsafe accessed frozen field."; - // first we create the unsafe write sink - if (unsafeWriteSinkFlow == null) { - UNSAFE_WRITE_SINK_FLOW_UPDATER.compareAndSet(this, null, new UnsafeWriteSinkTypeFlow(bb, field)); - } - // then we build the filter for it. - if (frozenFilterFlow == null) { - if (FROZEN_FILTER_FLOW_UPDATER.compareAndSet(this, null, new FrozenFieldFilterTypeFlow(bb, field, unsafeWriteSinkFlow))) { - frozenFilterFlow.addUse(bb, writeFlow()); - unsafeWriteSinkFlow.addUse(bb, frozenFilterFlow); - } - } - - return unsafeWriteSinkFlow; - } - } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java index 54b626d34d23..b8830cf8ee19 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java @@ -418,15 +418,6 @@ public boolean registerAsUnsafeAccessed(AnalysisField aField, Object reason) { return aField.registerAsUnsafeAccessed(reason); } - public void registerAsFrozenUnsafeAccessed(Field field, Object reason) { - registerAsFrozenUnsafeAccessed(getMetaAccess().lookupJavaField(field), reason); - } - - public void registerAsFrozenUnsafeAccessed(AnalysisField aField, Object reason) { - aField.registerAsFrozenUnsafeAccessed(); - registerAsUnsafeAccessed(aField, reason); - } - public void registerAsRoot(Executable method, boolean invokeSpecial, String reason, MultiMethod.MultiMethodKey... otherRoots) { bb.addRootMethod(method, invokeSpecial, reason, otherRoots); } 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 072afca74b73..ba1b22e6c41b 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 @@ -497,7 +497,6 @@ private static void addField(Field reflField, boolean writable, DuringAnalysisAc JNIAccessibleClass jniClass = addClass(reflField.getDeclaringClass(), access); AnalysisField field = access.getMetaAccess().lookupJavaField(reflField); jniClass.addFieldIfAbsent(field.getName(), name -> new JNIAccessibleField(jniClass, field.getJavaKind(), field.getModifiers())); - field.registerAsJNIAccessed(); field.registerAsRead("it is registered for as JNI accessed"); if (writable) { field.registerAsWritten("it is registered as JNI writable");