Skip to content

[GR-59403] Don't require all virtual roots to be installed in vtables. #10689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[GR-59403] Don't require all virtual roots to be installed in vtables. #10689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions ...atevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jni/access/JNIAccessibleMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,11 @@ CodePointer getCallWrapperAddress() {
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
CodePointer getJavaCallAddress(Object instance, boolean nonVirtual) {
if (!nonVirtual) {
if (SubstrateOptions.useClosedTypeWorldHubLayout()) {
assert vtableOffset != JNIAccessibleMethod.VTABLE_OFFSET_NOT_YET_COMPUTED;
if (vtableOffset != JNIAccessibleMethod.STATICALLY_BOUND_METHOD) {
assert vtableOffset != JNIAccessibleMethod.VTABLE_OFFSET_NOT_YET_COMPUTED;
if (vtableOffset != JNIAccessibleMethod.STATICALLY_BOUND_METHOD) {
if (SubstrateOptions.useClosedTypeWorldHubLayout()) {
return BarrieredAccess.readWord(instance.getClass(), vtableOffset, NamedLocationIdentity.FINAL_LOCATION);
}
} else {
assert vtableOffset != JNIAccessibleMethod.VTABLE_OFFSET_NOT_YET_COMPUTED;
if (vtableOffset != STATICALLY_BOUND_METHOD) {
} else {
long tableStartingOffset = LoadOpenTypeWorldDispatchTableStartingOffset.createOpenTypeWorldLoadDispatchTableStartingOffset(instance.getClass(), interfaceTypeID);

return BarrieredAccess.readWord(instance.getClass(), Word.pointer(tableStartingOffset + vtableOffset), NamedLocationIdentity.FINAL_LOCATION);
Expand Down
24 changes: 8 additions & 16 deletions ...tevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/SubstrateMethodAccessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,21 +106,13 @@ private CFunctionPointer invokeTarget(Object obj) {
* In case we have both a vtableOffset and a directTarget, the vtable lookup wins. For such
* methods, the directTarget is only used when doing an invokeSpecial.
*/
if (SubstrateOptions.useClosedTypeWorldHubLayout()) {
CFunctionPointer target;
if (vtableOffset == OFFSET_NOT_YET_COMPUTED) {
throw VMError.shouldNotReachHere("Missed vtableOffset recomputation at image build time");
} else if (vtableOffset != STATICALLY_BOUND) {
CFunctionPointer target;
if (vtableOffset == OFFSET_NOT_YET_COMPUTED) {
throw VMError.shouldNotReachHere("Missed vtableOffset recomputation at image build time");
} else if (vtableOffset != STATICALLY_BOUND) {
if (SubstrateOptions.useClosedTypeWorldHubLayout()) {
target = BarrieredAccess.readWord(obj.getClass(), vtableOffset, NamedLocationIdentity.FINAL_LOCATION);
} else {
target = directTarget;
}
return target;
} else {
CFunctionPointer target;
if (vtableOffset == OFFSET_NOT_YET_COMPUTED) {
throw VMError.shouldNotReachHere("Missed vtableOffset recomputation at image build time");
} else if (vtableOffset != STATICALLY_BOUND) {
long tableStartingOffset = LoadOpenTypeWorldDispatchTableStartingOffset.createOpenTypeWorldLoadDispatchTableStartingOffset(obj.getClass(), interfaceTypeID);

/*
Expand All @@ -129,11 +121,11 @@ private CFunctionPointer invokeTarget(Object obj) {
long methodOffset = tableStartingOffset + vtableOffset;

target = BarrieredAccess.readWord(obj.getClass(), Word.pointer(methodOffset), NamedLocationIdentity.FINAL_LOCATION);
} else {
target = directTarget;
}
return target;
} else {
target = directTarget;
}
return target;
}

@Override
Expand Down
47 changes: 36 additions & 11 deletions substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,25 @@ private List<HostedMethod> generateDispatchTable(HostedType type, int startingIn
// include only methods which will be indirect calls
includeMethod = m -> {
assert !m.isConstructor() : Assertions.errorMessage("Constructors should never be in dispatch tables", m);
return m.implementations.length > 1 || m.wrapped.isVirtualRootMethod();
if (m.implementations.length > 1) {
return true;
} else {
if (m.wrapped.isVirtualRootMethod()) {
return !m.canBeStaticallyBound();
} else {
return false;
}
}
};
} else {
includeMethod = m -> {
assert !m.isConstructor() : Assertions.errorMessage("Constructors should never be in dispatch tables", m);
return true;
/*
* We have to use the analysis method's canBeStaticallyBound implementation because
* within HostedMethod we sometimes do additional pruning when operating under the
* close type world assumption.
*/
return !m.getWrapped().canBeStaticallyBound();
};
}
var table = type.getWrapped().getOpenTypeWorldDispatchTableMethods().stream().map(hUniverse::lookup).filter(includeMethod).sorted(HostedUniverse.METHOD_COMPARATOR).toList();
Expand Down Expand Up @@ -466,18 +479,30 @@ private void buildVTable(HostedClass clazz, Map<HostedType, ArrayList<HostedMeth
}

private void assignImplementations(HostedType type, Map<HostedType, ArrayList<HostedMethod>> vtablesMap, Map<HostedType, BitSet> usedSlotsMap, Map<HostedMethod, Set<Integer>> vtablesSlots) {
/*
* Methods with 1 implementation do not need a vtable because invokes can be done as direct
* calls without the need for a vtable. Methods with 0 implementations are unreachable.
*
* However, virtual roots (even those with 0 implementations) that cannot be statically
* bound always need a vtable entry. This is because the vtable is used to invoke these
* methods via reflection and/or jni.
*/
Predicate<HostedMethod> vtableEntryRequired = (hMethod) -> {
if (hMethod.implementations.length > 1) {
return true;
} else {
if (hMethod.wrapped.isVirtualRootMethod()) {
return !hMethod.canBeStaticallyBound();
} else {
return false;
}
}
};

for (HostedMethod method : type.getAllDeclaredMethods()) {
/* We only need to look at methods that the static analysis registered as invoked. */
if (method.wrapped.isInvoked() || method.wrapped.isImplementationInvoked()) {
/*
* Methods with 1 implementation do not need a vtable because invokes can be done as
* direct calls without the need for a vtable. Methods with 0 implementations are
* unreachable.
*
* Methods manually registered as virtual root methods always need a vtable slot,
* even if there are 0 or 1 implementations.
*/
if (method.implementations.length > 1 || method.wrapped.isVirtualRootMethod()) {
if (vtableEntryRequired.test(method)) {
assert !method.isConstructor() : Assertions.errorMessage("Constructors should never be in vtables", method);
/*
* Find a suitable vtable slot for the method, taking the existing vtable
Expand Down
9 changes: 7 additions & 2 deletions ...ratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
import com.oracle.svm.core.hub.ClassForNameSupportFeature;
import com.oracle.svm.core.hub.DynamicHub;
import com.oracle.svm.core.meta.MethodPointer;
import com.oracle.svm.core.meta.SharedMethod;
import com.oracle.svm.core.reflect.ReflectionAccessorHolder;
import com.oracle.svm.core.reflect.SubstrateAccessor;
import com.oracle.svm.core.reflect.SubstrateConstructorAccessor;
Expand Down Expand Up @@ -220,6 +219,9 @@ private SubstrateAccessor createAccessor(AccessorKey key) {
if (!targetMethod.canBeStaticallyBound()) {
vtableOffset = SubstrateMethodAccessor.OFFSET_NOT_YET_COMPUTED;
}
if (callerSensitiveAdapter) {
VMError.guarantee(vtableOffset == SubstrateMethodAccessor.STATICALLY_BOUND, "Caller sensitive adapters should always be statically bound %s", targetMethod);
}
VMError.guarantee(directTarget != null || vtableOffset != SubstrateMethodAccessor.STATICALLY_BOUND, "Must have either a directTarget or a vtableOffset");
if (!targetMethod.isStatic()) {
receiverType = target.getDeclaringClass();
Expand Down Expand Up @@ -485,7 +487,10 @@ public Object transform(Object receiver, Object originalValue) {
SubstrateMethodAccessor accessor = (SubstrateMethodAccessor) receiver;

if (accessor.getVTableOffset() == SubstrateMethodAccessor.OFFSET_NOT_YET_COMPUTED) {
SharedMethod member = ImageSingletons.lookup(ReflectionFeature.class).hostedMetaAccess().lookupJavaMethod(accessor.getMember());
HostedMethod member = ImageSingletons.lookup(ReflectionFeature.class).hostedMetaAccess().lookupJavaMethod(accessor.getMember());
if (member.canBeStaticallyBound()) {
return SubstrateMethodAccessor.STATICALLY_BOUND;
}
if (SubstrateOptions.useClosedTypeWorldHubLayout()) {
return KnownOffsets.singleton().getVTableOffset(member.getVTableIndex(), true);
} else {
Expand Down
Loading