From 3e79a4e9acc7330bafc71168e6074c2cc5e9bd4d Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Thu, 14 Nov 2024 18:28:57 +0100 Subject: [PATCH] ensure vtables are calculated for all types when using open world analysis. --- .../graal/pointsto/meta/AnalysisType.java | 5 + .../LayeredDispatchTableSupport.java | 2 +- .../oracle/svm/hosted/meta/VTableBuilder.java | 93 ++++++++++++++++--- 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java index 0bbf0fd8c254..67609925d68d 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java @@ -1369,6 +1369,10 @@ public AnalysisMethod findConstructor(Signature signature) { return null; } + public boolean isOpenTypeWorldDispatchTableMethodsCalculated() { + return dispatchTableMethods != null; + } + public Set getOpenTypeWorldDispatchTableMethods() { Objects.requireNonNull(dispatchTableMethods); return dispatchTableMethods; @@ -1403,6 +1407,7 @@ public Set getOrCalculateOpenTypeWorldDispatchTableMethods() { } try { AnalysisMethod aMethod = universe.lookup(m); + assert aMethod != null : m; resultSet.add(aMethod); } catch (UnsupportedFeatureException t) { /* diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java index 6e8dd0f4653c..8de26bb55c08 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java @@ -318,7 +318,7 @@ public void registerWrittenDynamicHub(DynamicHub hub, AnalysisUniverse aUniverse assert hType.getWrapped().isReachable() : "All installed hubs should be reachable " + hType; int vtableLength = Array.getLength(vTable); - if (!VTableBuilder.needsDispatchTable(hType) && !hType.isArray()) { + if (VTableBuilder.hasEmptyDispatchTable(hType)) { assert vtableLength == 0 : hType; return; } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java index 9fbb3ab05491..e7af50518520 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java @@ -47,13 +47,13 @@ public final class VTableBuilder { private final HostedUniverse hUniverse; private final HostedMetaAccess hMetaAccess; - private final boolean closedTypeWorldHubLayout; - private final boolean imageLayer = ImageLayerBuildingSupport.buildingImageLayer(); + + private final OpenTypeWorldHubLayoutUtils openHubUtils; private VTableBuilder(HostedUniverse hUniverse, HostedMetaAccess hMetaAccess) { this.hUniverse = hUniverse; this.hMetaAccess = hMetaAccess; - closedTypeWorldHubLayout = SubstrateOptions.useClosedTypeWorldHubLayout(); + openHubUtils = SubstrateOptions.useClosedTypeWorldHubLayout() ? null : new OpenTypeWorldHubLayoutUtils(hUniverse); } public static void buildTables(HostedUniverse hUniverse, HostedMetaAccess hMetaAccess) { @@ -66,8 +66,67 @@ public static void buildTables(HostedUniverse hUniverse, HostedMetaAccess hMetaA } } - private static boolean shouldIncludeType(HostedType type) { - return type.getWrapped().isReachable() || type.getWrapped().isTrackedAcrossLayers(); + private static class OpenTypeWorldHubLayoutUtils { + private final boolean closedTypeWorld; + private final boolean registerTrackedTypes; + private final boolean registerAllTypes; + private final boolean filterVTableMethods; + + OpenTypeWorldHubLayoutUtils(HostedUniverse hUniverse) { + closedTypeWorld = SubstrateOptions.useClosedTypeWorld(); + + /* + * We only filter vtable methods when we are building a non-layered image under the + * closed type world assumption. For layered builds, we must keep all vtable methods to + * ensure consistency across layers. + * + * With the closed type world assumption we can filter out methods that we know will be + * simplified to direct calls (i.e., when at most a single implementation exists for the + * given target). See generateDispatchTable for the use of this filter. + */ + filterVTableMethods = closedTypeWorld && !ImageLayerBuildingSupport.buildingImageLayer(); + + registerTrackedTypes = hUniverse.hostVM().enableTrackAcrossLayers(); + registerAllTypes = ImageLayerBuildingSupport.buildingApplicationLayer(); + assert !(registerTrackedTypes && registerAllTypes) : "We expect these flags to be mutually exclusive"; + assert (registerTrackedTypes || registerAllTypes) == ImageLayerBuildingSupport.buildingImageLayer() : "Type information must be registered during layered image builds"; + } + + private boolean shouldIncludeType(HostedType type) { + if (closedTypeWorld) { + if (type.getWrapped().isInBaseLayer()) { + /* + * This check will be later removed. + * + * GR-60010 - We are currently loading base analysis types too late. + */ + return type.getWrapped().isOpenTypeWorldDispatchTableMethodsCalculated(); + } + + /* + * When using the closed type world we know calls to unreachable types will be + * removed via graph strengthening. It is also always possible to see base layer + * types. + */ + return type.getWrapped().isReachable() || type.getWrapped().isInBaseLayer(); + } else { + /* + * When using the open type world we are conservative and calculate metadata for all + * types seen during analysis. + */ + return type.getWrapped().isOpenTypeWorldDispatchTableMethodsCalculated(); + } + } + + private boolean shouldRegisterType(HostedType type) { + if (registerAllTypes) { + return true; + } + if (registerTrackedTypes && type.getWrapped().isTrackedAcrossLayers()) { + return true; + } + return false; + } } private boolean verifyOpenTypeWorldDispatchTables() { @@ -124,7 +183,7 @@ private List generateITable(HostedType type) { private List generateDispatchTable(HostedType type, int startingIndex) { Predicate includeMethod; - if (closedTypeWorldHubLayout) { + if (openHubUtils.filterVTableMethods) { // include only methods which will be indirect calls includeMethod = m -> m.implementations.length > 1 || m.wrapped.isVirtualRootMethod(); } else { @@ -141,7 +200,7 @@ private List generateDispatchTable(HostedType type, int startingIn index++; } - if (imageLayer) { + if (openHubUtils.shouldRegisterType(type)) { LayeredDispatchTableSupport.singleton().registerDeclaredDispatchInfo(type, table); } @@ -209,13 +268,13 @@ private void generateOpenTypeWorldDispatchTable(HostedInstanceClass type, Map