Skip to content

Commit

Permalink
[GR-53803] Only allow Unsafe.allocateInstance for types registered ex…
Browse files Browse the repository at this point in the history
…plicitly in the configuration.

PullRequest: graal/17643
  • Loading branch information
Christian Wimmer committed Jun 1, 2024
2 parents d3a06a2 + e2b81f7 commit 99920bd
Show file tree
Hide file tree
Showing 28 changed files with 173 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -536,9 +536,9 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
* NewInstanceNode.
*/
if (b.currentBlockCatchesOOM()) {
DynamicNewInstanceWithExceptionNode.createAndPush(b, clazz);
DynamicNewInstanceWithExceptionNode.createAndPush(b, clazz, true);
} else {
DynamicNewInstanceNode.createAndPush(b, clazz);
DynamicNewInstanceNode.createAndPush(b, clazz, true);
}
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import jdk.graal.compiler.nodes.spi.Canonicalizable;
import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
import jdk.graal.compiler.nodes.spi.CoreProviders;

import jdk.vm.ci.meta.JavaKind;
import jdk.vm.ci.meta.MetaAccessProvider;
import jdk.vm.ci.meta.ResolvedJavaType;
Expand All @@ -50,12 +49,12 @@ public final class DynamicNewInstanceNode extends AbstractNewObjectNode implemen

@Input ValueNode clazz;

