Skip to content

Commit 753b81f

Browse files
fangererfangerer
committed
[GR-60350] Correctly handle in-memory return for direct upcalls.
PullRequest: graal/20341
2 parents 3f079a0 + 724f69e commit 753b81f

File tree

4 files changed

+155
-35
lines changed

4 files changed

+155
-35
lines changed

‎substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/AbiUtils.java

Lines changed: 79 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,22 @@
3333
import java.lang.foreign.MemoryLayout;
3434
import java.lang.foreign.ValueLayout;
3535
import java.lang.invoke.MethodType;
36+
import java.lang.reflect.Method;
3637
import java.util.ArrayList;
3738
import java.util.Arrays;
3839
import java.util.Collections;
3940
import java.util.EnumMap;
4041
import java.util.List;
4142
import java.util.Map;
4243
import java.util.Objects;
44+
import java.util.Optional;
4345
import java.util.function.Consumer;
4446
import java.util.stream.Stream;
4547

4648
import org.graalvm.nativeimage.ImageSingletons;
4749
import org.graalvm.nativeimage.Isolate;
4850
import org.graalvm.nativeimage.Platform;
51+
import org.graalvm.nativeimage.Platform.HOSTED_ONLY;
4952
import org.graalvm.nativeimage.Platforms;
5053
import org.graalvm.word.Pointer;
5154

@@ -59,6 +62,7 @@
5962
import com.oracle.svm.core.util.BasedOnJDKClass;
6063
import com.oracle.svm.core.util.BasedOnJDKFile;
6164
import com.oracle.svm.core.util.VMError;
65+
import com.oracle.svm.util.ReflectionUtil;
6266

6367
import jdk.graal.compiler.api.replacements.Fold;
6468
import jdk.graal.compiler.asm.Label;
@@ -83,7 +87,6 @@
8387
import jdk.internal.foreign.abi.VMStorage;
8488
import jdk.internal.foreign.abi.aarch64.AArch64Architecture;
8589
import jdk.internal.foreign.abi.x64.X86_64Architecture;
86-
import jdk.internal.foreign.abi.x64.sysv.CallArranger;
8790
import jdk.vm.ci.aarch64.AArch64;
8891
import jdk.vm.ci.amd64.AMD64;
8992
import jdk.vm.ci.code.Register;
@@ -491,15 +494,31 @@ public static AbiUtils singleton() {
491494
return ImageSingletons.lookup(AbiUtils.class);
492495
}
493496

497+
/**
498+
* Specifies if a method handle invoked by an upcall stub needs to drop its return value in case
499+
* of an in-memory return type. See also: {@link java.lang.invoke.MethodHandles#dropReturn} and
500+
* {@code jdk.internal.foreign.abi.SharedUtils#adaptUpcallForIMR}
501+
*/
502+
@Platforms(Platform.HOSTED_ONLY.class)
503+
public abstract boolean dropReturn();
504+
505+
/**
506+
* Calls method {@code isInMemoryReturn} of the appropriate {@code CallArranger}. This method
507+
* determines, if a given return type requires an in-memory return on the current platform.
508+
*/
509+
@Platforms(Platform.HOSTED_ONLY.class)
510+
public abstract boolean isInMemoryReturn(Optional<MemoryLayout> returnLayout);
511+
494512
protected abstract CallingSequence makeCallingSequence(MethodType type, FunctionDescriptor desc, boolean forUpcall, LinkerOptions options);
495513

