Skip to content

Commit 077f075

Browse files
committed
[GR-61426] Ensure that when a reachability handler callback is executed a DuringAnalysisAccess object is available.
PullRequest: graal/20805
2 parents 89d1c5a + cc77fb8 commit 077f075

File tree

9 files changed

+127
-35
lines changed

9 files changed

+127
-35
lines changed

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -130,33 +130,36 @@ public void scanEmbeddedRoot(JavaConstant root, BytecodePosition position) {
130130
public void onFieldRead(AnalysisField field) {
131131
assert field.isRead() : field;
132132
/* Check if the value is available before accessing it. */
133-
AnalysisType declaringClass = field.getDeclaringClass();
134133
if (field.isStatic()) {
135-
FieldScan reason = new FieldScan(field);
136-
if (!field.installableInLayer()) {
137-
/*
138-
* For non-installable static fields we do not scan the constant value, but instead
139-
* inject its type state in the field flow. This will be propagated to any
140-
* corresponding field loads.
141-
*
142-
* GR-52421: the field state needs to be serialized from the base layer analysis
143-
*/
144-
if (field.getStorageKind().isObject()) {
145-
bb.injectFieldTypes(field, List.of(field.getType()), true);
146-
} else if (bb.trackPrimitiveValues() && field.getStorageKind().isPrimitive()) {
147-
((PointsToAnalysisField) field).saturatePrimitiveField();
148-
}
149-
} else if (isValueAvailable(field)) {
150-
JavaConstant fieldValue = readStaticFieldValue(field);
151-
if (fieldValue instanceof ImageHeapConstant imageHeapConstant && field.isFinal()) {
152-
AnalysisError.guarantee(imageHeapConstant.getOrigin() != null, "The origin of the constant %s should have been registered before", imageHeapConstant);
153-
}
154-
markReachable(fieldValue, reason);
155-
notifyAnalysis(field, null, fieldValue, reason);
156-
}
134+
postTask(() -> onStaticFieldRead(field));
157135
} else {
158136
/* Trigger field scanning for the already processed objects. */
159-
postTask(() -> onInstanceFieldRead(field, declaringClass));
137+
postTask(() -> onInstanceFieldRead(field, field.getDeclaringClass()));
138+
}
139+
}
140+
141+
private void onStaticFieldRead(AnalysisField field) {
142+
FieldScan reason = new FieldScan(field);
143+
if (!field.installableInLayer()) {
144+
/*
145+
* For non-installable static fields we do not scan the constant value, but instead
146+
* inject its type state in the field flow. This will be propagated to any corresponding
147+
* field loads.
148+
*
149+
* GR-52421: the field state needs to be serialized from the base layer analysis
150+
*/
151+
if (field.getStorageKind().isObject()) {
152+
bb.injectFieldTypes(field, List.of(field.getType()), true);
153+
} else if (bb.trackPrimitiveValues() && field.getStorageKind().isPrimitive()) {
154+
((PointsToAnalysisField) field).saturatePrimitiveField();
155+
}
156+
} else if (isValueAvailable(field)) {
157+
JavaConstant fieldValue = readStaticFieldValue(field);
158+
if (fieldValue instanceof ImageHeapConstant imageHeapConstant && field.isFinal()) {
159+
AnalysisError.guarantee(imageHeapConstant.getOrigin() != null, "The origin of the constant %s should have been registered before", imageHeapConstant);
160+
}
161+
markReachable(fieldValue, reason);
162+
notifyAnalysis(field, null, fieldValue, reason);
160163
}
161164
}
162165

@@ -605,7 +608,7 @@ protected void onObjectReachable(ImageHeapConstant imageHeapConstant, ScanReason
605608
* conditions, e.g., a started Thread should never be added to the image heap, but
606609
* the structure of the object is valid, as ensured by the validity check above.
607610
*/
608-
objectType.notifyObjectReachable(universe.getConcurrentAnalysisAccess(), object, reason);
611+
objectType.notifyObjectReachable(object, reason);
609612
} catch (UnsupportedFeatureException e) {
610613
/* Enhance the unsupported feature message with the object trace and rethrow. */
611614
StringBuilder backtrace = new StringBuilder();
@@ -890,9 +893,10 @@ protected AnalysisField lookupJavaField(String className, String fieldName) {
890893
*
891894
* In the (legacy) Feature.duringAnalysis state, the executor is not running and we must not
892895
* schedule new tasks, because that would be treated as "the analysis has not finished yet". So
893-
* in that case we execute the task directly.
896+
* in that case we execute the task directly. A task that runs in the Feature.duringAnalysis
897+
* stage and modifies the analysis state should itself trigger an additional analysis iteration.
894898
*/
895-
private void maybeRunInExecutor(CompletionExecutor.DebugContextRunnable task) {
899+
protected void maybeRunInExecutor(CompletionExecutor.DebugContextRunnable task) {
896900
if (bb.executorIsStarted()) {
897901
bb.postTask(task);
898902
} else {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -517,9 +517,9 @@ public JavaConstant getConstantValue() {
517517
}
518518

519519
/**
520-
* Ensure that all reachability handler that were present at the time the declaring type was
521-
* marked as reachable are executed before accessing field values. This allows field value
522-
* transformer to be installed reliably in reachability handler.
520+
* Ensure that all reachability handlers that were present at the time the declaring type was
521+
* marked as reachable are executed before accessing field values. This allows a field value
522+
* transformer to be installed reliably in a reachability handler.
523523
*/
524524
public void beforeFieldValueAccess() {
525525
declaringClass.registerAsReachable(this);

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -700,8 +700,9 @@ public <T> void registerObjectReachableCallback(ObjectReachableCallback<T> callb
700700
});
701701
}
702702

703-
public <T> void notifyObjectReachable(DuringAnalysisAccess access, T object, ScanReason reason) {
704-
ConcurrentLightHashSet.forEach(this, objectReachableCallbacksUpdater, (ObjectReachableCallback<T> c) -> c.doCallback(access, object, reason));
703+
public <T> void notifyObjectReachable(T object, ScanReason reason) {
704+
ConcurrentLightHashSet.forEach(this, objectReachableCallbacksUpdater,
705+
(ObjectReachableCallback<T> c) -> c.doCallback(universe.getConcurrentAnalysisAccess(), object, reason));
705706
}
706707

707708
public void registerInstantiatedCallback(Consumer<DuringAnalysisAccess> callback) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,9 @@ public void setConcurrentAnalysisAccess(DuringAnalysisAccess access) {
787787
}
788788

789789
public DuringAnalysisAccess getConcurrentAnalysisAccess() {
790+
AnalysisError.guarantee(concurrentAnalysisAccess != null, "The requested DuringAnalysisAccess object is not available. " +
791+
"This means that an analysis task is executed too eagerly, before analysis. " +
792+
"Make sure that all analysis tasks are posted to the analysis execution engine.");
790793
return concurrentAnalysisAccess;
791794
}
792795

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/BuildPhaseProvider.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public final class BuildPhaseProvider {
3535

3636
private boolean featureRegistrationFinished;
3737
private boolean setupFinished;
38+
private boolean analysisStarted;
3839
private boolean analysisFinished;
3940
private boolean hostedUniverseBuilt;
4041
private boolean readyForCompilation;
@@ -69,6 +70,14 @@ public static boolean isSetupFinished() {
6970
return ImageSingletons.contains(BuildPhaseProvider.class) && singleton().setupFinished;
7071
}
7172

73+
public static void markAnalysisStarted() {
74+
singleton().analysisStarted = true;
75+
}
76+
77+
public static boolean isAnalysisStarted() {
78+
return ImageSingletons.contains(BuildPhaseProvider.class) && singleton().analysisStarted;
79+
}
80+
7281
public static void markAnalysisFinished() {
7382
singleton().analysisFinished = true;
7483
}

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,13 @@ protected void createAbstractImage(NativeImageKind k, List<HostedMethod> hostedE
808808
@SuppressWarnings("try")
809809
protected boolean runPointsToAnalysis(String imageName, OptionValues options, DebugContext debug) {
810810
try (Indent ignored = debug.logAndIndent("run analysis")) {
811+
/*
812+
* Set the ConcurrentAnalysisAccessImpl before Feature.beforeAnalysis is executed. Some
813+
* features may already execute some pre-analysis tasks, e.g., reading a hosted field
814+
* value, that can trigger reachability callbacks.
815+
*/
816+
ConcurrentAnalysisAccessImpl concurrentConfig = new ConcurrentAnalysisAccessImpl(featureHandler, loader, bb, nativeLibraries, debug);
817+
aUniverse.setConcurrentAnalysisAccess(concurrentConfig);
811818
try (Indent ignored1 = debug.logAndIndent("process analysis initializers")) {
812819
BeforeAnalysisAccessImpl config = new BeforeAnalysisAccessImpl(featureHandler, loader, bb, nativeLibraries, debug);
813820
ServiceCatalogSupport.singleton().enableServiceCatalogMapTransformer(config);
@@ -825,8 +832,6 @@ protected boolean runPointsToAnalysis(String imageName, OptionValues options, De
825832
try (ReporterClosable c = ProgressReporter.singleton().printAnalysis(bb.getUniverse(), nativeLibraries.getLibraries())) {
826833
DuringAnalysisAccessImpl config = new DuringAnalysisAccessImpl(featureHandler, loader, bb, nativeLibraries, debug);
827834
try {
828-
ConcurrentAnalysisAccessImpl concurrentConfig = new ConcurrentAnalysisAccessImpl(featureHandler, loader, bb, nativeLibraries, debug);
829-
aUniverse.setConcurrentAnalysisAccess(concurrentConfig);
830835
if (ImageLayerBuildingSupport.buildingExtensionLayer()) {
831836
/*
832837
* All the field value transformers should be installed by this point.
@@ -835,6 +840,8 @@ protected boolean runPointsToAnalysis(String imageName, OptionValues options, De
835840
*/
836841
HostedImageLayerBuildingSupport.singleton().getLoader().relinkTransformedStaticFinalFieldValues();
837842
}
843+
/* All pre-analysis set-up is done and the fixed-point analysis can start. */
844+
BuildPhaseProvider.markAnalysisStarted();
838845
bb.runAnalysis(debug, (universe) -> {
839846
try (StopTimer t2 = TimerCollection.createTimerAndStart(TimerCollection.Registry.FEATURES)) {
840847
bb.getHostVM().notifyClassReachabilityListener(universe, config);
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation. Oracle designates this
8+
* particular file as subject to the "Classpath" exception as provided
9+
* by Oracle in the LICENSE file that accompanied this code.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
package com.oracle.svm.hosted;
26+
27+
import org.graalvm.nativeimage.hosted.Feature;
28+
29+
import com.oracle.svm.core.SubstrateUtil;
30+
import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
31+
import com.oracle.svm.core.feature.InternalFeature;
32+
import com.oracle.svm.core.util.VMError;
33+
34+
/**
35+
* Ensure that a {@link org.graalvm.nativeimage.hosted.Feature.DuringAnalysisAccess} object is
36+
* available when a reachability callback is executed for any of the reachable types.
37+
*/
38+
@AutomaticallyRegisteredFeature
39+
public class ReachabilityHandlerAssertFeature implements InternalFeature {
40+
@Override
41+
public boolean isInConfiguration(IsInConfigurationAccess access) {
42+
return SubstrateUtil.assertionsEnabled();
43+
}
44+
45+
@Override
46+
public void beforeAnalysis(Feature.BeforeAnalysisAccess access) {
47+
access.registerSubtypeReachabilityHandler(this::onTypeReachable, Object.class);
48+
}
49+
50+
private void onTypeReachable(DuringAnalysisAccess access, Class<?> cls) {
51+
VMError.guarantee(access != null, "DuringAnalysisAccess object is not available when processing class %s.", cls.getName());
52+
}
53+
}

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/NativeImagePointsToAnalysis.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public AnnotationSubstitutionProcessor getAnnotationSubstitutionProcessor() {
128128

129129
@Override
130130
public void onFieldAccessed(AnalysisField field) {
131-
customTypeFieldHandler.handleField(field);
131+
postTask(() -> customTypeFieldHandler.handleField(field));
132132
}
133133

134134
@Override

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapScanner.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
import com.oracle.graal.pointsto.heap.ImageHeapScanner;
4141
import com.oracle.graal.pointsto.meta.AnalysisField;
4242
import com.oracle.graal.pointsto.meta.AnalysisMetaAccess;
43+
import com.oracle.graal.pointsto.util.CompletionExecutor;
44+
import com.oracle.svm.core.BuildPhaseProvider;
4345
import com.oracle.svm.core.hub.DynamicHub;
4446
import com.oracle.svm.hosted.ImageClassLoader;
4547
import com.oracle.svm.hosted.ameta.AnalysisConstantReflectionProvider;
@@ -132,4 +134,17 @@ protected void onObjectReachable(ImageHeapConstant imageHeapConstant, ScanReason
132134
reflectionSupport.registerHeapDynamicHub(hub, reason);
133135
}
134136
}
137+
138+
@Override
139+
protected void maybeRunInExecutor(CompletionExecutor.DebugContextRunnable task) {
140+
if (BuildPhaseProvider.isAnalysisStarted()) {
141+
super.maybeRunInExecutor(task);
142+
} else {
143+
/*
144+
* Before the analysis is started post all scanning tasks to the executor. They will be
145+
* executed after the analysis starts.
146+
*/
147+
bb.postTask(task);
148+
}
149+
}
135150
}

0 commit comments

Comments
 (0)