Skip to content

Commit

Permalink
8305093: Linker cache should not take layout names into account
Browse files Browse the repository at this point in the history 
Reviewed-by: mcimadamore
  • Loading branch information
@JornVernee
JornVernee committed May 1, 2023
1 parent d437c61 commit 67dd841
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 5 deletions.
31 changes: 31 additions & 0 deletions src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java
View file
Expand Up @@ -34,7 +34,9 @@
import jdk.internal.foreign.abi.x64.sysv.SysVx64Linker;
import jdk.internal.foreign.abi.x64.windows.Windowsx64Linker;
import jdk.internal.foreign.layout.AbstractLayout;
import jdk.internal.foreign.layout.ValueLayouts;

import java.lang.foreign.AddressLayout;
import java.lang.foreign.GroupLayout;
import java.lang.foreign.MemoryLayout;
import java.lang.foreign.Arena;
Expand All @@ -48,6 +50,7 @@
import java.lang.foreign.ValueLayout;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodType;
import java.util.List;
import java.nio.ByteOrder;
import java.util.Objects;

Expand All @@ -69,6 +72,7 @@ public MethodHandle downcallHandle(FunctionDescriptor function, Option... option
Objects.requireNonNull(function);
Objects.requireNonNull(options);
checkLayouts(function);
function = stripNames(function);
LinkerOptions optionSet = LinkerOptions.forDowncall(function, options);

return DOWNCALL_CACHE.get(new LinkRequest(function, optionSet), linkRequest -> {
Expand All @@ -88,6 +92,7 @@ public MemorySegment upcallStub(MethodHandle target, FunctionDescriptor function
Objects.requireNonNull(function);
checkLayouts(function);
SharedUtils.checkExceptions(target);
function = stripNames(function);
LinkerOptions optionSet = LinkerOptions.forUpcall(function, options);

MethodType type = function.toMethodType();
Expand Down Expand Up @@ -174,6 +179,32 @@ private static void checkHasNaturalAlignment(MemoryLayout layout) {
}
}

private static MemoryLayout stripNames(MemoryLayout ml) {
// we don't care about transferring alignment and byte order here
// since the linker already restricts those such that they will always be the same
return switch (ml) {
case StructLayout sl -> MemoryLayout.structLayout(stripNames(sl.memberLayouts()));
case UnionLayout ul -> MemoryLayout.unionLayout(stripNames(ul.memberLayouts()));
case SequenceLayout sl -> MemoryLayout.sequenceLayout(sl.elementCount(), stripNames(sl.elementLayout()));
case AddressLayout al -> al.targetLayout()
.map(tl -> al.withoutName().withTargetLayout(stripNames(tl)))
.orElseGet(al::withoutName);
default -> ml.withoutName(); // ValueLayout and PaddingLayout
};
}

private static MemoryLayout[] stripNames(List<MemoryLayout> layouts) {
return layouts.stream()
.map(AbstractLinker::stripNames)
.toArray(MemoryLayout[]::new);
}

private static FunctionDescriptor stripNames(FunctionDescriptor function) {
return function.returnLayout()
.map(rl -> FunctionDescriptor.of(stripNames(rl), stripNames(function.argumentLayouts())))
.orElseGet(() -> FunctionDescriptor.ofVoid(stripNames(function.argumentLayouts())));
}

private void checkByteOrder(ValueLayout vl) {
if (vl.order() != linkerByteOrder()) {
throw new IllegalArgumentException("Layout does not have the right byte order: " + vl);
Expand Down
64 changes: 59 additions & 5 deletions test/jdk/java/foreign/TestLinker.java
View file
Expand Up @@ -35,20 +35,74 @@
import java.lang.foreign.Linker;
import java.lang.invoke.MethodHandle;

import static java.lang.foreign.MemoryLayout.*;
import static java.lang.foreign.ValueLayout.JAVA_CHAR;
import static java.lang.foreign.ValueLayout.JAVA_SHORT;
import static org.testng.Assert.assertSame;
import static org.testng.Assert.assertNotSame;

public class TestLinker extends NativeTestHelper {

@Test
public void testLinkerOptionsCache() {
record LinkRequest(FunctionDescriptor descriptor, Linker.Option... options) {}

@Test(dataProvider = "notSameCases")
public void testLinkerOptionsCache(LinkRequest l1, LinkRequest l2) {
Linker linker = Linker.nativeLinker();
FunctionDescriptor descriptor = FunctionDescriptor.ofVoid(C_INT, C_INT);
MethodHandle mh1 = linker.downcallHandle(descriptor);
MethodHandle mh2 = linker.downcallHandle(descriptor, Linker.Option.firstVariadicArg(1));
MethodHandle mh1 = linker.downcallHandle(l1.descriptor(), l1.options());
MethodHandle mh2 = linker.downcallHandle(l2.descriptor(), l2.options());
// assert that these are 2 distinct link request. No caching allowed
assertNotSame(mh1, mh2);
}

@DataProvider
public static Object[][] notSameCases() {
FunctionDescriptor fd_II_V = FunctionDescriptor.ofVoid(C_INT, C_INT);
return new Object[][]{
{new LinkRequest(fd_II_V), new LinkRequest(fd_II_V, Linker.Option.firstVariadicArg(1))},
{new LinkRequest(FunctionDescriptor.ofVoid(JAVA_SHORT)), new LinkRequest(FunctionDescriptor.ofVoid(JAVA_CHAR))},
{new LinkRequest(FunctionDescriptor.ofVoid(JAVA_SHORT)), new LinkRequest(FunctionDescriptor.ofVoid(JAVA_CHAR))},
};
}

@Test(dataProvider = "namedDescriptors")
public void testNamedLinkerCache(FunctionDescriptor f1, FunctionDescriptor f2) {
Linker linker = Linker.nativeLinker();
MethodHandle mh1 = linker.downcallHandle(f1);
MethodHandle mh2 = linker.downcallHandle(f2);
// assert that these are the same link request, even though layout names differ
assertSame(mh1, mh2);
}

@DataProvider
public static Object[][] namedDescriptors() {
return new Object[][]{
{ FunctionDescriptor.ofVoid(C_INT),
FunctionDescriptor.ofVoid(C_INT.withName("x")) },
{ FunctionDescriptor.ofVoid(structLayout(C_INT)),
FunctionDescriptor.ofVoid(structLayout(C_INT).withName("x")) },
{ FunctionDescriptor.ofVoid(structLayout(C_INT)),
FunctionDescriptor.ofVoid(structLayout(C_INT.withName("x"))) },
{ FunctionDescriptor.ofVoid(structLayout(C_INT, paddingLayout(32), C_LONG_LONG)),
FunctionDescriptor.ofVoid(structLayout(C_INT, paddingLayout(32), C_LONG_LONG.withName("x"))) },
{ FunctionDescriptor.ofVoid(structLayout(C_INT, paddingLayout(32), C_LONG_LONG)),
FunctionDescriptor.ofVoid(structLayout(C_INT, paddingLayout(32).withName("x"), C_LONG_LONG)) },
{ FunctionDescriptor.ofVoid(structLayout(sequenceLayout(1, C_INT))),
FunctionDescriptor.ofVoid(structLayout(sequenceLayout(1, C_INT).withName("x"))) },
{ FunctionDescriptor.ofVoid(structLayout(sequenceLayout(1, C_INT))),
FunctionDescriptor.ofVoid(structLayout(sequenceLayout(1, C_INT.withName("x")))) },
{ FunctionDescriptor.ofVoid(unionLayout(C_INT)),
FunctionDescriptor.ofVoid(unionLayout(C_INT).withName("x")) },
{ FunctionDescriptor.ofVoid(unionLayout(C_INT)),
FunctionDescriptor.ofVoid(unionLayout(C_INT.withName("x"))) },
{ FunctionDescriptor.ofVoid(C_POINTER),
FunctionDescriptor.ofVoid(C_POINTER.withName("x")) },
{ FunctionDescriptor.ofVoid(C_POINTER.withTargetLayout(C_INT)),
FunctionDescriptor.ofVoid(C_POINTER.withTargetLayout(C_INT.withName("x"))) },
{ FunctionDescriptor.ofVoid(C_POINTER.withTargetLayout(C_INT)),
FunctionDescriptor.ofVoid(C_POINTER.withName("x").withTargetLayout(C_INT.withName("x"))) },
};
}

@DataProvider
public static Object[][] invalidIndexCases() {
return new Object[][]{
Expand Down

1 comment on commit 67dd841

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.