From 57c3bb6091f8ba0caced6f5ecf21dc998ffeee9f Mon Sep 17 00:00:00 2001 From: Roland Westrelin Date: Wed, 6 Nov 2024 14:47:14 +0000 Subject: [PATCH 01/22] 8343068: C2: CastX2P Ideal transformation not always applied Reviewed-by: kvn, thartmann --- src/hotspot/share/opto/phaseX.cpp | 8 ++ .../c2/TestCastX2NotProcessedIGVN.java | 80 +++++++++++++++++++ .../compiler/lib/ir_framework/IRNode.java | 5 ++ 3 files changed, 93 insertions(+) create mode 100644 test/hotspot/jtreg/compiler/c2/TestCastX2NotProcessedIGVN.java diff --git a/src/hotspot/share/opto/phaseX.cpp b/src/hotspot/share/opto/phaseX.cpp index dae4a5c68a36f..f766146a894fc 100644 --- a/src/hotspot/share/opto/phaseX.cpp +++ b/src/hotspot/share/opto/phaseX.cpp @@ -1695,6 +1695,14 @@ void PhaseIterGVN::add_users_of_use_to_worklist(Node* n, Node* use, Unique_Node_ worklist.push(cmp); } } + if (use->Opcode() == Op_AddX) { + for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { + Node* u = use->fast_out(i2); + if (u->Opcode() == Op_CastX2P) { + worklist.push(u); + } + } + } } /** diff --git a/test/hotspot/jtreg/compiler/c2/TestCastX2NotProcessedIGVN.java b/test/hotspot/jtreg/compiler/c2/TestCastX2NotProcessedIGVN.java new file mode 100644 index 0000000000000..6ab45af39c5a1 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/TestCastX2NotProcessedIGVN.java @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2024, Red Hat, Inc. 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. + * + * 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 compiler.c2; + +import compiler.lib.ir_framework.*; +import jdk.internal.misc.Unsafe; + +/* + * @test + * @bug 8343068 + * @summary C2: CastX2P Ideal transformation not always applied + * @modules java.base/jdk.internal.misc + * @library /test/lib / + * @run driver compiler.c2.TestCastX2NotProcessedIGVN + */ + +public class TestCastX2NotProcessedIGVN { + private static final Unsafe UNSAFE = Unsafe.getUnsafe(); + static int size = 1024; + static long base = UNSAFE.allocateMemory(4 * size); + + + public static void main(String[] args) { + TestFramework.runWithFlags("--add-modules", "java.base", "--add-exports", "java.base/jdk.internal.misc=ALL-UNNAMED"); + } + + @Test + @IR(failOn = IRNode.ADD_L, counts = {IRNode.ADD_P, "1"}) + public static byte test1(long base) { + int offset = 0; + do { + offset++; + } while (offset < 100); + long longOffset = ((long) offset) * 2; + return UNSAFE.getByte(null, base + longOffset); + } + + @Run(test = "test1") + public static void test1Runner() { + test1(base); + } + + @Test + @IR(counts = {IRNode.LOAD_VECTOR_I, "> 1"}) + public static int test2(int stop, int[] array) { + int v = 0; + stop = Math.min(stop, Integer.MAX_VALUE / 4); + for (int i = 0; i < stop; i++) { + long offset = ((long)i) * 4; + array[i] = UNSAFE.getInt(null, offset + base); + } + return v; + } + + @Run(test = "test2") + public static void test2Runner() { + test2(size, new int[size]); + } +} diff --git a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java index 8c4b3c93343f6..fc55662d8013f 100644 --- a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java +++ b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java @@ -219,6 +219,11 @@ public class IRNode { beforeMatchingNameRegex(ADD_L, "AddL"); } + public static final String ADD_P = PREFIX + "ADD_P" + POSTFIX; + static { + beforeMatchingNameRegex(ADD_P, "AddP"); + } + public static final String ADD_VD = VECTOR_PREFIX + "ADD_VD" + POSTFIX; static { vectorNode(ADD_VD, "AddVD", TYPE_DOUBLE); From 72a45ddbad9c343200197348ccfcf74105e6fefa Mon Sep 17 00:00:00 2001 From: Roland Westrelin Date: Wed, 6 Nov 2024 14:49:30 +0000 Subject: [PATCH 02/22] 8341834: C2 compilation fails with "bad AD file" due to Replicate Reviewed-by: kvn, epeter --- .../share/opto/superwordVTransformBuilder.cpp | 8 +++- .../vectorization/TestReplicateAtConv.java | 48 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 test/hotspot/jtreg/compiler/vectorization/TestReplicateAtConv.java diff --git a/src/hotspot/share/opto/superwordVTransformBuilder.cpp b/src/hotspot/share/opto/superwordVTransformBuilder.cpp index 6c2d3a3be35c3..2e32ce28d3ccb 100644 --- a/src/hotspot/share/opto/superwordVTransformBuilder.cpp +++ b/src/hotspot/share/opto/superwordVTransformBuilder.cpp @@ -228,7 +228,13 @@ VTransformNode* SuperWordVTransformBuilder::get_or_make_vtnode_vector_input_at_i return shift_count; } else { // Replicate the scalar same_input to every vector element. - BasicType element_type = _vloop_analyzer.types().velt_basic_type(p0); + // In some rare case, p0 is Convert node such as a ConvL2I: all + // ConvL2I nodes in the pack only differ in their types. + // velt_basic_type(p0) is the output type of the pack. In the + // case of a ConvL2I, it can be int or some narrower type such + // as short etc. But given we replicate the input of the Convert + // node, we have to use the input type instead. + BasicType element_type = p0->is_Convert() ? p0->in(1)->bottom_type()->basic_type() : _vloop_analyzer.types().velt_basic_type(p0); if (index == 2 && VectorNode::is_scalar_rotate(p0) && element_type == T_LONG) { // Scalar rotate has int rotation value, but the scalar rotate expects longs. assert(same_input->bottom_type()->isa_int(), "scalar rotate expects int rotation"); diff --git a/test/hotspot/jtreg/compiler/vectorization/TestReplicateAtConv.java b/test/hotspot/jtreg/compiler/vectorization/TestReplicateAtConv.java new file mode 100644 index 0000000000000..e77b474bf66fb --- /dev/null +++ b/test/hotspot/jtreg/compiler/vectorization/TestReplicateAtConv.java @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2024, Red Hat, Inc. 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. + * + * 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. + */ + +/** + * @test + * @bug 8341834 + * @summary C2 compilation fails with "bad AD file" due to Replicate + * @run main/othervm -XX:CompileCommand=compileonly,TestReplicateAtConv::test -Xcomp TestReplicateAtConv + */ + +public class TestReplicateAtConv { + public static long val = 0; + + public static void test() { + int array[] = new int[500]; + + for (int i = 0; i < 100; i++) { + for (long l = 100; l > i; l--) { + val = 42 + (l + i); + array[(int)l] = (int)l - (int)val; + } + } + } + + public static void main(String[] args) { + test(); + } +} From 6811a11e278118b8b2781f1eaf45d363a3d2db49 Mon Sep 17 00:00:00 2001 From: Aggelos Biboudis Date: Wed, 6 Nov 2024 14:50:54 +0000 Subject: [PATCH 03/22] 8341408: Implement JEP 488: Primitive Types in Patterns, instanceof, and switch (Second Preview) Reviewed-by: vromero, jlahoda --- .../sun/tools/javac/comp/TransPatterns.java | 22 +++- .../com/sun/tools/javac/comp/TransTypes.java | 14 ++- .../com/sun/tools/javac/tree/JCTree.java | 2 + ...PrimitiveTypesInTestingContextErasure.java | 103 ++++++++++++++++++ 4 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 test/langtools/tools/javac/patterns/PrimitiveTypesInTestingContextErasure.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java index 0e3a2c0b1db82..93c4eaa03ee01 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java @@ -199,7 +199,27 @@ protected TransPatterns(Context context) { @Override public void visitTypeTest(JCInstanceOf tree) { - if (tree.pattern instanceof JCPattern pattern) { + // Translates regular instanceof type operation to instanceof pattern operator when + // the expression was originally T but was subsequently erased to Object. + // + // $expr instanceof $primitiveType + // => + // $expr instanceof T $temp && $temp instanceof $primitiveType + if (tree.erasedExprOriginalType!=null && !types.isSameType(tree.expr.type, tree.erasedExprOriginalType)) { + BindingSymbol temp = new BindingSymbol(Flags.FINAL | Flags.SYNTHETIC, + names.fromString("temp" + variableIndex++ + target.syntheticNameChar()), + tree.erasedExprOriginalType, + currentMethodSym); + + JCVariableDecl tempDecl = make.at(tree.pos()).VarDef(temp, null); + + JCTree resultExpr = + makeBinary(Tag.AND, + make.TypeTest(tree.expr, make.BindingPattern(tempDecl).setType(tree.erasedExprOriginalType)).setType(syms.booleanType), + make.TypeTest(make.Ident(tempDecl), tree.pattern).setType(syms.booleanType)); + + result = translate(resultExpr); + } else if (tree.pattern instanceof JCPattern pattern) { //first, resolve any record patterns: JCExpression extraConditions = null; if (pattern instanceof JCRecordPattern recordPattern) { diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java index d232f2e6d9f3d..a0b1ef27255f2 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java @@ -813,8 +813,7 @@ public void visitSwitch(JCSwitch tree) { Type selsuper = types.supertype(tree.selector.type); boolean enumSwitch = selsuper != null && selsuper.tsym == syms.enumSym; - Type target = enumSwitch ? erasure(tree.selector.type) : syms.intType; - tree.selector = translate(tree.selector, target); + tree.selector = translate(tree.selector, erasure(tree.selector.type)); tree.cases = translateCases(tree.cases); result = tree; } @@ -852,8 +851,7 @@ public void visitSwitchExpression(JCSwitchExpression tree) { Type selsuper = types.supertype(tree.selector.type); boolean enumSwitch = selsuper != null && selsuper.tsym == syms.enumSym; - Type target = enumSwitch ? erasure(tree.selector.type) : syms.intType; - tree.selector = translate(tree.selector, target); + tree.selector = translate(tree.selector, erasure(tree.selector.type)); tree.cases = translate(tree.cases, tree.type); tree.type = erasure(tree.type); result = retype(tree, tree.type, pt); @@ -1067,8 +1065,14 @@ public void visitTypeCast(JCTypeCast tree) { } public void visitTypeTest(JCInstanceOf tree) { - tree.expr = translate(tree.expr, null); tree.pattern = translate(tree.pattern, null); + if (tree.pattern.type.isPrimitive()) { + tree.erasedExprOriginalType = erasure(tree.expr.type); + tree.expr = translate(tree.expr, null); + } + else { + tree.expr = translate(tree.expr, null); + } result = tree; } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java index 6041da6723aa4..003f70eeecc2e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java @@ -2272,6 +2272,8 @@ public static class JCInstanceOf extends JCExpression implements InstanceOfTree /**{@code true} if this instanceof test should have * value {@code true} when the {@code expr} is {@code null}.*/ public boolean allowNull; + public Type erasedExprOriginalType; + protected JCInstanceOf(JCExpression expr, JCTree pattern) { this.expr = expr; this.pattern = pattern; diff --git a/test/langtools/tools/javac/patterns/PrimitiveTypesInTestingContextErasure.java b/test/langtools/tools/javac/patterns/PrimitiveTypesInTestingContextErasure.java new file mode 100644 index 0000000000000..40289f31d5620 --- /dev/null +++ b/test/langtools/tools/javac/patterns/PrimitiveTypesInTestingContextErasure.java @@ -0,0 +1,103 @@ +/* + * 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * 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. + */ +/* + * @test + * @bug 8341408 + * @summary Compiler Implementation for Primitive types in patterns, instanceof, and switch (Second Preview) + * @enablePreview + */ + +import java.util.List; + +public class PrimitiveTypesInTestingContextErasure { + public static void main(String[] args) { + erasureSwitch(); + erasureInstanceofTypeComparisonOperator(); + erasureInstanceofPatternMatchingOperator(); + + pollutedInstanceofPatternMatchingOperatorReference(); + pollutedInstanceofPatternMatchingOperator(); + pollutedInstanceofTypeComparisonOperator(); + pollutedSwitch(); + } + + public static void erasureSwitch() { + List ls = List.of((short) 42); + Short s = 42; + + assertTrue(switch(ls.get(0)) { + case int _ -> true; // Short to int + default -> false; + }); + } + + public static void erasureInstanceofTypeComparisonOperator() { + List ls = List.of((short) 42); + + assertTrue(ls.get(0) instanceof int); // Short to int + } + + public static void erasureInstanceofPatternMatchingOperator() { + List ls = List.of((short) 42); + + assertTrue(ls.get(0) instanceof int i); // Short to int + } + + public static void pollutedInstanceofPatternMatchingOperator() { + List ls = (List) List.of("42"); + + assertTrue(!(ls.get(0) instanceof int i)); + } + + public static void pollutedInstanceofTypeComparisonOperator() { + List ls = (List) List.of("42"); + + assertTrue(!(ls.get(0) instanceof int)); + } + + public static void pollutedInstanceofPatternMatchingOperatorReference() { + List ls = (List) List.of("42"); + + assertTrue(!(ls.get(0) instanceof Short)); + } + + public static void pollutedSwitch() { + List ls = (List) List.of("42"); + + try { + var res = switch(ls.get(0)) { + case int _ -> true; + default -> false; + }; + throw new AssertionError("Expected: ClassCastException"); + } catch (ClassCastException e) { + ; + } + } + + static void assertTrue(boolean actual) { + if (!actual) { + throw new AssertionError("Expected: true, but got false"); + } + } +} From 0be7118b2f761b416ebf8cbb11473d51e80be409 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Wed, 6 Nov 2024 16:38:55 +0000 Subject: [PATCH 04/22] 8279016: JFR Leak Profiler is broken with Shenandoah Reviewed-by: egahlin, rkennke, mgronlun, wkemper --- make/RunTests.gmk | 4 ++ .../share/jfr/leakprofiler/leakProfiler.cpp | 15 +++++ .../share/jfr/leakprofiler/leakProfiler.hpp | 1 + test/jdk/ProblemList-shenandoah.txt | 60 +++++++++++++++++++ 4 files changed, 80 insertions(+) create mode 100644 test/jdk/ProblemList-shenandoah.txt diff --git a/make/RunTests.gmk b/make/RunTests.gmk index 2d6392d783bd2..747d8f84dbc97 100644 --- a/make/RunTests.gmk +++ b/make/RunTests.gmk @@ -856,6 +856,10 @@ define SetupRunJtregTestBody JTREG_AUTO_PROBLEM_LISTS += ProblemList-zgc.txt endif + ifneq ($$(findstring -XX:+UseShenandoahGC, $$(JTREG_ALL_OPTIONS)), ) + JTREG_AUTO_PROBLEM_LISTS += ProblemList-shenandoah.txt + endif + ifneq ($$(JTREG_EXTRA_PROBLEM_LISTS), ) # Accept both absolute paths as well as relative to the current test root. $1_JTREG_BASIC_OPTIONS += $$(addprefix $$(JTREG_PROBLEM_LIST_PREFIX), $$(wildcard \ diff --git a/src/hotspot/share/jfr/leakprofiler/leakProfiler.cpp b/src/hotspot/share/jfr/leakprofiler/leakProfiler.cpp index 895eafc44099e..cf69dd56934ea 100644 --- a/src/hotspot/share/jfr/leakprofiler/leakProfiler.cpp +++ b/src/hotspot/share/jfr/leakprofiler/leakProfiler.cpp @@ -34,6 +34,15 @@ #include "runtime/javaThread.inline.hpp" #include "runtime/vmThread.hpp" +bool LeakProfiler::is_supported() { + if (UseShenandoahGC) { + // Leak Profiler uses mark words in the ways that might interfere + // with concurrent GC uses of them. This affects Shenandoah. + return false; + } + return true; +} + bool LeakProfiler::is_running() { return ObjectSampler::is_created(); } @@ -48,6 +57,12 @@ bool LeakProfiler::start(int sample_count) { return false; } + // Exit cleanly if not supported + if (!is_supported()) { + log_trace(jfr, system)("Object sampling is not supported"); + return false; + } + assert(!is_running(), "invariant"); assert(sample_count > 0, "invariant"); diff --git a/src/hotspot/share/jfr/leakprofiler/leakProfiler.hpp b/src/hotspot/share/jfr/leakprofiler/leakProfiler.hpp index 6290a10ff748b..08544f439488b 100644 --- a/src/hotspot/share/jfr/leakprofiler/leakProfiler.hpp +++ b/src/hotspot/share/jfr/leakprofiler/leakProfiler.hpp @@ -35,6 +35,7 @@ class LeakProfiler : public AllStatic { static bool start(int sample_count); static bool stop(); static bool is_running(); + static bool is_supported(); static void emit_events(int64_t cutoff_ticks, bool emit_all, bool skip_bfs); static void sample(HeapWord* object, size_t size, JavaThread* thread); diff --git a/test/jdk/ProblemList-shenandoah.txt b/test/jdk/ProblemList-shenandoah.txt new file mode 100644 index 0000000000000..063795d69e84f --- /dev/null +++ b/test/jdk/ProblemList-shenandoah.txt @@ -0,0 +1,60 @@ +# +# Copyright Amazon.com Inc. 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. +# +# 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. +# + +############################################################################# +# +# List of quarantined tests for testing with Shenandoah. +# +############################################################################# + +# Quiet all LeakProfiler tests + +jdk/jfr/api/consumer/TestRecordingFileWrite.java 8342951 generic-all +jdk/jfr/event/oldobject/TestAllocationTime.java 8342951 generic-all +jdk/jfr/event/oldobject/TestArrayInformation.java 8342951 generic-all +jdk/jfr/event/oldobject/TestCircularReference.java 8342951 generic-all +jdk/jfr/event/oldobject/TestClassLoaderLeak.java 8342951 generic-all +jdk/jfr/event/oldobject/TestFieldInformation.java 8342951 generic-all +jdk/jfr/event/oldobject/TestG1.java 8342951 generic-all +jdk/jfr/event/oldobject/TestHeapDeep.java 8342951 generic-all +jdk/jfr/event/oldobject/TestHeapShallow.java 8342951 generic-all +jdk/jfr/event/oldobject/TestLargeRootSet.java 8342951 generic-all +jdk/jfr/event/oldobject/TestLastKnownHeapUsage.java 8342951 generic-all +jdk/jfr/event/oldobject/TestListenerLeak.java 8342951 generic-all +jdk/jfr/event/oldobject/TestMetadataRetention.java 8342951 generic-all +jdk/jfr/event/oldobject/TestObjectAge.java 8342951 generic-all +jdk/jfr/event/oldobject/TestObjectDescription.java 8342951 generic-all +jdk/jfr/event/oldobject/TestObjectSize.java 8342951 generic-all +jdk/jfr/event/oldobject/TestParallel.java 8342951 generic-all +jdk/jfr/event/oldobject/TestReferenceChainLimit.java 8342951 generic-all +jdk/jfr/event/oldobject/TestSanityDefault.java 8342951 generic-all +jdk/jfr/event/oldobject/TestSerial.java 8342951 generic-all +jdk/jfr/event/oldobject/TestShenandoah.java 8342951 generic-all +jdk/jfr/event/oldobject/TestThreadLocalLeak.java 8342951 generic-all +jdk/jfr/event/oldobject/TestZ.java 8342951 generic-all +jdk/jfr/jcmd/TestJcmdDump.java 8342951 generic-all +jdk/jfr/jcmd/TestJcmdDumpPathToGCRoots.java 8342951 generic-all +jdk/jfr/jcmd/TestJcmdStartPathToGCRoots.java 8342951 generic-all +jdk/jfr/jvm/TestWaste.java 8342951 generic-all +jdk/jfr/startupargs/TestOldObjectQueueSize.java 8342951 generic-all + From c0e6c3b93c0d21debc538e0135805c2957053108 Mon Sep 17 00:00:00 2001 From: Srinivas Vamsi Parasa Date: Wed, 6 Nov 2024 16:41:41 +0000 Subject: [PATCH 05/22] 8343214: Fix encoding errors in APX New Data Destination Instructions Support Reviewed-by: jbhateja, sviswanathan --- src/hotspot/cpu/x86/assembler_x86.cpp | 115 ++++++++++++++++---------- 1 file changed, 71 insertions(+), 44 deletions(-) diff --git a/src/hotspot/cpu/x86/assembler_x86.cpp b/src/hotspot/cpu/x86/assembler_x86.cpp index e4ab99bf1c36f..03ce06726c6e9 100644 --- a/src/hotspot/cpu/x86/assembler_x86.cpp +++ b/src/hotspot/cpu/x86/assembler_x86.cpp @@ -1488,9 +1488,10 @@ void Assembler::addl(Register dst, Register src) { void Assembler::eaddl(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x01, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void)evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x03, 0xC0, src1, src2); } void Assembler::addr_nop_4() { @@ -1728,9 +1729,10 @@ void Assembler::andl(Register dst, Register src) { void Assembler::eandl(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x21, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x23, 0xC0, src1, src2); } void Assembler::andnl(Register dst, Register src1, Register src2) { @@ -2637,7 +2639,7 @@ void Assembler::eimull(Register dst, Address src, int32_t value, bool no_flags) InstructionMark im(this); InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); attributes.set_address_attributes(/* tuple_type */ EVEX_NOSCALE, /* input_size_in_bits */ EVEX_32bit); - evex_prefix_ndd(src, 0, dst->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); + evex_prefix_nf(src, 0, dst->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); if (is8bit(value)) { emit_int8((unsigned char)0x6B); emit_operand(dst, src, 1); @@ -4467,7 +4469,9 @@ void Assembler::enotl(Register dst, Register src) { void Assembler::eorw(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_66, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); emit_arith(0x0B, 0xC0, src1, src2); } @@ -4519,9 +4523,10 @@ void Assembler::orl(Register dst, Register src) { void Assembler::eorl(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x09, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x0B, 0xC0, src1, src2); } void Assembler::orl(Address dst, Register src) { @@ -6938,7 +6943,9 @@ void Assembler::shldl(Register dst, Register src) { void Assembler::eshldl(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - int encode = evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + int encode = evex_prefix_and_encode_ndd(src2->encoding(), dst->encoding(), src1->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); emit_int16(0xA5, (0xC0 | encode)); } @@ -6949,7 +6956,9 @@ void Assembler::shldl(Register dst, Register src, int8_t imm8) { void Assembler::eshldl(Register dst, Register src1, Register src2, int8_t imm8, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - int encode = evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + int encode = evex_prefix_and_encode_ndd(src2->encoding(), dst->encoding(), src1->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); emit_int24(0x24, (0xC0 | encode), imm8); } @@ -6960,7 +6969,9 @@ void Assembler::shrdl(Register dst, Register src) { void Assembler::eshrdl(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - int encode = evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + int encode = evex_prefix_and_encode_ndd(src2->encoding(), dst->encoding(), src1->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); emit_int16(0xAD, (0xC0 | encode)); } @@ -6971,7 +6982,9 @@ void Assembler::shrdl(Register dst, Register src, int8_t imm8) { void Assembler::eshrdl(Register dst, Register src1, Register src2, int8_t imm8, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - int encode = evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + int encode = evex_prefix_and_encode_ndd(src2->encoding(), dst->encoding(), src1->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); emit_int24(0x2C, (0xC0 | encode), imm8); } @@ -6983,7 +6996,9 @@ void Assembler::shldq(Register dst, Register src, int8_t imm8) { void Assembler::eshldq(Register dst, Register src1, Register src2, int8_t imm8, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - int encode = evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + int encode = evex_prefix_and_encode_ndd(src2->encoding(), dst->encoding(), src1->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); emit_int24(0x24, (0xC0 | encode), imm8); } @@ -6994,7 +7009,9 @@ void Assembler::shrdq(Register dst, Register src, int8_t imm8) { void Assembler::eshrdq(Register dst, Register src1, Register src2, int8_t imm8, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - int encode = evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + int encode = evex_prefix_and_encode_ndd(src2->encoding(), dst->encoding(), src1->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); emit_int24(0x2C, (0xC0 | encode), imm8); } #endif @@ -7155,11 +7172,12 @@ void Assembler::subl(Register dst, Register src) { emit_arith(0x2B, 0xC0, dst, src); } -void Assembler::esubl(Register dst, Register src2, Register src1, bool no_flags) { +void Assembler::esubl(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x29, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x2B, 0xC0, src1, src2); } void Assembler::subsd(XMMRegister dst, XMMRegister src) { @@ -7485,9 +7503,10 @@ void Assembler::xorl(Register dst, Register src) { void Assembler::exorl(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x31, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x33, 0xC0, src1, src2); } void Assembler::xorl(Address dst, Register src) { @@ -7548,10 +7567,11 @@ void Assembler::xorw(Register dst, Address src) { void Assembler::exorw(Register dst, Register src1, Address src2, bool no_flags) { InstructionMark im(this); - emit_int8(0x66); InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - attributes.set_address_attributes(/* tuple_type */ EVEX_NOSCALE, /* input_size_in_bits */ EVEX_64bit); - evex_prefix_ndd(src2, dst->encoding(), src1->encoding(), VEX_SIMD_66, VEX_OPCODE_0F_3C, &attributes, no_flags); + attributes.set_address_attributes(/* tuple_type */ EVEX_NOSCALE, /* input_size_in_bits */ EVEX_16bit); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + evex_prefix_ndd(src2, dst->encoding(), src1->encoding(), VEX_SIMD_66, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); emit_int8(0x33); emit_operand(src1, src2, 0); } @@ -15064,9 +15084,10 @@ void Assembler::addq(Register dst, Register src) { void Assembler::eaddq(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x01, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x03, 0xC0, src1, src2); } void Assembler::adcxq(Register dst, Register src) { @@ -15160,9 +15181,10 @@ void Assembler::andq(Register dst, Register src) { void Assembler::eandq(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x21, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x23, 0xC0, src1, src2); } void Assembler::andq(Address dst, Register src) { @@ -15591,7 +15613,7 @@ void Assembler::imulq(Register dst, Register src, int value) { void Assembler::eimulq(Register dst, Register src, int value, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - int encode = vex_prefix_and_encode(dst->encoding(), 0, src->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, /* src_is_gpr */ true, /* nds_is_ndd */ false, no_flags); + int encode = evex_prefix_and_encode_nf(dst->encoding(), 0, src->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); if (is8bit(value)) { emit_int24(0x6B, (0xC0 | encode), (value & 0xFF)); } else { @@ -15610,7 +15632,9 @@ void Assembler::imulq(Register dst, Address src) { void Assembler::eimulq(Register dst, Address src, bool no_flags) { InstructionMark im(this); InstructionAttr attributes(AVX_128bit, /* vex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - vex_prefix(src, 0, dst->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F, &attributes, /* nds_is_ndd */ false, no_flags); + attributes.set_address_attributes(/* tuple_type */ EVEX_NOSCALE, /* input_size_in_bits */ EVEX_64bit); + evex_prefix_nf(src, 0, dst->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_int8((unsigned char)0xAF); emit_operand(dst, src, 0); } @@ -16055,9 +16079,10 @@ void Assembler::orq(Register dst, Register src) { void Assembler::eorq(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x09, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x0B, 0xC0, src1, src2); } void Assembler::popcntq(Register dst, Address src) { @@ -16743,9 +16768,10 @@ void Assembler::subq(Register dst, Register src) { void Assembler::esubq(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x29, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x2B, 0xC0, src1, src2); } void Assembler::testq(Address dst, int32_t imm32) { @@ -16807,9 +16833,10 @@ void Assembler::xorq(Register dst, Register src) { void Assembler::exorq(Register dst, Register src1, Register src2, bool no_flags) { InstructionAttr attributes(AVX_128bit, /* vex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false); - (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags); - // opcode matches gcc - emit_arith(0x31, 0xC0, src1, src2); + // NDD shares its encoding bits with NDS bits for regular EVEX instruction. + // Therefore, DST is passed as the second argument to minimize changes in the leaf level routine. + (void) evex_prefix_and_encode_ndd(src1->encoding(), dst->encoding(), src2->encoding(), VEX_SIMD_NONE, /* MAP4 */VEX_OPCODE_0F_3C, &attributes, no_flags); + emit_arith(0x33, 0xC0, src1, src2); } void Assembler::xorq(Register dst, Address src) { From 78b378ad03d0f6c85468ac208e84fabea79fc7de Mon Sep 17 00:00:00 2001 From: Andrew Haley Date: Wed, 6 Nov 2024 17:52:07 +0000 Subject: [PATCH 06/22] 8342540: InterfaceCalls micro-benchmark gives misleading results Reviewed-by: shade, kvn --- .../bench/vm/compiler/InterfaceCalls.java | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/test/micro/org/openjdk/bench/vm/compiler/InterfaceCalls.java b/test/micro/org/openjdk/bench/vm/compiler/InterfaceCalls.java index 30ee9dc53b7b7..e98a3b37f5516 100644 --- a/test/micro/org/openjdk/bench/vm/compiler/InterfaceCalls.java +++ b/test/micro/org/openjdk/bench/vm/compiler/InterfaceCalls.java @@ -29,6 +29,7 @@ import org.openjdk.jmh.annotations.Measurement; import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; import org.openjdk.jmh.annotations.Scope; import org.openjdk.jmh.annotations.Setup; import org.openjdk.jmh.annotations.State; @@ -45,6 +46,11 @@ @Fork(value = 3) public class InterfaceCalls { + // Whether to step iteratively through the list of interfaces, or + // to select one in an unpredictable way. + @Param({"false", "true"}) + private boolean randomized; + interface FirstInterface { public int getIntFirst(); } @@ -241,44 +247,54 @@ public int testIfaceExtCall(Blackhole bh) { */ @Benchmark public int test1stInt2Types() { - FirstInterface ai = as[l]; - l = 1 - l; + FirstInterface ai = as[step(2)]; return ai.getIntFirst(); } @Benchmark public int test1stInt3Types() { - FirstInterface ai = as[l]; - l = ++ l % 3; + FirstInterface ai = as[step(3)]; return ai.getIntFirst(); } @Benchmark public int test1stInt5Types() { - FirstInterface ai = as[l]; - l = ++ l % asLength; + FirstInterface ai = as[step(asLength)]; return ai.getIntFirst(); } @Benchmark public int test2ndInt2Types() { - SecondInterface ai = (SecondInterface) as[l]; - l = 1 - l; + SecondInterface ai = (SecondInterface) as[step(2)]; return ai.getIntSecond(); } @Benchmark public int test2ndInt3Types() { - SecondInterface ai = (SecondInterface) as[l]; - l = ++ l % 3; + SecondInterface ai = (SecondInterface) as[step(3)]; return ai.getIntSecond(); } @Benchmark public int test2ndInt5Types() { - SecondInterface ai = (SecondInterface) as[l]; - l = ++ l % asLength; + SecondInterface ai = (SecondInterface) as[step(asLength)]; return ai.getIntSecond(); } + int step(int range) { + if (randomized) { + l = scramble(l); + } else { + l++; + } + return (l & Integer.MAX_VALUE) % range; + } + + static final int scramble(int n) { + int x = n; + x ^= x << 13; + x ^= x >>> 17; + x ^= x << 5; + return x == 0 ? 1 : x; + } } From 342fe42555a0e892d21d187287ab996be199abb1 Mon Sep 17 00:00:00 2001 From: Fernando Guallini Date: Wed, 6 Nov 2024 18:36:05 +0000 Subject: [PATCH 07/22] 8342270: Test sun/security/pkcs11/Provider/RequiredMechCheck.java needs write access to src tree Reviewed-by: rhalade, erikj --- test/jdk/sun/security/pkcs11/PKCS11Test.java | 34 ++++++++++++++++--- .../pkcs11/Provider/MultipleLogins.sh | 3 +- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/test/jdk/sun/security/pkcs11/PKCS11Test.java b/test/jdk/sun/security/pkcs11/PKCS11Test.java index 9cb29eb8507a1..63d486756e684 100644 --- a/test/jdk/sun/security/pkcs11/PKCS11Test.java +++ b/test/jdk/sun/security/pkcs11/PKCS11Test.java @@ -54,12 +54,14 @@ import java.util.ServiceConfigurationError; import java.util.ServiceLoader; import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; import jdk.test.lib.Platform; +import jdk.test.lib.Utils; import jdk.test.lib.artifacts.Artifact; import jdk.test.lib.artifacts.ArtifactResolver; import jdk.test.lib.artifacts.ArtifactResolverException; @@ -496,14 +498,13 @@ public static String getNssConfig() throws Exception { return null; } - String base = getBase(); - + String nssConfigDir = copyNssFiles(); String libfile = libdir + System.mapLibraryName(nss_library); String customDBdir = System.getProperty("CUSTOM_DB_DIR"); String dbdir = (customDBdir != null) ? customDBdir : - base + SEP + "nss" + SEP + "db"; + nssConfigDir + SEP + "db"; // NSS always wants forward slashes for the config path dbdir = dbdir.replace('\\', '/'); @@ -513,7 +514,7 @@ public static String getNssConfig() throws Exception { System.setProperty("pkcs11test.nss.db", dbdir); return (customConfig != null) ? customConfig : - base + SEP + "nss" + SEP + customConfigName; + nssConfigDir + SEP + customConfigName; } // Generate a vector of supported elliptic curves of a given provider @@ -796,6 +797,31 @@ private static Path findNSSLibrary(Path path, Path libraryName) throws IOExcepti } } + //Copy the nss config files to the current directory for tests. Returns the destination path + private static String copyNssFiles() throws Exception { + String nss = "nss"; + String db = "db"; + Path nssDirSource = Path.of(getBase()).resolve(nss); + Path nssDirDestination = Path.of(".").resolve(nss); + + // copy files from nss directory + copyFiles(nssDirSource, nssDirDestination); + // copy files from nss/db directory + copyFiles(nssDirSource.resolve(db), nssDirDestination.resolve(db)); + return nssDirDestination.toString(); + } + + private static void copyFiles(Path dirSource, Path dirDestination) throws IOException { + List sourceFiles = Arrays + .stream(dirSource.toFile().listFiles()) + .filter(File::isFile) + .map(File::toPath) + .collect(Collectors.toList()); + List destFiles = Utils.copyFiles(sourceFiles, dirDestination, + StandardCopyOption.REPLACE_EXISTING); + destFiles.forEach((Path file) -> file.toFile().setWritable(true)); + } + public abstract void main(Provider p) throws Exception; protected boolean skipTest(Provider p) { diff --git a/test/jdk/sun/security/pkcs11/Provider/MultipleLogins.sh b/test/jdk/sun/security/pkcs11/Provider/MultipleLogins.sh index 56a4fd6b80638..ada0ab7d59a3e 100644 --- a/test/jdk/sun/security/pkcs11/Provider/MultipleLogins.sh +++ b/test/jdk/sun/security/pkcs11/Provider/MultipleLogins.sh @@ -1,5 +1,5 @@ # -# Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2021, 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,6 +27,7 @@ # @library /test/lib/ # @build jdk.test.lib.util.ForceGC # jdk.test.lib.Platform +# jdk.test.lib.Utils # @run shell MultipleLogins.sh # set a few environment variables so that the shell-script can run stand-alone From d20ccd1aef4a36662cc9fcc91d1c14b6739187d6 Mon Sep 17 00:00:00 2001 From: Matias Saavedra Silva Date: Wed, 6 Nov 2024 18:46:06 +0000 Subject: [PATCH 08/22] 8335583: Avoid using pointers in CDS tables Reviewed-by: iklam, ccheung --- src/hotspot/share/cds/archiveBuilder.cpp | 7 ++ src/hotspot/share/cds/archiveBuilder.hpp | 10 ++- src/hotspot/share/cds/archiveUtils.hpp | 22 +++++ src/hotspot/share/cds/lambdaFormInvokers.cpp | 12 +-- src/hotspot/share/cds/lambdaFormInvokers.hpp | 2 +- .../share/cds/lambdaProxyClassDictionary.cpp | 34 +++++--- .../share/cds/lambdaProxyClassDictionary.hpp | 87 +++++++++++++++++-- src/hotspot/share/cds/runTimeClassInfo.cpp | 21 +++-- src/hotspot/share/cds/runTimeClassInfo.hpp | 35 ++++---- .../classfile/systemDictionaryShared.cpp | 39 ++++++--- .../classfile/systemDictionaryShared.hpp | 4 +- 11 files changed, 204 insertions(+), 69 deletions(-) diff --git a/src/hotspot/share/cds/archiveBuilder.cpp b/src/hotspot/share/cds/archiveBuilder.cpp index 3c67216d4c5bf..d68ce10d1c194 100644 --- a/src/hotspot/share/cds/archiveBuilder.cpp +++ b/src/hotspot/share/cds/archiveBuilder.cpp @@ -872,6 +872,13 @@ uintx ArchiveBuilder::any_to_offset(address p) const { return buffer_to_offset(p); } +address ArchiveBuilder::offset_to_buffered_address(u4 offset) const { + address requested_addr = _requested_static_archive_bottom + offset; + address buffered_addr = requested_addr - _buffer_to_requested_delta; + assert(is_in_buffer_space(buffered_addr), "bad offset"); + return buffered_addr; +} + #if INCLUDE_CDS_JAVA_HEAP narrowKlass ArchiveBuilder::get_requested_narrow_klass(Klass* k) { assert(CDSConfig::is_dumping_heap(), "sanity"); diff --git a/src/hotspot/share/cds/archiveBuilder.hpp b/src/hotspot/share/cds/archiveBuilder.hpp index c17090ee53d8f..4b3aa72611ebf 100644 --- a/src/hotspot/share/cds/archiveBuilder.hpp +++ b/src/hotspot/share/cds/archiveBuilder.hpp @@ -321,7 +321,7 @@ class ArchiveBuilder : public StackObj { } public: - static const uintx MAX_SHARED_DELTA = 0x7FFFFFFF; + static const uintx MAX_SHARED_DELTA = ArchiveUtils::MAX_SHARED_DELTA;; // The address p points to an object inside the output buffer. When the archive is mapped // at the requested address, what's the offset of this object from _requested_static_archive_bottom? @@ -331,6 +331,9 @@ class ArchiveBuilder : public StackObj { // inside the output buffer, or (b), an object in the currently mapped static archive. uintx any_to_offset(address p) const; + // The reverse of buffer_to_offset() + address offset_to_buffered_address(u4 offset) const; + template u4 buffer_to_offset_u4(T p) const { uintx offset = buffer_to_offset((address)p); @@ -343,6 +346,11 @@ class ArchiveBuilder : public StackObj { return to_offset_u4(offset); } + template + T offset_to_buffered(u4 offset) const { + return (T)offset_to_buffered_address(offset); + } + public: ArchiveBuilder(); ~ArchiveBuilder(); diff --git a/src/hotspot/share/cds/archiveUtils.hpp b/src/hotspot/share/cds/archiveUtils.hpp index 5a78bc26ee627..410b0b8400196 100644 --- a/src/hotspot/share/cds/archiveUtils.hpp +++ b/src/hotspot/share/cds/archiveUtils.hpp @@ -25,8 +25,10 @@ #ifndef SHARE_CDS_ARCHIVEUTILS_HPP #define SHARE_CDS_ARCHIVEUTILS_HPP +#include "cds/cds_globals.hpp" #include "cds/serializeClosure.hpp" #include "logging/log.hpp" +#include "memory/metaspace.hpp" #include "memory/virtualspace.hpp" #include "utilities/bitMap.hpp" #include "utilities/exceptions.hpp" @@ -247,7 +249,27 @@ class ReadClosure : public SerializeClosure { class ArchiveUtils { public: + static const uintx MAX_SHARED_DELTA = 0x7FFFFFFF; static void log_to_classlist(BootstrapInfo* bootstrap_specifier, TRAPS) NOT_CDS_RETURN; + + // offset must represent an object of type T in the mapped shared space. Return + // a direct pointer to this object. + template T static from_offset(u4 offset) { + T p = (T)(SharedBaseAddress + offset); + assert(Metaspace::is_in_shared_metaspace(p), "must be"); + return p; + } + + // p must be an archived object. Get its offset from SharedBaseAddress + template static u4 to_offset(T p) { + uintx pn = (uintx)p; + uintx base = (uintx)SharedBaseAddress; + assert(Metaspace::is_in_shared_metaspace(p), "must be"); + assert(pn > base, "sanity"); // No valid object is stored at 0 offset from SharedBaseAddress + uintx offset = pn - base; + assert(offset <= MAX_SHARED_DELTA, "range check"); + return static_cast(offset); + } }; class HeapRootSegments { diff --git a/src/hotspot/share/cds/lambdaFormInvokers.cpp b/src/hotspot/share/cds/lambdaFormInvokers.cpp index 8a556a48624d4..88b1f9277ff6e 100644 --- a/src/hotspot/share/cds/lambdaFormInvokers.cpp +++ b/src/hotspot/share/cds/lambdaFormInvokers.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 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 @@ -51,7 +51,7 @@ #include "runtime/mutexLocker.hpp" GrowableArrayCHeap* LambdaFormInvokers::_lambdaform_lines = nullptr; -Array*>* LambdaFormInvokers::_static_archive_invokers = nullptr; +Array* LambdaFormInvokers::_static_archive_invokers = nullptr; #define NUM_FILTER 4 static const char* filter[NUM_FILTER] = {"java.lang.invoke.Invokers$Holder", @@ -216,7 +216,7 @@ void LambdaFormInvokers::dump_static_archive_invokers() { } } if (count > 0) { - _static_archive_invokers = ArchiveBuilder::new_ro_array*>(count); + _static_archive_invokers = ArchiveBuilder::new_ro_array(count); int index = 0; for (int i = 0; i < len; i++) { char* str = _lambdaform_lines->at(i); @@ -225,8 +225,7 @@ void LambdaFormInvokers::dump_static_archive_invokers() { Array* line = ArchiveBuilder::new_ro_array((int)str_len); strncpy(line->adr_at(0), str, str_len); - _static_archive_invokers->at_put(index, line); - ArchivePtrMarker::mark_pointer(_static_archive_invokers->adr_at(index)); + _static_archive_invokers->at_put(index, ArchiveBuilder::current()->any_to_offset_u4(line)); index++; } } @@ -239,7 +238,8 @@ void LambdaFormInvokers::dump_static_archive_invokers() { void LambdaFormInvokers::read_static_archive_invokers() { if (_static_archive_invokers != nullptr) { for (int i = 0; i < _static_archive_invokers->length(); i++) { - Array* line = _static_archive_invokers->at(i); + u4 offset = _static_archive_invokers->at(i); + Array* line = ArchiveUtils::from_offset*>(offset); char* str = line->adr_at(0); append(str); } diff --git a/src/hotspot/share/cds/lambdaFormInvokers.hpp b/src/hotspot/share/cds/lambdaFormInvokers.hpp index e78ddb1a1bc47..583a863a1c20d 100644 --- a/src/hotspot/share/cds/lambdaFormInvokers.hpp +++ b/src/hotspot/share/cds/lambdaFormInvokers.hpp @@ -38,7 +38,7 @@ class LambdaFormInvokers : public AllStatic { private: static GrowableArrayCHeap* _lambdaform_lines; // For storing LF form lines (LF_RESOLVE only) in read only table. - static Array*>* _static_archive_invokers; + static Array* _static_archive_invokers; static void regenerate_class(char* name, ClassFileStream& st, TRAPS); public: static void append(char* line); diff --git a/src/hotspot/share/cds/lambdaProxyClassDictionary.cpp b/src/hotspot/share/cds/lambdaProxyClassDictionary.cpp index 2eef543f9db9f..b8220a34047e3 100644 --- a/src/hotspot/share/cds/lambdaProxyClassDictionary.cpp +++ b/src/hotspot/share/cds/lambdaProxyClassDictionary.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 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 @@ -34,17 +34,6 @@ DumpTimeLambdaProxyClassInfo::~DumpTimeLambdaProxyClassInfo() { } } -void LambdaProxyClassKey::init_for_archive(LambdaProxyClassKey& dumptime_key) { - ArchiveBuilder* b = ArchiveBuilder::current(); - - b->write_pointer_in_buffer(&_caller_ik, dumptime_key._caller_ik); - b->write_pointer_in_buffer(&_instantiated_method_type, dumptime_key._instantiated_method_type); - b->write_pointer_in_buffer(&_invoked_name, dumptime_key._invoked_name); - b->write_pointer_in_buffer(&_invoked_type, dumptime_key._invoked_type); - b->write_pointer_in_buffer(&_member_method, dumptime_key._member_method); - b->write_pointer_in_buffer(&_method_type, dumptime_key._method_type); -} - unsigned int LambdaProxyClassKey::hash() const { return SystemDictionaryShared::hash_for_shared_dictionary((address)_caller_ik) + SystemDictionaryShared::hash_for_shared_dictionary((address)_invoked_name) + @@ -53,6 +42,14 @@ unsigned int LambdaProxyClassKey::hash() const { SystemDictionaryShared::hash_for_shared_dictionary((address)_instantiated_method_type); } +unsigned int RunTimeLambdaProxyClassKey::hash() const { + return primitive_hash(_caller_ik) + + primitive_hash(_invoked_name) + + primitive_hash(_invoked_type) + + primitive_hash(_method_type) + + primitive_hash(_instantiated_method_type); +} + #ifndef PRODUCT void LambdaProxyClassKey::print_on(outputStream* st) const { ResourceMark rm; @@ -65,13 +62,24 @@ void LambdaProxyClassKey::print_on(outputStream* st) const { st->print_cr("_method_type : %s", _method_type->as_C_string()); } +void RunTimeLambdaProxyClassKey::print_on(outputStream* st) const { + ResourceMark rm; + st->print_cr("LambdaProxyClassKey : " INTPTR_FORMAT " hash: %0x08x", p2i(this), hash()); + st->print_cr("_caller_ik : %d", _caller_ik); + st->print_cr("_instantiated_method_type : %d", _instantiated_method_type); + st->print_cr("_invoked_name : %d", _invoked_name); + st->print_cr("_invoked_type : %d", _invoked_type); + st->print_cr("_member_method : %d", _member_method); + st->print_cr("_method_type : %d", _method_type); +} + void RunTimeLambdaProxyClassInfo::print_on(outputStream* st) const { _key.print_on(st); } #endif void RunTimeLambdaProxyClassInfo::init(LambdaProxyClassKey& key, DumpTimeLambdaProxyClassInfo& info) { - _key.init_for_archive(key); + _key = RunTimeLambdaProxyClassKey::init_for_dumptime(key); ArchiveBuilder::current()->write_pointer_in_buffer(&_proxy_klass_head, info._proxy_klasses->at(0)); } diff --git a/src/hotspot/share/cds/lambdaProxyClassDictionary.hpp b/src/hotspot/share/cds/lambdaProxyClassDictionary.hpp index b9766066484c6..2a9f87681dbe6 100644 --- a/src/hotspot/share/cds/lambdaProxyClassDictionary.hpp +++ b/src/hotspot/share/cds/lambdaProxyClassDictionary.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 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 @@ -101,9 +101,80 @@ class LambdaProxyClassKey { return (k1.equals(k2)); } - InstanceKlass* caller_ik() const { return _caller_ik; } + InstanceKlass* caller_ik() const { return _caller_ik; } + Symbol* invoked_name() const { return _invoked_name; } + Symbol* invoked_type() const { return _invoked_type; } + Symbol* method_type() const { return _method_type; } + Method* member_method() const { return _member_method; } + Symbol* instantiated_method_type() const { return _instantiated_method_type; } - void init_for_archive(LambdaProxyClassKey& dumptime_key); +#ifndef PRODUCT + void print_on(outputStream* st) const; +#endif +}; + +class RunTimeLambdaProxyClassKey { + u4 _caller_ik; + u4 _invoked_name; + u4 _invoked_type; + u4 _method_type; + u4 _member_method; + u4 _instantiated_method_type; + + RunTimeLambdaProxyClassKey(u4 caller_ik, + u4 invoked_name, + u4 invoked_type, + u4 method_type, + u4 member_method, + u4 instantiated_method_type) : + _caller_ik(caller_ik), + _invoked_name(invoked_name), + _invoked_type(invoked_type), + _method_type(method_type), + _member_method(member_method), + _instantiated_method_type(instantiated_method_type) {} + +public: + static RunTimeLambdaProxyClassKey init_for_dumptime(LambdaProxyClassKey& key) { + assert(ArchiveBuilder::is_active(), "sanity"); + ArchiveBuilder* b = ArchiveBuilder::current(); + + u4 caller_ik = b->any_to_offset_u4(key.caller_ik()); + u4 invoked_name = b->any_to_offset_u4(key.invoked_name()); + u4 invoked_type = b->any_to_offset_u4(key.invoked_type()); + u4 method_type = b->any_to_offset_u4(key.method_type()); + u4 member_method = b->any_to_offset_u4(key.member_method()); + u4 instantiated_method_type = b->any_to_offset_u4(key.instantiated_method_type()); + + return RunTimeLambdaProxyClassKey(caller_ik, invoked_name, invoked_type, method_type, + member_method, instantiated_method_type); + } + + static RunTimeLambdaProxyClassKey init_for_runtime(InstanceKlass* caller_ik, + Symbol* invoked_name, + Symbol* invoked_type, + Symbol* method_type, + Method* member_method, + Symbol* instantiated_method_type) { + // All parameters must be in shared space, or else you'd get an assert in + // ArchiveUtils::to_offset(). + return RunTimeLambdaProxyClassKey(ArchiveUtils::to_offset(caller_ik), + ArchiveUtils::to_offset(invoked_name), + ArchiveUtils::to_offset(invoked_type), + ArchiveUtils::to_offset(method_type), + ArchiveUtils::to_offset(member_method), + ArchiveUtils::to_offset(instantiated_method_type)); + } + + unsigned int hash() const; + bool equals(RunTimeLambdaProxyClassKey const& other) const { + return _caller_ik == other._caller_ik && + _invoked_name == other._invoked_name && + _invoked_type == other._invoked_type && + _method_type == other._method_type && + _member_method == other._member_method && + _instantiated_method_type == other._instantiated_method_type; + } #ifndef PRODUCT void print_on(outputStream* st) const; @@ -133,17 +204,17 @@ class DumpTimeLambdaProxyClassInfo { }; class RunTimeLambdaProxyClassInfo { - LambdaProxyClassKey _key; + RunTimeLambdaProxyClassKey _key; InstanceKlass* _proxy_klass_head; public: - RunTimeLambdaProxyClassInfo(LambdaProxyClassKey key, InstanceKlass* proxy_klass_head) : + RunTimeLambdaProxyClassInfo(RunTimeLambdaProxyClassKey key, InstanceKlass* proxy_klass_head) : _key(key), _proxy_klass_head(proxy_klass_head) {} InstanceKlass* proxy_klass_head() const { return _proxy_klass_head; } // Used by LambdaProxyClassDictionary to implement OffsetCompactHashtable::EQUALS static inline bool EQUALS( - const RunTimeLambdaProxyClassInfo* value, LambdaProxyClassKey* key, int len_unused) { + const RunTimeLambdaProxyClassInfo* value, RunTimeLambdaProxyClassKey* key, int len_unused) { return (value->_key.equals(*key)); } void init(LambdaProxyClassKey& key, DumpTimeLambdaProxyClassInfo& info); @@ -151,7 +222,7 @@ class RunTimeLambdaProxyClassInfo { unsigned int hash() const { return _key.hash(); } - LambdaProxyClassKey key() const { + RunTimeLambdaProxyClassKey key() const { return _key; } #ifndef PRODUCT @@ -173,7 +244,7 @@ class DumpTimeLambdaProxyClassDictionary }; class LambdaProxyClassDictionary : public OffsetCompactHashtable< - LambdaProxyClassKey*, + RunTimeLambdaProxyClassKey*, const RunTimeLambdaProxyClassInfo*, RunTimeLambdaProxyClassInfo::EQUALS> {}; diff --git a/src/hotspot/share/cds/runTimeClassInfo.cpp b/src/hotspot/share/cds/runTimeClassInfo.cpp index 2536b53308664..e1329d9d2f914 100644 --- a/src/hotspot/share/cds/runTimeClassInfo.cpp +++ b/src/hotspot/share/cds/runTimeClassInfo.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 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 @@ -30,9 +30,10 @@ void RunTimeClassInfo::init(DumpTimeClassInfo& info) { ArchiveBuilder* builder = ArchiveBuilder::current(); - builder->write_pointer_in_buffer(&_klass, info._klass); + InstanceKlass* k = info._klass; + _klass_offset = builder->any_to_offset_u4(k); - if (!SystemDictionaryShared::is_builtin(_klass)) { + if (!SystemDictionaryShared::is_builtin(k)) { CrcInfo* c = crc(); c->_clsfile_size = info._clsfile_size; c->_clsfile_crc32 = info._clsfile_crc32; @@ -61,10 +62,10 @@ void RunTimeClassInfo::init(DumpTimeClassInfo& info) { } } - if (_klass->is_hidden()) { - builder->write_pointer_in_buffer(nest_host_addr(), info.nest_host()); + if (k->is_hidden()) { + _nest_host_offset = builder->any_to_offset_u4(info.nest_host()); } - if (_klass->has_archived_enum_objs()) { + if (k->has_archived_enum_objs()) { int num = info.num_enum_klass_static_fields(); set_num_enum_klass_static_fields(num); for (int i = 0; i < num; i++) { @@ -74,6 +75,14 @@ void RunTimeClassInfo::init(DumpTimeClassInfo& info) { } } +InstanceKlass* RunTimeClassInfo::klass() const { + if (ArchiveBuilder::is_active() && ArchiveBuilder::current()->is_in_buffer_space((address)this)) { + return ArchiveBuilder::current()->offset_to_buffered(_klass_offset); + } else { + return ArchiveUtils::from_offset(_klass_offset); + } +} + size_t RunTimeClassInfo::crc_size(InstanceKlass* klass) { if (!SystemDictionaryShared::is_builtin(klass)) { return sizeof(CrcInfo); diff --git a/src/hotspot/share/cds/runTimeClassInfo.hpp b/src/hotspot/share/cds/runTimeClassInfo.hpp index 50cf3d528b9d6..d0f02d1c09559 100644 --- a/src/hotspot/share/cds/runTimeClassInfo.hpp +++ b/src/hotspot/share/cds/runTimeClassInfo.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 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,24 +52,24 @@ class RunTimeClassInfo { struct RTVerifierConstraint { u4 _name; u4 _from_name; - Symbol* name() { return (Symbol*)(SharedBaseAddress + _name);} - Symbol* from_name() { return (Symbol*)(SharedBaseAddress + _from_name); } + Symbol* name() { return ArchiveUtils::from_offset(_name); } + Symbol* from_name() { return ArchiveUtils::from_offset(_from_name); } }; struct RTLoaderConstraint { u4 _name; char _loader_type1; char _loader_type2; - Symbol* constraint_name() { - return (Symbol*)(SharedBaseAddress + _name); - } + Symbol* constraint_name() { return ArchiveUtils::from_offset(_name); } }; struct RTEnumKlassStaticFields { int _num; int _root_indices[1]; }; - InstanceKlass* _klass; +private: + u4 _klass_offset; + u4 _nest_host_offset; int _num_verifier_constraints; int _num_loader_constraints; @@ -80,7 +80,6 @@ class RunTimeClassInfo { // optional char _verifier_constraint_flags[_num_verifier_constraints] // optional RTEnumKlassStaticFields _enum_klass_static_fields; -private: static size_t header_size_size() { return align_up(sizeof(RunTimeClassInfo), wordSize); } @@ -108,6 +107,9 @@ class RunTimeClassInfo { static size_t crc_size(InstanceKlass* klass); public: + InstanceKlass* klass() const; + int num_verifier_constraints() const { return _num_verifier_constraints; } + int num_loader_constraints() const { return _num_loader_constraints; } static size_t byte_size(InstanceKlass* klass, int num_verifier_constraints, int num_loader_constraints, int num_enum_klass_static_fields) { return header_size_size() + @@ -125,11 +127,11 @@ class RunTimeClassInfo { } size_t nest_host_offset() const { - return crc_offset() + crc_size(_klass); + return crc_offset() + crc_size(klass()); } size_t loader_constraints_offset() const { - return nest_host_offset() + nest_host_size(_klass); + return nest_host_offset() + nest_host_size(klass()); } size_t verifier_constraints_offset() const { return loader_constraints_offset() + loader_constraints_size(_num_loader_constraints); @@ -150,13 +152,13 @@ class RunTimeClassInfo { } RTEnumKlassStaticFields* enum_klass_static_fields_addr() const { - assert(_klass->has_archived_enum_objs(), "sanity"); + assert(klass()->has_archived_enum_objs(), "sanity"); return (RTEnumKlassStaticFields*)(address(this) + enum_klass_static_fields_offset()); } public: CrcInfo* crc() const { - assert(crc_size(_klass) > 0, "must be"); + assert(crc_size(klass()) > 0, "must be"); return (CrcInfo*)(address(this) + crc_offset()); } RTVerifierConstraint* verifier_constraints() { @@ -173,12 +175,9 @@ class RunTimeClassInfo { return (char*)(address(this) + verifier_constraint_flags_offset()); } - InstanceKlass** nest_host_addr() { - assert(_klass->is_hidden(), "sanity"); - return (InstanceKlass**)(address(this) + nest_host_offset()); - } InstanceKlass* nest_host() { - return *nest_host_addr(); + assert(!ArchiveBuilder::is_active(), "not called when dumping archive"); + return ArchiveUtils::from_offset(_nest_host_offset); } RTLoaderConstraint* loader_constraints() { @@ -248,7 +247,7 @@ class RunTimeClassInfo { // Used by RunTimeSharedDictionary to implement OffsetCompactHashtable::EQUALS static inline bool EQUALS( const RunTimeClassInfo* value, Symbol* key, int len_unused) { - return (value->_klass->name() == key); + return (value->klass()->name() == key); } }; diff --git a/src/hotspot/share/classfile/systemDictionaryShared.cpp b/src/hotspot/share/classfile/systemDictionaryShared.cpp index 05f6ace9b8a95..5b4e6163bc13c 100644 --- a/src/hotspot/share/classfile/systemDictionaryShared.cpp +++ b/src/hotspot/share/classfile/systemDictionaryShared.cpp @@ -134,7 +134,7 @@ InstanceKlass* SystemDictionaryShared::lookup_from_stream(Symbol* class_name, return nullptr; } - return acquire_class_for_current_thread(record->_klass, class_loader, + return acquire_class_for_current_thread(record->klass(), class_loader, protection_domain, cfs, THREAD); } @@ -789,9 +789,20 @@ InstanceKlass* SystemDictionaryShared::get_shared_lambda_proxy_class(InstanceKla Symbol* method_type, Method* member_method, Symbol* instantiated_method_type) { + if (!caller_ik->is_shared() || + !invoked_name->is_shared() || + !invoked_type->is_shared() || + !method_type->is_shared() || + !member_method->is_shared() || + !instantiated_method_type->is_shared()) { + // These can't be represented as u4 offset, but we wouldn't have archived a lambda proxy in this case anyway. + return nullptr; + } + MutexLocker ml(CDSLambda_lock, Mutex::_no_safepoint_check_flag); - LambdaProxyClassKey key(caller_ik, invoked_name, invoked_type, - method_type, member_method, instantiated_method_type); + RunTimeLambdaProxyClassKey key = + RunTimeLambdaProxyClassKey::init_for_runtime(caller_ik, invoked_name, invoked_type, + method_type, member_method, instantiated_method_type); // Try to retrieve the lambda proxy class from static archive. const RunTimeLambdaProxyClassInfo* info = _static_archive.lookup_lambda_proxy_class(&key); @@ -899,7 +910,7 @@ void SystemDictionaryShared::check_verification_constraints(InstanceKlass* klass assert(!CDSConfig::is_dumping_static_archive() && CDSConfig::is_using_archive(), "called at run time with CDS enabled only"); RunTimeClassInfo* record = RunTimeClassInfo::get_for(klass); - int length = record->_num_verifier_constraints; + int length = record->num_verifier_constraints(); if (length > 0) { for (int i = 0; i < length; i++) { RunTimeClassInfo::RTVerifierConstraint* vc = record->verifier_constraint_at(i); @@ -1015,9 +1026,9 @@ bool SystemDictionaryShared::check_linking_constraints(Thread* current, Instance if (klass->is_shared_platform_class() || klass->is_shared_app_class()) { RunTimeClassInfo* info = RunTimeClassInfo::get_for(klass); assert(info != nullptr, "Sanity"); - if (info->_num_loader_constraints > 0) { + if (info->num_loader_constraints() > 0) { HandleMark hm(current); - for (int i = 0; i < info->_num_loader_constraints; i++) { + for (int i = 0; i < info->num_loader_constraints(); i++) { RunTimeClassInfo::RTLoaderConstraint* lc = info->loader_constraint_at(i); Symbol* name = lc->constraint_name(); Handle loader1(current, get_class_loader_by(lc->_loader_type1)); @@ -1333,14 +1344,14 @@ InstanceKlass* SystemDictionaryShared::find_builtin_class(Symbol* name) { &_dynamic_archive._builtin_dictionary, name); if (record != nullptr) { - assert(!record->_klass->is_hidden(), "hidden class cannot be looked up by name"); - assert(check_alignment(record->_klass), "Address not aligned"); + assert(!record->klass()->is_hidden(), "hidden class cannot be looked up by name"); + assert(check_alignment(record->klass()), "Address not aligned"); // We did not save the classfile data of the generated LambdaForm invoker classes, // so we cannot support CLFH for such classes. - if (record->_klass->is_generated_shared_class() && JvmtiExport::should_post_class_file_load_hook()) { + if (record->klass()->is_generated_shared_class() && JvmtiExport::should_post_class_file_load_hook()) { return nullptr; } - return record->_klass; + return record->klass(); } else { return nullptr; } @@ -1378,10 +1389,10 @@ class SharedDictionaryPrinter : StackObj { void do_value(const RunTimeClassInfo* record) { ResourceMark rm; - _st->print_cr("%4d: %s %s", _index++, record->_klass->external_name(), - class_loader_name_for_shared(record->_klass)); - if (record->_klass->array_klasses() != nullptr) { - record->_klass->array_klasses()->cds_print_value_on(_st); + _st->print_cr("%4d: %s %s", _index++, record->klass()->external_name(), + class_loader_name_for_shared(record->klass())); + if (record->klass()->array_klasses() != nullptr) { + record->klass()->array_klasses()->cds_print_value_on(_st); _st->cr(); } } diff --git a/src/hotspot/share/classfile/systemDictionaryShared.hpp b/src/hotspot/share/classfile/systemDictionaryShared.hpp index 070f671e7a7ff..c79821036e038 100644 --- a/src/hotspot/share/classfile/systemDictionaryShared.hpp +++ b/src/hotspot/share/classfile/systemDictionaryShared.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 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 @@ -145,7 +145,7 @@ class SystemDictionaryShared: public SystemDictionary { RunTimeSharedDictionary _unregistered_dictionary; LambdaProxyClassDictionary _lambda_proxy_class_dictionary; - const RunTimeLambdaProxyClassInfo* lookup_lambda_proxy_class(LambdaProxyClassKey* key) { + const RunTimeLambdaProxyClassInfo* lookup_lambda_proxy_class(RunTimeLambdaProxyClassKey* key) { return _lambda_proxy_class_dictionary.lookup(key, key->hash(), 0); } From e33dc13567a4f0d9a6c1ae63fa0424ca27d52584 Mon Sep 17 00:00:00 2001 From: Martin Doerr Date: Wed, 6 Nov 2024 19:36:20 +0000 Subject: [PATCH 09/22] 8343343: Misc crash dump improvements on more platforms after JDK-8294160 Co-authored-by: Boris Ulasevich Reviewed-by: mbaesken, jkern, dlong --- src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp | 29 ++++----------- .../os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 23 ++++-------- src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp | 23 ++++-------- src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp | 4 --- .../os_cpu/linux_aarch64/os_linux_aarch64.cpp | 17 --------- src/hotspot/os_cpu/linux_arm/os_linux_arm.cpp | 35 ++++++++++--------- src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp | 23 ++++-------- .../os_cpu/linux_riscv/os_linux_riscv.cpp | 17 --------- .../os_cpu/linux_s390/os_linux_s390.cpp | 23 ++++-------- src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp | 17 --------- .../os_cpu/linux_zero/os_linux_zero.cpp | 15 -------- .../windows_aarch64/os_windows_aarch64.cpp | 18 ---------- .../os_cpu/windows_x86/os_windows_x86.cpp | 17 --------- src/hotspot/share/runtime/os.cpp | 20 +++++++++++ 14 files changed, 68 insertions(+), 213 deletions(-) diff --git a/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp b/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp index f83aa6030622a..45d91c60ed42a 100644 --- a/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp +++ b/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp @@ -123,13 +123,13 @@ frame os::fetch_frame_from_context(const void* ucVoid) { intptr_t* sp; intptr_t* fp; address epc = fetch_frame_from_context(ucVoid, &sp, &fp); - // Avoid crash during crash if pc broken. - if (epc) { - frame fr(sp, epc, frame::kind::unknown); - return fr; + if (epc == nullptr || !is_readable_pointer(epc)) { + // Try to recover from calling into bad memory + // Assume new frame has not been set up, the same as + // compiled frame stack bang + return fetch_compiled_frame_from_context(ucVoid); } - frame fr(sp); - return fr; + return frame(sp, epc, frame::kind::unknown); } frame os::fetch_compiled_frame_from_context(const void* ucVoid) { @@ -447,23 +447,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const ucontext_t* uc = (const ucontext_t*)context; - - address sp = (address)os::Aix::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::Posix::ucontext_get_pc(uc); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = 32 /* r0-r32 */ + 3 /* pc, lr, sp */; int n = continuation; diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index ba14cb2ac1280..7702dbd17ad69 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -159,6 +159,12 @@ frame os::fetch_frame_from_context(const void* ucVoid) { intptr_t* sp; intptr_t* fp; address epc = fetch_frame_from_context(ucVoid, &sp, &fp); + if (!is_readable_pointer(epc)) { + // Try to recover from calling into bad memory + // Assume new frame has not been set up, the same as + // compiled frame stack bang + return fetch_compiled_frame_from_context(ucVoid); + } return frame(sp, fp, epc); } @@ -442,23 +448,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const ucontext_t* uc = (const ucontext_t*)context; - - address sp = (address)os::Bsd::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::Posix::ucontext_get_pc(uc); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = 29 /* x0-x28 */ + 3 /* fp, lr, sp */; int n = continuation; diff --git a/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp b/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp index 437274a2cb126..153c5ad7e2b76 100644 --- a/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp +++ b/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp @@ -333,6 +333,12 @@ frame os::fetch_frame_from_context(const void* ucVoid) { intptr_t* sp; intptr_t* fp; address epc = fetch_frame_from_context(ucVoid, &sp, &fp); + if (!is_readable_pointer(epc)) { + // Try to recover from calling into bad memory + // Assume new frame has not been set up, the same as + // compiled frame stack bang + return fetch_compiled_frame_from_context(ucVoid); + } return frame(sp, fp, epc); } @@ -836,23 +842,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const ucontext_t* uc = (const ucontext_t*)context; - - address sp = (address)os::Bsd::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::Posix::ucontext_get_pc(uc); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = AMD64_ONLY(16) NOT_AMD64(8); int n = continuation; diff --git a/src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp b/src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp index 0fc9484ce23ef..4ecdbe93ebfdc 100644 --- a/src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp +++ b/src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp @@ -228,10 +228,6 @@ void os::print_context(outputStream* st, const void* context) { ShouldNotCallThis(); } -void os::print_tos_pc(outputStream *st, const void *context) { - ShouldNotCallThis(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { ShouldNotCallThis(); } diff --git a/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp b/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp index 9f44fe3aa2520..a7ec163f78553 100644 --- a/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp +++ b/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp @@ -354,23 +354,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const ucontext_t* uc = (const ucontext_t*)context; - - address sp = (address)os::Linux::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::fetch_frame_from_context(uc).pc(); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = 32 /* r0-r31 */; int n = continuation; diff --git a/src/hotspot/os_cpu/linux_arm/os_linux_arm.cpp b/src/hotspot/os_cpu/linux_arm/os_linux_arm.cpp index 807fa76589713..861d0d20153f7 100644 --- a/src/hotspot/os_cpu/linux_arm/os_linux_arm.cpp +++ b/src/hotspot/os_cpu/linux_arm/os_linux_arm.cpp @@ -188,9 +188,27 @@ frame os::fetch_frame_from_context(const void* ucVoid) { intptr_t* sp; intptr_t* fp; address epc = fetch_frame_from_context(ucVoid, &sp, &fp); + if (!is_readable_pointer(epc)) { + // Try to recover from calling into bad memory + // Assume new frame has not been set up, the same as + // compiled frame stack bang + return fetch_compiled_frame_from_context(ucVoid); + } return frame(sp, fp, epc); } +frame os::fetch_compiled_frame_from_context(const void* ucVoid) { + const ucontext_t* uc = (const ucontext_t*)ucVoid; + // In compiled code, the stack banging is performed before LR + // has been saved in the frame. LR is live, and SP and FP + // belong to the caller. + intptr_t* fp = os::Linux::ucontext_get_fp(uc); + intptr_t* sp = os::Linux::ucontext_get_sp(uc); + address pc = (address)(uc->uc_mcontext.arm_lr + - NativeInstruction::instruction_size); + return frame(sp, fp, pc); +} + frame os::get_sender_for_C_frame(frame* fr) { #ifdef __thumb__ // We can't reliably get anything from a thumb C frame. @@ -474,23 +492,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const ucontext_t* uc = (const ucontext_t*)context; - - address sp = (address)os::Linux::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::Posix::ucontext_get_pc(uc); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = ARM_REGS_IN_CONTEXT; int n = continuation; diff --git a/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp b/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp index 1857097bd52cd..f3f9a3a88df67 100644 --- a/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp +++ b/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp @@ -155,6 +155,12 @@ frame os::fetch_frame_from_context(const void* ucVoid) { intptr_t* sp; intptr_t* fp; address epc = fetch_frame_from_context(ucVoid, &sp, &fp); + if (!is_readable_pointer(epc)) { + // Try to recover from calling into bad memory + // Assume new frame has not been set up, the same as + // compiled frame stack bang + return fetch_compiled_frame_from_context(ucVoid); + } return frame(sp, epc, frame::kind::unknown); } @@ -461,23 +467,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const ucontext_t* uc = (const ucontext_t*)context; - - address sp = (address)os::Linux::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::Posix::ucontext_get_pc(uc); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = 32 /* r0-r32 */ + 3 /* pc, lr, ctr */; int n = continuation; diff --git a/src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp b/src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp index fcb0e170af16a..a00659f37cb42 100644 --- a/src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp +++ b/src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp @@ -352,23 +352,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const ucontext_t* uc = (const ucontext_t*)context; - - address sp = (address)os::Linux::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::fetch_frame_from_context(uc).pc(); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = 32; int n = continuation; diff --git a/src/hotspot/os_cpu/linux_s390/os_linux_s390.cpp b/src/hotspot/os_cpu/linux_s390/os_linux_s390.cpp index 9ac1152a013f5..bc8e3d104310a 100644 --- a/src/hotspot/os_cpu/linux_s390/os_linux_s390.cpp +++ b/src/hotspot/os_cpu/linux_s390/os_linux_s390.cpp @@ -140,6 +140,12 @@ frame os::fetch_frame_from_context(const void* ucVoid) { intptr_t* sp; intptr_t* fp; address epc = fetch_frame_from_context(ucVoid, &sp, &fp); + if (!is_readable_pointer(epc)) { + // Try to recover from calling into bad memory + // Assume new frame has not been set up, the same as + // compiled frame stack bang + return fetch_compiled_frame_from_context(ucVoid); + } return frame(sp, epc); } @@ -442,23 +448,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const ucontext_t* uc = (const ucontext_t*)context; - - address sp = (address)os::Linux::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::Posix::ucontext_get_pc(uc); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = 16 /* r0-r15 */ + 1 /* pc */; int n = continuation; diff --git a/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp b/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp index 8fdcbe63c7e8b..e357747bfea46 100644 --- a/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp +++ b/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp @@ -578,23 +578,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const ucontext_t* uc = (const ucontext_t*)context; - - address sp = (address)os::Linux::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::fetch_frame_from_context(uc).pc(); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = AMD64_ONLY(16) NOT_AMD64(8); int n = continuation; diff --git a/src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp b/src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp index d593c46d15d91..d8498f104c26c 100644 --- a/src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp +++ b/src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp @@ -376,21 +376,6 @@ void os::print_context(outputStream* st, const void* ucVoid) { st->print_cr("No context information."); } -void os::print_tos_pc(outputStream *st, const void* ucVoid) { - const ucontext_t* uc = (const ucontext_t*)ucVoid; - - address sp = (address)os::Linux::ucontext_get_sp(uc); - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::Posix::ucontext_get_pc(uc); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { st->print_cr("No register info."); } diff --git a/src/hotspot/os_cpu/windows_aarch64/os_windows_aarch64.cpp b/src/hotspot/os_cpu/windows_aarch64/os_windows_aarch64.cpp index 722c3c8dd608d..24410ed920314 100644 --- a/src/hotspot/os_cpu/windows_aarch64/os_windows_aarch64.cpp +++ b/src/hotspot/os_cpu/windows_aarch64/os_windows_aarch64.cpp @@ -205,24 +205,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const CONTEXT* uc = (const CONTEXT*)context; - - address sp = (address)uc->Sp; - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = (address)uc->Pc; - st->print_cr("Instructions: (pc=" PTR_FORMAT ")", pc); - print_hex_dump(st, pc - 32, pc + 32, sizeof(char), /* print_ascii=*/false); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = 29 /* X0-X28 */; int n = continuation; diff --git a/src/hotspot/os_cpu/windows_x86/os_windows_x86.cpp b/src/hotspot/os_cpu/windows_x86/os_windows_x86.cpp index de59a74cc24cc..f67e5df8e3ea3 100644 --- a/src/hotspot/os_cpu/windows_x86/os_windows_x86.cpp +++ b/src/hotspot/os_cpu/windows_x86/os_windows_x86.cpp @@ -464,23 +464,6 @@ void os::print_context(outputStream *st, const void *context) { st->cr(); } -void os::print_tos_pc(outputStream *st, const void *context) { - if (context == nullptr) return; - - const CONTEXT* uc = (const CONTEXT*)context; - - address sp = (address)uc->REG_SP; - print_tos(st, sp); - st->cr(); - - // Note: it may be unsafe to inspect memory near pc. For example, pc may - // point to garbage if entry point in an nmethod is corrupted. Leave - // this at the end, and hope for the best. - address pc = os::fetch_frame_from_context(uc).pc(); - print_instructions(st, pc); - st->cr(); -} - void os::print_register_info(outputStream *st, const void *context, int& continuation) { const int register_count = AMD64_ONLY(16) NOT_AMD64(8); int n = continuation; diff --git a/src/hotspot/share/runtime/os.cpp b/src/hotspot/share/runtime/os.cpp index 2395510f27f81..31f671e742e7a 100644 --- a/src/hotspot/share/runtime/os.cpp +++ b/src/hotspot/share/runtime/os.cpp @@ -1083,6 +1083,26 @@ void os::print_dhm(outputStream* st, const char* startStr, long sec) { st->print_cr("%s %ld days %ld:%02ld hours", startStr, days, hours, minutes); } +void os::print_tos_pc(outputStream* st, const void* context) { + if (context == nullptr) return; + + // First of all, carefully determine sp without inspecting memory near pc. + // See comment below. + intptr_t* sp = nullptr; + fetch_frame_from_context(context, &sp, nullptr); + print_tos(st, (address)sp); + st->cr(); + + // Note: it may be unsafe to inspect memory near pc. For example, pc may + // point to garbage if entry point in an nmethod is corrupted. Leave + // this at the end, and hope for the best. + // This version of fetch_frame_from_context finds the caller pc if the actual + // one is bad. + address pc = fetch_frame_from_context(context).pc(); + print_instructions(st, pc); + st->cr(); +} + void os::print_tos(outputStream* st, address sp) { st->print_cr("Top of Stack: (sp=" PTR_FORMAT ")", p2i(sp)); print_hex_dump(st, sp, sp + 512, sizeof(intptr_t)); From 9e31e78e39a4b573c158ef31af3ab4e9a1e229de Mon Sep 17 00:00:00 2001 From: Jaikiran Pai Date: Thu, 7 Nov 2024 00:51:06 +0000 Subject: [PATCH 10/22] 8342647: [macosx] Clean up the NSInvocation based call to NSProcessInfo.operatingSystemVersion Reviewed-by: bchristi --- .../macosx/native/libjava/java_props_macosx.c | 61 +++++++------------ 1 file changed, 21 insertions(+), 40 deletions(-) diff --git a/src/java.base/macosx/native/libjava/java_props_macosx.c b/src/java.base/macosx/native/libjava/java_props_macosx.c index f50daac13cb6a..07bc2be9d54d4 100644 --- a/src/java.base/macosx/native/libjava/java_props_macosx.c +++ b/src/java.base/macosx/native/libjava/java_props_macosx.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 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 @@ -222,54 +222,35 @@ char *setupMacOSXLocale(int cat) { } } -// 10.9 SDK does not include the NSOperatingSystemVersion struct. -// For now, create our own -typedef struct { - NSInteger majorVersion; - NSInteger minorVersion; - NSInteger patchVersion; -} OSVerStruct; - void setOSNameAndVersion(java_props_t *sprops) { // Hardcode os_name, and fill in os_version sprops->os_name = strdup("Mac OS X"); NSString *nsVerStr = NULL; char* osVersionCStr = NULL; - // Mac OS 10.9 includes the [NSProcessInfo operatingSystemVersion] function, - // but it's not in the 10.9 SDK. So, call it via NSInvocation. - if ([[NSProcessInfo processInfo] respondsToSelector:@selector(operatingSystemVersion)]) { - OSVerStruct osVer; - NSMethodSignature *sig = [[NSProcessInfo processInfo] methodSignatureForSelector: - @selector(operatingSystemVersion)]; - NSInvocation *invoke = [NSInvocation invocationWithMethodSignature:sig]; - invoke.selector = @selector(operatingSystemVersion); - [invoke invokeWithTarget:[NSProcessInfo processInfo]]; - [invoke getReturnValue:&osVer]; - - // Copy out the char* if running on version other than 10.16 Mac OS (10.16 == 11.x) - // or explicitly requesting version compatibility - if (!((long)osVer.majorVersion == 10 && (long)osVer.minorVersion >= 16) || - (getenv("SYSTEM_VERSION_COMPAT") != NULL)) { - if (osVer.patchVersion == 0) { // Omit trailing ".0" - nsVerStr = [NSString stringWithFormat:@"%ld.%ld", - (long)osVer.majorVersion, (long)osVer.minorVersion]; - } else { - nsVerStr = [NSString stringWithFormat:@"%ld.%ld.%ld", - (long)osVer.majorVersion, (long)osVer.minorVersion, (long)osVer.patchVersion]; - } + NSOperatingSystemVersion osVer = [[NSProcessInfo processInfo] operatingSystemVersion]; + // Copy out the char* if running on version other than 10.16 Mac OS (10.16 == 11.x) + // or explicitly requesting version compatibility + if (!((long)osVer.majorVersion == 10 && (long)osVer.minorVersion >= 16) || + (getenv("SYSTEM_VERSION_COMPAT") != NULL)) { + if (osVer.patchVersion == 0) { // Omit trailing ".0" + nsVerStr = [NSString stringWithFormat:@"%ld.%ld", + (long)osVer.majorVersion, (long)osVer.minorVersion]; } else { - // Version 10.16, without explicit env setting of SYSTEM_VERSION_COMPAT - // AKA 11+ Read the *real* ProductVersion from the hidden link to avoid SYSTEM_VERSION_COMPAT - // If not found, fallback below to the SystemVersion.plist - NSDictionary *version = [NSDictionary dictionaryWithContentsOfFile : - @"/System/Library/CoreServices/.SystemVersionPlatform.plist"]; - if (version != NULL) { - nsVerStr = [version objectForKey : @"ProductVersion"]; - } + nsVerStr = [NSString stringWithFormat:@"%ld.%ld.%ld", + (long)osVer.majorVersion, (long)osVer.minorVersion, (long)osVer.patchVersion]; + } + } else { + // Version 10.16, without explicit env setting of SYSTEM_VERSION_COMPAT + // AKA 11+ Read the *real* ProductVersion from the hidden link to avoid SYSTEM_VERSION_COMPAT + // If not found, fallback below to the SystemVersion.plist + NSDictionary *version = [NSDictionary dictionaryWithContentsOfFile : + @"/System/Library/CoreServices/.SystemVersionPlatform.plist"]; + if (version != NULL) { + nsVerStr = [version objectForKey : @"ProductVersion"]; } } - // Fallback if running on pre-10.9 Mac OS + // Fallback to reading the SystemVersion.plist if (nsVerStr == NULL) { NSDictionary *version = [NSDictionary dictionaryWithContentsOfFile : @"/System/Library/CoreServices/SystemVersion.plist"]; From f2316f6829c9b671e992401ee39d7a1a1805857e Mon Sep 17 00:00:00 2001 From: SendaoYan Date: Thu, 7 Nov 2024 03:12:32 +0000 Subject: [PATCH 11/22] 8343505: Problemlist java/lang/Thread/jni/AttachCurrentThread/AttachTest.java Reviewed-by: dholmes, lmesnik --- test/jdk/ProblemList.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt index ea74cd40fb865..59d86c646d230 100644 --- a/test/jdk/ProblemList.txt +++ b/test/jdk/ProblemList.txt @@ -518,6 +518,8 @@ java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java 8151492 generic- java/lang/invoke/LFCaching/LFGarbageCollectedTest.java 8078602 generic-all java/lang/invoke/lambda/LambdaFileEncodingSerialization.java 8249079 linux-all java/lang/invoke/RicochetTest.java 8251969 generic-all +java/lang/Thread/jni/AttachCurrentThread/AttachTest.java#id0 8343244 generic-all +java/lang/Thread/jni/AttachCurrentThread/AttachTest.java#id1 8343244 generic-all ############################################################################ From 97b681e93a9469d8d16122dc10bbf2f5b5fe2266 Mon Sep 17 00:00:00 2001 From: Axel Boldt-Christmas Date: Thu, 7 Nov 2024 06:28:02 +0000 Subject: [PATCH 12/22] 8340586: JdkJfrEvent::get_all_klasses stores non-strong oops in JNI handles Reviewed-by: coleenp, stefank, mgronlun --- .../share/jfr/support/jfrJdkJfrEvent.cpp | 28 ++++++++----------- src/hotspot/share/oops/klass.hpp | 2 ++ src/hotspot/share/oops/klass.inline.hpp | 8 ++++++ 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/hotspot/share/jfr/support/jfrJdkJfrEvent.cpp b/src/hotspot/share/jfr/support/jfrJdkJfrEvent.cpp index 0c2fb0206ecee..a6cff1609a458 100644 --- a/src/hotspot/share/jfr/support/jfrJdkJfrEvent.cpp +++ b/src/hotspot/share/jfr/support/jfrJdkJfrEvent.cpp @@ -35,6 +35,7 @@ #include "oops/klass.inline.hpp" #include "runtime/handles.inline.hpp" #include "runtime/javaThread.hpp" +#include "runtime/safepointVerifiers.hpp" #include "utilities/stack.inline.hpp" static jobject empty_java_util_arraylist = nullptr; @@ -80,30 +81,25 @@ static bool is_allowed(const Klass* k) { return !(k->is_abstract() || k->should_be_initialized()); } -static void fill_klasses(GrowableArray& event_subklasses, const InstanceKlass* event_klass, JavaThread* thread) { +static void fill_klasses(GrowableArray& event_subklasses, const InstanceKlass* event_klass, JavaThread* thread) { assert(event_subklasses.length() == 0, "invariant"); assert(event_klass != nullptr, "invariant"); DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(thread)); + // Do not safepoint while walking the ClassHierarchy, keeping klasses alive and storing their mirrors in JNI handles. + NoSafepointVerifier nsv; for (ClassHierarchyIterator iter(const_cast(event_klass)); !iter.done(); iter.next()) { Klass* subk = iter.klass(); if (is_allowed(subk)) { - event_subklasses.append(subk); + // We are walking the class hierarchy and saving the relevant klasses in JNI handles. + // To be allowed to store the java mirror, we must ensure that the klass and its oops are kept alive, + // and perform the store before the next safepoint. + subk->keep_alive(); + event_subklasses.append((jclass)JfrJavaSupport::local_jni_handle(subk->java_mirror(), thread)); } } } -static void transform_klasses_to_local_jni_handles(GrowableArray& event_subklasses, JavaThread* thread) { - assert(event_subklasses.is_nonempty(), "invariant"); - DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(thread)); - - for (int i = 0; i < event_subklasses.length(); ++i) { - const InstanceKlass* k = static_cast(event_subklasses.at(i)); - assert(is_allowed(k), "invariant"); - event_subklasses.at_put(i, JfrJavaSupport::local_jni_handle(k->java_mirror(), thread)); - } -} - jobject JdkJfrEvent::get_all_klasses(TRAPS) { DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(THREAD)); initialize(THREAD); @@ -126,15 +122,13 @@ jobject JdkJfrEvent::get_all_klasses(TRAPS) { } ResourceMark rm(THREAD); - GrowableArray event_subklasses(initial_array_size); + GrowableArray event_subklasses(initial_array_size); fill_klasses(event_subklasses, InstanceKlass::cast(klass), THREAD); if (event_subklasses.is_empty()) { return empty_java_util_arraylist; } - transform_klasses_to_local_jni_handles(event_subklasses, THREAD); - Handle h_array_list(THREAD, new_java_util_arraylist(THREAD)); if (h_array_list.is_null()) { return empty_java_util_arraylist; @@ -152,7 +146,7 @@ jobject JdkJfrEvent::get_all_klasses(TRAPS) { JavaValue result(T_BOOLEAN); for (int i = 0; i < event_subklasses.length(); ++i) { - const jclass clazz = (jclass)event_subklasses.at(i); + const jclass clazz = event_subklasses.at(i); assert(JdkJfrEvent::is_subklass(clazz), "invariant"); JfrJavaArguments args(&result, array_list_klass, add_method_sym, add_method_sig_sym); args.set_receiver(h_array_list()); diff --git a/src/hotspot/share/oops/klass.hpp b/src/hotspot/share/oops/klass.hpp index 4fc670d85f1f7..e5447ed41eece 100644 --- a/src/hotspot/share/oops/klass.hpp +++ b/src/hotspot/share/oops/klass.hpp @@ -581,6 +581,8 @@ class Klass : public Metadata { inline oop klass_holder() const; + inline void keep_alive() const; + protected: // Error handling when length > max_length or length < 0 diff --git a/src/hotspot/share/oops/klass.inline.hpp b/src/hotspot/share/oops/klass.inline.hpp index ea9af6d69289f..828c5697da2bf 100644 --- a/src/hotspot/share/oops/klass.inline.hpp +++ b/src/hotspot/share/oops/klass.inline.hpp @@ -37,6 +37,13 @@ inline oop Klass::klass_holder() const { return class_loader_data()->holder(); } +inline void Klass::keep_alive() const { + // Resolving the holder (a WeakHandle) will keep the klass alive until the next safepoint. + // Making the klass's CLD handle oops (e.g. the java_mirror), safe to store in the object + // graph and its roots (e.g. Handles). + static_cast(klass_holder()); +} + inline bool Klass::is_non_strong_hidden() const { return is_hidden() && class_loader_data()->has_class_mirror_holder(); } @@ -52,6 +59,7 @@ inline bool Klass::is_loader_alive() const { return class_loader_data()->is_alive(); } +// Loading the java_mirror does not keep its holder alive. See Klass::keep_alive(). inline oop Klass::java_mirror() const { return _java_mirror.resolve(); } From a6c85daa1c5e685ab64cbf9860a022aaa4a0d7f8 Mon Sep 17 00:00:00 2001 From: Christian Hagedorn Date: Thu, 7 Nov 2024 07:05:20 +0000 Subject: [PATCH 13/22] 8342945: Replace predicate walking code in get_assertion_predicates() used for Loop Unswitching and cleaning useless Template Assertion Predicates with a predicate visitor Reviewed-by: thartmann, roland, kvn --- src/hotspot/share/opto/loopPredicate.cpp | 44 ++++++------------------ src/hotspot/share/opto/loopnode.cpp | 4 +-- src/hotspot/share/opto/loopnode.hpp | 4 +-- src/hotspot/share/opto/predicates.hpp | 22 ++++++++++++ 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/hotspot/share/opto/loopPredicate.cpp b/src/hotspot/share/opto/loopPredicate.cpp index 9a639b1f9a1f2..d60d44ff09ce6 100644 --- a/src/hotspot/share/opto/loopPredicate.cpp +++ b/src/hotspot/share/opto/loopPredicate.cpp @@ -288,7 +288,7 @@ IfProjNode* PhaseIdealLoop::clone_parse_predicate_to_unswitched_loop(ParsePredic // cloned predicates. void PhaseIdealLoop::clone_assertion_predicates_to_unswitched_loop(IdealLoopTree* loop, const Node_List& old_new, Deoptimization::DeoptReason reason, - IfProjNode* old_predicate_proj, + ParsePredicateSuccessProj* old_parse_predicate_proj, ParsePredicateSuccessProj* fast_loop_parse_predicate_proj, ParsePredicateSuccessProj* slow_loop_parse_predicate_proj) { assert(fast_loop_parse_predicate_proj->in(0)->is_ParsePredicate() && @@ -297,17 +297,15 @@ void PhaseIdealLoop::clone_assertion_predicates_to_unswitched_loop(IdealLoopTree // and doing loop unrolling. Push the original predicates on a list to later process them in reverse order to keep the // original predicate order. Unique_Node_List list; - get_assertion_predicates(old_predicate_proj, list); + get_assertion_predicates(old_parse_predicate_proj, list); Node_List to_process; - IfNode* iff = old_predicate_proj->in(0)->as_If(); - IfProjNode* uncommon_proj = iff->proj_out(1 - old_predicate_proj->as_Proj()->_con)->as_IfProj(); // Process in reverse order such that 'create_new_if_for_predicate' can be used in // 'clone_assertion_predicate_for_unswitched_loops' and the original order is maintained. for (int i = list.size() - 1; i >= 0; i--) { Node* predicate = list.at(i); assert(predicate->in(0)->is_If(), "must be If node"); - iff = predicate->in(0)->as_If(); + IfNode* iff = predicate->in(0)->as_If(); assert(predicate->is_Proj() && predicate->as_Proj()->is_IfProj(), "predicate must be a projection of an if node"); IfProjNode* predicate_proj = predicate->as_IfProj(); @@ -337,34 +335,14 @@ void PhaseIdealLoop::clone_assertion_predicates_to_unswitched_loop(IdealLoopTree } // Put all Assertion Predicate projections on a list, starting at 'predicate' and going up in the tree. If 'get_opaque' -// is set, then the OpaqueTemplateAssertionPredicate nodes of the Assertion Predicates are put on the list instead of -// the projections. -void PhaseIdealLoop::get_assertion_predicates(Node* predicate, Unique_Node_List& list, bool get_opaque) { - ParsePredicateNode* parse_predicate = predicate->in(0)->as_ParsePredicate(); - ProjNode* uncommon_proj = parse_predicate->proj_out(1 - predicate->as_Proj()->_con); - Node* rgn = uncommon_proj->unique_ctrl_out(); - assert(rgn->is_Region() || rgn->is_Call(), "must be a region or call uct"); - predicate = parse_predicate->in(0); - while (predicate != nullptr && predicate->is_Proj() && predicate->in(0)->is_If()) { - IfNode* iff = predicate->in(0)->as_If(); - uncommon_proj = iff->proj_out(1 - predicate->as_Proj()->_con); - if (uncommon_proj->unique_ctrl_out() != rgn) { - break; - } - Node* bol = iff->in(1); - assert(!bol->is_OpaqueInitializedAssertionPredicate(), "should not find an Initialized Assertion Predicate"); - if (bol->is_OpaqueTemplateAssertionPredicate()) { - assert(assertion_predicate_has_loop_opaque_node(iff), "must find OpaqueLoop* nodes"); - if (get_opaque) { - // Collect the OpaqueTemplateAssertionPredicateNode. - list.push(bol); - } else { - // Collect the predicate projection. - list.push(predicate); - } - } - predicate = predicate->in(0)->in(0); - } +// is set, then the OpaqueTemplateAssertionPredicateNode nodes of the Assertion Predicates are put on the list instead +// of the projections. +void PhaseIdealLoop::get_assertion_predicates(ParsePredicateSuccessProj* parse_predicate_proj, Unique_Node_List& list, + const bool get_opaque) { + Deoptimization::DeoptReason deopt_reason = parse_predicate_proj->in(0)->as_ParsePredicate()->deopt_reason(); + PredicateBlockIterator predicate_iterator(parse_predicate_proj, deopt_reason); + TemplateAssertionPredicateCollector template_assertion_predicate_collector(list, get_opaque); + predicate_iterator.for_each(template_assertion_predicate_collector); } // Clone an Assertion Predicate for an unswitched loop. OpaqueLoopInit and OpaqueLoopStride nodes are cloned and uncommon diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index e2db179676df8..225d04366004f 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -4469,7 +4469,7 @@ void PhaseIdealLoop::collect_useful_template_assertion_predicates_for_loop(Ideal if (UseProfiledLoopPredicate) { const PredicateBlock* profiled_loop_predicate_block = predicates.profiled_loop_predicate_block(); if (profiled_loop_predicate_block->has_parse_predicate()) { - IfProjNode* parse_predicate_proj = profiled_loop_predicate_block->parse_predicate_success_proj(); + ParsePredicateSuccessProj* parse_predicate_proj = profiled_loop_predicate_block->parse_predicate_success_proj(); get_assertion_predicates(parse_predicate_proj, useful_predicates, true); } } @@ -4477,7 +4477,7 @@ void PhaseIdealLoop::collect_useful_template_assertion_predicates_for_loop(Ideal if (UseLoopPredicate) { const PredicateBlock* loop_predicate_block = predicates.loop_predicate_block(); if (loop_predicate_block->has_parse_predicate()) { - IfProjNode* parse_predicate_proj = loop_predicate_block->parse_predicate_success_proj(); + ParsePredicateSuccessProj* parse_predicate_proj = loop_predicate_block->parse_predicate_success_proj(); get_assertion_predicates(parse_predicate_proj, useful_predicates, true); } } diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 487c447328089..d4f8e2e254a12 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -947,7 +947,7 @@ class PhaseIdealLoop : public PhaseTransform { DEBUG_ONLY(static bool assertion_predicate_has_loop_opaque_node(IfNode* iff);) private: DEBUG_ONLY(static void count_opaque_loop_nodes(Node* n, uint& init, uint& stride);) - static void get_assertion_predicates(Node* predicate, Unique_Node_List& list, bool get_opaque = false); + static void get_assertion_predicates(ParsePredicateSuccessProj* parse_predicate_proj, Unique_Node_List& list, bool get_opaque = false); void update_main_loop_assertion_predicates(Node* ctrl, CountedLoopNode* loop_head, Node* init, int stride_con); void initialize_assertion_predicates_for_peeled_loop(CountedLoopNode* peeled_loop_head, CountedLoopNode* remaining_loop_head, @@ -1672,7 +1672,7 @@ class PhaseIdealLoop : public PhaseTransform { IfProjNode* clone_parse_predicate_to_unswitched_loop(ParsePredicateSuccessProj* parse_predicate_proj, Node* new_entry, Deoptimization::DeoptReason reason, bool slow_loop); void clone_assertion_predicates_to_unswitched_loop(IdealLoopTree* loop, const Node_List& old_new, - Deoptimization::DeoptReason reason, IfProjNode* old_predicate_proj, + Deoptimization::DeoptReason reason, ParsePredicateSuccessProj* old_parse_predicate_proj, ParsePredicateSuccessProj* fast_loop_parse_predicate_proj, ParsePredicateSuccessProj* slow_loop_parse_predicate_proj); IfProjNode* clone_assertion_predicate_for_unswitched_loops(IfNode* template_assertion_predicate, IfProjNode* predicate, diff --git a/src/hotspot/share/opto/predicates.hpp b/src/hotspot/share/opto/predicates.hpp index f57948f8ab902..dc834a18399d2 100644 --- a/src/hotspot/share/opto/predicates.hpp +++ b/src/hotspot/share/opto/predicates.hpp @@ -1010,4 +1010,26 @@ class CreateAssertionPredicatesVisitor : public PredicateVisitor { } }; +// This visitor collects all Template Assertion Predicates If nodes or the corresponding Opaque nodes, depending on the +// provided 'get_opaque' flag, to the provided list. +class TemplateAssertionPredicateCollector : public PredicateVisitor { + Unique_Node_List& _list; + const bool _get_opaque; + + public: + TemplateAssertionPredicateCollector(Unique_Node_List& list, const bool get_opaque) + : _list(list), + _get_opaque(get_opaque) {} + + using PredicateVisitor::visit; + + void visit(const TemplateAssertionPredicate& template_assertion_predicate) override { + if (_get_opaque) { + _list.push(template_assertion_predicate.opaque_node()); + } else { + _list.push(template_assertion_predicate.tail()); + } + } +}; + #endif // SHARE_OPTO_PREDICATES_HPP From 619b4d596634ee84b9bf5884b97a69eb01661657 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Thu, 7 Nov 2024 07:25:41 +0000 Subject: [PATCH 14/22] 8334107: Specification for MemorySegment::get/setString could use some clarification Reviewed-by: jvernee --- .../share/classes/java/lang/foreign/MemorySegment.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/java.base/share/classes/java/lang/foreign/MemorySegment.java b/src/java.base/share/classes/java/lang/foreign/MemorySegment.java index 7ecbe2504f373..cd38bd1922775 100644 --- a/src/java.base/share/classes/java/lang/foreign/MemorySegment.java +++ b/src/java.base/share/classes/java/lang/foreign/MemorySegment.java @@ -1307,7 +1307,8 @@ MemorySegment reinterpret(long newSize, * @param offset offset in bytes (relative to this segment address) at which this * access operation will occur * @param charset the charset used to {@linkplain Charset#newDecoder() decode} the - * string bytes + * string bytes. The {@code charset} must be a + * {@linkplain StandardCharsets standard charset} * @return a Java string constructed from the bytes read from the given starting * address up to (but not including) the first {@code '\0'} terminator * character (assuming one is found) @@ -1375,7 +1376,9 @@ MemorySegment reinterpret(long newSize, * access operation will occur, the final address of this write * operation can be expressed as {@code address() + offset} * @param str the Java string to be written into this segment - * @param charset the charset used to {@linkplain Charset#newEncoder() encode} the string bytes + * @param charset the charset used to {@linkplain Charset#newEncoder() encode} the + * string bytes. The {@code charset} must be a + * {@linkplain StandardCharsets standard charset} * @throws IndexOutOfBoundsException if {@code offset < 0} * @throws IndexOutOfBoundsException if {@code offset > byteSize() - (B + N)}, where: *
    From 0e1c1b793d43064aabe9571057284899c9580f30 Mon Sep 17 00:00:00 2001 From: theoweidmannoracle Date: Thu, 7 Nov 2024 07:46:52 +0000 Subject: [PATCH 15/22] 8343452: Incorrect WINDOWS build variable is used in macroAssembler_x86.cpp Reviewed-by: kvn, chagedorn, jwaters --- src/hotspot/cpu/x86/macroAssembler_x86.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/cpu/x86/macroAssembler_x86.cpp b/src/hotspot/cpu/x86/macroAssembler_x86.cpp index 2da8fe68502ee..d6c6735abc33e 100644 --- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp +++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp @@ -4092,7 +4092,7 @@ RegSet MacroAssembler::call_clobbered_gp_registers() { RegSet regs; #ifdef _LP64 regs += RegSet::of(rax, rcx, rdx); -#ifndef WINDOWS +#ifndef _WINDOWS regs += RegSet::of(rsi, rdi); #endif regs += RegSet::range(r8, r11); @@ -4109,7 +4109,7 @@ RegSet MacroAssembler::call_clobbered_gp_registers() { XMMRegSet MacroAssembler::call_clobbered_xmm_registers() { int num_xmm_registers = XMMRegister::available_xmm_registers(); -#if defined(WINDOWS) && defined(_LP64) +#if defined(_WINDOWS) && defined(_LP64) XMMRegSet result = XMMRegSet::range(xmm0, xmm5); if (num_xmm_registers > 16) { result += XMMRegSet::range(xmm16, as_XMMRegister(num_xmm_registers - 1)); From c3df050b88ecef123199a4e96f6d9884d064ae45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6len?= Date: Thu, 7 Nov 2024 08:16:17 +0000 Subject: [PATCH 16/22] 8343726: [BACKOUT] NMT should not use ThreadCritical Reviewed-by: shade, dholmes --- src/hotspot/os/posix/perfMemory_posix.cpp | 2 +- src/hotspot/os/windows/os_windows.cpp | 3 +-- src/hotspot/os/windows/perfMemory_windows.cpp | 2 +- src/hotspot/share/nmt/memBaseline.cpp | 2 +- src/hotspot/share/nmt/memReporter.cpp | 2 +- src/hotspot/share/nmt/memTracker.hpp | 19 ++++++++++--------- src/hotspot/share/nmt/memoryFileTracker.cpp | 11 +++++++++++ src/hotspot/share/nmt/memoryFileTracker.hpp | 7 +++++++ src/hotspot/share/nmt/nmtCommon.hpp | 10 ---------- src/hotspot/share/nmt/threadStackTracker.cpp | 5 +++-- .../share/nmt/virtualMemoryTracker.cpp | 4 ++-- src/hotspot/share/runtime/mutexLocker.cpp | 4 +--- src/hotspot/share/runtime/mutexLocker.hpp | 1 - src/hotspot/share/runtime/os.cpp | 8 ++++---- src/hotspot/share/utilities/vmError.cpp | 4 ---- 15 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/hotspot/os/posix/perfMemory_posix.cpp b/src/hotspot/os/posix/perfMemory_posix.cpp index 17bf63092c208..4eb46169878cd 100644 --- a/src/hotspot/os/posix/perfMemory_posix.cpp +++ b/src/hotspot/os/posix/perfMemory_posix.cpp @@ -1086,7 +1086,7 @@ static char* mmap_create_shared(size_t size) { static void unmap_shared(char* addr, size_t bytes) { int res; if (MemTracker::enabled()) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; res = ::munmap(addr, bytes); if (res == 0) { MemTracker::record_virtual_memory_release((address)addr, bytes); diff --git a/src/hotspot/os/windows/os_windows.cpp b/src/hotspot/os/windows/os_windows.cpp index 71efb57e0f2c5..4dafef0c09823 100644 --- a/src/hotspot/os/windows/os_windows.cpp +++ b/src/hotspot/os/windows/os_windows.cpp @@ -3825,8 +3825,7 @@ bool os::pd_release_memory(char* addr, size_t bytes) { if (err != nullptr) { log_warning(os)("bad release: [" PTR_FORMAT "-" PTR_FORMAT "): %s", p2i(start), p2i(end), err); #ifdef ASSERT - fileStream fs(stdout); - os::print_memory_mappings((char*)start, bytes, &fs); + os::print_memory_mappings((char*)start, bytes, tty); assert(false, "bad release: [" PTR_FORMAT "-" PTR_FORMAT "): %s", p2i(start), p2i(end), err); #endif return false; diff --git a/src/hotspot/os/windows/perfMemory_windows.cpp b/src/hotspot/os/windows/perfMemory_windows.cpp index 959be982fabbe..06b057315cbdd 100644 --- a/src/hotspot/os/windows/perfMemory_windows.cpp +++ b/src/hotspot/os/windows/perfMemory_windows.cpp @@ -1803,7 +1803,7 @@ void PerfMemory::detach(char* addr, size_t bytes) { if (MemTracker::enabled()) { // it does not go through os api, the operation has to record from here - NmtVirtualMemoryLocker ml; + ThreadCritical tc; remove_file_mapping(addr); MemTracker::record_virtual_memory_release((address)addr, bytes); } else { diff --git a/src/hotspot/share/nmt/memBaseline.cpp b/src/hotspot/share/nmt/memBaseline.cpp index 270601e9c057b..6f82b2de9f106 100644 --- a/src/hotspot/share/nmt/memBaseline.cpp +++ b/src/hotspot/share/nmt/memBaseline.cpp @@ -141,7 +141,7 @@ void MemBaseline::baseline_summary() { MallocMemorySummary::snapshot(&_malloc_memory_snapshot); VirtualMemorySummary::snapshot(&_virtual_memory_snapshot); { - NmtVirtualMemoryLocker ml; + MemoryFileTracker::Instance::Locker lock; MemoryFileTracker::Instance::summary_snapshot(&_virtual_memory_snapshot); } diff --git a/src/hotspot/share/nmt/memReporter.cpp b/src/hotspot/share/nmt/memReporter.cpp index 15767da276c1b..6ce6206ebcc2a 100644 --- a/src/hotspot/share/nmt/memReporter.cpp +++ b/src/hotspot/share/nmt/memReporter.cpp @@ -465,7 +465,7 @@ void MemDetailReporter::report_virtual_memory_region(const ReservedMemoryRegion* void MemDetailReporter::report_memory_file_allocations() { stringStream st; { - NmtVirtualMemoryLocker ml; + MemoryFileTracker::Instance::Locker lock; MemoryFileTracker::Instance::print_all_reports_on(&st, scale()); } output()->print_raw(st.freeze()); diff --git a/src/hotspot/share/nmt/memTracker.hpp b/src/hotspot/share/nmt/memTracker.hpp index 92640430e1c7b..6ba1db2e7ffe6 100644 --- a/src/hotspot/share/nmt/memTracker.hpp +++ b/src/hotspot/share/nmt/memTracker.hpp @@ -31,6 +31,7 @@ #include "nmt/threadStackTracker.hpp" #include "nmt/virtualMemoryTracker.hpp" #include "runtime/mutexLocker.hpp" +#include "runtime/threadCritical.hpp" #include "utilities/debug.hpp" #include "utilities/nativeCallStack.hpp" @@ -124,7 +125,7 @@ class MemTracker : AllStatic { assert_post_init(); if (!enabled()) return; if (addr != nullptr) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; VirtualMemoryTracker::add_reserved_region((address)addr, size, stack, mem_tag); } } @@ -150,7 +151,7 @@ class MemTracker : AllStatic { assert_post_init(); if (!enabled()) return; if (addr != nullptr) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; VirtualMemoryTracker::add_reserved_region((address)addr, size, stack, mem_tag); VirtualMemoryTracker::add_committed_region((address)addr, size, stack); } @@ -161,7 +162,7 @@ class MemTracker : AllStatic { assert_post_init(); if (!enabled()) return; if (addr != nullptr) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; VirtualMemoryTracker::add_committed_region((address)addr, size, stack); } } @@ -169,7 +170,7 @@ class MemTracker : AllStatic { static inline MemoryFileTracker::MemoryFile* register_file(const char* descriptive_name) { assert_post_init(); if (!enabled()) return nullptr; - NmtVirtualMemoryLocker ml; + MemoryFileTracker::Instance::Locker lock; return MemoryFileTracker::Instance::make_file(descriptive_name); } @@ -177,7 +178,7 @@ class MemTracker : AllStatic { assert_post_init(); if (!enabled()) return; assert(file != nullptr, "must be"); - NmtVirtualMemoryLocker ml; + MemoryFileTracker::Instance::Locker lock; MemoryFileTracker::Instance::free_file(file); } @@ -186,7 +187,7 @@ class MemTracker : AllStatic { assert_post_init(); if (!enabled()) return; assert(file != nullptr, "must be"); - NmtVirtualMemoryLocker ml; + MemoryFileTracker::Instance::Locker lock; MemoryFileTracker::Instance::allocate_memory(file, offset, size, stack, mem_tag); } @@ -195,7 +196,7 @@ class MemTracker : AllStatic { assert_post_init(); if (!enabled()) return; assert(file != nullptr, "must be"); - NmtVirtualMemoryLocker ml; + MemoryFileTracker::Instance::Locker lock; MemoryFileTracker::Instance::free_memory(file, offset, size); } @@ -209,7 +210,7 @@ class MemTracker : AllStatic { assert_post_init(); if (!enabled()) return; if (addr != nullptr) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; VirtualMemoryTracker::split_reserved_region((address)addr, size, split, mem_tag, split_tag); } } @@ -218,7 +219,7 @@ class MemTracker : AllStatic { assert_post_init(); if (!enabled()) return; if (addr != nullptr) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; VirtualMemoryTracker::set_reserved_region_type((address)addr, mem_tag); } } diff --git a/src/hotspot/share/nmt/memoryFileTracker.cpp b/src/hotspot/share/nmt/memoryFileTracker.cpp index ab4f8b6d1f3c6..ede483ed33727 100644 --- a/src/hotspot/share/nmt/memoryFileTracker.cpp +++ b/src/hotspot/share/nmt/memoryFileTracker.cpp @@ -29,11 +29,13 @@ #include "nmt/nmtCommon.hpp" #include "nmt/nmtNativeCallStackStorage.hpp" #include "nmt/vmatree.hpp" +#include "runtime/mutex.hpp" #include "utilities/growableArray.hpp" #include "utilities/nativeCallStack.hpp" #include "utilities/ostream.hpp" MemoryFileTracker* MemoryFileTracker::Instance::_tracker = nullptr; +PlatformMutex* MemoryFileTracker::Instance::_mutex = nullptr; MemoryFileTracker::MemoryFileTracker(bool is_detailed_mode) : _stack_storage(is_detailed_mode), _files() {} @@ -130,6 +132,7 @@ bool MemoryFileTracker::Instance::initialize(NMT_TrackingLevel tracking_level) { _tracker = static_cast(os::malloc(sizeof(MemoryFileTracker), mtNMT)); if (_tracker == nullptr) return false; new (_tracker) MemoryFileTracker(tracking_level == NMT_TrackingLevel::NMT_detail); + _mutex = new PlatformMutex(); return true; } @@ -190,3 +193,11 @@ void MemoryFileTracker::summary_snapshot(VirtualMemorySnapshot* snapshot) const void MemoryFileTracker::Instance::summary_snapshot(VirtualMemorySnapshot* snapshot) { _tracker->summary_snapshot(snapshot); } + +MemoryFileTracker::Instance::Locker::Locker() { + MemoryFileTracker::Instance::_mutex->lock(); +} + +MemoryFileTracker::Instance::Locker::~Locker() { + MemoryFileTracker::Instance::_mutex->unlock(); +} diff --git a/src/hotspot/share/nmt/memoryFileTracker.hpp b/src/hotspot/share/nmt/memoryFileTracker.hpp index 911f10baf9806..42902701a16df 100644 --- a/src/hotspot/share/nmt/memoryFileTracker.hpp +++ b/src/hotspot/share/nmt/memoryFileTracker.hpp @@ -30,6 +30,7 @@ #include "nmt/nmtNativeCallStackStorage.hpp" #include "nmt/virtualMemoryTracker.hpp" #include "nmt/vmatree.hpp" +#include "runtime/mutex.hpp" #include "runtime/os.inline.hpp" #include "utilities/growableArray.hpp" #include "utilities/nativeCallStack.hpp" @@ -80,8 +81,14 @@ class MemoryFileTracker { class Instance : public AllStatic { static MemoryFileTracker* _tracker; + static PlatformMutex* _mutex; public: + class Locker : public StackObj { + public: + Locker(); + ~Locker(); + }; static bool initialize(NMT_TrackingLevel tracking_level); diff --git a/src/hotspot/share/nmt/nmtCommon.hpp b/src/hotspot/share/nmt/nmtCommon.hpp index a6fd29e83f27b..3f72960f21fde 100644 --- a/src/hotspot/share/nmt/nmtCommon.hpp +++ b/src/hotspot/share/nmt/nmtCommon.hpp @@ -29,8 +29,6 @@ #include "memory/allStatic.hpp" #include "nmt/memTag.hpp" -#include "runtime/mutexLocker.hpp" -#include "runtime/thread.hpp" #include "utilities/align.hpp" #include "utilities/globalDefinitions.hpp" @@ -139,13 +137,5 @@ class NMTUtil : AllStatic { static S _strings[mt_number_of_tags]; }; -// Same as MutexLocker but can be used during VM init. -// Performs no action if given a null mutex or with detached threads. -class NmtVirtualMemoryLocker: public ConditionalMutexLocker { -public: - NmtVirtualMemoryLocker() : - ConditionalMutexLocker(NmtVirtualMemory_lock, Thread::current_or_null_safe() != nullptr, Mutex::_no_safepoint_check_flag) { - } -}; #endif // SHARE_NMT_NMTCOMMON_HPP diff --git a/src/hotspot/share/nmt/threadStackTracker.cpp b/src/hotspot/share/nmt/threadStackTracker.cpp index 42af67d64649b..6f112fa8fc5c2 100644 --- a/src/hotspot/share/nmt/threadStackTracker.cpp +++ b/src/hotspot/share/nmt/threadStackTracker.cpp @@ -29,6 +29,7 @@ #include "nmt/threadStackTracker.hpp" #include "nmt/virtualMemoryTracker.hpp" #include "runtime/os.hpp" +#include "runtime/threadCritical.hpp" #include "utilities/align.hpp" #include "utilities/debug.hpp" #include "utilities/globalDefinitions.hpp" @@ -52,7 +53,7 @@ void ThreadStackTracker::new_thread_stack(void* base, size_t size, const NativeC assert(base != nullptr, "Should have been filtered"); align_thread_stack_boundaries_inward(base, size); - NmtVirtualMemoryLocker ml; + ThreadCritical tc; VirtualMemoryTracker::add_reserved_region((address)base, size, stack, mtThreadStack); _thread_count++; } @@ -62,7 +63,7 @@ void ThreadStackTracker::delete_thread_stack(void* base, size_t size) { assert(base != nullptr, "Should have been filtered"); align_thread_stack_boundaries_inward(base, size); - NmtVirtualMemoryLocker ml; + ThreadCritical tc; VirtualMemoryTracker::remove_released_region((address)base, size); _thread_count--; } diff --git a/src/hotspot/share/nmt/virtualMemoryTracker.cpp b/src/hotspot/share/nmt/virtualMemoryTracker.cpp index 8b0f2f4d7a4c3..d298381f1038f 100644 --- a/src/hotspot/share/nmt/virtualMemoryTracker.cpp +++ b/src/hotspot/share/nmt/virtualMemoryTracker.cpp @@ -30,6 +30,7 @@ #include "nmt/threadStackTracker.hpp" #include "nmt/virtualMemoryTracker.hpp" #include "runtime/os.hpp" +#include "runtime/threadCritical.hpp" #include "utilities/ostream.hpp" VirtualMemorySnapshot VirtualMemorySummary::_snapshot; @@ -620,7 +621,6 @@ class SnapshotThreadStackWalker : public VirtualMemoryWalker { SnapshotThreadStackWalker() {} bool do_allocation_site(const ReservedMemoryRegion* rgn) { - assert_lock_strong(NmtVirtualMemory_lock); if (rgn->mem_tag() == mtThreadStack) { address stack_bottom = rgn->thread_stack_uncommitted_bottom(); address committed_start; @@ -661,7 +661,7 @@ void VirtualMemoryTracker::snapshot_thread_stacks() { bool VirtualMemoryTracker::walk_virtual_memory(VirtualMemoryWalker* walker) { assert(_reserved_regions != nullptr, "Sanity check"); - NmtVirtualMemoryLocker ml; + ThreadCritical tc; // Check that the _reserved_regions haven't been deleted. if (_reserved_regions != nullptr) { LinkedListNode* head = _reserved_regions->head(); diff --git a/src/hotspot/share/runtime/mutexLocker.cpp b/src/hotspot/share/runtime/mutexLocker.cpp index 76d0674e8c6f8..a0a6e5626e45b 100644 --- a/src/hotspot/share/runtime/mutexLocker.cpp +++ b/src/hotspot/share/runtime/mutexLocker.cpp @@ -135,7 +135,6 @@ Mutex* SharedDecoder_lock = nullptr; Mutex* DCmdFactory_lock = nullptr; Mutex* NMTQuery_lock = nullptr; Mutex* NMTCompilationCostHistory_lock = nullptr; -Mutex* NmtVirtualMemory_lock = nullptr; #if INCLUDE_CDS #if INCLUDE_JVMTI @@ -294,11 +293,10 @@ void mutex_init() { MUTEX_DEFN(CodeHeapStateAnalytics_lock , PaddedMutex , safepoint); MUTEX_DEFN(ThreadsSMRDelete_lock , PaddedMonitor, service-2); // Holds ConcurrentHashTableResize_lock MUTEX_DEFN(ThreadIdTableCreate_lock , PaddedMutex , safepoint); - MUTEX_DEFN(SharedDecoder_lock , PaddedMutex , service-5); // Must be lower than NmtVirtualMemory_lock due to MemTracker::print_containing_region + MUTEX_DEFN(SharedDecoder_lock , PaddedMutex , tty-1); MUTEX_DEFN(DCmdFactory_lock , PaddedMutex , nosafepoint); MUTEX_DEFN(NMTQuery_lock , PaddedMutex , safepoint); MUTEX_DEFN(NMTCompilationCostHistory_lock , PaddedMutex , nosafepoint); - MUTEX_DEFN(NmtVirtualMemory_lock , PaddedMutex , service-4); #if INCLUDE_CDS #if INCLUDE_JVMTI MUTEX_DEFN(CDSClassFileStream_lock , PaddedMutex , safepoint); diff --git a/src/hotspot/share/runtime/mutexLocker.hpp b/src/hotspot/share/runtime/mutexLocker.hpp index cc465bfc1915a..7cca1d94bf2e0 100644 --- a/src/hotspot/share/runtime/mutexLocker.hpp +++ b/src/hotspot/share/runtime/mutexLocker.hpp @@ -117,7 +117,6 @@ extern Mutex* SharedDecoder_lock; // serializes access to the dec extern Mutex* DCmdFactory_lock; // serialize access to DCmdFactory information extern Mutex* NMTQuery_lock; // serialize NMT Dcmd queries extern Mutex* NMTCompilationCostHistory_lock; // guards NMT compilation cost history -extern Mutex* NmtVirtualMemory_lock; // guards NMT virtual memory updates #if INCLUDE_CDS #if INCLUDE_JVMTI extern Mutex* CDSClassFileStream_lock; // FileMapInfo::open_stream_for_jvmti diff --git a/src/hotspot/share/runtime/os.cpp b/src/hotspot/share/runtime/os.cpp index 31f671e742e7a..d8e539ca115cd 100644 --- a/src/hotspot/share/runtime/os.cpp +++ b/src/hotspot/share/runtime/os.cpp @@ -2186,7 +2186,7 @@ bool os::uncommit_memory(char* addr, size_t bytes, bool executable) { assert_nonempty_range(addr, bytes); bool res; if (MemTracker::enabled()) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; res = pd_uncommit_memory(addr, bytes, executable); if (res) { MemTracker::record_virtual_memory_uncommit((address)addr, bytes); @@ -2208,7 +2208,7 @@ bool os::release_memory(char* addr, size_t bytes) { assert_nonempty_range(addr, bytes); bool res; if (MemTracker::enabled()) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; res = pd_release_memory(addr, bytes); if (res) { MemTracker::record_virtual_memory_release((address)addr, bytes); @@ -2293,7 +2293,7 @@ char* os::map_memory(int fd, const char* file_name, size_t file_offset, bool os::unmap_memory(char *addr, size_t bytes) { bool result; if (MemTracker::enabled()) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; result = pd_unmap_memory(addr, bytes); if (result) { MemTracker::record_virtual_memory_release((address)addr, bytes); @@ -2332,7 +2332,7 @@ char* os::reserve_memory_special(size_t size, size_t alignment, size_t page_size bool os::release_memory_special(char* addr, size_t bytes) { bool res; if (MemTracker::enabled()) { - NmtVirtualMemoryLocker ml; + ThreadCritical tc; res = pd_release_memory_special(addr, bytes); if (res) { MemTracker::record_virtual_memory_release((address)addr, bytes); diff --git a/src/hotspot/share/utilities/vmError.cpp b/src/hotspot/share/utilities/vmError.cpp index 0df77689a048f..23db59f766ae6 100644 --- a/src/hotspot/share/utilities/vmError.cpp +++ b/src/hotspot/share/utilities/vmError.cpp @@ -717,10 +717,6 @@ void VMError::report(outputStream* st, bool _verbose) { address lastpc = nullptr; BEGIN - if (MemTracker::enabled() && NmtVirtualMemory_lock != nullptr && NmtVirtualMemory_lock->owned_by_self()) { - // Manually unlock to avoid reentrancy due to mallocs in detailed mode. - NmtVirtualMemory_lock->unlock(); - } STEP("printing fatal error message") st->print_cr("#"); From 592a48b163ed582872b686e7a606cf8b96fcbcbc Mon Sep 17 00:00:00 2001 From: Tobias Holenstein Date: Thu, 7 Nov 2024 08:55:55 +0000 Subject: [PATCH 17/22] 8321997: Increase upper limit of LoopOptsCount flag Reviewed-by: shade, chagedorn --- src/hotspot/share/opto/c2_globals.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp index 7f6b7c2dcebae..45a067a830ba6 100644 --- a/src/hotspot/share/opto/c2_globals.hpp +++ b/src/hotspot/share/opto/c2_globals.hpp @@ -419,7 +419,7 @@ \ product(intx, LoopOptsCount, 43, \ "Set level of loop optimization for tier 1 compiles") \ - range(5, 43) \ + range(5, max_jint) \ \ product(bool, OptimizeUnstableIf, true, DIAGNOSTIC, \ "Optimize UnstableIf traps") \ From 7620b129888d57514d9ef588e0681f1d43377236 Mon Sep 17 00:00:00 2001 From: Theo Weidmann Date: Thu, 7 Nov 2024 10:04:03 +0000 Subject: [PATCH 18/22] 8323803: ConstantOopReadValue::print_on should print 'null' instead of 'nullptr' Reviewed-by: chagedorn, kvn --- src/hotspot/share/code/debugInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/code/debugInfo.cpp b/src/hotspot/share/code/debugInfo.cpp index 5376e8ade9baa..ccee142c93808 100644 --- a/src/hotspot/share/code/debugInfo.cpp +++ b/src/hotspot/share/code/debugInfo.cpp @@ -382,7 +382,7 @@ void ConstantOopReadValue::print_on(outputStream* st) const { if (value()() != nullptr) { value()()->print_value_on(st); } else { - st->print("nullptr"); + st->print("null"); } } From 4244682309e7ae1be892280dfd6a6f70ccecc760 Mon Sep 17 00:00:00 2001 From: Nizar Benalla Date: Thu, 7 Nov 2024 10:30:12 +0000 Subject: [PATCH 19/22] 8339190: Parameter arrays that are capped during annotation processing report incorrect length Reviewed-by: vromero --- .../com/sun/tools/javac/jvm/ClassFile.java | 1 + .../com/sun/tools/javac/jvm/ClassWriter.java | 8 + .../tools/javac/resources/compiler.properties | 5 + .../annotations/ParameterArrayLimit.java | 150 ++++++++++++++++++ .../tools/javac/diags/examples.not-yet.txt | 1 + 5 files changed, 165 insertions(+) create mode 100644 test/langtools/tools/javac/annotations/ParameterArrayLimit.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassFile.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassFile.java index 7e8b96d58560f..c7e71f74d8a78 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassFile.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassFile.java @@ -103,6 +103,7 @@ public class ClassFile { public static final int MAX_STACK = 0xffff; public static final int PREVIEW_MINOR_VERSION = 0xffff; + public static final int MAX_ANNOTATIONS = 0xffff; public enum Version { V45_3(45, 3), // base level for all attributes diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java index 6679bb43fb807..d70032353b686 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java @@ -649,11 +649,19 @@ void writeCompoundAttribute(Attribute.Compound c) { databuf.appendChar(poolWriter.putDescriptor(c.type)); databuf.appendChar(c.values.length()); for (Pair p : c.values) { + checkAnnotationArraySizeInternal(p); databuf.appendChar(poolWriter.putName(p.fst.name)); p.snd.accept(awriter); } } + private void checkAnnotationArraySizeInternal(Pair p) { + if (p.snd instanceof Attribute.Array arrAttr && + arrAttr.values.length > ClassFile.MAX_ANNOTATIONS) { + log.error(Errors.AnnotationArrayTooLarge(p.fst.owner)); + } + } + void writeTypeAnnotation(Attribute.TypeCompound c) { writePosition(c.position); writeCompoundAttribute(c); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties index a991ae60601e9..a24dd2f95bd48 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties @@ -887,6 +887,10 @@ compiler.err.limit.stack=\ compiler.err.limit.string=\ constant string too long +# 0: symbol +compiler.err.annotation.array.too.large=\ + Annotation array element too large in \"{0}\" + # 0: string compiler.err.limit.string.overflow=\ UTF8 representation for string \"{0}...\" is too long for the constant pool @@ -2235,6 +2239,7 @@ compiler.warn.proc.duplicate.option.name=\ compiler.warn.proc.duplicate.supported.annotation=\ Duplicate supported annotation interface ''{0}'' returned by annotation processor ''{1}'' + # 0: string compiler.warn.proc.redundant.types.with.wildcard=\ Annotation processor ''{0}'' redundantly supports both ''*'' and other annotation interfaces diff --git a/test/langtools/tools/javac/annotations/ParameterArrayLimit.java b/test/langtools/tools/javac/annotations/ParameterArrayLimit.java new file mode 100644 index 0000000000000..6e782dd0b8391 --- /dev/null +++ b/test/langtools/tools/javac/annotations/ParameterArrayLimit.java @@ -0,0 +1,150 @@ +/* + * 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * 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. + */ + +/* + * @test + * @summary Check if error is thrown if annotation array exceeds limit + * @library /tools/lib + * @run main ParameterArrayLimit + */ + +import java.io.BufferedWriter; +import java.io.IOException; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.text.MessageFormat; +import java.util.Collections; +import java.util.List; +import javax.tools.*; + +import com.sun.source.util.JavacTask; + +public class ParameterArrayLimit { + + public static void main(String[] args) throws IOException { + + int[] values = new int[]{65536, 65537, 512000}; + String[] retPolicies = {"RUNTIME", "CLASS"}; + + for (var value : values) { + Path tmpDir = Paths.get(System.getProperty("java.io.tmpdir")); + + for (String retPolicy : retPolicies) { + String className = MessageFormat.format("ClassAnnotationWithLength_{0,number,#}_{1}.java", + value, + retPolicy); + Path out = tmpDir.resolve(className); + createAnnotationFile(out, value, retPolicy, false); + checkParamArrayWarning(className, out); + } + + for (String retPolicy : retPolicies) { + String className = MessageFormat.format("TypeAnnotationWithLength_{0,number,#}_{1}.java", + value, + retPolicy); + Path out = tmpDir.resolve(className); + createAnnotationFile(out, value, retPolicy, true); + checkParamArrayWarning(className, out); + } + } + } + + private static void checkParamArrayWarning(String className, Path out) throws IOException { + JavaCompiler javaCompiler = ToolProvider.getSystemJavaCompiler(); + DiagnosticCollector d = new DiagnosticCollector<>(); + JavacTask task = (JavacTask) javaCompiler.getTask( + null, + null, + d, + null, + null, + Collections.singletonList( + SimpleJavaFileObject.forSource( + URI.create("myfo:/" + className), + Files.readString(out) + ))); + task.call(); + + List> diagnosticList = d.getDiagnostics(); + if (diagnosticList.isEmpty()) { + throw new RuntimeException("No diagnostic found"); + } + + for (Diagnostic diagnostic : diagnosticList) { + if (!(diagnostic.getKind() == Diagnostic.Kind.ERROR + && diagnostic.getCode() + .equals("compiler.err.annotation.array.too.large"))) { + throw new RuntimeException("Unexpected diagnostic: " + diagnostic.getMessage(null)); + } + } + } + + private static void createAnnotationFile(Path out, int value, String retPolicy, boolean isTypeAnnotation) throws IOException { + StringBuilder sb = new StringBuilder(); + + if (isTypeAnnotation) { + sb.append(MessageFormat.format(""" + import java.lang.annotation.*; + @Retention(RetentionPolicy.{0}) + @Target(ElementType.TYPE_USE) + @interface TypeAnno '{' + long[] arr(); + '}' + """, retPolicy)); + sb.append(MessageFormat.format(""" + public class TypeAnnotationWithLength_{0,number,#}_{1}'{' + @TypeAnno(arr = '{' + """, value, retPolicy)); + } else { + sb.append(MessageFormat.format(""" + import java.lang.annotation.*; + @Retention(RetentionPolicy.{0}) + @interface MyCustomAnno '{' + String value() default "default value"; + long[] arr(); + int count() default 0; + '}' + """, retPolicy)); + sb.append(MessageFormat.format(""" + public class ClassAnnotationWithLength_{0,number,#}_{1}'{' + @MyCustomAnno(value = "custom", count = 42, arr = '{' + """, value, retPolicy)); + } + + sb.append("-1,".repeat(Math.max(0, value - 1))); + sb.append("-1})"); + + sb.append(""" + static int x = 3; + + public void myAnnotatedMethod() { } + } + """); + + try (BufferedWriter bufferedWriter = Files.newBufferedWriter(out)) { + bufferedWriter.write(sb.toString()); + } + } +} diff --git a/test/langtools/tools/javac/diags/examples.not-yet.txt b/test/langtools/tools/javac/diags/examples.not-yet.txt index 329e716a78073..e158366a17133 100644 --- a/test/langtools/tools/javac/diags/examples.not-yet.txt +++ b/test/langtools/tools/javac/diags/examples.not-yet.txt @@ -16,6 +16,7 @@ compiler.err.limit.code # Code compiler.err.limit.code.too.large.for.try.stmt # Gen compiler.err.limit.dimensions # Gen compiler.err.limit.locals # Code +compiler.err.annotation.array.too.large # Code compiler.err.limit.parameters # Gen compiler.err.limit.pool # Gen,JavaCompiler compiler.err.limit.pool.in.class # UNUSED? From f0b251d76078e8d5b47e967b0449c4cbdcb5a005 Mon Sep 17 00:00:00 2001 From: Volker Simonis Date: Thu, 7 Nov 2024 12:10:50 +0000 Subject: [PATCH 20/22] 8343531: Improve print_location for invalid heap pointers Reviewed-by: shade, tschatzl, ayang --- src/hotspot/share/gc/shared/locationPrinter.inline.hpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/gc/shared/locationPrinter.inline.hpp b/src/hotspot/share/gc/shared/locationPrinter.inline.hpp index d368aa1c6c673..ae873d52cb5d1 100644 --- a/src/hotspot/share/gc/shared/locationPrinter.inline.hpp +++ b/src/hotspot/share/gc/shared/locationPrinter.inline.hpp @@ -37,7 +37,7 @@ oop BlockLocationPrinter::base_oop_or_null(void* addr) { return cast_to_oop(addr); } - // Try to find addr using block_start. + // Try to find addr using block_start (not implemented for all GCs/generations). HeapWord* p = CollectedHeapT::heap()->block_start(addr); if (p != nullptr && CollectedHeapT::heap()->block_is_obj(p)) { if (!is_valid_obj(p)) { @@ -52,7 +52,9 @@ oop BlockLocationPrinter::base_oop_or_null(void* addr) { template bool BlockLocationPrinter::print_location(outputStream* st, void* addr) { // Check if addr points into Java heap. - if (CollectedHeapT::heap()->is_in(addr)) { + bool in_heap = CollectedHeapT::heap()->is_in(addr); + if (in_heap) { + // base_oop_or_null() might be unimplemented and return NULL for some GCs/generations oop o = base_oop_or_null(addr); if (o != nullptr) { if ((void*)o == addr) { @@ -83,6 +85,10 @@ bool BlockLocationPrinter::print_location(outputStream* st, void } #endif + if (in_heap) { + st->print_cr(PTR_FORMAT " is an unknown heap location", p2i(addr)); + return true; + } return false; } From ac82a8f89c7066fb1d379b12bcfd68053cb39ba4 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Thu, 7 Nov 2024 12:32:34 +0000 Subject: [PATCH 21/22] 8343610: InOutPathTest jpackage test produces invalid app image on macOS Reviewed-by: almatvee --- .../jdk/jpackage/test/JPackageCommand.java | 5 ++-- .../helpers/jdk/jpackage/test/MacHelper.java | 24 +++++++++++++-- .../helpers/jdk/jpackage/test/TKit.java | 14 +++++++-- .../tools/jpackage/share/InOutPathTest.java | 29 +++++++++++++++---- 4 files changed, 59 insertions(+), 13 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java index d4219704934f8..9b25c9058d184 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java @@ -897,8 +897,9 @@ public JPackageCommand setAppLayoutAsserts(AppLayoutAssert ... asserts) { } public JPackageCommand excludeAppLayoutAsserts(AppLayoutAssert... asserts) { - return setAppLayoutAsserts(Stream.of(asserts).filter(Predicate.not( - appLayoutAsserts::contains)).toArray(AppLayoutAssert[]::new)); + var asSet = Set.of(asserts); + return setAppLayoutAsserts(appLayoutAsserts.stream().filter(Predicate.not( + asSet::contains)).toArray(AppLayoutAssert[]::new)); } JPackageCommand assertAppLayout() { diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java index 8068e1d858f48..63afb6cf9f765 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java @@ -259,7 +259,7 @@ static PackageHandlers createPkgPackageHandlers() { } static void verifyBundleStructure(JPackageCommand cmd) { - Path bundleRoot; + final Path bundleRoot; if (cmd.isImagePackageType()) { bundleRoot = cmd.outputBundle(); } else { @@ -268,8 +268,26 @@ static void verifyBundleStructure(JPackageCommand cmd) { } TKit.assertDirectoryContent(bundleRoot).match(Path.of("Contents")); - TKit.assertDirectoryContent(bundleRoot.resolve("Contents")).match( - cmd.isRuntime() ? RUNTIME_BUNDLE_CONTENTS : APP_BUNDLE_CONTENTS); + + final var contentsDir = bundleRoot.resolve("Contents"); + final var expectedContentsItems = cmd.isRuntime() ? RUNTIME_BUNDLE_CONTENTS : APP_BUNDLE_CONTENTS; + + var contentsVerifier = TKit.assertDirectoryContent(contentsDir); + if (!cmd.hasArgument("--app-content")) { + contentsVerifier.match(expectedContentsItems); + } else { + // Additional content added to the bundle. + // Verify there is no period (.) char in the names of additional directories if any. + contentsVerifier.contains(expectedContentsItems); + contentsVerifier = contentsVerifier.removeAll(expectedContentsItems); + contentsVerifier.match(contentsVerifier.items().stream().filter(path -> { + if (Files.isDirectory(contentsDir.resolve(path))) { + return !path.getFileName().toString().contains("."); + } else { + return true; + } + }).collect(toSet())); + } } static String getBundleName(JPackageCommand cmd) { diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java index ca1224aafd78c..e332480e35ae4 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java @@ -786,7 +786,7 @@ public void match(Set expected) { baseDir, format(comm.common), format(comm.unique1), format(comm.unique2))); } else if (!comm.unique1.isEmpty()) { error(String.format( - "assertDirectoryContentEquals%s: Expected %s. Unexpected %s", + "assertDirectoryContentEquals(%s): Expected %s. Unexpected %s", baseDir, format(comm.common), format(comm.unique1))); } else if (!comm.unique2.isEmpty()) { error(String.format( @@ -818,12 +818,20 @@ public void contains(Set expected) { } } - public DirectoryContentVerifier removeAll(Path ... paths) { + public DirectoryContentVerifier removeAll(Collection paths) { Set newContent = new HashSet<>(content); - newContent.removeAll(List.of(paths)); + newContent.removeAll(paths); return new DirectoryContentVerifier(baseDir, newContent); } + public DirectoryContentVerifier removeAll(Path ... paths) { + return removeAll(List.of(paths)); + } + + public Set items() { + return content; + } + private DirectoryContentVerifier(Path baseDir, Set contents) { this.baseDir = baseDir; this.content = contents; diff --git a/test/jdk/tools/jpackage/share/InOutPathTest.java b/test/jdk/tools/jpackage/share/InOutPathTest.java index 8ea70a0603451..f8cb983bd1657 100644 --- a/test/jdk/tools/jpackage/share/InOutPathTest.java +++ b/test/jdk/tools/jpackage/share/InOutPathTest.java @@ -72,11 +72,20 @@ public static Collection input() { data.addAll(additionalContentInput(packageTypes, "--app-content")); } - data.addAll(List.of(new Object[][]{ - {PackageType.IMAGE.toString(), wrap(cmd -> { - additionalContent(cmd, "--app-content", cmd.outputBundle()); - }, "--app-content same as output bundle")}, - })); + if (!TKit.isOSX()) { + data.addAll(List.of(new Object[][]{ + {PackageType.IMAGE.toString(), wrap(cmd -> { + additionalContent(cmd, "--app-content", cmd.outputBundle()); + }, "--app-content same as output bundle")}, + })); + } else { + var contentsFolder = "Contents/MacOS"; + data.addAll(List.of(new Object[][]{ + {PackageType.IMAGE.toString(), wrap(cmd -> { + additionalContent(cmd, "--app-content", cmd.outputBundle().resolve(contentsFolder)); + }, String.format("--app-content same as the \"%s\" folder in the output bundle", contentsFolder))}, + })); + } if (TKit.isOSX()) { data.addAll(additionalContentInput(PackageType.MAC_DMG.toString(), @@ -172,6 +181,16 @@ private static void runTest(Set packageTypes, if (isAppImageValid(cmd)) { verifyAppImage(cmd); } + + if (cmd.hasArgument("--app-content")) { + // `--app-content` can be set to the app image directory which + // should not exist before jpackage is executed: + // jpackage --name Foo --dest output --app-content output/Foo + // Verify the directory exists after jpackage execution. + // At least this will catch the case when the value of + // `--app-content` option refers to a path unrelated to jpackage I/O. + TKit.assertDirectoryExists(Path.of(cmd.getArgumentValue("--app-content"))); + } } else { new PackageTest() .forTypes(packageTypes) From d2b681d4557109158fbbce9db995f4146c344c97 Mon Sep 17 00:00:00 2001 From: Kevin Walls Date: Thu, 7 Nov 2024 13:10:26 +0000 Subject: [PATCH 22/22] 8343730: JMX cleanups Reviewed-by: cjplummer --- .../com/sun/jmx/mbeanserver/MBeanServerDelegateImpl.java | 2 +- .../classes/javax/management/MBeanServerDelegate.java | 2 +- .../share/classes/javax/management/Notification.java | 6 +++--- .../management/remote/JMXConnectionNotification.java | 9 ++------- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/java.management/share/classes/com/sun/jmx/mbeanserver/MBeanServerDelegateImpl.java b/src/java.management/share/classes/com/sun/jmx/mbeanserver/MBeanServerDelegateImpl.java index c4837c8ea89ef..f5925a25d35e4 100644 --- a/src/java.management/share/classes/com/sun/jmx/mbeanserver/MBeanServerDelegateImpl.java +++ b/src/java.management/share/classes/com/sun/jmx/mbeanserver/MBeanServerDelegateImpl.java @@ -100,7 +100,7 @@ public MBeanServerDelegateImpl () { super(); delegateInfo = new MBeanInfo("javax.management.MBeanServerDelegate", - "Represents the MBean server from the management "+ + "Represents the MBean server from the management "+ "point of view.", MBeanServerDelegateImpl.attributeInfos, null, null,getNotificationInfo()); diff --git a/src/java.management/share/classes/javax/management/MBeanServerDelegate.java b/src/java.management/share/classes/javax/management/MBeanServerDelegate.java index 443e9a7a06c83..6e089f4164d50 100644 --- a/src/java.management/share/classes/javax/management/MBeanServerDelegate.java +++ b/src/java.management/share/classes/javax/management/MBeanServerDelegate.java @@ -31,7 +31,7 @@ import com.sun.jmx.mbeanserver.Util; /** - * Represents the MBean server from the management point of view. + * Represents the MBean server from the management point of view. * The MBeanServerDelegate MBean emits the MBeanServerNotifications when * an MBean is registered/unregistered in the MBean server. * diff --git a/src/java.management/share/classes/javax/management/Notification.java b/src/java.management/share/classes/javax/management/Notification.java index 80cf4b464d268..337628aeec82c 100644 --- a/src/java.management/share/classes/javax/management/Notification.java +++ b/src/java.management/share/classes/javax/management/Notification.java @@ -59,7 +59,7 @@ public class Notification extends EventObject { /** * @serialField type String The notification type. * A string expressed in a dot notation similar to Java properties. - * An example of a notification type is network.alarm.router + * An example of a notification type is com.sun.management.gc.notification * @serialField sequenceNumber long The notification sequence number. * A serial number which identify particular instance * of notification in the context of the notification source. @@ -83,7 +83,7 @@ public class Notification extends EventObject { /** * @serial The notification type. * A string expressed in a dot notation similar to Java properties. - * An example of a notification type is network.alarm.router + * An example of a notification type is com.sun.management.gc.notification */ private String type; @@ -239,7 +239,7 @@ public void setSequenceNumber(long sequenceNumber) { * @return The notification type. It's a string expressed in a dot notation * similar to Java properties. It is recommended that the notification type * should follow the reverse-domain-name convention used by Java package - * names. An example of a notification type is com.example.alarm.router. + * names. An example of a notification type is com.sun.management.gc.notification */ public String getType() { return type ; diff --git a/src/java.management/share/classes/javax/management/remote/JMXConnectionNotification.java b/src/java.management/share/classes/javax/management/remote/JMXConnectionNotification.java index 0ba100ec8b297..b7bb5a3dc8cac 100644 --- a/src/java.management/share/classes/javax/management/remote/JMXConnectionNotification.java +++ b/src/java.management/share/classes/javax/management/remote/JMXConnectionNotification.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 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 @@ -159,12 +159,7 @@ public JMXConnectionNotification(String type, long sequenceNumber, String message, Object userData) { - /* We don't know whether the parent class (Notification) will - throw an exception if the type or source is null, because - JMX 1.2 doesn't specify that. So we make sure it is not - null, in case it would throw the wrong exception - (e.g. IllegalArgumentException instead of - NullPointerException). Likewise for the sequence number. */ + // Do not pass null source to super, as EventObject will throw IllegalArgumentException. super((String) nonNull(type), nonNull(source), Math.max(0, sequenceNumber),