public static void createAndPush(GraphBuilderContext b, ValueNode clazz) {
public static void createAndPush(GraphBuilderContext b, ValueNode clazz, boolean validateClass) {
ResolvedJavaType constantType = tryConvertToNonDynamic(clazz, b);
if (constantType != null) {
b.addPush(JavaKind.Object, new NewInstanceNode(constantType, true));
} else {
ValueNode clazzLegal = b.add(new ValidateNewInstanceClassNode(clazz));
ValueNode clazzLegal = validateClass ? b.add(new ValidateNewInstanceClassNode(clazz)) : clazz;
b.addPush(JavaKind.Object, new DynamicNewInstanceNode(clazzLegal, true));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ public class DynamicNewInstanceWithExceptionNode extends AllocateWithExceptionNo
public static final NodeClass<DynamicNewInstanceWithExceptionNode> TYPE = NodeClass.create(DynamicNewInstanceWithExceptionNode.class);
protected boolean fillContents;

public static void createAndPush(GraphBuilderContext b, ValueNode clazz) {
public static void createAndPush(GraphBuilderContext b, ValueNode clazz, boolean validateClass) {
ResolvedJavaType constantType = tryConvertToNonDynamic(clazz, b);
if (constantType != null) {
b.addPush(JavaKind.Object, new NewInstanceWithExceptionNode(constantType, true));
} else {
ValueNode clazzLegal = b.add(new ValidateNewInstanceClassNode(clazz));
ValueNode clazzLegal = validateClass ? b.add(new ValidateNewInstanceClassNode(clazz)) : clazz;
b.addPush(JavaKind.Object, new DynamicNewInstanceWithExceptionNode(clazzLegal, true));
}
}
Expand Down
1 change: 1 addition & 0 deletions sdk/src/org.graalvm.nativeimage/snapshot.sigtest
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,7 @@ meth public abstract !varargs void registerReachabilityHandler(java.util.functio
meth public abstract void registerAsAccessed(java.lang.reflect.Field)
meth public abstract void registerAsInHeap(java.lang.Class<?>)
meth public abstract void registerAsUnsafeAccessed(java.lang.reflect.Field)
meth public abstract void registerAsUnsafeAllocated(java.lang.Class<?>)
meth public abstract void registerAsUsed(java.lang.Class<?>)
meth public abstract void registerClassInitializerReachabilityHandler(java.util.function.Consumer<org.graalvm.nativeimage.hosted.Feature$DuringAnalysisAccess>,java.lang.Class<?>)
meth public abstract void registerFieldValueTransformer(java.lang.reflect.Field,org.graalvm.nativeimage.hosted.FieldValueTransformer)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* The Universal Permissive License (UPL), Version 1.0
Expand Down Expand Up @@ -220,6 +220,14 @@ interface BeforeAnalysisAccess extends FeatureAccess {
*/
void registerAsInHeap(Class<?> type);

/**
* Registers the provided type as allocatable without running a constructor, via
* Unsafe.allocateInstance or via the JNI function AllocObject.
*
* @since 24.1
*/
void registerAsUnsafeAllocated(Class<?> type);

/**
* Registers the provided field as accesses, i.e., the static analysis assumes the field is
* used even if there are no explicit reads or writes in the bytecodes.
Expand Down
1 change: 1 addition & 0 deletions substratevm/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ This changelog summarizes major changes to GraalVM Native Image.
* (GR-52534) Change the digest (used e.g. for symbol names) from SHA-1 encoded as a hex string (40 bytes) to 128-bit Murmur3 as a Base-62 string (22 bytes).
* (GR-52578) Print information about embedded resources into `embedded-resources.json` using the `-H:+GenerateEmbeddedResourcesFile` option.
* (GR-51172) Add support to catch OutOfMemoryError exceptions on native image if there is no memory left.
* (GR-53803) In the strict reflection configuration mode (when `ThrowMissingRegistrationErrors` is enabled), only allow `Unsafe.allocateInstance` for types registered explicitly in the configuration.
* (GR-43837) `--report-unsupported-elements-at-runtime` is now enabled by default and the option is deprecated.
* (GR-53359) Provide the `.debug_gdb_scripts` section that triggers auto-loading of `svmhelpers.py` in GDB. Remove single and double quotes from `ClassLoader.nameAndId` in the debuginfo.
* (GR-47365) Include dynamic proxy metadata in the reflection metadata with the syntax `"type": { "proxy": [<interface list>] }`. This allows members of proxy classes to be accessed reflectively. `proxy-config.json` is now deprecated but will still be honored.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ public void registerAsInHeap(AnalysisType aType, Object reason) {
aType.registerAsInstantiated(reason);
}

@Override
public void registerAsUnsafeAllocated(Class<?> type) {
getMetaAccess().lookupJavaType(type).registerAsUnsafeAllocated("registered from Feature API");
}

@Override
public void registerAsAccessed(Field field) {
registerAsAccessed(getMetaAccess().lookupJavaField(field), "registered from Feature API");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav
private static final AtomicReferenceFieldUpdater<AnalysisType, Object> isInstantiatedUpdater = AtomicReferenceFieldUpdater
.newUpdater(AnalysisType.class, Object.class, "isInstantiated");

private static final AtomicReferenceFieldUpdater<AnalysisType, Object> isUnsafeAllocatedUpdater = AtomicReferenceFieldUpdater
.newUpdater(AnalysisType.class, Object.class, "isUnsafeAllocated");

private static final AtomicReferenceFieldUpdater<AnalysisType, Object> isReachableUpdater = AtomicReferenceFieldUpdater
.newUpdater(AnalysisType.class, Object.class, "isReachable");

Expand All @@ -112,6 +115,8 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav
private final String unqualifiedName;

@SuppressWarnings("unused") private volatile Object isInstantiated;
/** Can be allocated via Unsafe or JNI, i.e., without executing a constructor. */
@SuppressWarnings("unused") private volatile Object isUnsafeAllocated;
@SuppressWarnings("unused") private volatile Object isReachable;
@SuppressWarnings("unused") private volatile int isAnySubtypeInstantiated;
private boolean reachabilityListenerNotified;
Expand Down Expand Up @@ -526,6 +531,11 @@ protected void onInstantiated() {
processMethodOverrides();
}

public boolean registerAsUnsafeAllocated(Object reason) {
registerAsInstantiated(reason);
return AtomicUtils.atomicSet(this, reason, isUnsafeAllocatedUpdater);
}

private void processMethodOverrides() {
/*
* Walk up the type hierarchy from this type keeping track of all processed types. For each
Expand Down Expand Up @@ -815,6 +825,10 @@ public Object getInstantiatedReason() {
return isInstantiated;
}

public boolean isUnsafeAllocated() {
return AtomicUtils.isSet(this, isUnsafeAllocatedUpdater);
}

/** Returns true if this type or any of its subtypes was marked as instantiated. */
public boolean isAnySubtypeInstantiated() {
return AtomicUtils.isSet(this, isAnySubtypeInstantiatedUpdater);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,22 @@ private static JNIObjectHandle findClass(JNIEnvironment env, CCharPointer name)
return result;
}

@CEntryPoint(name = "AllocObject")
@CEntryPointOptions(prologue = AgentIsolate.Prologue.class)
static JNIObjectHandle allocObject(JNIEnvironment env, JNIObjectHandle clazz) {
InterceptedState state = initInterceptedState();
JNIObjectHandle callerClass = getCallerClass(state, env);
JNIObjectHandle result = jniFunctions().getAllocObject().invoke(env, clazz);
if (nullHandle().equal(result) || clearException(env)) {
result = nullHandle();
}
if (shouldTrace()) {
traceCall(env, "AllocObject", clazz, nullHandle(), callerClass, result.notEqual(nullHandle()), state);
}
return result;

}

@CEntryPoint(name = "GetMethodID")
@CEntryPointOptions(prologue = AgentIsolate.Prologue.class)
private static JNIMethodId getMethodID(JNIEnvironment env, JNIObjectHandle clazz, CCharPointer name, CCharPointer signature) {
Expand Down Expand Up @@ -316,6 +332,7 @@ public static void onVMStart(JvmtiEnv jvmti) {
JNINativeInterface functions = functionsPtr.read();
functions.setDefineClass(defineClassLiteral.getFunctionPointer());
functions.setFindClass(findClassLiteral.getFunctionPointer());
functions.setAllocObject(allocObjectLiteral.getFunctionPointer());
functions.setGetMethodID(getMethodIDLiteral.getFunctionPointer());
functions.setGetStaticMethodID(getStaticMethodIDLiteral.getFunctionPointer());
functions.setGetFieldID(getFieldIDLiteral.getFunctionPointer());
Expand All @@ -342,6 +359,9 @@ public static void onUnload() {
private static final CEntryPointLiteral<FindClassFunctionPointer> findClassLiteral = CEntryPointLiteral.create(JniCallInterceptor.class,
"findClass", JNIEnvironment.class, CCharPointer.class);

private static final CEntryPointLiteral<FindClassFunctionPointer> allocObjectLiteral = CEntryPointLiteral.create(JniCallInterceptor.class,
"allocObject", JNIEnvironment.class, JNIObjectHandle.class);

private static final CEntryPointLiteral<GetMethodIDFunctionPointer> getMethodIDLiteral = CEntryPointLiteral.create(JniCallInterceptor.class,
"getMethodID", JNIEnvironment.class, JNIObjectHandle.class, CCharPointer.class, CCharPointer.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ void processEntry(EconomicMap<String, ?> entry, ConfigurationSet configurationSe
ConfigurationMemberDeclaration declaration = (declaringClass != null) ? ConfigurationMemberDeclaration.DECLARED : ConfigurationMemberDeclaration.PRESENT;
TypeConfiguration config = configurationSet.getJniConfiguration();
switch (function) {
case "AllocObject":
expectSize(args, 0);
/*
* AllocObject is implemented via Unsafe.allocateInstance, so we need to set the
* "unsafe allocated" flag in the reflection configuration file.
*/
configurationSet.getReflectionConfiguration().getOrCreateType(condition, clazz).setUnsafeAllocated();
break;
case "GetStaticMethodID":
case "GetMethodID": {
expectSize(args, 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@
*/
package com.oracle.svm.core;

import jdk.graal.compiler.api.replacements.Fold;
import org.graalvm.nativeimage.ImageSingletons;
import org.graalvm.nativeimage.Platform;
import org.graalvm.nativeimage.Platforms;

import com.oracle.svm.core.option.OptionClassFilter;

import jdk.graal.compiler.api.replacements.Fold;

public class MissingRegistrationSupport {
private final OptionClassFilter classFilter;

Expand All @@ -48,7 +49,11 @@ public boolean reportMissingRegistrationErrors(StackTraceElement responsibleClas
return reportMissingRegistrationErrors(responsibleClass.getModuleName(), getPackageName(responsibleClass.getClassName()), responsibleClass.getClassName());
}

public boolean reportMissingRegistrationErrors(String moduleName, String packageName, String className) {
public boolean reportMissingRegistrationErrors(Class<?> clazz) {
return reportMissingRegistrationErrors(clazz.getModule().getName(), clazz.getPackageName(), clazz.getName());
}

private boolean reportMissingRegistrationErrors(String moduleName, String packageName, String className) {
return classFilter.isIncluded(moduleName, packageName, className) != null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1223,4 +1223,8 @@ public static class TruffleStableOptions {
@Option(help = "Reduce the amount of metadata in the image for implicit exceptions by removing inlining information from the stack trace. " +
"This makes the image smaller, but also the stack trace of implicit exceptions less precise.", type = OptionType.Expert)//
public static final HostedOptionKey<Boolean> ReduceImplicitExceptionStackTraceInformation = new HostedOptionKey<>(false);

@Option(help = "Allow all instantiated types to be allocated via Unsafe.allocateInstance().", type = OptionType.Expert, //
deprecated = true, deprecationMessage = "ThrowMissingRegistrationErrors is the preferred way of configuring this on a per-type level.") //
public static final HostedOptionKey<Boolean> AllowUnsafeAllocationOfAllInstantiatedTypes = new HostedOptionKey<>(null);
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@
import jdk.graal.compiler.options.Option;
import jdk.graal.compiler.word.BarrieredAccess;
import jdk.graal.compiler.word.Word;
import jdk.internal.misc.Unsafe;
import jdk.vm.ci.code.InstalledCode;
import jdk.vm.ci.meta.DeoptimizationAction;
import jdk.vm.ci.meta.DeoptimizationReason;
Expand Down Expand Up @@ -1042,11 +1041,7 @@ private Object materializeObject(int virtualObjectId, FrameInfoQueryResult sourc
if (!LayoutEncoding.isPureInstance(layoutEncoding)) {
throw fatalDeoptimizationError("Non-pure instance layout encoding: " + layoutEncoding, sourceFrame);
}
try {
obj = Unsafe.getUnsafe().allocateInstance(DynamicHub.toClass(hub));
} catch (InstantiationException ex) {
throw fatalDeoptimizationError("Instantiation exception: " + ex, sourceFrame);
}
obj = KnownIntrinsics.unvalidatedAllocateInstance(DynamicHub.toClass(hub));
curOffset = WordFactory.unsigned(objectLayout.getFirstFieldOffset());
curIdx = 1;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@
import jdk.graal.compiler.replacements.nodes.ObjectClone;
import jdk.graal.compiler.word.BarrieredAccess;
import jdk.graal.compiler.word.ObjectAccess;
import jdk.internal.misc.Unsafe;
import jdk.vm.ci.meta.ResolvedJavaType;

public final class SubstrateObjectCloneSnippets extends SubstrateTemplates implements Snippets {
Expand All @@ -92,7 +91,7 @@ public static void registerForeignCalls(SubstrateForeignCallsProvider foreignCal
}

@SubstrateForeignCallTarget(stubCallingConvention = false)
private static Object doClone(Object original) throws CloneNotSupportedException, InstantiationException {
private static Object doClone(Object original) throws CloneNotSupportedException {
if (original == null) {
throw new NullPointerException();
} else if (!(original instanceof Cloneable)) {
Expand Down Expand Up @@ -122,7 +121,7 @@ private static Object doClone(Object original) throws CloneNotSupportedException
throw VMError.shouldNotReachHere("Hybrid classes do not support Object.clone().");
}
} else {
result = Unsafe.getUnsafe().allocateInstance(DynamicHub.toClass(hub));
result = KnownIntrinsics.unvalidatedAllocateInstance(DynamicHub.toClass(hub));
}

int firstFieldOffset = ConfigurationValues.getObjectLayout().getFirstFieldOffset();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ protected Object newmultiarray(DynamicHub hub, @ConstantParameter int rank, @Con
public DynamicHub validateNewInstanceClass(DynamicHub hub) {
if (probability(EXTREMELY_FAST_PATH_PROBABILITY, hub != null)) {
DynamicHub nonNullHub = (DynamicHub) PiNode.piCastNonNull(hub, SnippetAnchorNode.anchor());
if (probability(EXTREMELY_FAST_PATH_PROBABILITY, nonNullHub.canInstantiateAsInstance())) {
if (probability(EXTREMELY_FAST_PATH_PROBABILITY, nonNullHub.canUnsafeInstantiateAsInstance())) {
return nonNullHub;
}
}
Expand Down Expand Up @@ -317,7 +317,7 @@ private static void instanceHubErrorStub(DynamicHub hub) throws InstantiationExc
throw new NullPointerException("Allocation type is null.");
} else if (!hub.isInstanceClass() || LayoutEncoding.isSpecial(hub.getLayoutEncoding())) {
throw new InstantiationException("Can only allocate instance objects for concrete classes.");
} else if (!hub.isInstantiated()) {
} else if (!hub.canUnsafeInstantiateAsInstance()) {
if (MissingRegistrationUtils.throwMissingRegistrationErrors()) {
MissingReflectionRegistrationUtils.forClass(hub.getTypeName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ public final class DynamicHub implements AnnotatedElement, java.lang.reflect.Typ
/** Indicates whether the type has been discovered as instantiated by the static analysis. */
private static final int IS_INSTANTIATED_BIT = 0;
/** Can this class be instantiated as an instance. */
private static final int CAN_INSTANTIATE_AS_INSTANCE_BIT = 1;
private static final int CAN_UNSAFE_INSTANTIATE_AS_INSTANCE_BIT = 1;

private static final int IS_REGISTERED_FOR_SERIALIZATION = 2;

Expand Down Expand Up @@ -492,8 +492,8 @@ public void setClassInitializationInfo(ClassInitializationInfo classInitializati

@Platforms(Platform.HOSTED_ONLY.class)
public void setSharedData(int layoutEncoding, int monitorOffset, int identityHashOffset, long referenceMapIndex,
boolean isInstantiated, boolean canInstantiateAsInstance, boolean isRegisteredForSerialization) {
assert !(!isInstantiated && canInstantiateAsInstance);
boolean isInstantiated, boolean canUnsafeInstantiateAsInstance, boolean isRegisteredForSerialization) {
assert !(!isInstantiated && canUnsafeInstantiateAsInstance);
VMError.guarantee(monitorOffset == (char) monitorOffset, "Class %s has an invalid monitor field offset. Most likely, its objects are larger than supported.", name);
VMError.guarantee(identityHashOffset == (char) identityHashOffset, "Class %s has an invalid identity hash code field offset. Most likely, its objects are larger than supported.", name);

Expand All @@ -506,7 +506,7 @@ public void setSharedData(int layoutEncoding, int monitorOffset, int identityHas
}
this.referenceMapIndex = (int) referenceMapIndex;
this.additionalFlags = NumUtil.safeToUByte(makeFlag(IS_INSTANTIATED_BIT, isInstantiated) |
makeFlag(CAN_INSTANTIATE_AS_INSTANCE_BIT, canInstantiateAsInstance) |
makeFlag(CAN_UNSAFE_INSTANTIATE_AS_INSTANCE_BIT, canUnsafeInstantiateAsInstance) |
makeFlag(IS_REGISTERED_FOR_SERIALIZATION, isRegisteredForSerialization));
}

Expand Down Expand Up @@ -748,8 +748,8 @@ public boolean isInstantiated() {
return isFlagSet(additionalFlags, IS_INSTANTIATED_BIT);
}

public boolean canInstantiateAsInstance() {
return isFlagSet(additionalFlags, CAN_INSTANTIATE_AS_INSTANCE_BIT);
public boolean canUnsafeInstantiateAsInstance() {
return isFlagSet(additionalFlags, CAN_UNSAFE_INSTANTIATE_AS_INSTANCE_BIT);
}

public boolean isProxyClass() {
Expand Down
Loading

0 comments on commit 99920bd

Please sign in to comment.