Skip to content

Commit 0ce355e

Browse files
committed
Address review comments
1 parent 96fde9a commit 0ce355e

File tree

17 files changed

+189
-238
lines changed

17 files changed

+189
-238
lines changed

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/nodes/MethodHandleWithExceptionNode.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ public WithExceptionNode trySimplify(MethodHandleAccessProvider methodHandleAcce
8787
return invoke;
8888
}
8989

90+
/** {@link MethodHandleNode#tryResolveTargetInvoke}. */
9091
protected <T extends Invoke> T tryResolveTargetInvoke(GraphAdder adder, InvokeFactory<T> factory, MethodHandleAccessProvider methodHandleAccess, ValueNode... argumentsArray) {
9192
return MethodHandleNode.tryResolveTargetInvoke(adder, factory, methodHandleAccess, intrinsicMethod, targetMethod, bci, returnStamp, argumentsArray);
9293
}

substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/ForeignFunctionsRuntime.java

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -224,24 +224,20 @@ public void registerSafeArenaAccessorMethod(ResolvedJavaMethod method) {
224224
neverAccessesSharedArenaMethods.add(method);
225225
}
226226

227-
FunctionPointerHolder getDowncallStubPointerHolder(NativeEntryPointInfo nep) {
227+
public CFunctionPointer getDowncallStubPointer(NativeEntryPointInfo nep) {
228228
FunctionPointerHolder holder = downcallStubs.get(nep);
229229
if (holder == null) {
230230
throw reportMissingDowncall(nep);
231231
}
232-
return holder;
233-
}
234-
235-
public CFunctionPointer getDowncallStubPointer(NativeEntryPointInfo nep) {
236-
return getDowncallStubPointerHolder(nep).functionPointer;
232+
return holder.functionPointer;
237233
}
238234

239-
FunctionPointerHolder getDowncallStubInvokerPointerHolder(MethodType methodType) {
235+
CFunctionPointer getDowncallStubInvokerPointer(MethodType methodType) {
240236
FunctionPointerHolder holder = downcallStubInvokers.get(methodType);
241237
if (holder == null) {
242238
throw reportMissingDowncall(methodType);
243239
}
244-
return holder;
240+
return holder.functionPointer;
245241
}
246242

247243
CFunctionPointer getUpcallStubPointer(JavaEntryPointInfo jep) {
@@ -331,6 +327,9 @@ void freeTrampoline(long addr) {
331327
private MissingForeignRegistrationError reportMissingDowncall(NativeEntryPointInfo nep) {
332328
LinkRequest currentLinkRequest = null;
333329
for (LinkRequest linkRequest : currentLinkRequests) {
330+
if (!Thread.currentThread().equals(linkRequest.requester)) {
331+
continue;
332+
}
334333
NativeEntryPointInfo nativeEntryPointInfo = abiUtils.makeNativeEntrypoint(linkRequest.functionDescriptor, linkRequest.linkerOptions);
335334
if (nep.equals(nativeEntryPointInfo)) {
336335
currentLinkRequest = linkRequest;
@@ -391,10 +390,10 @@ private static MissingForeignRegistrationError report(boolean upcall, LinkReques
391390
"upcallStub"));
392391
}
393392

394-
record LinkRequest(boolean upcall, FunctionDescriptor functionDescriptor, LinkerOptions linkerOptions) implements AutoCloseable, JsonPrintable {
393+
record LinkRequest(boolean upcall, FunctionDescriptor functionDescriptor, LinkerOptions linkerOptions, Thread requester) implements AutoCloseable, JsonPrintable {
395394

396395
static LinkRequest create(boolean upcall, FunctionDescriptor functionDescriptor, LinkerOptions linkerOptions) {
397-
LinkRequest linkRequest = new LinkRequest(upcall, functionDescriptor, linkerOptions);
396+
LinkRequest linkRequest = new LinkRequest(upcall, functionDescriptor, linkerOptions, Thread.currentThread());
398397
ForeignFunctionsRuntime.singleton().currentLinkRequests.push(linkRequest);
399398
return linkRequest;
400399
}
@@ -438,8 +437,10 @@ public void printJson(JsonWriter writer) throws IOException {
438437
@Override
439438
public Object linkToNative(Object... args) throws Throwable {
440439
Target_jdk_internal_foreign_abi_NativeEntryPoint nep = (Target_jdk_internal_foreign_abi_NativeEntryPoint) args[args.length - 1];
441-
/* The nep argument will be dropped in the invoked function */
442-
return ((StubInvokerPointer) nep.downcallInvokerPointerHolder.functionPointer).invoke(nep.downcallStubPointerHolder.functionPointer, args);
440+
StubInvokerPointer invoker = (StubInvokerPointer) nep.downcallInvokerPointer;
441+
CFunctionPointer stub = nep.downcallStubPointer;
442+
/* The nep argument will be dropped in the invoked downcall stub */
443+
return invoker.invoke(stub, args);
443444
}
444445

445446
@Override

substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/NativeEntryPointHelper.java

Lines changed: 8 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,31 +25,6 @@
2525

2626
package com.oracle.svm.core.foreign;
2727

28-
/*
29-
* Copyright (c) 2026, 2026, Oracle and/or its affiliates. All rights reserved.
30-
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
31-
*
32-
* This code is free software; you can redistribute it and/or modify it
33-
* under the terms of the GNU General Public License version 2 only, as
34-
* published by the Free Software Foundation. Oracle designates this
35-
* particular file as subject to the "Classpath" exception as provided
36-
* by Oracle in the LICENSE file that accompanied this code.
37-
*
38-
* This code is distributed in the hope that it will be useful, but WITHOUT
39-
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
40-
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
41-
* version 2 for more details (a copy is included in the LICENSE file that
42-
* accompanied this code).
43-
*
44-
* You should have received a copy of the GNU General Public License version
45-
* 2 along with this work; if not, write to the Free Software Foundation,
46-
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
47-
*
48-
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
49-
* or visit www.oracle.com if you need additional information or have any
50-
* questions.
51-
*/
52-
5328
import java.lang.invoke.MethodType;
5429
import java.util.Arrays;
5530
import java.util.Objects;
@@ -58,7 +33,6 @@
5833
import org.graalvm.nativeimage.Platforms;
5934

6035
import com.oracle.svm.shared.util.BasedOnJDKClass;
61-
import com.oracle.svm.shared.util.VMError;
6236
import com.oracle.svm.util.GuestAccess;
6337
import com.oracle.svm.util.GuestElements;
6438
import com.oracle.svm.util.JVMCIReflectionUtil;
@@ -69,7 +43,7 @@
6943

7044
@BasedOnJDKClass(NativeEntryPoint.class)
7145
@BasedOnJDKClass(className = "jdk.internal.foreign.abi.SoftReferenceCache")
72-
@BasedOnJDKClass(className = "jdk.internal.foreign.abi.NativeEntryPoint$CacheKey")
46+
@BasedOnJDKClass(className = "jdk.internal.foreign.abi.NativeEntryPoint", innerClass = "CacheKey")
7347
@Platforms(Platform.HOSTED_ONLY.class)
7448
public final class NativeEntryPointHelper {
7549

@@ -105,11 +79,14 @@ public static NativeEntryPointInfo extractNativeEntryPointInfo(JavaConstant nati
10579
}
10680

10781
/*
108-
* Any instance of NativeEntryPoint is created via 'NativeEntryPoint.make' and will
109-
* therefore be added to NEP_CACHE. Further, the keys of NEP_CACHE are strongly referenced.
110-
* This means that there must be a cache key for any NativeEntryPoint that exists.
82+
* In the common case, any instance of NativeEntryPoint is created via
83+
* 'NativeEntryPoint.make' and will therefore be added to NEP_CACHE and the keys of
84+
* NEP_CACHE are strongly referenced. However, we are defensive and do not fail if we
85+
* couldn't find the key.
11186
*/
112-
VMError.guarantee(cacheKeyConstant != null, "Cannot extract info for NativeEntryPoint because it is not in NEP_CACHE");
87+
if (cacheKeyConstant == null) {
88+
return null;
89+
}
11390

11491
/*
11592
* Field 'CacheKey.abi' is ignored because the ABIDescriptor is JDK's equivalent of our

substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/NativeEntryPointInfo.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2023, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -28,7 +28,8 @@
2828
import java.util.Arrays;
2929
import java.util.Objects;
3030

31-
import com.oracle.svm.core.FunctionPointerHolder;
31+
import org.graalvm.nativeimage.c.function.CFunctionPointer;
32+
3233
import com.oracle.svm.shared.util.VMError;
3334

3435
import jdk.internal.foreign.abi.ABIDescriptor;
@@ -113,8 +114,8 @@ public static Target_jdk_internal_foreign_abi_NativeEntryPoint makeEntryPoint(
113114
boolean needsTransition,
114115
boolean allowHeapAccess) {
115116
var info = make(argMoves, returnMoves, methodType, needsReturnBuffer, capturedStateMask, needsTransition, allowHeapAccess);
116-
FunctionPointerHolder holder = ForeignFunctionsRuntime.singleton().getDowncallStubPointerHolder(info);
117-
return new Target_jdk_internal_foreign_abi_NativeEntryPoint(info.methodType(), holder, capturedStateMask);
117+
CFunctionPointer downcallStubPointer = ForeignFunctionsRuntime.singleton().getDowncallStubPointer(info);
118+
return new Target_jdk_internal_foreign_abi_NativeEntryPoint(info.methodType(), downcallStubPointer, capturedStateMask);
118119
}
119120

120121
public MethodType methodType() {

substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/SubstrateForeignUtil.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import com.oracle.svm.configure.UnresolvedAccessCondition;
4141
import com.oracle.svm.configure.config.ForeignConfiguration.ConfigurationFunctionDescriptor;
4242
import com.oracle.svm.configure.config.ForeignConfiguration.StubDesc;
43-
import com.oracle.svm.shared.AlwaysInline;
4443
import com.oracle.svm.core.ArenaIntrinsics;
4544
import com.oracle.svm.core.NeverInline;
4645
import com.oracle.svm.core.SubstrateOptions;
@@ -49,6 +48,7 @@
4948
import com.oracle.svm.core.nodes.foreign.ScopedMemExceptionHandlerClusterNode.ExceptionInputNode;
5049
import com.oracle.svm.core.nodes.foreign.ScopedMemExceptionHandlerClusterNode.ExceptionPathNode;
5150
import com.oracle.svm.core.nodes.foreign.ScopedMemExceptionHandlerClusterNode.RegularPathNode;
51+
import com.oracle.svm.shared.AlwaysInline;
5252
import com.oracle.svm.shared.util.LogUtils;
5353
import com.oracle.svm.shared.util.VMError;
5454

@@ -61,7 +61,7 @@
6161
/**
6262
* For details on the implementation of shared arenas on substrate see
6363
* {@link Target_jdk_internal_misc_ScopedMemoryAccess}.
64-
*
64+
* <p>
6565
* Note that this code should only be called by substitutions in
6666
* {@link Target_jdk_internal_misc_ScopedMemoryAccess}.
6767
*/
@@ -111,7 +111,7 @@ public static void checkValidStateRawInRuntimeCompiledCode(MemorySessionImpl ses
111111

112112
/**
113113
* Handles exceptions related to memory sessions within a specific arena scope.
114-
*
114+
* <p>
115115
* This method checks if the {@link java.lang.foreign.Arena} associated with {@code session} is
116116
* in a valid state. If validation fails, it logs the exception and propagates it through the
117117
* exception path.
@@ -205,12 +205,12 @@ private static String memoryLayoutToString(MemoryLayout memoryLayout) {
205205
}
206206

207207
@NeverInline("inlining cut off")
208-
static IllegalArgumentException getSymbolIsNullException(MemorySegment symbol) {
209-
return new IllegalArgumentException("Symbol is NULL: " + symbol);
208+
static void throwSymbolIsNullException(MemorySegment symbol) {
209+
throw new IllegalArgumentException("Symbol is NULL: " + symbol);
210210
}
211211

212212
@NeverInline("inlining cut off")
213-
static IllegalArgumentException getHeapSegmentNotAllowedException(MemorySegment segment) {
214-
return new IllegalArgumentException("Heap segment not allowed: " + segment);
213+
static void throwHeapSegmentNotAllowedException(MemorySegment segment) {
214+
throw new IllegalArgumentException("Heap segment not allowed: " + segment);
215215
}
216216
}

substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/Target_jdk_internal_foreign_abi_NativeEntryPoint.java

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727
import java.lang.invoke.MethodType;
2828

2929
import org.graalvm.nativeimage.MissingForeignRegistrationError;
30+
import org.graalvm.nativeimage.c.function.CFunctionPointer;
3031
import org.graalvm.nativeimage.hosted.FieldValueTransformer;
3132

32-
import com.oracle.svm.core.FunctionPointerHolder;
3333
import com.oracle.svm.core.annotate.RecomputeFieldValue;
3434
import com.oracle.svm.core.annotate.Substitute;
3535
import com.oracle.svm.core.annotate.TargetClass;
36+
import com.oracle.svm.core.util.UserError;
37+
import com.oracle.svm.core.util.UserError.UserException;
3638
import com.oracle.svm.shared.util.VMError;
3739
import com.oracle.svm.util.GuestAccess;
3840

@@ -42,10 +44,9 @@
4244
import jdk.vm.ci.meta.JavaConstant;
4345

4446
/**
45-
* Packs the address of a {@link com.oracle.svm.hosted.foreign.DowncallStub} with some extra
47+
* Packs the address of a {@code com.oracle.svm.hosted.foreign.DowncallStub} with some extra
4648
* information.
4749
*/
48-
@SuppressWarnings("javadoc")
4950
@TargetClass(value = NativeEntryPoint.class, onlyWith = ForeignAPIPredicates.Enabled.class)
5051
@Substitute
5152
public final class Target_jdk_internal_foreign_abi_NativeEntryPoint {
@@ -55,18 +56,18 @@ public final class Target_jdk_internal_foreign_abi_NativeEntryPoint {
5556
private MethodType methodType;
5657

5758
@RecomputeFieldValue(isFinal = true, kind = RecomputeFieldValue.Kind.Custom, declClass = DowncallAddressTransformer.class) //
58-
final FunctionPointerHolder downcallStubPointerHolder;
59+
final CFunctionPointer downcallStubPointer;
5960

6061
@RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset) //
6162
final int captureMask;
6263

6364
@RecomputeFieldValue(isFinal = true, kind = RecomputeFieldValue.Kind.Custom, declClass = DowncallInvokerAddressTransformer.class) //
64-
final FunctionPointerHolder downcallInvokerPointerHolder;
65+
final CFunctionPointer downcallInvokerPointer;
6566

66-
Target_jdk_internal_foreign_abi_NativeEntryPoint(MethodType methodType, FunctionPointerHolder downcallStubPointerHolder, int captureMask) {
67+
Target_jdk_internal_foreign_abi_NativeEntryPoint(MethodType methodType, CFunctionPointer downcallStubPointer, int captureMask) {
6768
this.methodType = methodType;
68-
this.downcallStubPointerHolder = downcallStubPointerHolder;
69-
this.downcallInvokerPointerHolder = ForeignFunctionsRuntime.singleton().getDowncallStubInvokerPointerHolder(methodType);
69+
this.downcallStubPointer = downcallStubPointer;
70+
this.downcallInvokerPointer = ForeignFunctionsRuntime.singleton().getDowncallStubInvokerPointer(methodType);
7071
this.captureMask = captureMask;
7172
}
7273

@@ -92,8 +93,8 @@ public static Target_jdk_internal_foreign_abi_NativeEntryPoint make(ABIDescripto
9293
* construction in the NativeEntryPointInfo.make function.
9394
*/
9495
boolean allowHeapAccess = false;
95-
for (int i = 0; i < argMoves.length; i++) {
96-
if (argMoves[i] == null) {
96+
for (VMStorage argMove : argMoves) {
97+
if (argMove == null) {
9798
allowHeapAccess = true;
9899
break;
99100
}
@@ -109,35 +110,43 @@ public MethodType type() {
109110
static final class MethodTypeTransformer implements FieldValueTransformer {
110111
@Override
111112
public Object transform(Object receiver, Object originalValue) {
112-
VMError.guarantee(receiver.getClass() == NativeEntryPoint.class);
113+
assert receiver.getClass() == NativeEntryPoint.class;
113114
return ((NativeEntryPoint) receiver).type();
114115
}
115116
}
116117

117118
static final class DowncallAddressTransformer implements FieldValueTransformer {
118119
@Override
119120
public Object transform(Object receiver, Object originalValue) {
120-
VMError.guarantee(receiver.getClass() == NativeEntryPoint.class);
121+
assert receiver.getClass() == NativeEntryPoint.class;
121122
try {
122123
JavaConstant nativeEntryPoint = GuestAccess.get().getSnippetReflection().forObject(receiver);
123124
NativeEntryPointInfo nativeEntryPointInfo = NativeEntryPointHelper.extractNativeEntryPointInfo(nativeEntryPoint);
124-
return ForeignFunctionsRuntime.singleton().getDowncallStubPointerHolder(nativeEntryPointInfo);
125+
VMError.guarantee(nativeEntryPointInfo != null, "Cannot extract info for NativeEntryPoint because it is not in NEP_CACHE");
126+
return ForeignFunctionsRuntime.singleton().getDowncallStubPointer(nativeEntryPointInfo);
125127
} catch (MissingForeignRegistrationError e) {
126-
// explicitly catch and rethrow with VMError; otherwise, it may be ignored
127-
throw VMError.shouldNotReachHere(e);
128+
throw rethrowMissingForeignRegistrationError(e);
128129
}
129130
}
131+
132+
static UserException rethrowMissingForeignRegistrationError(MissingForeignRegistrationError e) {
133+
/*
134+
* MissingForeignRegistrationError is a LinkageError, which are deliberately ignored in
135+
* the ImageHeapScanner when reading a hosted field value. Therefore, explicitly catch
136+
* and rethrow with UserError.
137+
*/
138+
throw UserError.abort(e, "Missing downcall stub registration");
139+
}
130140
}
131141

132142
static final class DowncallInvokerAddressTransformer implements FieldValueTransformer {
133143
@Override
134144
public Object transform(Object receiver, Object originalValue) {
135-
VMError.guarantee(receiver.getClass() == NativeEntryPoint.class);
145+
assert receiver.getClass() == NativeEntryPoint.class;
136146
try {
137-
return ForeignFunctionsRuntime.singleton().getDowncallStubInvokerPointerHolder(((NativeEntryPoint) receiver).type());
147+
return ForeignFunctionsRuntime.singleton().getDowncallStubInvokerPointer(((NativeEntryPoint) receiver).type());
138148
} catch (MissingForeignRegistrationError e) {
139-
// explicitly catch and rethrow with VMError; otherwise, it may be ignored
140-
throw VMError.shouldNotReachHere(e);
149+
throw DowncallAddressTransformer.rethrowMissingForeignRegistrationError(e);
141150
}
142151
}
143152
}

substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/Target_jdk_internal_foreign_abi_SharedUtils.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,27 @@
3434
import jdk.internal.foreign.abi.SharedUtils;
3535
import jdk.internal.vm.annotation.ForceInline;
3636

37+
/**
38+
* Substitutions of FFM API helper functions. Those substitutions are not required for correctness.
39+
* They prevent the exception path (which is a slow path) from being inlined during method handle
40+
* intrinsification and reduce the code size in the intrinsification scope.
41+
*/
3742
@TargetClass(value = SharedUtils.class, onlyWith = ForeignAPIPredicates.Enabled.class)
3843
final class Target_jdk_internal_foreign_abi_SharedUtils {
3944
@ForceInline
4045
@Substitute
4146
public static void checkSymbol(MemorySegment symbol) {
4247
Objects.requireNonNull(symbol);
4348
if (MemorySegment.NULL.equals(symbol)) {
44-
throw SubstrateForeignUtil.getSymbolIsNullException(symbol);
49+
SubstrateForeignUtil.throwSymbolIsNullException(symbol);
4550
}
4651
}
4752

4853
@ForceInline
4954
@Substitute
5055
public static void checkNative(MemorySegment segment) {
5156
if (!segment.isNative()) {
52-
throw SubstrateForeignUtil.getHeapSegmentNotAllowedException(segment);
57+
SubstrateForeignUtil.throwHeapSegmentNotAllowedException(segment);
5358
}
5459
}
5560
}

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/invoke/MethodHandleUtils.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,17 @@
2828

2929
import java.lang.invoke.MethodHandle;
3030
import java.lang.invoke.MethodType;
31+
import java.lang.reflect.Executable;
32+
33+
import org.graalvm.nativeimage.Platform.HOSTED_ONLY;
34+
import org.graalvm.nativeimage.Platforms;
3135

3236
import com.oracle.svm.core.ForeignSupport;
3337
import com.oracle.svm.core.hub.RuntimeClassLoading;
3438
import com.oracle.svm.core.methodhandles.MethodHandleInterpreterUtils;
3539
import com.oracle.svm.shared.AlwaysInline;
3640

41+
import jdk.vm.ci.meta.JavaKind;
3742
import sun.invoke.util.Wrapper;
3843

3944
public class MethodHandleUtils {
@@ -178,4 +183,14 @@ public static ResolvedMember getResolvedMember(Target_java_lang_invoke_MemberNam
178183
}
179184
return null;
180185
}
186+
187+
@Platforms(HOSTED_ONLY.class)
188+
public static Executable getUnboxMethod(JavaKind returnKind, boolean crema, Class<?>... parameterTypes) throws NoSuchMethodException {
189+
return MethodHandleUtils.class.getMethod(returnKind + (crema ? "UnboxCrema" : "Unbox"), parameterTypes);
190+
}
191+
192+
@Platforms(HOSTED_ONLY.class)
193+
public static Executable getUnboxForeignMethod(JavaKind returnKind) throws NoSuchMethodException {
194+
return MethodHandleUtils.class.getMethod(returnKind + "UnboxForeign", Object.class, Object.class);
195+
}
181196
}

0 commit comments

Comments
 (0)