Skip to content

Commit 8f34ae1

Browse files
cstancuOracleLabsAutomation
authored andcommitted
[GR-80689] Reject uncovered direct field writes in extension layers.
PullRequest: graal/25740
2 parents 7916f6b + 50d85ad commit 8f34ae1

13 files changed

Lines changed: 232 additions & 34 deletions

File tree

‎substratevm/docs/architecture/PARTIALLY-OPEN-WORLD-ANALYSIS.md‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ An **extension policy** describes what that extension is allowed to introduce or
1818
An effect is **covered** when an analyzed method graph or a conservative summary represents all of its relevant runtime behavior.
1919
An **entry summary**, for example, represents the possible arguments of calls whose call sites are unavailable to the analysis.
2020

21-
A method can be known to the builder while analysis of its body is deferred to a later image layer.
21+
A method can be known to the builder while its body remains outside the current analysis, for example when deferred to a later image layer.
2222
Conversely, at run time an analyzed method can receive and use a concrete value supplied by an extension.
2323
The component that declares a class therefore does not, by itself, determine which effects the analysis covers.
2424

@@ -148,6 +148,40 @@ Enumerating known instantiated types is insufficient when a field can hold value
148148
Such a field requires an open value approximation independently of the number of known types.
149149
A closed value bound permits a finite approximation only under the conditions in [Representing Possible Runtime Types](#representing-possible-runtime-types).
150150

151+
### Effects of Unanalyzed Method Bodies
152+
153+
Whether a method's entry is **open** (callers outside the current analysis may invoke it) or **closed** (all permitted calls are represented by analyzed call sites) is separate from whether its body is analyzed.
154+
An open entry requires a summary of arguments from callers whose call sites are unavailable to the analysis; when the body is analyzed, its stores propagate those values into fields.
155+
If a permitted method body is outside the current analysis, its effects require conservative summaries even when its declaring component is closed.
156+
157+
Suppose `ClassX` has a closed type hierarchy, but the extension policy permits the implementation of `ClassX.foo` to remain outside the current analysis.
158+
Knowing all subtypes of `ClassX` does not account for writes performed by that implementation.
159+
The extension policy must specify whether `ClassX.foo` may write `ClassX.f`.
160+
If writes are permitted, compiled readers of the field must account for them through analyzed stores or a conservative mutation summary.
161+
The extension mechanism must reject writes that are forbidden or whose effects are not covered.
162+
Summarizing the call's return values and continuation does not establish mutation coverage.
163+
164+
A field summary must represent unknown extension types when the effective field bound is open.
165+
If the value can be an array, the summary must cover its element values as well as its reference, including values reached through nested arrays.
166+
These requirements apply independently of how the extension is supplied.
167+
For example, a method deferred to a later image layer supplies no analyzed stores when the shared layer is compiled.
168+
Readers retained from that layer must already account for its permitted writes; analyzing the writer later cannot repair their assumptions.
169+
170+
### Conservative Validation of Layered Closed-Field Writes
171+
172+
Layered builds reject uncovered direct writes to fields deliberately excluded from shared-layer mutation summaries.
173+
The check uses stores observed after parsing and the parsing-support hook, before static-final-field folding, pre-analysis escape analysis, and final canonicalization.
174+
This is a conservative validation boundary: a recorded write can cause rejection even if a later optimization removes its store or enclosing branch.
175+
Parsing-time simplifications have already occurred, so the recorded writes are not a complete bytecode-level summary.
176+
177+
An immutable metadata record accompanies each persisted analysis graph and retains these validation obligations even when the optimized graph no longer contains the stores.
178+
Freshly parsed extension methods and reused graphs validate writes from this same collection boundary, before caller-specific specialization.
179+
The metadata does not mark fields as written in analysis or add mutation summaries.
180+
181+
Allowing eliminated writes requires distinguishing eliminations justified by facts that remain valid across layers from those relying on closed-world assumptions protected by this check.
182+
The absence of a store in an optimized graph alone does not establish that distinction.
183+
Until such a boundary is established, validation conservatively retains all writes observed at the current collection point.
184+
151185
## Architectural Requirements
152186

153187
These requirements describe the intended contract for every supported configuration.

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ExtensionPolicy.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ public interface ExtensionPolicy {
5656
/**
5757
* Returns whether the field is closed to direct writes from the open world. Closed fields still
5858
* receive values through analyzed stores, including values originating in the open world.
59-
* Unsafe access must be accounted for by the corresponding analysis handling.
59+
* Unsafe access must be accounted for by the corresponding analysis handling. A method whose
60+
* body is not analyzed at the current boundary may still write fields declared by a closed
61+
* component. Every permitted write must be covered by analyzed code or a conservative mutation
62+
* summary. The extension mechanism must reject writes for which that coverage cannot be
63+
* established.
6064
*/
6165
boolean isFieldClosed(ResolvedJavaField field);
6266

@@ -65,7 +69,8 @@ public interface ExtensionPolicy {
6569
* open-world entry seeding.
6670
* <p>
6771
* This does not close the method's parameter or return types, or the targets of calls made by
68-
* its body.
72+
* its body. Entry classification does not establish whether the body is analyzed or its effects
73+
* are covered by summaries.
6974
*/
7075
boolean isMethodEntryClosed(AnalysisMethod method);
7176

@@ -74,8 +79,9 @@ public interface ExtensionPolicy {
7479
* kind. This handling covers possible returned values and predicates indicating that the call
7580
* may return normally, including void calls.
7681
* <p>
77-
* This decision applies independently of the containing method's entry classification. Delayed
78-
* callees require additional handling when they are linked.
82+
* This decision applies independently of the containing method's entry classification. Callees
83+
* whose bodies are not analyzed require additional result and continuation coverage when they
84+
* are linked.
7985
*/
8086
boolean requiresOpenInvokeResult(AnalysisType receiverType, boolean directInvoke);
8187
}

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/HostVM.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141

4242
import com.oracle.graal.pointsto.BigBang;
4343
import com.oracle.graal.pointsto.PointsToAnalysis;
44+
import com.oracle.graal.pointsto.flow.AnalysisParsedGraph.GraphMetadata;
4445
import com.oracle.graal.pointsto.flow.AnalysisParsedGraph.Stage;
4546
import com.oracle.graal.pointsto.flow.InvokeTypeFlow;
4647
import com.oracle.graal.pointsto.meta.AnalysisField;
@@ -244,7 +245,7 @@ public void addMethodAfterParsingListener(BiConsumer<AnalysisMethod, StructuredG
244245
* {@link com.oracle.graal.pointsto.flow.AnalysisParsedGraph.Stage#isRequiredStage(Stage, AnalysisMethod)}
245246
* is true for the given method and stage {@link Stage#BYTECODE_PARSED}. If the hook is invoked,
246247
* it is guaranteed to be invoked before
247-
* {@link #methodAfterParsingHook(BigBang, AnalysisMethod, StructuredGraph)} .
248+
* {@link #methodAfterParsingHook}.
248249
*
249250
* @param bb the analysis engine
250251
* @param method the newly parsed method
@@ -258,14 +259,15 @@ public void methodAfterBytecodeParsedHook(BigBang bb, AnalysisMethod method, Str
258259
}
259260

260261
/**
261-
* Can be overwritten to run code after a method is parsed and all pre-analysis optimizations
262-
* are finished. This hook will be invoked before the graph is made available to the analysis.
262+
* Processes a freshly parsed method graph before encoding and publication to the analysis.
263+
* Implementations may apply pre-analysis optimizations and collect metadata for this parse.
263264
*
264265
* @param bb the analysis engine
265266
* @param method the newly parsed method
266267
* @param graph the method graph
268+
* @param metadata collects metadata to retain with this graph's encoding
267269
*/
268-
public void methodAfterParsingHook(BigBang bb, AnalysisMethod method, StructuredGraph graph) {
270+
public void methodAfterParsingHook(BigBang bb, AnalysisMethod method, StructuredGraph graph, GraphMetadata.Builder metadata) {
269271
for (BiConsumer<AnalysisMethod, StructuredGraph> listener : methodAfterParsingListeners) {
270272
listener.accept(method, graph);
271273
}

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AnalysisParsedGraph.java‎

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2021, 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
@@ -24,9 +24,12 @@
2424
*/
2525
package com.oracle.graal.pointsto.flow;
2626

27+
import java.util.List;
28+
2729
import com.oracle.graal.pointsto.BigBang;
2830
import com.oracle.graal.pointsto.api.HostVM;
2931
import com.oracle.graal.pointsto.infrastructure.GraphProvider.Purpose;
32+
import com.oracle.graal.pointsto.meta.AnalysisField;
3033
import com.oracle.graal.pointsto.meta.AnalysisMethod;
3134
import com.oracle.graal.pointsto.phases.SubstrateIntrinsicGraphBuilder;
3235
import com.oracle.graal.pointsto.util.AnalysisError;
@@ -128,20 +131,57 @@ public static boolean isRequiredStage(Stage stage, AnalysisMethod method) {
128131
HOST_ARCHITECTURE = compiler.getGraalRuntime().getCapability(RuntimeProvider.class).getHostBackend().getTarget().arch;
129132
}
130133

131-
private static final AnalysisParsedGraph EMPTY = new AnalysisParsedGraph(null, false);
134+
/**
135+
* Immutable metadata captured for one parsed graph, retained with its encoding so subsequent
136+
* parsing of the same method cannot replace it. Closed-field writes describe stores observed
137+
* at the collection boundary, including stores eliminated before encoding. They conservatively
138+
* retain validation obligations rather than describing only the encoded graph's remaining effects.
139+
*
140+
* @param closedFieldWrites always-closed fields written before optimizations can remove their
141+
* stores; this is validation metadata for later layers and does not mark fields as
142+
* written in the analysis
143+
*/
144+
public record GraphMetadata(List<AnalysisField> closedFieldWrites) {
145+
private static final GraphMetadata EMPTY = new GraphMetadata(List.of());
146+
147+
public GraphMetadata {
148+
closedFieldWrites = List.copyOf(closedFieldWrites);
149+
}
150+
151+
/** Collects metadata during one parsing operation, before publishing the encoded graph. */
152+
public static final class Builder {
153+
private List<AnalysisField> closedFieldWrites = List.of();
154+
155+
public void setClosedFieldWrites(List<AnalysisField> closedFieldWrites) {
156+
this.closedFieldWrites = List.copyOf(closedFieldWrites);
157+
}
158+
159+
public GraphMetadata build() {
160+
return closedFieldWrites.isEmpty() ? EMPTY : new GraphMetadata(closedFieldWrites);
161+
}
162+
}
163+
}
164+
165+
private static final AnalysisParsedGraph EMPTY = new AnalysisParsedGraph(null, false, GraphMetadata.EMPTY);
132166

133167
private final EncodedGraph encodedGraph;
134168
private final boolean isIntrinsic;
169+
private final GraphMetadata metadata;
135170

136-
public AnalysisParsedGraph(EncodedGraph encodedGraph, boolean isIntrinsic) {
171+
public AnalysisParsedGraph(EncodedGraph encodedGraph, boolean isIntrinsic, GraphMetadata metadata) {
137172
this.isIntrinsic = isIntrinsic;
138173
this.encodedGraph = encodedGraph;
174+
this.metadata = metadata;
139175
}
140176

141177
public EncodedGraph getEncodedGraph() {
142178
return encodedGraph;
143179
}
144180

181+
public GraphMetadata getMetadata() {
182+
return metadata;
183+
}
184+
145185
public boolean isIntrinsic() {
146186
return isIntrinsic;
147187
}
@@ -251,6 +291,7 @@ public static AnalysisParsedGraph createFinalStage(BigBang bb, AnalysisMethod me
251291
return optimizeAndEncode(bb, method, decodeParsedGraph(bb, method, stage1Graph), stage1Graph.isIntrinsic, Stage.OPTIMIZATIONS_APPLIED);
252292
}
253293

294+
/** Decode a non-empty parsed graph without inlining or applying optimizations. */
254295
@SuppressWarnings("try")
255296
private static StructuredGraph decodeParsedGraph(BigBang bb, AnalysisMethod method, AnalysisParsedGraph analysisParsedGraph) {
256297
// Persisted analysis graphs do not consume the structured optimization log, so avoid
@@ -281,13 +322,14 @@ private static AnalysisParsedGraph optimizeAndEncode(BigBang bb, AnalysisMethod
281322
* Must be called before any other thread can access the graph, i.e., before the graph
282323
* is published.
283324
*/
325+
GraphMetadata.Builder metadata = new GraphMetadata.Builder();
284326
switch (stage) {
285327
case BYTECODE_PARSED -> bb.getHostVM().methodAfterBytecodeParsedHook(bb, method, graph);
286-
case OPTIMIZATIONS_APPLIED -> bb.getHostVM().methodAfterParsingHook(bb, method, graph);
328+
case OPTIMIZATIONS_APPLIED -> bb.getHostVM().methodAfterParsingHook(bb, method, graph, metadata);
287329
}
288330

289331
EncodedGraph encodedGraph = GraphEncoder.encodeSingleGraph(graph, HOST_ARCHITECTURE);
290-
return new AnalysisParsedGraph(encodedGraph, isIntrinsic);
332+
return new AnalysisParsedGraph(encodedGraph, isIntrinsic, metadata.build());
291333
} catch (Throwable e) {
292334
throw graph.getDebug().handle(e);
293335
}

‎substratevm/src/com.oracle.svm.hosted/resources/SharedLayerSnapshotCapnProtoSchema.capnp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ struct PersistedAnalysisMethod {
107107
strengthenedGraphLocation @26 :Text;
108108
hostedMethodIndex @27 :HostedMethodIndex;
109109
compilationBehaviorOrdinal @28 :Int8;
110+
# IDs of always-closed fields written by the unspecialized graph, captured before caller-specific inlining and graph optimizations.
111+
closedFieldWrites @43 :List(FieldId);
110112
wrappedMethod :union {
111113
none @29 :Void; # default
112114
factoryMethod :group {

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ExtensionLayerImageFeature.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2024, 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
@@ -32,12 +32,15 @@
3232
import org.graalvm.collections.EconomicSet;
3333

3434
import com.oracle.graal.pointsto.api.HostVM;
35+
import com.oracle.graal.pointsto.meta.AnalysisField;
36+
import com.oracle.graal.pointsto.meta.AnalysisMethod;
3537
import com.oracle.graal.pointsto.meta.AnalysisType;
3638
import com.oracle.graal.pointsto.meta.AnalysisUniverse;
3739
import com.oracle.svm.core.c.BoxedRelocatedPointer;
3840
import com.oracle.svm.core.code.ImageCodeInfo;
3941
import com.oracle.svm.core.feature.InternalFeature;
4042
import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
43+
import com.oracle.svm.core.util.UserError;
4144
import com.oracle.svm.hosted.FeatureImpl.BeforeAnalysisAccessImpl;
4245
import com.oracle.svm.hosted.FeatureImpl.BeforeCompilationAccessImpl;
4346
import com.oracle.svm.hosted.imagelayer.HostedImageLayerBuildingSupport;
@@ -93,6 +96,27 @@ public void beforeAnalysis(BeforeAnalysisAccess a) {
9396
access.registerAsInHeap(ReflectionUtil.lookupClass(false, "java.util.concurrent.ConcurrentHashMap$CounterCell"));
9497
}
9598

99+
/** Validate writes to always-closed fields recorded from a newly parsed or persisted method graph. */
100+
static void checkNewClosedFieldWrites(AnalysisMethod method, Iterable<AnalysisField> closedFieldWrites) {
101+
if (!method.analyzedInPriorLayer()) {
102+
for (AnalysisField field : closedFieldWrites) {
103+
/*
104+
* These fields are deliberately excluded from shared-layer mutation summaries to
105+
* retain closed-world optimizations. Reject new writes because their compatibility
106+
* with assumptions used to compile the shared layer is not established. A future
107+
* mechanism could record those assumptions and allow otherwise permitted writes
108+
* after verifying that they preserve all affected assumptions; compatibility with
109+
* the field's type bound alone is insufficient.
110+
*/
111+
if (field.isInSharedLayer()) {
112+
throw UserError.abort("Method %s writes field %s, which must remain closed across image layers. " +
113+
"Analyze the write in the initial layer instead of delaying it to an extension layer.",
114+
method.format("%H.%n(%p)"), field.format("%H.%n"));
115+
}
116+
}
117+
}
118+
}
119+
96120
@Override
97121
public void beforeCompilation(BeforeCompilationAccess a) {
98122
BeforeCompilationAccessImpl access = (BeforeCompilationAccessImpl) a;

0 commit comments

Comments
 (0)