Skip to content

Commit a8c0f43

Browse files
entlicherentlicher
committed
[GR-65696] Move the method callees inspection to duringAnalysis() and register uninterpretable methods as roots.
PullRequest: graal/21243
2 parents 1216ead + f8a5d16 commit a8c0f43

File tree

2 files changed

+95
-38
lines changed

2 files changed

+95
-38
lines changed

‎substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/BuildTimeInterpreterUniverse.java‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2023, 2025, 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
@@ -25,8 +25,6 @@
2525
package com.oracle.svm.interpreter;
2626

2727
import static com.oracle.svm.interpreter.metadata.Bytecodes.INVOKEINTERFACE;
28-
import static com.oracle.svm.interpreter.metadata.Bytecodes.INVOKESPECIAL;
29-
import static com.oracle.svm.interpreter.metadata.Bytecodes.INVOKESTATIC;
3028
import static com.oracle.svm.interpreter.metadata.Bytecodes.INVOKEVIRTUAL;
3129

3230
import java.lang.reflect.Modifier;
@@ -274,44 +272,38 @@ public static LocalVariableTable processLocalVariableTable(LocalVariableTable ho
274272

275273
/* Not thread-safe, only call in single thread context */
276274
@Platforms(Platform.HOSTED_ONLY.class)
277-
public static void setNeedMethodBody(InterpreterResolvedJavaMethod thiz, boolean needMethodBody, MetaAccessProvider metaAccessProvider) {
278-
if (thiz.needMethodBody) {
275+
private static void setNeedMethodBody(InterpreterResolvedJavaMethod thiz, boolean needMethodBody, MetaAccessProvider metaAccessProvider) {
276+
if (thiz.needMethodBody && needMethodBody) {
279277
// skip, already scanned
280278
return;
281279
} else if (!needMethodBody) {
282280
// nothing to do
281+
thiz.needMethodBody = needMethodBody;
283282
return;
284283
}
285284

286-
if (thiz.getInterpretedCode() == null) {
285+
byte[] code = thiz.getInterpretedCode();
286+
if (code == null) {
287287
// nothing to scan, method is not interpreterExecutable
288288
return;
289289
}
290290

291291
thiz.needMethodBody = true;
292292

293-
for (int bci = 0; bci < BytecodeStream.endBCI(thiz.getInterpretedCode()); bci = BytecodeStream.nextBCI(thiz.getInterpretedCode(), bci)) {
294-
int opcode = BytecodeStream.opcode(thiz.getInterpretedCode(), bci);
293+
for (int bci = 0; bci < BytecodeStream.endBCI(code); bci = BytecodeStream.nextBCI(code, bci)) {
294+
int opcode = BytecodeStream.opcode(code, bci);
295295
switch (opcode) {
296296
/* GR-53540: Handle invokedyanmic too */
297-
case INVOKESPECIAL, INVOKESTATIC, INVOKEVIRTUAL, INVOKEINTERFACE -> {
298-
int originalCPI = BytecodeStream.readCPI(thiz.getInterpretedCode(), bci);
297+
case INVOKEVIRTUAL, INVOKEINTERFACE -> {
298+
int originalCPI = BytecodeStream.readCPI(code, bci);
299299
try {
300300
JavaMethod method = thiz.getOriginalMethod().getConstantPool().lookupMethod(originalCPI, opcode);
301301
if (!(method instanceof ResolvedJavaMethod resolvedJavaMethod)) {
302302
continue;
303303
}
304304
if (!InterpreterFeature.callableByInterpreter(resolvedJavaMethod, metaAccessProvider)) {
305-
InterpreterUtil.log("[process invokes] cannot execute %s due to call-site (%s) @ bci=%s is not callable by interpreter%n", thiz.getName(), bci, method);
306-
thiz.setCode(null);
307-
thiz.needMethodBody = false;
308305
return;
309306
}
310-
311-
if (opcode == INVOKESPECIAL || opcode == INVOKESTATIC) {
312-
continue;
313-
}
314-
315307
BuildTimeInterpreterUniverse.singleton().getOrCreateMethodWithMethodBody(resolvedJavaMethod, metaAccessProvider);
316308
} catch (UnsupportedFeatureException | UserError.UserException e) {
317309
InterpreterUtil.log("[process invokes] lookup in method %s failed due to:", thiz.getOriginalMethod());

‎substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/DebuggerFeature.java‎

Lines changed: 85 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2023, 2025, 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,8 +24,13 @@
2424
*/
2525
package com.oracle.svm.interpreter;
2626

27+
import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException;
2728
import static com.oracle.svm.hosted.pltgot.GOTEntryAllocator.GOT_NO_ENTRY;
2829
import static com.oracle.svm.interpreter.metadata.Bytecodes.INVOKEDYNAMIC;
30+
import static com.oracle.svm.interpreter.metadata.Bytecodes.INVOKEINTERFACE;
31+
import static com.oracle.svm.interpreter.metadata.Bytecodes.INVOKESPECIAL;
32+
import static com.oracle.svm.interpreter.metadata.Bytecodes.INVOKESTATIC;
33+
import static com.oracle.svm.interpreter.metadata.Bytecodes.INVOKEVIRTUAL;
2934
import static com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod.EST_NO_ENTRY;
3035
import static com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod.VTBL_NO_ENTRY;
3136
import static com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod.VTBL_ONE_IMPL;
@@ -35,6 +40,7 @@
3540
import java.lang.reflect.Constructor;
3641
import java.lang.reflect.Field;
3742
import java.lang.reflect.Method;
43+
import java.lang.reflect.Modifier;
3844
import java.nio.charset.StandardCharsets;
3945
import java.nio.file.FileSystems;
4046
import java.nio.file.Files;
@@ -72,6 +78,7 @@
7278
import com.oracle.svm.core.hub.DynamicHub;
7379
import com.oracle.svm.core.meta.MethodPointer;
7480
import com.oracle.svm.core.option.HostedOptionValues;
81+
import com.oracle.svm.core.util.UserError;
7582
import com.oracle.svm.core.util.VMError;
7683
import com.oracle.svm.graal.hosted.DeoptimizationFeature;
7784
import com.oracle.svm.hosted.FeatureImpl;
@@ -113,6 +120,7 @@
113120
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugins;
114121
import jdk.graal.compiler.options.OptionValues;
115122
import jdk.graal.compiler.phases.util.Providers;
123+
import jdk.vm.ci.meta.ConstantPool;
116124
import jdk.vm.ci.meta.JavaConstant;
117125
import jdk.vm.ci.meta.JavaKind;
118126
import jdk.vm.ci.meta.JavaMethod;
@@ -257,9 +265,18 @@ static boolean isReachable(AnalysisMethod m) {
257265
return m.isReachable() || m.isDirectRootMethod() || m.isVirtualRootMethod();
258266
}
259267

268+
private static boolean isInvokeSpecial(AnalysisMethod method) {
269+
boolean invokeSpecial = method.isConstructor();
270+
if (!invokeSpecial) {
271+
invokeSpecial = Modifier.isPrivate(method.getModifiers());
272+
}
273+
return invokeSpecial;
274+
}
275+
260276
@Override
261277
public void duringAnalysis(DuringAnalysisAccess access) {
262278
FeatureImpl.DuringAnalysisAccessImpl accessImpl = (FeatureImpl.DuringAnalysisAccessImpl) access;
279+
MetaAccessProvider metaAccessProvider = accessImpl.getMetaAccess();
263280

264281
boolean addedIndyHelper = false;
265282
for (AnalysisMethod m : accessImpl.getUniverse().getMethods()) {
@@ -308,12 +325,12 @@ public void duringAnalysis(DuringAnalysisAccess access) {
308325
methodsProcessedDuringAnalysis.add(method);
309326
if (method.wrapped instanceof SubstitutionMethod subMethod && subMethod.isUserSubstitution()) {
310327
if (subMethod.getOriginal().isNative()) {
311-
accessImpl.registerAsRoot(method, false, "compiled entry point of substitution needed for interpreter");
328+
accessImpl.registerAsRoot(method, isInvokeSpecial(method), "compiled entry point of substitution needed for interpreter");
312329
continue;
313330
}
314331
}
315332
byte[] code = method.getCode();
316-
if (code == null) {
333+
if (code == null || !InterpreterFeature.callableByInterpreter(method, metaAccessProvider)) {
317334
continue;
318335
}
319336
AnalysisType declaringClass = method.getDeclaringClass();
@@ -329,39 +346,87 @@ public void duringAnalysis(DuringAnalysisAccess access) {
329346
accessImpl.registerAsUsed(declaringClass, "interpreter needs dynamic hub at runtime for this class");
330347
access.requireAnalysisIteration();
331348
}
349+
InvocationPlugin invocationPlugin = accessImpl.getBigBang().getProviders(method).getGraphBuilderPlugins().getInvocationPlugins().lookupInvocation(method,
350+
accessImpl.getBigBang().getOptions());
351+
if (invocationPlugin != null) {
352+
// There's an invocation plugin for this method.
353+
continue;
354+
}
355+
boolean analyzeCallees = true;
332356
for (int bci = 0; bci < BytecodeStream.endBCI(code); bci = BytecodeStream.nextBCI(code, bci)) {
333357
int bytecode = BytecodeStream.currentBC(code, bci);
334358
if (bytecode == INVOKEDYNAMIC) {
335359
int targetMethodCPI = BytecodeStream.readCPI4(code, bci);
336-
JavaMethod targetMethod = method.getConstantPool().lookupMethod(targetMethodCPI, bytecode);
337-
/*
338-
* SVM optimizes away javac's INVOKDYNAMIC-based String concatenation e.g.
339-
* MH.makeConcatWithConstants(...) . The CP method entry remains unresolved.
340-
*
341-
* Only reachable call sites should have its method and appendix included in
342-
* the image, for now, ALL INVOKEDYNAMIC call sites of reachable methods are
343-
* included.
344-
*/
345-
if (targetMethod instanceof UnresolvedJavaMethod) {
346-
method.getConstantPool().loadReferencedType(targetMethodCPI, bytecode);
347-
targetMethod = method.getConstantPool().lookupMethod(targetMethodCPI, bytecode);
348-
}
349-
if (targetMethod instanceof AnalysisMethod analysisMethod) {
350-
accessImpl.registerAsRoot(analysisMethod, true, "forced for indy support in interpreter");
351-
InterpreterUtil.log("[during analysis] force %s mark as reachable", targetMethod);
360+
AnalysisMethod analysisMethod = getAnalysisMethodAt(method.getConstantPool(), targetMethodCPI, bytecode);
361+
if (analysisMethod != null) {
362+
accessImpl.registerAsRoot(analysisMethod, false, "forced for indy support in interpreter");
363+
InterpreterUtil.log("[during analysis] force %s mark as reachable", analysisMethod);
352364
}
353365

354366
JavaConstant appendixConstant = method.getConstantPool().lookupAppendix(targetMethodCPI, bytecode);
355367
if (appendixConstant instanceof ImageHeapConstant imageHeapConstant) {
356368
supportImpl.ensureConstantIsInImageHeap(snippetReflection, imageHeapConstant);
357369
}
358370
}
371+
if (analyzeCallees) {
372+
switch (bytecode) {
373+
/* GR-53540: Handle invokedyanmic too */
374+
case INVOKESPECIAL, INVOKESTATIC, INVOKEVIRTUAL, INVOKEINTERFACE -> {
375+
int originalCPI = BytecodeStream.readCPI(code, bci);
376+
377+
try {
378+
AnalysisMethod calleeMethod = getAnalysisMethodAt(method.getConstantPool(), originalCPI, bytecode);
379+
if (calleeMethod == null || !calleeMethod.isReachable()) {
380+
continue;
381+
}
382+
383+
if (!InterpreterFeature.callableByInterpreter(calleeMethod, metaAccessProvider)) {
384+
InterpreterUtil.log("[process invokes] cannot execute %s due to call-site (%s) @ bci=%s is not callable by interpreter%n", method.getName(), bci, calleeMethod);
385+
if (method.getAnalyzedGraph() == null) {
386+
accessImpl.registerAsRoot(method, isInvokeSpecial(method), "method handle for interpreter");
387+
accessImpl.registerAsUsed(method.getDeclaringClass().getJavaClass());
388+
access.requireAnalysisIteration();
389+
}
390+
if (method.reachableInCurrentLayer()) {
391+
SubstrateCompilationDirectives.singleton().registerForcedCompilation(method);
392+
}
393+
analyzeCallees = false;
394+
break;
395+
}
396+
} catch (UnsupportedFeatureException | UserError.UserException e) {
397+
InterpreterUtil.log("[process invokes] lookup in method %s failed due to:", method);
398+
InterpreterUtil.log(e);
399+
// ignore, call will fail at run-time if reached
400+
}
401+
}
402+
}
403+
}
359404
}
360405
}
361406
}
362407
supportImpl.trimForcedReferencesInImageHeap();
363408
}
364409

410+
private static AnalysisMethod getAnalysisMethodAt(ConstantPool constantPool, int targetMethodCPI, int bytecode) {
411+
JavaMethod targetMethod = constantPool.lookupMethod(targetMethodCPI, bytecode);
412+
/*
413+
* SVM optimizes away javac's INVOKDYNAMIC-based String concatenation e.g.
414+
* MH.makeConcatWithConstants(...) . The CP method entry remains unresolved.
415+
*
416+
* Only reachable call sites should have its method and appendix included in the image, for
417+
* now, ALL INVOKEDYNAMIC call sites of reachable methods are included.
418+
*/
419+
if (targetMethod instanceof UnresolvedJavaMethod) {
420+
constantPool.loadReferencedType(targetMethodCPI, bytecode);
421+
targetMethod = constantPool.lookupMethod(targetMethodCPI, bytecode);
422+
}
423+
if (targetMethod instanceof AnalysisMethod analysisMethod) {
424+
return analysisMethod;
425+
} else {
426+
return null;
427+
}
428+
}
429+
365430
@Override
366431
public void afterAnalysis(AfterAnalysisAccess access) {
367432
VMError.guarantee(InterpreterToVM.wordJavaKind() == JavaKind.Long ||
@@ -407,7 +472,7 @@ public void beforeCompilation(BeforeCompilationAccess access) {
407472
for (HostedMethod hMethod : hUniverse.getMethods()) {
408473
AnalysisMethod aMethod = hMethod.getWrapped();
409474
if (isReachable(aMethod)) {
410-
boolean needsMethodBody = InterpreterFeature.executableByInterpreter(aMethod);
475+
boolean needsMethodBody = InterpreterFeature.executableByInterpreter(aMethod) && InterpreterFeature.callableByInterpreter(hMethod, hMetaAccess);
411476
// Test if the methods needs to be compiled for execution in the interpreter:
412477
if (aMethod.getAnalyzedGraph() != null && //
413478
(aMethod.wrapped instanceof SubstitutionMethod subMethod && subMethod.isUserSubstitution() ||

0 commit comments

Comments
 (0)