496514
/**
497515
* This method re-implements a part of the logic from the JDK so that we can get the callee-type
498516
* (i.e. the ABI low-level type) of a function from its descriptor.
499517
*/
518+
@Platforms(HOSTED_ONLY.class)
500519
@BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-25+18/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java#L99")
501520
@BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-25+18/src/java.base/share/classes/jdk/internal/foreign/abi/DowncallLinker.java#L71-L85")
502-
public NativeEntryPointInfo makeNativeEntrypoint(FunctionDescriptor desc, LinkerOptions linkerOptions) {
521+
public final NativeEntryPointInfo makeNativeEntrypoint(FunctionDescriptor desc, LinkerOptions linkerOptions) {
503522
// From Linker.downcallHandle implemented in AbstractLinker.downcallHandle:
504523
// From AbstractLinker.downcallHandle0
505524
MethodType type = desc.toMethodType();
@@ -519,9 +538,9 @@ public NativeEntryPointInfo makeNativeEntrypoint(FunctionDescriptor desc, Linker
519538
linkerOptions.allowsHeapAccess());
520539
}
521540

522-
@BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-25+18/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java#L124")
541+
@BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-25+13/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java#L126")
523542
@BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-25+18/src/java.base/share/classes/jdk/internal/foreign/abi/UpcallLinker.java#L62-L110")
524-
public JavaEntryPointInfo makeJavaEntryPoint(FunctionDescriptor desc, LinkerOptions linkerOptions) {
543+
public final JavaEntryPointInfo makeJavaEntryPoint(FunctionDescriptor desc, LinkerOptions linkerOptions) {
525544
// Linker.upcallStub implemented in AbstractLinker.upcallStub
526545
MethodType type = desc.toMethodType();
527546

@@ -659,7 +678,7 @@ public Pointer write(Pointer at, Isolate isolate, Word methodHandle, Word stubPo
659678
}
660679

661680
@Platforms(Platform.HOSTED_ONLY.class)
662-
public abstract TrampolineTemplate generateTrampolineTemplate();
681+
abstract TrampolineTemplate generateTrampolineTemplate();
663682
}
664683

665684
class ABIs {
@@ -716,12 +735,26 @@ public int trampolineSize() {
716735
public TrampolineTemplate generateTrampolineTemplate() {
717736
return null;
718737
}
738+
739+
@Override
740+
public boolean dropReturn() {
741+
return fail();
742+
}
743+
744+
@Override
745+
public boolean isInMemoryReturn(Optional<MemoryLayout> returnLayout) {
746+
return fail();
747+
}
719748
}
720749

721750
@BasedOnJDKClass(AArch64Architecture.class)
722751
@BasedOnJDKClass(jdk.internal.foreign.abi.DowncallLinker.class)
723752
@BasedOnJDKClass(jdk.internal.foreign.abi.UpcallLinker.class)
724753
abstract static class ARM64 extends AbiUtils {
754+
755+
@Platforms(Platform.HOSTED_ONLY.class) //
756+
private static final Method IS_IN_MEMORY_RETURN = ReflectionUtil.lookupMethod(jdk.internal.foreign.abi.aarch64.CallArranger.class, "isInMemoryReturn", Optional.class);
757+
725758
@Override
726759
public Registers upcallSpecialArgumentsRegisters() {
727760
return new Registers(SubstrateAArch64MacroAssembler.scratch1, SubstrateAArch64MacroAssembler.scratch2);
@@ -815,6 +848,17 @@ public TrampolineTemplate generateTrampolineTemplate() {
815848

816849
return new TrampolineTemplate(assembly, posIsolate, posMHArray, posCallTarget);
817850
}
851+
852+
@BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-25+13/src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/CallArranger.java#L195")
853+
@Override
854+
public boolean dropReturn() {
855+
return true;
856+
}
857+
858+
@Override
859+
public boolean isInMemoryReturn(Optional<MemoryLayout> returnLayout) {
860+
return ReflectionUtil.invokeMethod(IS_IN_MEMORY_RETURN, null, returnLayout);
861+
}
818862
}
819863

820864
@BasedOnJDKClass(jdk.internal.foreign.abi.aarch64.linux.LinuxAArch64Linker.class)
@@ -961,6 +1005,10 @@ public TrampolineTemplate generateTrampolineTemplate() {
9611005
@BasedOnJDKClass(jdk.internal.foreign.abi.x64.sysv.SysVx64Linker.class)
9621006
@BasedOnJDKClass(jdk.internal.foreign.abi.x64.sysv.CallArranger.class)
9631007
static final class SysV extends X86_64 {
1008+
1009+
@Platforms(Platform.HOSTED_ONLY.class) //
1010+
private static final Method IS_IN_MEMORY_RETURN = ReflectionUtil.lookupMethod(jdk.internal.foreign.abi.x64.sysv.CallArranger.class, "isInMemoryReturn", Optional.class);
1011+
9641012
@Override
9651013
protected CallingSequence makeCallingSequence(MethodType type, FunctionDescriptor desc, boolean forUpcall, LinkerOptions options) {
9661014
return jdk.internal.foreign.abi.x64.sysv.CallArranger.getBindings(type, desc, forUpcall, options).callingSequence();
@@ -1002,11 +1050,26 @@ public void checkLibrarySupport() {
10021050
public Map<String, MemoryLayout> canonicalLayouts() {
10031051
return SharedUtils.canonicalLayouts(ValueLayout.JAVA_LONG, ValueLayout.JAVA_LONG, ValueLayout.JAVA_INT);
10041052
}
1053+
1054+
@BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-25+13/src/java.base/share/classes/jdk/internal/foreign/abi/x64/sysv/CallArranger.java#L147")
1055+
@Override
1056+
public boolean dropReturn() {
1057+
return true;
1058+
}
1059+
1060+
@Override
1061+
public boolean isInMemoryReturn(Optional<MemoryLayout> returnLayout) {
1062+
return ReflectionUtil.invokeMethod(IS_IN_MEMORY_RETURN, null, returnLayout);
1063+
}
10051064
}
10061065

10071066
@BasedOnJDKClass(jdk.internal.foreign.abi.x64.windows.Windowsx64Linker.class)
10081067
@BasedOnJDKClass(jdk.internal.foreign.abi.x64.windows.CallArranger.class)
10091068
static final class Win64 extends X86_64 {
1069+
1070+
@Platforms(Platform.HOSTED_ONLY.class) //
1071+
private static final Method IS_IN_MEMORY_RETURN = ReflectionUtil.lookupMethod(jdk.internal.foreign.abi.x64.windows.CallArranger.class, "isInMemoryReturn", Optional.class);
1072+
10101073
@Override
10111074
protected CallingSequence makeCallingSequence(MethodType type, FunctionDescriptor desc, boolean forUpcall, LinkerOptions options) {
10121075
return jdk.internal.foreign.abi.x64.windows.CallArranger.getBindings(type, desc, forUpcall, options).callingSequence();
@@ -1058,6 +1121,17 @@ public void checkLibrarySupport() {
10581121
public Map<String, MemoryLayout> canonicalLayouts() {
10591122
return SharedUtils.canonicalLayouts(ValueLayout.JAVA_INT, ValueLayout.JAVA_LONG, ValueLayout.JAVA_CHAR);
10601123
}
1124+
1125+
@BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-25+13/src/java.base/share/classes/jdk/internal/foreign/abi/x64/windows/CallArranger.java#L139")
1126+
@Override
1127+
public boolean dropReturn() {
1128+
return false;
1129+
}
1130+
1131+
@Override
1132+
public boolean isInMemoryReturn(Optional<MemoryLayout> returnLayout) {
1133+
return ReflectionUtil.invokeMethod(IS_IN_MEMORY_RETURN, null, returnLayout);
1134+
}
10611135
}
10621136

10631137
@BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-25+11/src/java.base/share/classes/jdk/internal/foreign/abi/DowncallLinker.java#L122-L140")

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static jdk.graal.compiler.core.common.spi.ForeignCallDescriptor.CallSideEffect.HAS_SIDE_EFFECT;
2828

2929
import java.lang.constant.DirectMethodHandleDesc;
30+
import java.lang.foreign.FunctionDescriptor;
3031
import java.lang.foreign.MemorySegment;
3132
import java.lang.foreign.MemorySegment.Scope;
3233
import java.lang.invoke.MethodHandle;
@@ -36,6 +37,7 @@
3637
import java.util.function.BiConsumer;
3738

3839
import org.graalvm.collections.EconomicMap;
40+
import org.graalvm.collections.Pair;
3941
import org.graalvm.nativeimage.ImageSingletons;
4042
import org.graalvm.nativeimage.Platform;
4143
import org.graalvm.nativeimage.Platforms;
@@ -64,6 +66,7 @@
6466
import jdk.internal.foreign.CABI;
6567
import jdk.internal.foreign.MemorySessionImpl;
6668
import jdk.internal.foreign.abi.CapturableState;
69+
import jdk.internal.foreign.abi.LinkerOptions;
6770

6871
public class ForeignFunctionsRuntime implements ForeignSupport {
6972
@Fold
@@ -73,7 +76,7 @@ public static ForeignFunctionsRuntime singleton() {
7376

7477
private final AbiUtils.TrampolineTemplate trampolineTemplate;
7578
private final EconomicMap<NativeEntryPointInfo, FunctionPointerHolder> downcallStubs = EconomicMap.create();
76-
private final EconomicMap<DirectMethodHandleDesc, FunctionPointerHolder> directUpcallStubs = EconomicMap.create();
79+
private final EconomicMap<Pair<DirectMethodHandleDesc, JavaEntryPointInfo>, FunctionPointerHolder> directUpcallStubs = EconomicMap.create();
7780
private final EconomicMap<JavaEntryPointInfo, FunctionPointerHolder> upcallStubs = EconomicMap.create();
7881

7982
private final Map<Long, TrampolineSet> trampolines = new HashMap<>();
@@ -115,9 +118,10 @@ public void addUpcallStubPointer(JavaEntryPointInfo jep, CFunctionPointer ptr) {
115118
}
116119

117120
@Platforms(Platform.HOSTED_ONLY.class)
118-
public void addDirectUpcallStubPointer(DirectMethodHandleDesc desc, CFunctionPointer ptr) {
119-
VMError.guarantee(!directUpcallStubs.containsKey(desc), "Seems like multiple stubs were generated for %s", desc);
120-
VMError.guarantee(directUpcallStubs.put(desc, new FunctionPointerHolder(ptr)) == null);
121+
public void addDirectUpcallStubPointer(DirectMethodHandleDesc desc, JavaEntryPointInfo jep, CFunctionPointer ptr) {
122+
var key = Pair.create(desc, jep);
123+
VMError.guarantee(!directUpcallStubs.containsKey(key), "Seems like multiple stubs were generated for %s", desc);
124+
VMError.guarantee(directUpcallStubs.put(key, new FunctionPointerHolder(ptr)) == null);
121125
}
122126

123127
/**
@@ -168,8 +172,9 @@ Pointer registerForUpcall(MethodHandle methodHandle, JavaEntryPointInfo jep) {
168172
* @param trampolineAddress The address of the upcall trampoline.
169173
* @param desc A direct method handle descriptor used to lookup the direct upcall stub.
170174
*/
171-
void patchForDirectUpcall(long trampolineAddress, DirectMethodHandleDesc desc) {
172-
FunctionPointerHolder functionPointerHolder = directUpcallStubs.get(desc);
175+
void patchForDirectUpcall(long trampolineAddress, DirectMethodHandleDesc desc, FunctionDescriptor functionDescriptor, LinkerOptions options) {
176+
JavaEntryPointInfo jep = AbiUtils.singleton().makeJavaEntryPoint(functionDescriptor, options);
177+
FunctionPointerHolder functionPointerHolder = directUpcallStubs.get(Pair.create(desc, jep));
173178
if (functionPointerHolder == null) {
174179
return;
175180
}

‎substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/Target_jdk_internal_foreign_abi_AbstractLinker.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ final class Target_jdk_internal_foreign_abi_SoftReferenceCache {
7171
*
7272
* @param delegate The original upcall stub factory as created by JDK's call arranger.
7373
*/
74-
record UpcallStubFactoryDecorator(UpcallStubFactory delegate) implements UpcallStubFactory {
74+
record UpcallStubFactoryDecorator(UpcallStubFactory delegate, FunctionDescriptor function, LinkerOptions options) implements UpcallStubFactory {
7575

7676
@Override
7777
public MemorySegment makeStub(MethodHandle target, Arena arena) {
@@ -86,7 +86,7 @@ public MemorySegment makeStub(MethodHandle target, Arena arena) {
8686
*/
8787
Optional<MethodHandleDesc> methodHandleDesc = target.describeConstable();
8888
if (methodHandleDesc.isPresent() && methodHandleDesc.get() instanceof DirectMethodHandleDesc desc) {
89-
ForeignFunctionsRuntime.singleton().patchForDirectUpcall(segment.address(), desc);
89+
ForeignFunctionsRuntime.singleton().patchForDirectUpcall(segment.address(), desc, function, options);
9090
}
9191
return segment;
9292
}
@@ -97,7 +97,7 @@ final class Target_jdk_internal_foreign_abi_x64_sysv_SysVx64Linker {
9797

9898
@Substitute
9999
UpcallStubFactory arrangeUpcall(MethodType targetType, FunctionDescriptor function, LinkerOptions options) {
100-
return new UpcallStubFactoryDecorator(jdk.internal.foreign.abi.x64.sysv.CallArranger.arrangeUpcall(targetType, function, options));
100+
return new UpcallStubFactoryDecorator(jdk.internal.foreign.abi.x64.sysv.CallArranger.arrangeUpcall(targetType, function, options), function, options);
101101
}
102102
}
103103

@@ -106,7 +106,7 @@ final class Target_jdk_internal_foreign_abi_x64_windows_Windowsx64Linker {
106106

107107
@Substitute
108108
UpcallStubFactory arrangeUpcall(MethodType targetType, FunctionDescriptor function, LinkerOptions options) {
109-
return new UpcallStubFactoryDecorator(jdk.internal.foreign.abi.x64.windows.CallArranger.arrangeUpcall(targetType, function, options));
109+
return new UpcallStubFactoryDecorator(jdk.internal.foreign.abi.x64.windows.CallArranger.arrangeUpcall(targetType, function, options), function, options);
110110
}
111111
}
112112

@@ -115,7 +115,7 @@ final class Target_jdk_internal_foreign_abi_aarch64_macos_MacOsAArch64Linker {
115115

116116
@Substitute
117117
UpcallStubFactory arrangeUpcall(MethodType targetType, FunctionDescriptor function, LinkerOptions options) {
118-
return new UpcallStubFactoryDecorator(jdk.internal.foreign.abi.aarch64.CallArranger.MACOS.arrangeUpcall(targetType, function, options));
118+
return new UpcallStubFactoryDecorator(jdk.internal.foreign.abi.aarch64.CallArranger.MACOS.arrangeUpcall(targetType, function, options), function, options);
119119
}
120120
}
121121

@@ -124,6 +124,6 @@ final class Target_jdk_internal_foreign_abi_aarch64_linux_LinuxAArch64Linker {
124124

125125
@Substitute
126126
UpcallStubFactory arrangeUpcall(MethodType targetType, FunctionDescriptor function, LinkerOptions options) {
127-
return new UpcallStubFactoryDecorator(jdk.internal.foreign.abi.aarch64.CallArranger.LINUX.arrangeUpcall(targetType, function, options));
127+
return new UpcallStubFactoryDecorator(jdk.internal.foreign.abi.aarch64.CallArranger.LINUX.arrangeUpcall(targetType, function, options), function, options);
128128
}
129129
}

0 commit comments

Comments
 (0)