diff --git a/common.json b/common.json index 366f532f95b0..2baebf2d591e 100644 --- a/common.json +++ b/common.json @@ -4,7 +4,7 @@ "Jsonnet files should not include this file directly but use ci/common.jsonnet instead." ], - "mx_version": "7.25.0", + "mx_version": "7.25.5", "COMMENT.jdks": "When adding or removing JDKs keep in sync with JDKs in ci/common.jsonnet", "jdks": { diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/aarch64/AArch64ArithmeticLIRGenerator.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/aarch64/AArch64ArithmeticLIRGenerator.java index 95dc2f91376d..854f8fd5e80e 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/aarch64/AArch64ArithmeticLIRGenerator.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/aarch64/AArch64ArithmeticLIRGenerator.java @@ -450,7 +450,8 @@ public static boolean isAddSubtractConstant(JavaConstant constValue) { case Long: return AArch64MacroAssembler.isAddSubtractImmediate(constValue.asLong(), true); case Object: - return constValue.isNull(); + /* Object constants can't be encoded as immediates in add/subtract. */ + return false; default: throw GraalError.shouldNotReachHereUnexpectedValue(constValue.getJavaKind().getStackKind()); // ExcludeFromJacocoGeneratedReport } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/gen/LIRCompilerBackend.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/gen/LIRCompilerBackend.java index 7c39e4b14815..668e8a71ff9f 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/gen/LIRCompilerBackend.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/gen/LIRCompilerBackend.java @@ -29,17 +29,18 @@ import jdk.graal.compiler.code.CompilationResult; import jdk.graal.compiler.core.LIRGenerationPhase; +import jdk.graal.compiler.core.common.GraalOptions; import jdk.graal.compiler.core.common.alloc.LinearScanOrder; import jdk.graal.compiler.core.common.alloc.RegisterAllocationConfig; +import jdk.graal.compiler.core.common.cfg.CodeEmissionOrder; +import jdk.graal.compiler.core.common.cfg.CodeEmissionOrder.ComputationTime; +import jdk.graal.compiler.core.target.Backend; +import jdk.graal.compiler.debug.Assertions; import jdk.graal.compiler.debug.CounterKey; import jdk.graal.compiler.debug.DebugCloseable; import jdk.graal.compiler.debug.DebugContext; import jdk.graal.compiler.debug.GraalError; import jdk.graal.compiler.debug.TimerKey; -import jdk.graal.compiler.core.common.GraalOptions; -import jdk.graal.compiler.core.common.cfg.CodeEmissionOrder; -import jdk.graal.compiler.core.common.cfg.CodeEmissionOrder.ComputationTime; -import jdk.graal.compiler.core.target.Backend; import jdk.graal.compiler.lir.ComputeCodeEmissionOrder; import jdk.graal.compiler.lir.LIR; import jdk.graal.compiler.lir.alloc.OutOfRegistersException; @@ -59,8 +60,6 @@ import jdk.graal.compiler.nodes.StructuredGraph.ScheduleResult; import jdk.graal.compiler.nodes.cfg.HIRBlock; import jdk.graal.compiler.nodes.spi.NodeLIRBuilderTool; -import jdk.graal.compiler.debug.Assertions; - import jdk.vm.ci.code.RegisterConfig; import jdk.vm.ci.code.TargetDescription; import jdk.vm.ci.code.site.ConstantReference; @@ -168,6 +167,11 @@ private static LIRGenerationResult emitLIR0(Backend backend, // Dump LIR along with HIR (the LIR is looked up from context) debug.dump(DebugContext.BASIC_LEVEL, graph.getLastSchedule(), "After LIR generation"); LIRGenerationResult result = emitLowLevel(backend.getTarget(), lirGenRes, lirGen, lirSuites, registerAllocationConfig, blockOrder); + /* + * LIRGeneration may have changed the CFG, so in case a schedule is needed afterward + * we make sure it will be recomputed. + */ + graph.clearLastSchedule(); return result; } catch (Throwable e) { throw debug.handle(e); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/ProfileReplaySupport.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/ProfileReplaySupport.java index 386a1d7d4cc2..77b1dcf6080e 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/ProfileReplaySupport.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/ProfileReplaySupport.java @@ -225,10 +225,14 @@ public void profileReplayEpilogue(DebugContext debug, CompilationResult result, String codeSignature = null; String graphSignature = null; if (result != null) { - codeSignature = result.getCodeSignature(); - assert graph != null; - String s = getCanonicalGraphString(graph); - graphSignature = CompilationResult.getSignature(s.getBytes(StandardCharsets.UTF_8)); + try { + codeSignature = result.getCodeSignature(); + assert graph != null; + String s = getCanonicalGraphString(graph); + graphSignature = CompilationResult.getSignature(s.getBytes(StandardCharsets.UTF_8)); + } catch (Throwable t) { + throw debug.handle(t); + } } if (Options.WarnAboutCodeSignatureMismatch.getValue(debug.getOptions())) { if (expectedCodeSignature != null && !Objects.equals(codeSignature, expectedCodeSignature)) { diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/EncodedGraph.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/EncodedGraph.java index b18520c408d4..aedcafb6f507 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/EncodedGraph.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/EncodedGraph.java @@ -97,7 +97,14 @@ public void clear() { protected int[] nodeStartOffsets; public EncodedGraph(byte[] encoding, int startOffset, Object[] objects, NodeClass[] types, StructuredGraph sourceGraph) { - this(encoding, startOffset, objects, types, sourceGraph.getAssumptions(), sourceGraph.getMethods(), sourceGraph.hasUnsafeAccess(), sourceGraph.trackNodeSourcePosition()); + this(encoding, + startOffset, + objects, + types, + sourceGraph.getAssumptions(), + sourceGraph.isRecordingInlinedMethods() ? sourceGraph.getMethods() : null, + sourceGraph.hasUnsafeAccess(), + sourceGraph.trackNodeSourcePosition()); } public EncodedGraph(byte[] encoding, int startOffset, Object[] objects, NodeClass[] types, Assumptions assumptions, List inlinedMethods, @@ -140,6 +147,10 @@ public Assumptions getAssumptions() { return assumptions; } + public boolean isRecordingInlinedMethods() { + return inlinedMethods != null; + } + public List getInlinedMethods() { return inlinedMethods; } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/IntrinsicGraphBuilder.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/IntrinsicGraphBuilder.java index 27ecb362b89d..c40d0e530816 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/IntrinsicGraphBuilder.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/IntrinsicGraphBuilder.java @@ -131,6 +131,8 @@ protected IntrinsicGraphBuilder(OptionValues options, if (graphBuilderConfig != null && !method.isNative()) { graph.start().setStateAfter(createStateAfterStartOfReplacementGraph(method, graphBuilderConfig)); } + // Record method dependency in the graph + graph.recordMethod(method); Signature sig = method.getSignature(); int max = sig.getParameterCount(false); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/classfile/Classfile.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/classfile/Classfile.java index 3799fd5a7ac6..fcb2e9d3d85f 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/classfile/Classfile.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/classfile/Classfile.java @@ -49,9 +49,13 @@ public class Classfile { private final List codeAttributes; private static final int MAJOR_VERSION_JAVA_MIN = 51; // JDK7 - private static final int MAJOR_VERSION_JAVA_MAX = 67; // JDK23 private static final int MAGIC = 0xCAFEBABE; + // An upper limit on the major class version is not set as it doesn't + // make much of a difference whether a ClassFormatError or the more specific + // a UnsupportedClassVersionError is thrown and it avoids the need for an + // Graal change when updating to a newer JDK. + /** * Creates a {@link Classfile} by parsing the class file bytes for {@code type} loadable from * {@code context}. @@ -67,26 +71,30 @@ public Classfile(ResolvedJavaType type, DataInputStream stream, ClassfileBytecod int minor = stream.readUnsignedShort(); int major = stream.readUnsignedShort(); - if (major < MAJOR_VERSION_JAVA_MIN || major > MAJOR_VERSION_JAVA_MAX) { + if (major < MAJOR_VERSION_JAVA_MIN) { throw new UnsupportedClassVersionError("Unsupported class file version: " + major + "." + minor); } - ClassfileConstantPool cp = new ClassfileConstantPool(stream, context); + try { + ClassfileConstantPool cp = new ClassfileConstantPool(stream, context); - // access_flags, this_class, super_class - skipFully(stream, 6); + // access_flags, this_class, super_class + skipFully(stream, 6); - // interfaces - skipFully(stream, stream.readUnsignedShort() * 2); + // interfaces + skipFully(stream, stream.readUnsignedShort() * 2); - // fields - skipFields(stream); + // fields + skipFields(stream); - // methods - codeAttributes = readMethods(stream, cp); + // methods + codeAttributes = readMethods(stream, cp); - // attributes - skipAttributes(stream); + // attributes + skipAttributes(stream); + } catch (ClassFormatError cfe) { + throw (ClassFormatError) new ClassFormatError("Error reading class file with version " + major + "." + minor).initCause(cfe); + } } public ClassfileBytecode getCode(String name, String descriptor) { diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/classfile/ClassfileConstantPool.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/classfile/ClassfileConstantPool.java index e462bd8261d8..ef0556a1068d 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/classfile/ClassfileConstantPool.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/classfile/ClassfileConstantPool.java @@ -130,7 +130,7 @@ static final ClassfileConstant readConstant(DataInputStream stream) throws IOExc skipFully(stream, 4); // bootstrap_method_attr_index, name_and_type_index return new ClassfileConstant.Unsupported(tag, "CONSTANT_InvokeDynamic_info"); default: - throw new GraalError("Invalid constant pool tag: " + tag); + throw new ClassFormatError("Invalid constant pool tag: " + tag); } } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/TruffleCompilerImpl.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/TruffleCompilerImpl.java index a2844ac92297..227996e1e286 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/TruffleCompilerImpl.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/TruffleCompilerImpl.java @@ -79,10 +79,7 @@ import jdk.graal.compiler.graph.Node; import jdk.graal.compiler.lir.asm.CompilationResultBuilderFactory; import jdk.graal.compiler.lir.phases.LIRSuites; -import jdk.graal.compiler.nodes.NodeView; import jdk.graal.compiler.nodes.StructuredGraph; -import jdk.graal.compiler.nodes.calc.NarrowNode; -import jdk.graal.compiler.nodes.calc.ZeroExtendNode; import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration; import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration.BytecodeExceptionMode; import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration.Plugins; @@ -96,8 +93,6 @@ import jdk.graal.compiler.phases.tiers.Suites; import jdk.graal.compiler.phases.util.Providers; import jdk.graal.compiler.serviceprovider.GraalServices; -import jdk.graal.compiler.truffle.nodes.AnyExtendNode; -import jdk.graal.compiler.truffle.nodes.AnyNarrowNode; import jdk.graal.compiler.truffle.nodes.TruffleAssumption; import jdk.graal.compiler.truffle.phases.InstrumentPhase; import jdk.graal.compiler.truffle.phases.InstrumentationSuite; @@ -559,6 +554,7 @@ private StructuredGraph truffleTier(TruffleCompilationWrapper wrapper, DebugCont } catch (Throwable e) { throw context.debug.handle(e); } + } if (wrapper.statistics != null) { wrapper.statistics.afterTruffleTier(wrapper.compilable, graph); @@ -569,17 +565,6 @@ private StructuredGraph truffleTier(TruffleCompilationWrapper wrapper, DebugCont return graph; } - private static void replaceAnyExtendNodes(StructuredGraph graph) { - // replace all AnyExtendNodes with ZeroExtendNodes - for (AnyExtendNode node : graph.getNodes(AnyExtendNode.TYPE)) { - node.replaceAndDelete(graph.addOrUnique(ZeroExtendNode.create(node.getValue(), 64, NodeView.DEFAULT))); - } - // replace all AnyNarrowNodes with NarrowNodes - for (AnyNarrowNode node : graph.getNodes(AnyNarrowNode.TYPE)) { - node.replaceAndDelete(graph.addOrUnique(NarrowNode.create(node.getValue(), 32, NodeView.DEFAULT))); - } - } - /** * Compiles a graph produced by {@link TruffleTier}, i.e. * {@link #truffleTier(TruffleCompilationWrapper, DebugContext)}. @@ -601,7 +586,6 @@ public CompilationResult compilePEGraph(StructuredGraph graph, InstalledCode[] outInstalledCode, TruffleInliningScope inlining) { - replaceAnyExtendNodes(graph); DebugContext debug = graph.getDebug(); try (DebugContext.Scope s = debug.scope("TruffleFinal")) { debug.dump(DebugContext.BASIC_LEVEL, graph, "After TruffleTier"); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/ReplaceAnyExtendNodePhase.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/ReplaceAnyExtendNodePhase.java new file mode 100644 index 000000000000..83522ec9bd96 --- /dev/null +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/ReplaceAnyExtendNodePhase.java @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package jdk.graal.compiler.truffle.phases; + +import jdk.graal.compiler.nodes.NodeView; +import jdk.graal.compiler.nodes.StructuredGraph; +import jdk.graal.compiler.nodes.calc.NarrowNode; +import jdk.graal.compiler.nodes.calc.ZeroExtendNode; +import jdk.graal.compiler.phases.BasePhase; +import jdk.graal.compiler.truffle.TruffleTierContext; +import jdk.graal.compiler.truffle.nodes.AnyExtendNode; +import jdk.graal.compiler.truffle.nodes.AnyNarrowNode; + +public final class ReplaceAnyExtendNodePhase extends BasePhase { + @Override + protected void run(StructuredGraph graph, TruffleTierContext context) { + // replace all AnyExtendNodes with ZeroExtendNodes + for (AnyExtendNode node : graph.getNodes(AnyExtendNode.TYPE)) { + node.replaceAndDelete(graph.addOrUnique(ZeroExtendNode.create(node.getValue(), 64, NodeView.DEFAULT))); + } + // replace all AnyNarrowNodes with NarrowNodes + for (AnyNarrowNode node : graph.getNodes(AnyNarrowNode.TYPE)) { + node.replaceAndDelete(graph.addOrUnique(NarrowNode.create(node.getValue(), 32, NodeView.DEFAULT))); + } + } +} diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/TruffleTier.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/TruffleTier.java index 70d8b426a061..4f235128bb62 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/TruffleTier.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/TruffleTier.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -43,6 +43,7 @@ public TruffleTier(OptionValues options, PartialEvaluator partialEvaluator, Inst appendPhase(new NeverPartOfCompilationPhase()); appendPhase(new MaterializeFramesPhase()); appendPhase(new SetIdentityForValueTypesPhase()); + appendPhase(new ReplaceAnyExtendNodePhase()); if (!TruffleCompilerOptions.InlineAcrossTruffleBoundary.getValue(options)) { appendPhase(new InliningAcrossTruffleBoundaryPhase()); } diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/JDWPContext.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/JDWPContext.java index 1788e898dd89..7f6be97e1e8b 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/JDWPContext.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/JDWPContext.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -421,10 +421,11 @@ public interface JDWPContext { /** * Returns the entry count for the monitor on the current thread. * + * @param monitorOwnerThread the owner thread of the monitor * @param monitor the monitor * @return entry count of monitor */ - int getMonitorEntryCount(Object monitor); + int getMonitorEntryCount(Object monitorOwnerThread, Object monitor); /** * Returns all owned guest-language monitor object of the input call frames. diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/VMEventListenerImpl.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/VMEventListenerImpl.java index fe6866ae8c7f..87ba43a47c0c 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/VMEventListenerImpl.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/VMEventListenerImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -52,7 +52,6 @@ import com.oracle.truffle.espresso.jdwp.impl.SocketConnection; import com.oracle.truffle.espresso.jdwp.impl.SteppingInfo; import com.oracle.truffle.espresso.jdwp.impl.SuspendStrategy; -import com.oracle.truffle.espresso.jdwp.impl.SuspendedInfo; import com.oracle.truffle.espresso.jdwp.impl.TypeTag; public final class VMEventListenerImpl implements VMEventListener { @@ -662,8 +661,6 @@ public void monitorWait(Object monitor, long timeout) { Object guestThread = context.asGuestThread(Thread.currentThread()); // a call to wait marks the monitor as contended currentContendedMonitor.put(guestThread, monitor); - // capture the call frames before entering a known blocking state - debuggerController.captureCallFramesBeforeBlocking(guestThread); if (monitorWaitRequests.isEmpty()) { return; @@ -675,12 +672,9 @@ public void monitorWait(Object monitor, long timeout) { // create the call frame for the caller location of Object.wait(timeout) CallFrame frame = context.locateObjectWaitFrame(); - debuggerController.immediateSuspend(guestThread, filter.getSuspendPolicy(), new Callable() { - @Override - public Void call() { - sendMonitorWaitEvent(monitor, timeout, filter, frame); - return null; - } + debuggerController.immediateSuspend(guestThread, filter.getSuspendPolicy(), () -> { + sendMonitorWaitEvent(monitor, timeout, filter, frame); + return null; }); } } @@ -739,7 +733,6 @@ public void monitorWaited(Object monitor, boolean timedOut) { Object currentThread = context.asGuestThread(Thread.currentThread()); // remove contended monitor from the thread currentContendedMonitor.remove(currentThread); - debuggerController.cancelBlockingCallFrames(currentThread); if (monitorWaitedRequests.isEmpty()) { return; @@ -771,25 +764,20 @@ public void onContendedMonitorEnter(Object monitor) { Object guestThread = context.asGuestThread(Thread.currentThread()); currentContendedMonitor.put(guestThread, monitor); - final CallFrame topFrame = debuggerController.captureCallFramesBeforeBlocking(guestThread)[0]; - if (monitorContendedRequests.isEmpty()) { return; } - Object currentThread = context.asGuestThread(Thread.currentThread()); for (Map.Entry entry : monitorContendedRequests.entrySet()) { RequestFilter filter = entry.getValue(); - if (currentThread == filter.getThread()) { + if (guestThread == filter.getThread()) { // monitor is contended on a requested thread MonitorEvent event = new MonitorEvent(monitor, filter); + final CallFrame topFrame = context.getStackTrace(guestThread)[0]; - debuggerController.immediateSuspend(currentThread, filter.getSuspendPolicy(), new Callable() { - @Override - public Void call() { - sendMonitorContendedEnterEvent(event, topFrame); - return null; - } + debuggerController.immediateSuspend(guestThread, filter.getSuspendPolicy(), () -> { + sendMonitorContendedEnterEvent(event, topFrame); + return null; }); } } @@ -804,27 +792,20 @@ public void onContendedMonitorEntered(Object monitor) { Object guestThread = context.asGuestThread(Thread.currentThread()); currentContendedMonitor.remove(guestThread); - SuspendedInfo info = debuggerController.getSuspendedInfo(guestThread); - final CallFrame topFrame = info != null ? info.getStackFrames()[0] : debuggerController.captureCallFramesBeforeBlocking(guestThread)[0]; - debuggerController.cancelBlockingCallFrames(guestThread); - if (monitorContendedEnteredRequests.isEmpty()) { return; } - Object currentThread = context.asGuestThread(Thread.currentThread()); for (Map.Entry entry : monitorContendedEnteredRequests.entrySet()) { RequestFilter filter = entry.getValue(); - if (currentThread == filter.getThread()) { + if (guestThread == filter.getThread()) { // monitor is contended on a requested thread MonitorEvent event = new MonitorEvent(monitor, filter); + final CallFrame topFrame = context.getStackTrace(guestThread)[0]; - debuggerController.immediateSuspend(currentThread, filter.getSuspendPolicy(), new Callable() { - @Override - public Void call() { - sendMonitorContendedEnteredEvent(event, topFrame); - return null; - } + debuggerController.immediateSuspend(guestThread, filter.getSuspendPolicy(), () -> { + sendMonitorContendedEnteredEvent(event, topFrame); + return null; }); } } @@ -979,7 +960,6 @@ private void suspend(byte suspendPolicy, Object thread) { return; case SuspendStrategy.ALL: debuggerController.suspendAll(); - return; } } } diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java index 86ee59bcbec5..a85dae015d35 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java @@ -52,7 +52,6 @@ import com.oracle.truffle.api.debug.SuspensionFilter; import com.oracle.truffle.api.frame.Frame; import com.oracle.truffle.api.frame.FrameInstance; -import com.oracle.truffle.api.frame.FrameInstanceVisitor; import com.oracle.truffle.api.instrumentation.ContextsListener; import com.oracle.truffle.api.nodes.LanguageInfo; import com.oracle.truffle.api.nodes.Node; @@ -63,7 +62,6 @@ import com.oracle.truffle.espresso.jdwp.api.JDWPOptions; import com.oracle.truffle.espresso.jdwp.api.KlassRef; import com.oracle.truffle.espresso.jdwp.api.MethodRef; -import com.oracle.truffle.espresso.jdwp.api.MonitorStackInfo; import com.oracle.truffle.espresso.jdwp.api.VMEventListener; public final class DebuggerController implements ContextsListener { @@ -266,9 +264,11 @@ public boolean popFrames(Object guestThread, CallFrame frameToPop, int packetId) return false; } - public boolean forceEarlyReturn(Object guestThread, CallFrame frame, Object returnValue) { - SuspendedInfo susp = suspendedInfos.get(guestThread); - if (susp != null && !(susp instanceof UnknownSuspendedInfo)) { + public boolean forceEarlyReturn(SuspendedInfo susp, Object guestThread, CallFrame frame, Object returnValue) { + assert susp != null; + assert frame != null; + + if (!(susp instanceof UnknownSuspendedInfo)) { // Truffle unwind will take us to exactly the right location in the caller method susp.getEvent().prepareUnwindFrame(frame.getDebugStackFrame(), frame.asDebugValue(returnValue)); susp.setForceEarlyReturnInProgress(); @@ -403,11 +403,7 @@ public void suspend(Object guestThread) { // even if the guestThread is executing. If the guestThread is blocked or waiting we // still need to suspend it, thus we manage this with a hard suspend mechanism threadSuspension.addHardSuspendedThread(guestThread); - if (suspendedInfos.get(guestThread) == null) { - // if already set, we have captured a blocking suspendedInfo already - // so don't overwrite that information - suspendedInfos.put(guestThread, new UnknownSuspendedInfo(guestThread, getContext())); - } + suspendedInfos.put(guestThread, new UnknownSuspendedInfo(context, guestThread)); } catch (Exception e) { fine(() -> "not able to suspend guestThread: " + getThreadName(guestThread)); } @@ -677,74 +673,60 @@ public void postJobForThread(ThreadJob job) { } } - public CallFrame[] captureCallFramesBeforeBlocking(Object guestThread) { + public CallFrame[] getCallFrames(Object guestThread) { List callFrames = new ArrayList<>(); - Truffle.getRuntime().iterateFrames(new FrameInstanceVisitor<>() { - @Override - public Object visitFrame(FrameInstance frameInstance) { - KlassRef klass; - MethodRef method; - RootNode root = getRootNode(frameInstance); - if (root == null) { - return null; - } - method = getContext().getMethodFromRootNode(root); - if (method == null) { - return null; - } + Truffle.getRuntime().iterateFrames(frameInstance -> { + KlassRef klass; + MethodRef method; + RootNode root = getRootNode(frameInstance); + if (root == null) { + return null; + } + method = getContext().getMethodFromRootNode(root); + if (method == null) { + return null; + } - klass = method.getDeclaringKlassRef(); - long klassId = ids.getIdAsLong(klass); - long methodId = ids.getIdAsLong(method); - byte typeTag = TypeTag.getKind(klass); - - Frame frame = frameInstance.getFrame(FrameInstance.FrameAccess.READ_WRITE); - // for bytecode-based languages (Espresso) we can read the precise bci from the - // frame - long codeIndex = -1; - try { - codeIndex = context.readBCIFromFrame(root, frame); - } catch (Throwable t) { - fine(() -> "Unable to read current BCI from frame in method: " + klass.getNameAsString() + "." + method.getNameAsString()); - } - if (codeIndex == -1) { - // fall back to start of the method then - codeIndex = 0; - } + klass = method.getDeclaringKlassRef(); + long klassId = ids.getIdAsLong(klass); + long methodId = ids.getIdAsLong(method); + byte typeTag = TypeTag.getKind(klass); - // check if current bci is higher than the first index on the last line, - // in which case we must report the last line index instead - long lastLineBCI = method.getBCIFromLine(method.getLastLine()); - if (codeIndex > lastLineBCI) { - codeIndex = lastLineBCI; - } - Node currentNode = frameInstance.getCallNode(); - if (currentNode == null) { - CallTarget callTarget = frameInstance.getCallTarget(); - if (callTarget instanceof RootCallTarget) { - currentNode = ((RootCallTarget) callTarget).getRootNode(); - } - } - if (currentNode instanceof RootNode) { - currentNode = context.getInstrumentableNode((RootNode) currentNode); + Frame frame = frameInstance.getFrame(FrameInstance.FrameAccess.READ_WRITE); + // for bytecode-based languages (Espresso) we can read the precise bci from the + // frame + long codeIndex = -1; + try { + codeIndex = context.readBCIFromFrame(root, frame); + } catch (Throwable t) { + fine(() -> "Unable to read current BCI from frame in method: " + klass.getNameAsString() + "." + method.getNameAsString()); + } + if (codeIndex == -1) { + // fall back to start of the method then + codeIndex = 0; + } + + // check if current bci is higher than the first index on the last line, + // in which case we must report the last line index instead + long lastLineBCI = method.getBCIFromLine(method.getLastLine()); + if (codeIndex > lastLineBCI) { + codeIndex = lastLineBCI; + } + Node currentNode = frameInstance.getCallNode(); + if (currentNode == null) { + CallTarget callTarget = frameInstance.getCallTarget(); + if (callTarget instanceof RootCallTarget) { + currentNode = ((RootCallTarget) callTarget).getRootNode(); } - callFrames.add(new CallFrame(context.getIds().getIdAsLong(guestThread), typeTag, klassId, method, methodId, codeIndex, frame, currentNode, root, null, context, - DebuggerController.this)); - return null; } + if (currentNode instanceof RootNode) { + currentNode = context.getInstrumentableNode((RootNode) currentNode); + } + callFrames.add(new CallFrame(context.getIds().getIdAsLong(guestThread), typeTag, klassId, method, methodId, codeIndex, frame, currentNode, root, null, context, + DebuggerController.this)); + return null; }); - CallFrame[] result = callFrames.toArray(new CallFrame[callFrames.size()]); - - // collect monitor info - MonitorStackInfo[] ownedMonitorInfos = context.getOwnedMonitors(result); - HashMap entryCounts = new HashMap<>(ownedMonitorInfos.length); - for (MonitorStackInfo ownedMonitorInfo : ownedMonitorInfos) { - Object monitor = ownedMonitorInfo.getMonitor(); - entryCounts.put(monitor, context.getMonitorEntryCount(monitor)); - } - - suspendedInfos.put(guestThread, new SuspendedInfo(context, result, guestThread, entryCounts)); - return result; + return callFrames.toArray(new CallFrame[0]); } private RootNode getRootNode(FrameInstance frameInstance) { @@ -752,8 +734,7 @@ private RootNode getRootNode(FrameInstance frameInstance) { if (callTarget == null) { return null; } - if (callTarget instanceof RootCallTarget) { - RootCallTarget rootCallTarget = (RootCallTarget) callTarget; + if (callTarget instanceof RootCallTarget rootCallTarget) { RootNode rootNode = rootCallTarget.getRootNode(); // check if we can read the current bci to validate try { @@ -767,10 +748,6 @@ private RootNode getRootNode(FrameInstance frameInstance) { return null; } - public void cancelBlockingCallFrames(Object guestThread) { - suspendedInfos.remove(guestThread); - } - private class SuspendedCallbackImpl implements SuspendedCallback { @Override @@ -806,7 +783,7 @@ public void onSuspend(SuspendedEvent event) { CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), -1, steppingInfo); RootNode callerRootNode = callFrames.length > 1 ? callFrames[1].getRootNode() : null; - SuspendedInfo suspendedInfo = new SuspendedInfo(DebuggerController.this, event, callFrames, currentThread, callerRootNode); + SuspendedInfo suspendedInfo = new SuspendedInfo(context, event, callFrames, currentThread, callerRootNode); suspendedInfos.put(currentThread, suspendedInfo); byte suspendPolicy = SuspendStrategy.EVENT_THREAD; diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java index cebba1fde8f3..161411105dac 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -50,7 +50,6 @@ public final class JDWP { public static final String JAVA_LANG_OBJECT = "Ljava/lang/Object;"; private static final boolean CAN_GET_INSTANCE_INFO = false; - private static final long SUSPEND_TIMEOUT = 400; private static final int ACC_SYNTHETIC = 0x00001000; private static final int JDWP_SYNTHETIC = 0xF0000000; @@ -194,14 +193,6 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { PacketStream reply = new PacketStream().replyPacket().id(packet.id); controller.suspendAll(); - - // give threads time to suspend before returning - for (Object guestThread : controller.getVisibleGuestThreads()) { - SuspendedInfo info = controller.getSuspendedInfo(guestThread); - if (info instanceof UnknownSuspendedInfo) { - awaitSuspendedInfo(controller, guestThread, info); - } - } return new CommandResult(reply); } } @@ -1721,23 +1712,14 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { } else { reply.writeLong(context.getIds().getIdAsLong(monitorOwnerThread)); - // go through the suspended info to obtain the entry count + // check if thread not suspended SuspendedInfo info = controller.getSuspendedInfo(monitorOwnerThread); if (info == null) { reply.errorCode(ErrorCodes.THREAD_NOT_SUSPENDED); return new CommandResult(reply); } - - if (info instanceof UnknownSuspendedInfo) { - awaitSuspendedInfo(controller, monitorOwnerThread, info); - if (info instanceof UnknownSuspendedInfo) { - // still no known suspension state - reply.errorCode(ErrorCodes.THREAD_NOT_SUSPENDED); - return new CommandResult(reply); - } - } - int entryCount = info.getMonitorEntryCount(monitor); + int entryCount = context.getMonitorEntryCount(monitorOwnerThread, monitor); if (entryCount == -1) { reply.errorCode(ErrorCodes.INVALID_OBJECT); @@ -2154,16 +2136,6 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { return new CommandResult(reply); } - if (suspendedInfo instanceof UnknownSuspendedInfo) { - controller.fine(() -> "Unknown suspension info for thread: " + controller.getContext().getThreadName(thread)); - suspendedInfo = awaitSuspendedInfo(controller, thread, suspendedInfo); - if (suspendedInfo instanceof UnknownSuspendedInfo) { - // we can't return any frames for a not yet suspended thread - reply.errorCode(ErrorCodes.THREAD_NOT_SUSPENDED); - return new CommandResult(reply); - } - } - CallFrame[] frames = suspendedInfo.getStackFrames(); if (length == -1 || length > frames.length) { @@ -2206,11 +2178,8 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { return new CommandResult(reply); } - if (suspendedInfo instanceof UnknownSuspendedInfo) { - suspendedInfo = awaitSuspendedInfo(controller, thread, suspendedInfo); - } int length = suspendedInfo.getStackFrames().length; - reply.writeInt(suspendedInfo.getStackFrames().length); + reply.writeInt(length); controller.fine(() -> "current frame count: " + length + " for thread: " + controller.getContext().getThreadName(thread)); return new CommandResult(reply); @@ -2239,15 +2208,6 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { return new CommandResult(reply); } - if (info instanceof UnknownSuspendedInfo) { - info = awaitSuspendedInfo(controller, thread, info); - if (info instanceof UnknownSuspendedInfo) { - // still no known suspension state - reply.errorCode(ErrorCodes.THREAD_NOT_SUSPENDED); - return new CommandResult(reply); - } - } - // fetch all monitors on current stack MonitorStackInfo[] ownedMonitors = context.getOwnedMonitors(info.getStackFrames()); @@ -2379,14 +2339,6 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { SuspendedInfo suspendedInfo = controller.getSuspendedInfo(thread); - if (suspendedInfo instanceof UnknownSuspendedInfo) { - suspendedInfo = awaitSuspendedInfo(controller, thread, suspendedInfo); - if (suspendedInfo instanceof UnknownSuspendedInfo) { - reply.errorCode(ErrorCodes.THREAD_NOT_SUSPENDED); - return new CommandResult(reply); - } - } - MonitorStackInfo[] ownedMonitorInfos = context.getOwnedMonitors(suspendedInfo.getStackFrames()); // filter out monitors not owned by thread ArrayList filtered = new ArrayList<>(ownedMonitorInfos.length); @@ -2429,17 +2381,6 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { return new CommandResult(reply); } - if (info instanceof UnknownSuspendedInfo) { - info = awaitSuspendedInfo(controller, thread, info); - if (info instanceof UnknownSuspendedInfo) { - // still no known suspension state - reply.errorCode(ErrorCodes.THREAD_NOT_SUSPENDED); - return new CommandResult(reply); - } - } - - final SuspendedInfo suspendedInfo = info; - Object returnValue = readValue(input, controller.getContext()); if (returnValue == Void.TYPE) { // we have to use an Interop value, so simply use @@ -2447,8 +2388,9 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { // return type methods anyway returnValue = controller.getContext().getNullObject(); } - CallFrame topFrame = suspendedInfo.getStackFrames().length > 0 ? suspendedInfo.getStackFrames()[0] : null; - if (!controller.forceEarlyReturn(thread, topFrame, returnValue)) { + CallFrame[] stackFrames = info.getStackFrames(); + CallFrame topFrame = stackFrames.length > 0 ? stackFrames[0] : null; + if (topFrame == null || !controller.forceEarlyReturn(info, thread, topFrame, returnValue)) { reply.errorCode(ErrorCodes.OPAQUE_FRAME); } @@ -2993,32 +2935,6 @@ static CommandResult createReply(Packet packet) { } } - private static SuspendedInfo awaitSuspendedInfo(DebuggerController controller, Object thread, SuspendedInfo suspendedInfo) { - // OK, we hard suspended this thread, but it hasn't yet actually suspended - // in a code location known to Truffle - // let's check if the thread is RUNNING and give it a moment to reach - // the suspended state - SuspendedInfo result = suspendedInfo; - Thread hostThread = controller.getContext().asHostThread(thread); - if (hostThread.getState() == Thread.State.RUNNABLE) { - controller.fine(() -> "Awaiting suspended info for thread " + controller.getContext().getThreadName(thread)); - - long timeout = System.currentTimeMillis() + SUSPEND_TIMEOUT; - while (result instanceof UnknownSuspendedInfo && System.currentTimeMillis() < timeout) { - try { - Thread.sleep(10); - result = controller.getSuspendedInfo(thread); - } catch (InterruptedException e) { - // ignore this here - } - } - } - if (result instanceof UnknownSuspendedInfo) { - controller.fine(() -> "Still no suspended info for thread " + controller.getContext().getThreadName(thread)); - } - return result; - } - private static Object readValue(byte valueKind, PacketStream input, JDWPContext context) { switch (valueKind) { case TagConstants.BOOLEAN: @@ -3092,7 +3008,7 @@ public static void writeValue(byte tag, Object value, PacketStream reply, boolea case TagConstants.BOOLEAN: if (value.getClass() == Long.class) { long unboxed = (long) value; - reply.writeBoolean(unboxed > 0 ? true : false); + reply.writeBoolean(unboxed > 0); } else { reply.writeBoolean((boolean) value); } diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SuspendedInfo.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SuspendedInfo.java index b215b2cd5156..8e78ec3f0637 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SuspendedInfo.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SuspendedInfo.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -22,9 +22,6 @@ */ package com.oracle.truffle.espresso.jdwp.impl; -import java.util.HashMap; -import java.util.concurrent.Callable; - import com.oracle.truffle.api.debug.SuspendedEvent; import com.oracle.truffle.api.frame.Frame; import com.oracle.truffle.api.nodes.RootNode; @@ -34,34 +31,28 @@ public class SuspendedInfo { protected final JDWPContext context; - private final DebuggerController controller; private final SuspendedEvent event; private final CallFrame[] stackFrames; private final Object thread; private final RootNode callerRootNode; private boolean forceEarlyInProgress; - private final HashMap monitorEntryCounts; - SuspendedInfo(DebuggerController controller, SuspendedEvent event, CallFrame[] stackFrames, Object thread, RootNode callerRootNode) { - this.controller = controller; - this.context = controller.getContext(); + SuspendedInfo(JDWPContext context, SuspendedEvent event, CallFrame[] stackFrames, Object thread, RootNode callerRootNode) { + this.context = context; this.event = event; this.stackFrames = stackFrames; this.thread = thread; this.callerRootNode = callerRootNode; - this.monitorEntryCounts = null; } // used for pre-collected thread suspension data, before the thread // disappears to native code - SuspendedInfo(JDWPContext context, CallFrame[] stackFrames, Object thread, HashMap monitorEntryCounts) { - this.controller = null; + SuspendedInfo(JDWPContext context, CallFrame[] stackFrames, Object thread) { this.context = context; this.event = null; this.stackFrames = stackFrames; this.thread = thread; this.callerRootNode = null; - this.monitorEntryCounts = monitorEntryCounts; } public SuspendedEvent getEvent() { @@ -91,19 +82,4 @@ public void setForceEarlyReturnInProgress() { public boolean isForceEarlyReturnInProgress() { return forceEarlyInProgress; } - - public int getMonitorEntryCount(Object monitor) { - if (monitorEntryCounts != null) { - return monitorEntryCounts.get(monitor); - } else { - ThreadJob job = new ThreadJob<>(thread, new Callable() { - @Override - public Integer call() { - return context.getMonitorEntryCount(monitor); - } - }); - controller.postJobForThread(job); - return job.getResult().getResult(); - } - } } diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/UnknownSuspendedInfo.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/UnknownSuspendedInfo.java index 496bde7597bf..c59453b9a343 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/UnknownSuspendedInfo.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/UnknownSuspendedInfo.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -27,8 +27,8 @@ public final class UnknownSuspendedInfo extends SuspendedInfo { - UnknownSuspendedInfo(Object thread, JDWPContext jdwpContext) { - super(jdwpContext, null, thread, null); + UnknownSuspendedInfo(JDWPContext context, Object thread) { + super(context, null, thread); } @Override diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/JDWPContextImpl.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/JDWPContextImpl.java index 8ce25fa7dffc..891d9156454a 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/JDWPContextImpl.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/JDWPContextImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -29,8 +29,13 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.regex.Matcher; +import com.oracle.truffle.api.ThreadLocalAction; import com.oracle.truffle.api.TruffleLanguage; import com.oracle.truffle.api.debug.Debugger; import com.oracle.truffle.api.frame.Frame; @@ -63,6 +68,7 @@ import com.oracle.truffle.espresso.jdwp.impl.DebuggerController; import com.oracle.truffle.espresso.jdwp.impl.JDWPInstrument; import com.oracle.truffle.espresso.jdwp.impl.TypeTag; +import com.oracle.truffle.espresso.meta.EspressoError; import com.oracle.truffle.espresso.meta.Meta; import com.oracle.truffle.espresso.nodes.BciProvider; import com.oracle.truffle.espresso.nodes.EspressoInstrumentableRootNode; @@ -82,6 +88,8 @@ public final class JDWPContextImpl implements JDWPContext { public static final String JAVA_LANG_STRING = "Ljava/lang/String;"; private static final InteropLibrary UNCACHED = InteropLibrary.getUncached(); + private static final long SUSPEND_TIMEOUT = 100; + private final EspressoContext context; private final Ids ids; private final JDWPSetup setup; @@ -126,8 +134,7 @@ public boolean isString(Object string) { @Override public boolean isValidThread(Object thread, boolean checkTerminated) { - if (thread instanceof StaticObject) { - StaticObject staticObject = (StaticObject) thread; + if (thread instanceof StaticObject staticObject) { if (context.getMeta().java_lang_Thread.isAssignableFrom(staticObject.getKlass())) { if (checkTerminated) { // check if thread has been terminated @@ -141,8 +148,7 @@ public boolean isValidThread(Object thread, boolean checkTerminated) { @Override public boolean isValidThreadGroup(Object threadGroup) { - if (threadGroup instanceof StaticObject) { - StaticObject staticObject = (StaticObject) threadGroup; + if (threadGroup instanceof StaticObject staticObject) { return context.getMeta().java_lang_ThreadGroup.isAssignableFrom(staticObject.getKlass()); } else { return false; @@ -240,8 +246,7 @@ public List getInitiatedClasses(Object classLoader) { @Override public boolean isValidClassLoader(Object object) { - if (object instanceof StaticObject) { - StaticObject loader = (StaticObject) object; + if (object instanceof StaticObject loader) { return context.getRegistries().isClassLoader(loader); } return false; @@ -284,8 +289,7 @@ public Object[] getAllGuestThreads() { @Override public String getStringValue(Object object) { - if (object instanceof StaticObject) { - StaticObject staticObject = (StaticObject) object; + if (object instanceof StaticObject staticObject) { return (String) InteropLibrary.getUncached().toDisplayString(staticObject, false); } return object.toString(); @@ -310,8 +314,7 @@ public KlassRef getRefType(Object object) { @Override public KlassRef getReflectedType(Object classObject) { - if (classObject instanceof StaticObject) { - StaticObject staticObject = (StaticObject) classObject; + if (classObject instanceof StaticObject staticObject) { if (staticObject.getKlass().getType() == Symbol.Type.java_lang_Class) { return (KlassRef) context.getMeta().HIDDEN_MIRROR_KLASS.getHiddenObject(staticObject); } @@ -321,9 +324,8 @@ public KlassRef getReflectedType(Object classObject) { @Override public KlassRef[] getNestedTypes(KlassRef klass) { - if (klass instanceof ObjectKlass) { + if (klass instanceof ObjectKlass objectKlass) { ArrayList result = new ArrayList<>(); - ObjectKlass objectKlass = (ObjectKlass) klass; List> nestedTypeNames = objectKlass.getNestedTypeNames(); StaticObject classLoader = objectKlass.getDefiningClassLoader(); @@ -345,8 +347,7 @@ public byte getTag(Object object) { return TagConstants.OBJECT; } byte tag = TagConstants.OBJECT; - if (object instanceof StaticObject) { - StaticObject staticObject = (StaticObject) object; + if (object instanceof StaticObject staticObject) { if (object == StaticObject.NULL) { return tag; } @@ -424,8 +425,7 @@ public T getUnboxedArray(Object array) { @Override public boolean isArray(Object object) { - if (object instanceof StaticObject) { - StaticObject staticObject = (StaticObject) object; + if (object instanceof StaticObject staticObject) { return staticObject.isArray(); } return false; @@ -484,7 +484,7 @@ public Object getArrayValue(Object array, int index) { // For JDWP we have to have a ref type, so here we have to create a copy // value when possible as a StaticObject based on the foreign type. // Note: we only support Host String conversion for now - if (String.class.isInstance(value)) { + if (value instanceof String) { return context.getMeta().toGuestString((String) value); } else { throw new IllegalStateException("foreign object conversion not supported"); @@ -523,8 +523,7 @@ public Object toGuestString(String string) { @Override public Object getGuestException(Throwable exception) { - if (exception instanceof EspressoException) { - EspressoException ex = (EspressoException) exception; + if (exception instanceof EspressoException ex) { return ex.getGuestException(); } else { throw new RuntimeException("unknown exception type: " + exception.getClass(), exception); @@ -533,9 +532,42 @@ public Object getGuestException(Throwable exception) { @Override public CallFrame[] getStackTrace(Object thread) { - // TODO(Gregersen) - implement this method when we can get stack frames - // for arbitrary threads. - return new CallFrame[0]; + Thread hostThread = asHostThread(thread); + + if (Thread.currentThread() == hostThread) { + // on current thread, we can just fetch the frames directly + return controller.getCallFrames(thread); + } else { + // on other threads, we have to utilize Truffle safe points + CollectStackFramesAction action = new CollectStackFramesAction(thread); + Future future = context.getEnv().submitThreadLocal(new Thread[]{hostThread}, action); + try { + future.get(SUSPEND_TIMEOUT, TimeUnit.MILLISECONDS); + return action.result; + } catch (ExecutionException e) { + throw EspressoError.shouldNotReachHere(e); + } catch (InterruptedException | TimeoutException e) { + // OK, when interrupted we can't get stack frames + future.cancel(true); + return new CallFrame[0]; + } + } + } + + private final class CollectStackFramesAction extends ThreadLocalAction { + CallFrame[] result; + + final Object guestThread; + + CollectStackFramesAction(Object guestThread) { + super(false, false); + this.guestThread = guestThread; + } + + @Override + protected void perform(Access access) { + result = controller.getCallFrames(guestThread); + } } @Override @@ -604,8 +636,7 @@ public int getCatchLocation(MethodRef method, Object guestException, int bci) { @Override public int getNextBCI(RootNode callerRoot, Frame frame) { - if (callerRoot instanceof EspressoRootNode) { - EspressoRootNode espressoRootNode = (EspressoRootNode) callerRoot; + if (callerRoot instanceof EspressoRootNode espressoRootNode) { int bci = (int) readBCIFromFrame(callerRoot, frame); if (bci >= 0) { BytecodeStream bs = new BytecodeStream(espressoRootNode.getMethodVersion().getOriginalCode()); @@ -617,8 +648,7 @@ public int getNextBCI(RootNode callerRoot, Frame frame) { @Override public long readBCIFromFrame(RootNode root, Frame frame) { - if (root instanceof EspressoRootNode && frame != null) { - EspressoRootNode rootNode = (EspressoRootNode) root; + if (root instanceof EspressoRootNode rootNode && frame != null) { return rootNode.readBCI(frame); } return -1; @@ -642,12 +672,50 @@ public Object getMonitorOwnerThread(Object object) { } @Override - public int getMonitorEntryCount(Object monitor) { - if (monitor instanceof StaticObject) { - EspressoLock lock = ((StaticObject) monitor).getLock(context); + public int getMonitorEntryCount(Object monitorOwnerThread, Object monitor) { + if (!(monitor instanceof StaticObject theMonitor)) { + return -1; + } + Thread hostThread = asHostThread(monitorOwnerThread); + if (Thread.currentThread() == hostThread) { + // on current thread, we can get the results directly + EspressoLock lock = theMonitor.getLock(context); return lock.getEntryCount(); + } else { + // on other threads, we have to utilize Truffle safe points + GetMonitorEntryCountAction action = new GetMonitorEntryCountAction(theMonitor); + Future future = context.getEnv().submitThreadLocal(new Thread[]{hostThread}, action); + try { + future.get(SUSPEND_TIMEOUT, TimeUnit.MILLISECONDS); + return action.result; + } catch (ExecutionException e) { + throw EspressoError.shouldNotReachHere(e); + } catch (InterruptedException | TimeoutException e) { + future.cancel(true); + // OK, not possible to get accurate result, but since we know the monitor is + // currently owned by the owner thread, we return 1 because it's currently locked at + // least once + return 1; + } + } + } + + private final class GetMonitorEntryCountAction extends ThreadLocalAction { + int result; + StaticObject monitor; + + GetMonitorEntryCountAction(StaticObject monitor) { + super(false, false); + this.monitor = monitor; + } + + @Override + protected void perform(Access access) { + // Since by the time we get here this thread might not own the lock, so we'll return 0. + // In this case it's up to the caller to handle that. + EspressoLock lock = monitor.getLock(context); + result = lock.getEntryCount(); } - return -1; } @Override @@ -656,8 +724,7 @@ public MonitorStackInfo[] getOwnedMonitors(CallFrame[] callFrames) { int stackDepth = 0; for (CallFrame callFrame : callFrames) { RootNode rootNode = callFrame.getRootNode(); - if (rootNode instanceof EspressoRootNode) { - EspressoRootNode espressoRootNode = (EspressoRootNode) rootNode; + if (rootNode instanceof EspressoRootNode espressoRootNode) { if (espressoRootNode.usesMonitors()) { StaticObject[] monitors = espressoRootNode.getMonitorsOnFrame(callFrame.getFrame()); for (StaticObject monitor : monitors) { @@ -675,8 +742,7 @@ public MonitorStackInfo[] getOwnedMonitors(CallFrame[] callFrames) { @Override public void clearFrameMonitors(CallFrame frame) { RootNode rootNode = frame.getRootNode(); - if (rootNode instanceof EspressoRootNode) { - EspressoRootNode espressoRootNode = (EspressoRootNode) rootNode; + if (rootNode instanceof EspressoRootNode espressoRootNode) { espressoRootNode.abortInternalMonitors(frame.getFrame()); } } @@ -732,8 +798,7 @@ public Node getInstrumentableNode(RootNode rootNode) { @Override public boolean isMemberOf(Object guestObject, KlassRef klass) { - if (guestObject instanceof StaticObject) { - StaticObject staticObject = (StaticObject) guestObject; + if (guestObject instanceof StaticObject staticObject) { return klass.isAssignable(staticObject.getKlass()); } else { return false; diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py index 8c320c88bc74..234393cd2d06 100644 --- a/substratevm/mx.substratevm/mx_substratevm.py +++ b/substratevm/mx.substratevm/mx_substratevm.py @@ -2024,9 +2024,10 @@ def native_image_on_jvm(args, **kwargs): args.append("-D" + key + "=" + value) jacoco_args = mx_gate.get_jacoco_agent_args(agent_option_prefix='-J') + passedArgs = args if jacoco_args is not None: - arg += jacoco_args - mx.run([executable] + _debug_args() + args, **kwargs) + passedArgs += jacoco_args + mx.run([executable] + _debug_args() + passedArgs, **kwargs) @mx.command(suite.name, 'native-image-configure') def native_image_configure_on_jvm(args, **kwargs): diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AnalysisParsedGraph.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AnalysisParsedGraph.java index 8361c75c5fbb..316e9a0c05f2 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AnalysisParsedGraph.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AnalysisParsedGraph.java @@ -125,7 +125,7 @@ public static AnalysisParsedGraph parseBytecode(BigBang bb, AnalysisMethod metho graph = new StructuredGraph.Builder(options, debug) .method(method) - .recordInlinedMethods(false) + .recordInlinedMethods(bb.getHostVM().recordInlinedMethods(method)) .build(); try (DebugContext.Scope s = debug.scope("ClosedWorldAnalysis", graph, method)) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java index 740a1c1df1b4..1d7031f07617 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java @@ -953,23 +953,25 @@ public StructuredGraph decodeAnalyzedGraph(DebugContext debug, Iterable nodeReferences, boolean trackNodeSourcePosition, + public StructuredGraph decodeAnalyzedGraph(DebugContext debug, Iterable nodeReferences, boolean trackNodeSourcePosition, boolean recordInlinedMethods, BiFunction decoderProvider) { if (analyzedGraph == null) { return null; } var allowAssumptions = getUniverse().hostVM().allowAssumptions(this); - // Note we never record inlined methods. This is correct even for runtime compiled methods - StructuredGraph result = new StructuredGraph.Builder(debug.getOptions(), debug, allowAssumptions).method(this).recordInlinedMethods(false).trackNodeSourcePosition( - trackNodeSourcePosition).build(); + StructuredGraph result = new StructuredGraph.Builder(debug.getOptions(), debug, allowAssumptions) + .method(this) + .trackNodeSourcePosition(trackNodeSourcePosition) + .recordInlinedMethods(recordInlinedMethods) + .build(); GraphDecoder decoder = decoderProvider.apply(AnalysisParsedGraph.HOST_ARCHITECTURE, result); decoder.decode(analyzedGraph, nodeReferences); /* diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysis.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysis.java index 4e186c2155e9..555d11df74ab 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysis.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysis.java @@ -64,8 +64,8 @@ public static StructuredGraph decodeGraph(BigBang bb, AnalysisMethod method, Ana StructuredGraph result = new StructuredGraph.Builder(bb.getOptions(), debug, bb.getHostVM().allowAssumptions(method)) .method(method) - .recordInlinedMethods(bb.getHostVM().recordInlinedMethods(method)) .trackNodeSourcePosition(analysisParsedGraph.getEncodedGraph().trackNodeSourcePosition()) + .recordInlinedMethods(analysisParsedGraph.getEncodedGraph().isRecordingInlinedMethods()) .build(); try (DebugContext.Scope s = debug.scope("InlineBeforeAnalysis", result)) { diff --git a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/AMD64SubstrateSuitesCreator.java b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/AMD64SubstrateSuitesCreator.java index 11e75c709ae1..b02a799daa5d 100644 --- a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/AMD64SubstrateSuitesCreator.java +++ b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/AMD64SubstrateSuitesCreator.java @@ -27,7 +27,9 @@ import jdk.graal.compiler.core.amd64.AMD64SuitesCreator; import jdk.graal.compiler.debug.GraalError; import jdk.graal.compiler.java.GraphBuilderPhase; +import jdk.graal.compiler.lir.phases.LIRSuites; import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration; +import jdk.graal.compiler.options.OptionValues; import jdk.graal.compiler.phases.tiers.CompilerConfiguration; public class AMD64SubstrateSuitesCreator extends AMD64SuitesCreator { @@ -41,4 +43,12 @@ protected GraphBuilderPhase createGraphBuilderPhase(GraphBuilderConfiguration gr throw GraalError.shouldNotReachHere("this path is unused"); } + @Override + public LIRSuites createLIRSuites(OptionValues options) { + LIRSuites lirSuites = super.createLIRSuites(options); + /* Enable support for methods that need a frame pointer for stack unwinding purposes. */ + lirSuites.getPreAllocationOptimizationStage().appendPhase(new FramePointerPhase()); + lirSuites.getFinalCodeAnalysisStage().appendPhase(new VerifyFramePointerPhase()); + return lirSuites; + } } diff --git a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/FramePointerPhase.java b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/FramePointerPhase.java new file mode 100644 index 000000000000..d73ce5b8fdc4 --- /dev/null +++ b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/FramePointerPhase.java @@ -0,0 +1,149 @@ +/* + * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package com.oracle.svm.core.graal.amd64; + +import java.util.ArrayList; + +import com.oracle.svm.core.SubstrateOptions; + +import jdk.graal.compiler.asm.amd64.AMD64MacroAssembler; +import jdk.graal.compiler.core.common.cfg.BasicBlock; +import jdk.graal.compiler.lir.LIR; +import jdk.graal.compiler.lir.LIRInsertionBuffer; +import jdk.graal.compiler.lir.LIRInstruction; +import jdk.graal.compiler.lir.LIRInstructionClass; +import jdk.graal.compiler.lir.Opcode; +import jdk.graal.compiler.lir.amd64.AMD64Call; +import jdk.graal.compiler.lir.amd64.AMD64LIRInstruction; +import jdk.graal.compiler.lir.asm.CompilationResultBuilder; +import jdk.graal.compiler.lir.framemap.FrameMapBuilderTool; +import jdk.graal.compiler.lir.gen.LIRGenerationResult; +import jdk.graal.compiler.lir.phases.PreAllocationOptimizationPhase; +import jdk.vm.ci.amd64.AMD64; +import jdk.vm.ci.code.TargetDescription; + +/** + * This phase ensures that methods that modify the stack pointer in a non-standard way have a frame + * pointer, which is necessary to properly support stack unwinding. + * + * @see LIRInstruction#modifiesStackPointer + */ +class FramePointerPhase extends PreAllocationOptimizationPhase { + @Override + protected void run(TargetDescription target, LIRGenerationResult lirGenRes, PreAllocationOptimizationContext context) { + LIR lir = lirGenRes.getLIR(); + if (!modifiesStackPointer(lir)) { + return; + } + + /* + * The stack pointer is being modified in a way that requires the use of a frame pointer, so + * for this method we configure the register allocator to preserve rbp, and the method's + * prologue and epilogue to use rbp as the frame pointer. + */ + ((SubstrateAMD64Backend.SubstrateAMD64RegisterAllocationConfig) lirGenRes.getRegisterAllocationConfig()).setPreserveFramePointer(); + ((SubstrateAMD64Backend.SubstrateAMD64FrameMap) ((FrameMapBuilderTool) lirGenRes.getFrameMapBuilder()).getFrameMap()).setNeedsFramePointer(); + + /* + * Additionally, if rbp is not globally preserved, we must also preserve it around calls + * that may destroy it. + */ + if (!SubstrateOptions.PreserveFramePointer.getValue()) { + LIRInsertionBuffer buffer = new LIRInsertionBuffer(); + for (int blockId : lir.getBlocks()) { + if (LIR.isBlockDeleted(blockId)) { + continue; + } + BasicBlock block = lir.getBlockById(blockId); + ArrayList instructions = lir.getLIRforBlock(block); + + /* + * Note that we need to restore rbp after both calls and exceptions, so for + * simplicity we spill it before each call (not just the ones that might destroy it) + * because then we can simply reload it in each exception handler. + */ + buffer.init(instructions); + if (block.isExceptionEntry()) { + buffer.append(lirGenRes.getFirstInsertPosition(), new ReloadFramePointerOp()); + } + for (int i = 0; i < instructions.size(); i++) { + if (instructions.get(i) instanceof AMD64Call.CallOp callOp) { + buffer.append(i, new SpillFramePointerOp()); + if (callOp.destroysCallerSavedRegisters()) { + buffer.append(i + 1, new ReloadFramePointerOp()); + } + } + } + buffer.finish(); + } + } + } + + /** Returns true if any LIR instruction modifies the stack pointer, false otherwise. */ + private static boolean modifiesStackPointer(LIR lir) { + for (int blockId : lir.getBlocks()) { + if (LIR.isBlockDeleted(blockId)) { + continue; + } + BasicBlock block = lir.getBlockById(blockId); + for (LIRInstruction op : lir.getLIRforBlock(block)) { + if (op.modifiesStackPointer()) { + return true; + } + } + } + return false; + } + + @Opcode("SPILL_FRAME_POINTER") + public static class SpillFramePointerOp extends AMD64LIRInstruction { + public static final LIRInstructionClass TYPE = LIRInstructionClass.create(SpillFramePointerOp.class); + + SpillFramePointerOp() { + super(TYPE); + } + + @Override + public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) { + var frameMap = (SubstrateAMD64Backend.SubstrateAMD64FrameMap) crb.frameMap; + masm.movq(masm.makeAddress(AMD64.rsp, frameMap.getFramePointerSaveAreaOffset()), AMD64.rbp); + } + } + + @Opcode("RELOAD_FRAME_POINTER") + public static class ReloadFramePointerOp extends AMD64LIRInstruction { + public static final LIRInstructionClass TYPE = LIRInstructionClass.create(ReloadFramePointerOp.class); + + ReloadFramePointerOp() { + super(TYPE); + } + + @Override + public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) { + var frameMap = (SubstrateAMD64Backend.SubstrateAMD64FrameMap) crb.frameMap; + masm.movq(AMD64.rbp, masm.makeAddress(AMD64.rsp, frameMap.getFramePointerSaveAreaOffset())); + } + } +} diff --git a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java index 3e87c935beda..d1be8b681ba3 100644 --- a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java +++ b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/SubstrateAMD64Backend.java @@ -38,6 +38,7 @@ import static jdk.vm.ci.code.ValueUtil.asRegister; import static jdk.vm.ci.code.ValueUtil.isRegister; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.EnumSet; @@ -110,6 +111,7 @@ import jdk.graal.compiler.core.common.CompressEncoding; import jdk.graal.compiler.core.common.GraalOptions; import jdk.graal.compiler.core.common.LIRKind; +import jdk.graal.compiler.core.common.NumUtil; import jdk.graal.compiler.core.common.Stride; import jdk.graal.compiler.core.common.alloc.RegisterAllocationConfig; import jdk.graal.compiler.core.common.memory.MemoryExtendKind; @@ -189,6 +191,7 @@ import jdk.vm.ci.code.CompilationRequest; import jdk.vm.ci.code.CompiledCode; import jdk.vm.ci.code.Register; +import jdk.vm.ci.code.RegisterArray; import jdk.vm.ci.code.RegisterAttributes; import jdk.vm.ci.code.RegisterConfig; import jdk.vm.ci.code.RegisterValue; @@ -1112,12 +1115,17 @@ private static ForeignCallDescriptor chooseCPUFeatureVariant(ForeignCallDescript /** * Generates a method's prologue and epilogue. *

- * Depending on the {@link SubstrateAMD64FrameMap frame map} properties and whether the rbp + * Depending on the {@linkplain SubstrateAMD64FrameMap frame map} properties and whether the rbp * register is saved by the caller or the callee, we use different forms of prologue and * epilogue. + *

+ * If a method doesn't need a frame pointer, we use the following forms: * * 

-     *
+     *          |        needsFramePointer        |
+     *          +---------------------------------+
+     *          |              false              |
+     *          +---------------------------------+
      *          |      preserveFramePointer       |
      *          +----------------+----------------+
      *          |     false      |      true      |
@@ -1142,7 +1150,58 @@ private static ForeignCallDescriptor chooseCPUFeatureVariant(ForeignCallDescript
      *          |  pop rbp       |  pop rbp       |
      *          |  ret           |  ret           |
      *  --------+----------------+----------------+
+     *
+     *  Legend:
+     *    #fs - frame size
      *
+ *

+ * If a method does need a frame pointer, we use the following forms: + * + * 

+     *          |                 needsFramePointer                 |
+     *          +---------------------------------------------------+
+     *          |                       true                        |
+     *          +---------------------------------------------------+
+     *          |               preserveFramePointer                |
+     *          +-------------------------+-------------------------+
+     *          |          false          |          true           |
+     *  --------+-------------------------+-------------------------+
+     *          |  ; prologue             |  ; prologue             |
+     *          |  sub rsp, #fs           |  push rbp               |
+     *          |  mov rbp, rsp           |  sub rsp, #fs           |
+     *          |                         |  mov #fp[rsp], rbp      |
+     *  rbp is  |                         |  mov rbp, #fs+8[rsp]    |
+     *  caller  |                         |  mov #fp+8[rsp], rbp    |
+     *  saved   |                         |  lea rbp, #fp[rsp]      |
+     *          |                         |                         |
+     *          |  ; epilogue             |  ; epilogue             |
+     *          |  lea rsp, #fs[rbp]      |  lea rsp, #fs-#fp[rbp]  |
+     *          |  ret                    |  pop rbp                |
+     *          |                         |  ret                    |
+     *  --------+-------------------------+-------------------------+
+     *          |  ; prologue             |  ; prologue             |
+     *          |  push rbp               |  push rbp               |
+     *          |  sub rsp, #fs           |  sub rsp, #fs           |
+     *          |  mov rbp, rsp           |  mov #fp[rsp], rbp      |
+     *  rbp is  |                         |  mov rbp, #fs+8[rsp]    |
+     *  callee  |                         |  mov #fp+8[rsp], rbp    |
+     *  saved   |                         |  lea rbp, #fp[rsp]      |
+     *          |                         |                         |
+     *          |  ; epilogue             |  ; epilogue             |
+     *          |  lea rsp, #fs[rbp]      |  lea rsp, #fs-#fp[rbp]  |
+     *          |  pop rbp                |  pop rbp                |
+     *          |  ret                    |  ret                    |
+     *  --------+-------------------------+-------------------------+
+     *
+     *  Legend:
+     *    #fs - frame size
+     *    #fp - frame pointer save area offset
+     *
+ * + * Note that the platform ABI may require a certain form of prologue/epilogue (e.g., see + * x64 + * prolog and epilog for Windows), so any changes must take all such requirements into + * account. */ protected static class SubstrateAMD64FrameContext implements FrameContext { @@ -1161,6 +1220,8 @@ public void enter(CompilationResultBuilder crb) { makeFrame(crb, asm); crb.recordMark(PROLOGUE_DECD_RSP); + maybeSetFramePointer(crb, asm); + if (method.hasCalleeSavedRegisters()) { VMError.guarantee(!method.isDeoptTarget(), "Deoptimization runtime cannot fill the callee saved registers"); AMD64CalleeSavedRegisters.singleton().emitSave((AMD64MacroAssembler) crb.asm, crb.frameMap.totalFrameSize(), crb); @@ -1180,16 +1241,36 @@ protected final void reserveStackFrame(CompilationResultBuilder crb, AMD64MacroA protected void maybePushBasePointer(CompilationResultBuilder crb, AMD64MacroAssembler asm) { SubstrateAMD64FrameMap frameMap = (SubstrateAMD64FrameMap) crb.frameMap; - if (frameMap.preserveFramePointer()) { - /* - * Note that we never use the `enter` instruction so that we have a predictable code - * pattern at each method prologue. And `enter` seems to be slower than the explicit - * code. - */ + if (frameMap.preserveFramePointer() || isCalleeSaved(rbp, frameMap.getRegisterConfig(), method)) { asm.push(rbp); + } + if (frameMap.preserveFramePointer() && !frameMap.needsFramePointer()) { + /* We won't be using rbp as a frame pointer, so we form a frame chain here. */ asm.movq(rbp, rsp); - } else if (isCalleeSaved(rbp, frameMap.getRegisterConfig(), method)) { - asm.push(rbp); + } + } + + /** Establishes rbp as the frame pointer if needed, while taking care of frame chaining. */ + private static void maybeSetFramePointer(CompilationResultBuilder crb, AMD64MacroAssembler asm) { + SubstrateAMD64FrameMap frameMap = (SubstrateAMD64FrameMap) crb.frameMap; + if (frameMap.needsFramePointer()) { + if (frameMap.preserveFramePointer()) { + /* + * We need to form a frame chain using the frame pointer save area because we + * will be using rbp as the frame pointer. + */ + int framePointerSaveAreaOffset = frameMap.getFramePointerSaveAreaOffset(); + /* So we first store rbp ... */ + asm.movq(asm.makeAddress(rsp, framePointerSaveAreaOffset), rbp); + /* ... then copy the return address ... */ + asm.movq(rbp, asm.makeAddress(rsp, frameMap.frameSize() + frameMap.getTarget().wordSize)); + asm.movq(asm.makeAddress(rsp, framePointerSaveAreaOffset + frameMap.getTarget().wordSize), rbp); + /* ... and set the frame pointer to [rsp + framePointerSaveAreaOffset]. */ + asm.leaq(rbp, asm.makeAddress(rsp, framePointerSaveAreaOffset)); + } else { + /* Set the frame pointer to [rsp]. */ + asm.movq(rbp, rsp); + } } } @@ -1208,7 +1289,12 @@ public void leave(CompilationResultBuilder crb) { AMD64CalleeSavedRegisters.singleton().emitRestore(asm, frameMap.totalFrameSize(), returnRegister, crb); } - asm.incrementq(rsp, frameMap.frameSize()); + if (frameMap.needsFramePointer()) { + int framePointerOffset = frameMap.preserveFramePointer() ? frameMap.getFramePointerSaveAreaOffset() : 0; + asm.leaq(rsp, asm.makeAddress(rbp, frameMap.frameSize() - framePointerOffset)); + } else { + asm.incrementq(rsp, frameMap.frameSize()); + } if (frameMap.preserveFramePointer() || isCalleeSaved(rbp, frameMap.getRegisterConfig(), method)) { asm.pop(rbp); } @@ -1472,9 +1558,63 @@ private FrameMapBuilder newFrameMapBuilder(RegisterConfig registerConfig, Shared * AMD64 Substrate VM specific frame map. *

* The layout is basically the same as {@link AMD64FrameMap} except that space for rbp is also - * reserved when rbp is callee saved, not just if {@link #preserveFramePointer} is true. + * reserved when rbp is callee saved, not just if {@link #preserveFramePointer} is true, and + * that space for the frame pointer save area is reserved at the end of the overflow argument + * area if {@link #needsFramePointer} is true. + * + * 

+     *   Base       Contents
+     *
+     *            :                                :  -----
+     *   caller   | incoming overflow argument n   |    ^
+     *   frame    :     ...                        :    | positive
+     *            | incoming overflow argument 0   |    | offsets
+     *   ---------+--------------------------------+---------------------
+     *   current  | return address                 |    |            ^
+     *   frame    +--------------------------------+    |            |
+     *            | preserved rbp                  |    |            |
+     *            | iff preserveFramePointer       |    |            |
+     *            |     || rbp is callee saved     |    |            |
+     *            +--------------------------------+    |            |    -----
+     *            |                                |    |            |      |
+     *            : callee save area               :    |            |      |
+     *            |                                |    |            |      |
+     *            +--------------------------------+    |            |      |
+     *            | spill slot 0                   |    | negative   |      |
+     *            :     ...                        :    v offsets    |      |
+     *            | spill slot n                   |  -----        total  frame
+     *            +--------------------------------+               frame  size
+     *            | alignment padding              |               size     |
+     *            +--------------------------------+  -----          |      |
+     *            | frame pointer save area        |    ^            |      |
+     *            | iff needsFramePointer          |    |            |      |
+     *            +--------------------------------+    |            |      |
+     *            | outgoing overflow argument n   |    |            |      |
+     *            :     ...                        :    | positive   |      |
+     *            | outgoing overflow argument 0   |    | offsets    v      v
+     *    %sp-->  +--------------------------------+---------------------------
+     *
+     *
+ * + * The frame pointer save area actually serves two purposes: + *
    + *
  • If {@link #preserveFramePointer} is true, it is used for + * {@linkplain SubstrateAMD64FrameContext#maybeSetFramePointer frame chaining}. + *
  • If {@link #preserveFramePointer} is false, it is used as a spill slot in + * {@linkplain FramePointerPhase}. + *
*/ static class SubstrateAMD64FrameMap extends AMD64FrameMap { + /** + * If true, space for the {@linkplain #allocateFramePointerSaveArea frame pointer save area} + * is reserved, and the {@linkplain SubstrateAMD64FrameContext frame context} establishes + * rbp as the {@linkplain SubstrateAMD64FrameContext#maybeSetFramePointer frame pointer}. + */ + private boolean needsFramePointer; + + /** The offset at which the frame pointer save area is located. */ + private int framePointerSaveAreaOffset; + SubstrateAMD64FrameMap(CodeCacheProvider codeCache, SubstrateAMD64RegisterConfig registerConfig, ReferenceMapBuilderFactory referenceMapFactory, SharedMethod method) { super(codeCache, registerConfig, referenceMapFactory, registerConfig.shouldUseBasePointer()); if (!preserveFramePointer() && isCalleeSaved(rbp, registerConfig, method)) { @@ -1483,6 +1623,47 @@ static class SubstrateAMD64FrameMap extends AMD64FrameMap { spillSize += getTarget().wordSize; } } + + private boolean finalized; + + @Override + public void finish() { + finalized = true; + if (needsFramePointer) { + allocateFramePointerSaveArea(); + } + super.finish(); + } + + /** + * Reserves space for the frame pointer save area at the end of the overflow argument area. + *

+ * If {@link #preserveFramePointer} is true, it also includes an additional slot for the + * return address. + */ + private void allocateFramePointerSaveArea() { + int framePointerSaveAreaSize = getTarget().wordSize; + if (preserveFramePointer()) { + framePointerSaveAreaSize += returnAddressSize(); + } + framePointerSaveAreaOffset = NumUtil.roundUp(outgoingSize, framePointerSaveAreaSize); + reserveOutgoing(framePointerSaveAreaOffset + framePointerSaveAreaSize); + } + + void setNeedsFramePointer() { + assert !finalized; + needsFramePointer = true; + } + + boolean needsFramePointer() { + assert finalized; + return needsFramePointer; + } + + int getFramePointerSaveAreaOffset() { + assert needsFramePointer() : "no frame pointer save area"; + return framePointerSaveAreaOffset; + } } private static boolean isCalleeSaved(Register register, RegisterConfig config, SharedMethod method) { @@ -1680,4 +1861,44 @@ private void optimizeLongJumps(CompilationResultBuilder crb) { crb.emitLIR(); } } + + @Override + public RegisterAllocationConfig newRegisterAllocationConfig(RegisterConfig registerConfig, String[] allocationRestrictedTo, Object stub) { + RegisterConfig registerConfigNonNull = registerConfig == null ? getCodeCache().getRegisterConfig() : registerConfig; + return new SubstrateAMD64RegisterAllocationConfig(registerConfigNonNull, allocationRestrictedTo); + } + + static class SubstrateAMD64RegisterAllocationConfig extends RegisterAllocationConfig { + /** + * If true, rbp is removed from the set of allocatable registers. + */ + private boolean preserveFramePointer; + + SubstrateAMD64RegisterAllocationConfig(RegisterConfig registerConfig, String[] allocationRestrictedTo) { + super(registerConfig, allocationRestrictedTo); + } + + private boolean initialized; + + @Override + protected RegisterArray initAllocatable(RegisterArray registers) { + initialized = true; + if (preserveFramePointer) { + var allocatableRegisters = new ArrayList<>(registers.asList()); + allocatableRegisters.remove(rbp); + return super.initAllocatable(new RegisterArray(allocatableRegisters)); + } + return super.initAllocatable(registers); + } + + void setPreserveFramePointer() { + assert !initialized; + preserveFramePointer = true; + } + + boolean preserveFramePointer() { + assert initialized; + return preserveFramePointer; + } + } } diff --git a/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/VerifyFramePointerPhase.java b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/VerifyFramePointerPhase.java new file mode 100644 index 000000000000..e5f80a1f8091 --- /dev/null +++ b/substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/VerifyFramePointerPhase.java @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package com.oracle.svm.core.graal.amd64; + +import com.oracle.svm.core.util.VMError; + +import jdk.graal.compiler.core.common.cfg.BasicBlock; +import jdk.graal.compiler.lir.LIR; +import jdk.graal.compiler.lir.LIRInstruction; +import jdk.graal.compiler.lir.gen.LIRGenerationResult; +import jdk.graal.compiler.lir.phases.FinalCodeAnalysisPhase; +import jdk.vm.ci.code.TargetDescription; + +/** + * This phase verifies that no LIR instructions that {@linkplain LIRInstruction#modifiesStackPointer + * modify the stack pointer} were added after {@linkplain FramePointerPhase} to a LIR that did not + * have them before. + */ +class VerifyFramePointerPhase extends FinalCodeAnalysisPhase { + @Override + protected void run(TargetDescription target, LIRGenerationResult lirGenRes, FinalCodeAnalysisContext context) { + LIR lir = lirGenRes.getLIR(); + for (int blockId : lir.getBlocks()) { + if (LIR.isBlockDeleted(blockId)) { + continue; + } + BasicBlock block = lir.getBlockById(blockId); + for (LIRInstruction op : lir.getLIRforBlock(block)) { + if (op.modifiesStackPointer()) { + var registerAllocationConfig = (SubstrateAMD64Backend.SubstrateAMD64RegisterAllocationConfig) lirGenRes.getRegisterAllocationConfig(); + var frameMap = (SubstrateAMD64Backend.SubstrateAMD64FrameMap) lirGenRes.getFrameMap(); + if (registerAllocationConfig.preserveFramePointer() && frameMap.needsFramePointer()) { + return; /* Frame pointer is used, no need to check further. */ + } + throw VMError.shouldNotReachHere("The following instruction modifies the stack pointer, but was added after " + FramePointerPhase.class.getSimpleName() + ": " + op); + } + } + } + } +} diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java index 8dedba47bb19..0540984e64a6 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java @@ -227,7 +227,7 @@ public StructuredGraph getSnippet(ResolvedJavaMethod method, Object[] args, bool StructuredGraph result = new StructuredGraph.Builder(optionValues, debug) .method(method) .trackNodeSourcePosition(trackNodeSourcePosition) - .recordInlinedMethods(false) + .recordInlinedMethods(true) .setIsSubstitution(true) .build(); diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/replacements/SubstrateGraphKit.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/replacements/SubstrateGraphKit.java index c480c08aba53..53c2eed7d988 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/replacements/SubstrateGraphKit.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/replacements/SubstrateGraphKit.java @@ -109,10 +109,8 @@ public SubstrateGraphKit(DebugContext debug, ResolvedJavaMethod stubMethod, Prov frameState.initializeForMethodStart(null, true, graphBuilderPlugins, collectedArguments); initialArguments = Collections.unmodifiableList(collectedArguments); graph.start().setStateAfter(frameState.create(bci(), graph.start())); - } - - public SubstrateGraphKit(DebugContext debug, ResolvedJavaMethod stubMethod, Providers providers, GraphBuilderConfiguration.Plugins graphBuilderPlugins, CompilationIdentifier compilationId) { - this(debug, stubMethod, providers, graphBuilderPlugins, compilationId, false); + // Record method dependency in the graph + graph.recordMethod(graph.method()); } @Override diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/VarHandleSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/VarHandleSupport.java index e24d29f9c413..5b20036c3f0e 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/VarHandleSupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/VarHandleSupport.java @@ -30,7 +30,9 @@ import com.oracle.svm.core.annotate.Alias; import com.oracle.svm.core.annotate.RecomputeFieldValue; import com.oracle.svm.core.annotate.RecomputeFieldValue.Kind; +import com.oracle.svm.core.annotate.Substitute; import com.oracle.svm.core.annotate.TargetClass; +import com.oracle.svm.core.classinitialization.EnsureClassInitializedNode; import com.oracle.svm.core.fieldvaluetransformer.FieldValueTransformerWithAvailability; import com.oracle.svm.core.graal.nodes.FieldOffsetNode; import com.oracle.svm.core.util.VMError; @@ -369,3 +371,20 @@ final class Target_java_lang_invoke_DirectMethodHandle_StaticAccessor { @Alias @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Custom, declClass = VarHandleFieldOffsetAsLongComputer.class) // long staticOffset; } + +@TargetClass(className = "java.lang.invoke.LazyInitializingVarHandle", onlyWith = JDK23OrLater.class) +final class Target_java_lang_invoke_LazyInitializingVarHandle { + @Alias @RecomputeFieldValue(isFinal = true, kind = RecomputeFieldValue.Kind.None) // + Class refc; + + @Substitute + void ensureInitialized() { + /* + * Without JIT compilation, there is no point in speculating on a @Stable initialized flag. + * By emitting a EnsureClassInitializedNode, a VarHandle access to a static field is + * optimized like a direct access of a static field, e.g., the class initialization check is + * removed when the class initializer can be simulated. + */ + EnsureClassInitializedNode.ensureClassInitialized(refc); + } +} diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompiledMethodSupport.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompiledMethodSupport.java index 6994b98652d5..77b1c7221479 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompiledMethodSupport.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompiledMethodSupport.java @@ -220,7 +220,7 @@ private void compileRuntimeCompiledMethod(DebugContext debug) { boolean trackNodeSourcePosition = SubstrateOptions.IncludeNodeSourcePositions.getValue(); AnalysisMethod aMethod = method.getWrapped(); - StructuredGraph graph = aMethod.decodeAnalyzedGraph(debug, null, trackNodeSourcePosition, + StructuredGraph graph = aMethod.decodeAnalyzedGraph(debug, null, trackNodeSourcePosition, false, (arch, analyzedGraph) -> new RuntimeCompilationGraphDecoder(arch, analyzedGraph, compilationState.heapScanner)); if (graph == null) { throw VMError.shouldNotReachHere("Method not parsed during static analysis: " + aMethod.format("%r %H.%n(%p)")); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ameta/AnalysisConstantReflectionProvider.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ameta/AnalysisConstantReflectionProvider.java index 50445482c999..36a4e61e99dc 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ameta/AnalysisConstantReflectionProvider.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ameta/AnalysisConstantReflectionProvider.java @@ -42,6 +42,7 @@ import com.oracle.graal.pointsto.meta.AnalysisType; import com.oracle.graal.pointsto.meta.AnalysisUniverse; import com.oracle.graal.pointsto.util.AnalysisError; +import com.oracle.svm.core.classinitialization.TypeReachedProvider; import com.oracle.svm.core.hub.DynamicHub; import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.SVMHost; @@ -50,7 +51,6 @@ import com.oracle.svm.hosted.meta.RelocatableConstant; import jdk.graal.compiler.nodes.spi.IdentityHashCodeProvider; -import com.oracle.svm.core.classinitialization.TypeReachedProvider; import jdk.vm.ci.meta.Constant; import jdk.vm.ci.meta.ConstantReflectionProvider; import jdk.vm.ci.meta.JavaConstant; diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/SimulateClassInitializerGraphDecoder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/SimulateClassInitializerGraphDecoder.java index bac1bcdfaec7..95d4547bb2dc 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/SimulateClassInitializerGraphDecoder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/SimulateClassInitializerGraphDecoder.java @@ -746,7 +746,7 @@ protected static int asIntegerOrMinusOne(ValueNode node) { * sub-integer types are often just integer constants in the Graal IR, i.e., we cannot rely on * the JavaKind of the constant to match the type of the field or array. */ - private static JavaConstant adaptForImageHeap(JavaConstant value, JavaKind storageKind) { + static JavaConstant adaptForImageHeap(JavaConstant value, JavaKind storageKind) { if (value.getJavaKind() != storageKind) { assert value instanceof PrimitiveConstant && value.getJavaKind().getStackKind() == storageKind.getStackKind() : "only sub-int values can have a mismatch of the JavaKind: " + value.getJavaKind() + ", " + storageKind; diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/SimulateClassInitializerSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/SimulateClassInitializerSupport.java index 57b481c722d6..86c6dfe289b6 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/SimulateClassInitializerSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/SimulateClassInitializerSupport.java @@ -24,6 +24,8 @@ */ package com.oracle.svm.hosted.classinitialization; +import static com.oracle.svm.hosted.classinitialization.SimulateClassInitializerGraphDecoder.adaptForImageHeap; + import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.stream.Collectors; @@ -466,8 +468,8 @@ private StructuredGraph decodeGraph(SimulateClassInitializerClusterMember cluste var result = new StructuredGraph.Builder(bb.getOptions(), debug) .method(classInitializer) - .recordInlinedMethods(false) .trackNodeSourcePosition(analysisParsedGraph.getEncodedGraph().trackNodeSourcePosition()) + .recordInlinedMethods(analysisParsedGraph.getEncodedGraph().isRecordingInlinedMethods()) .build(); try (var scope = debug.scope("SimulateClassInitializerGraphDecoder", result)) { @@ -510,13 +512,14 @@ private static void processEffectsOfNode(SimulateClassInitializerClusterMember c return; } else if (node instanceof MarkStaticFinalFieldInitializedNode) { return; - } else if (node instanceof StoreFieldNode storeFieldNode) { var field = (AnalysisField) storeFieldNode.field(); if (field.isStatic() && field.getDeclaringClass().equals(clusterMember.type)) { var constantValue = storeFieldNode.value().asJavaConstant(); if (constantValue != null) { - clusterMember.staticFieldValues.put(field, constantValue); + // We use the java kind here and not the storage kind since that's what the + // users of (Analysis)ConstantReflectionProvider expect. + clusterMember.staticFieldValues.put(field, adaptForImageHeap(constantValue, field.getJavaKind())); return; } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompilationInfo.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompilationInfo.java index 9bbcda6f261e..ac004a8a9192 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompilationInfo.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompilationInfo.java @@ -108,17 +108,18 @@ public CompilationGraph getCompilationGraph() { @SuppressWarnings("try") public StructuredGraph createGraph(DebugContext debug, OptionValues options, CompilationIdentifier compilationId, boolean decode) { + var encodedGraph = getCompilationGraph().getEncodedGraph(); var graph = new StructuredGraph.Builder(options, debug) .method(method) - .recordInlinedMethods(false) - .trackNodeSourcePosition(getCompilationGraph().getEncodedGraph().trackNodeSourcePosition()) + .trackNodeSourcePosition(encodedGraph.trackNodeSourcePosition()) + .recordInlinedMethods(encodedGraph.isRecordingInlinedMethods()) .compilationId(compilationId) .build(); if (decode) { try (var s = debug.scope("CreateGraph", graph, method)) { var decoder = new GraphDecoder(AnalysisParsedGraph.HOST_ARCHITECTURE, graph); - decoder.decode(getCompilationGraph().getEncodedGraph()); + decoder.decode(encodedGraph); } catch (Throwable ex) { throw debug.handle(ex); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/HostedGraphKit.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/HostedGraphKit.java index 2ee25aa64573..848fc07ff16e 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/HostedGraphKit.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/HostedGraphKit.java @@ -29,6 +29,7 @@ import java.util.List; import com.oracle.graal.pointsto.meta.AnalysisMetaAccess; +import com.oracle.graal.pointsto.meta.AnalysisMethod; import com.oracle.graal.pointsto.meta.HostedProviders; import com.oracle.svm.core.c.BoxedRelocatedPointer; import com.oracle.svm.core.classinitialization.EnsureClassInitializedNode; @@ -36,7 +37,6 @@ import com.oracle.svm.core.graal.replacements.SubstrateGraphKit; import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.classinitialization.ClassInitializationSupport; -import com.oracle.svm.hosted.code.SubstrateCompilationDirectives; import jdk.graal.compiler.core.common.type.StampFactory; import jdk.graal.compiler.core.common.type.StampPair; @@ -65,7 +65,7 @@ public class HostedGraphKit extends SubstrateGraphKit { public HostedGraphKit(DebugContext debug, HostedProviders providers, ResolvedJavaMethod method) { super(debug, method, providers, providers.getGraphBuilderPlugins(), new SubstrateCompilationIdentifier(), - SubstrateCompilationDirectives.isRuntimeCompiledMethod(method)); + ((AnalysisMetaAccess) providers.getMetaAccess()).getUniverse().hostVM().recordInlinedMethods((AnalysisMethod) method)); } @Override diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SharedGraphBuilderPhase.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SharedGraphBuilderPhase.java index 0f992900384b..2a895d18b64b 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SharedGraphBuilderPhase.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SharedGraphBuilderPhase.java @@ -1153,7 +1153,12 @@ protected Object resolveLinkedObject(int bci, int cpi, int opcode, BootstrapMeth int argCpi = primitiveConstant.asInt(); Object argConstant = loadConstantDynamic(argCpi, opcode == Opcodes.INVOKEDYNAMIC ? Opcodes.LDC : opcode); if (argConstant instanceof ValueNode valueNode) { - currentNode = valueNode; + ResolvedJavaMethod.Parameter[] parameters = bootstrapMethod.getParameters(); + if (valueNode.getStackKind().isPrimitive() && i + 3 <= parameters.length && !parameters[i + 3].getKind().isPrimitive()) { + currentNode = append(BoxNode.create(valueNode, getMetaAccess().lookupJavaType(valueNode.getStackKind().toBoxedJavaClass()), valueNode.getStackKind())); + } else { + currentNode = valueNode; + } } else if (argConstant instanceof Throwable || argConstant instanceof UnresolvedJavaType) { /* A nested constant dynamic threw. */ return argConstant; diff --git a/vm/mx.vm/suite.py b/vm/mx.vm/suite.py index de05f74a1fac..0a40f118a8de 100644 --- a/vm/mx.vm/suite.py +++ b/vm/mx.vm/suite.py @@ -65,7 +65,7 @@ }, { "name": "graalpython", - "version": "1417114cd981679edbfd64ee9165e4d0d1a10212", + "version": "459788314eea1a8ed92017485de7c646795d55dd", "dynamic": True, "urls": [ {"url": "https://github.com/graalvm/graalpython.git", "kind": "git"},