From 88775f95f2b25323a75c5acad43ec9131c5e80fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Gr=C3=B6nlund?= Date: Wed, 17 Jul 2024 12:16:22 +0000 Subject: [PATCH 1/3] 8334781: JFR crash: assert(((((JfrTraceIdBits::load(klass)) & ((JfrTraceIdEpoch::this_epoch_method_and_class_bits()))) != 0))) failed: invariant Reviewed-by: egahlin Backport-of: 67979eb0771ff834d6d3d18ba5a8bfe161cfc2ce --- .../recorder/checkpoint/types/jfrTypeSet.cpp | 4 ++++ .../checkpoint/types/jfrTypeSetUtils.hpp | 4 ++++ .../checkpoint/types/traceid/jfrTraceId.cpp | 18 +++++++++--------- .../traceid/jfrTraceIdLoadBarrier.inline.hpp | 4 ++-- .../types/traceid/jfrTraceIdMacros.hpp | 7 ++++++- .../share/jfr/support/jfrTraceIdExtension.hpp | 9 +++++---- .../share/prims/jvmtiRedefineClasses.cpp | 2 +- 7 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp index 0c947655752..203ec3a3ec4 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp @@ -310,7 +310,9 @@ static void set_serialized(const T* ptr) { assert(ptr != nullptr, "invariant"); if (current_epoch()) { CLEAR_THIS_EPOCH_CLEARED_BIT(ptr); + assert(!IS_THIS_EPOCH_CLEARED_BIT_SET(ptr), "invariant"); } + assert(IS_PREVIOUS_EPOCH_CLEARED_BIT_SET(ptr), "invariant"); SET_SERIALIZED(ptr); assert(IS_SERIALIZED(ptr), "invariant"); } @@ -929,9 +931,11 @@ void set_serialized(MethodPtr method) { assert(method != nullptr, "invariant"); if (current_epoch()) { CLEAR_THIS_EPOCH_METHOD_CLEARED_BIT(method); + assert(!IS_THIS_EPOCH_METHOD_CLEARED_BIT_SET(method), "invariant"); } assert(unloading() ? true : METHOD_IS_NOT_SERIALIZED(method), "invariant"); SET_METHOD_SERIALIZED(method); + assert(IS_PREVIOUS_EPOCH_METHOD_CLEARED_BIT_SET(method), "invariant"); assert(METHOD_IS_SERIALIZED(method), "invariant"); } diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp index 237745b13d9..bfff7eaaf1b 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp @@ -96,6 +96,8 @@ class ClearArtifact { assert(IS_NOT_TRANSIENT(value), "invariant"); SET_PREVIOUS_EPOCH_CLEARED_BIT(value); CLEAR_PREVIOUS_EPOCH_METHOD_AND_CLASS(value); + assert(IS_THIS_EPOCH_CLEARED_BIT_SET(value), "invariant"); + assert(IS_PREVIOUS_EPOCH_CLEARED_BIT_SET(value), "invariant"); return true; } }; @@ -111,6 +113,8 @@ class ClearArtifact { assert(METHOD_IS_NOT_TRANSIENT(method), "invariant"); SET_PREVIOUS_EPOCH_METHOD_CLEARED_BIT(method); CLEAR_PREVIOUS_EPOCH_METHOD_FLAG(method); + assert(IS_THIS_EPOCH_METHOD_CLEARED_BIT_SET(method), "invariant"); + assert(IS_PREVIOUS_EPOCH_METHOD_CLEARED_BIT_SET(method), "invariant"); return true; } }; diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.cpp b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.cpp index f07078eaf06..d2973f74872 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -50,22 +50,22 @@ static traceid atomic_inc(traceid volatile* const dest, traceid stride = 1) { static traceid next_class_id() { static volatile traceid class_id_counter = LAST_TYPE_ID + 1; // + 1 is for the void.class primitive - return atomic_inc(&class_id_counter) << TRACE_ID_SHIFT; + return (atomic_inc(&class_id_counter) << TRACE_ID_SHIFT) | EPOCH_CLEARED_BITS; } static traceid next_module_id() { static volatile traceid module_id_counter = 0; - return atomic_inc(&module_id_counter) << TRACE_ID_SHIFT; + return (atomic_inc(&module_id_counter) << TRACE_ID_SHIFT) | EPOCH_CLEARED_BITS; } static traceid next_package_id() { static volatile traceid package_id_counter = 0; - return atomic_inc(&package_id_counter) << TRACE_ID_SHIFT; + return (atomic_inc(&package_id_counter) << TRACE_ID_SHIFT) | EPOCH_CLEARED_BITS; } static traceid next_class_loader_data_id() { static volatile traceid cld_id_counter = 0; - return atomic_inc(&cld_id_counter) << TRACE_ID_SHIFT; + return (atomic_inc(&cld_id_counter) << TRACE_ID_SHIFT) | EPOCH_CLEARED_BITS; } static bool found_jdk_internal_event_klass = false; @@ -201,18 +201,18 @@ traceid JfrTraceId::load_raw(jclass jc) { // used by CDS / APPCDS as part of "remove_unshareable_info" void JfrTraceId::remove(const Klass* k) { assert(k != nullptr, "invariant"); - // Mask off and store the event flags. + // Mask off and store the event flags and epoch clear bits. // This mechanism will retain the event specific flags // in the archive, allowing for event flag restoration // when renewing the traceid on klass revival. - k->set_trace_id(EVENT_KLASS_MASK(k)); + k->set_trace_id(EPOCH_CLEARED_BITS | EVENT_KLASS_MASK(k)); } // used by CDS / APPCDS as part of "remove_unshareable_info" void JfrTraceId::remove(const Method* method) { assert(method != nullptr, "invariant"); - // Clear all bits. - method->set_trace_flags(0); + // Clear tag bits and set epoch cleared bits. + method->set_trace_flags(static_cast(EPOCH_CLEARED_BITS)); } // used by CDS / APPCDS as part of "restore_unshareable_info" diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdLoadBarrier.inline.hpp b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdLoadBarrier.inline.hpp index ed302335274..5f986121ea0 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdLoadBarrier.inline.hpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdLoadBarrier.inline.hpp @@ -157,9 +157,9 @@ inline traceid JfrTraceIdLoadBarrier::load_leakp(const Klass* klass) { inline traceid JfrTraceIdLoadBarrier::load_leakp(const Klass* klass, const Method* method) { assert(klass != nullptr, "invariant"); - assert(METHOD_AND_CLASS_USED_THIS_EPOCH(klass), "invariant"); assert(method != nullptr, "invariant"); assert(klass == method->method_holder(), "invariant"); + assert(METHOD_AND_CLASS_USED_THIS_EPOCH(klass), "invariant"); if (should_tag(method)) { // the method is already logically tagged, just like the klass, // but because of redefinition, the latest Method* @@ -174,9 +174,9 @@ inline traceid JfrTraceIdLoadBarrier::load_leakp(const Klass* klass, const Metho inline traceid JfrTraceIdLoadBarrier::load_leakp_previuos_epoch(const Klass* klass, const Method* method) { assert(klass != nullptr, "invariant"); - assert(METHOD_AND_CLASS_USED_PREVIOUS_EPOCH(klass), "invariant"); assert(method != nullptr, "invariant"); assert(klass == method->method_holder(), "invariant"); + assert(METHOD_AND_CLASS_USED_PREVIOUS_EPOCH(klass), "invariant"); if (METHOD_FLAG_NOT_USED_PREVIOUS_EPOCH(method)) { // the method is already logically tagged, just like the klass, // but because of redefinition, the latest Method* diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdMacros.hpp b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdMacros.hpp index 3736c070f0a..f53eecad2cf 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdMacros.hpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdMacros.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 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 @@ -48,6 +48,7 @@ #define EPOCH_0_CLEARED_BIT (EPOCH_0_CLEARED_META_BIT << META_SHIFT) #define EPOCH_1_CLEARED_META_BIT (BIT << 1) #define EPOCH_1_CLEARED_BIT (EPOCH_1_CLEARED_META_BIT << META_SHIFT) +#define EPOCH_CLEARED_BITS (EPOCH_1_CLEARED_BIT | EPOCH_0_CLEARED_BIT) #define LEAKP_META_BIT (BIT << 2) #define LEAKP_BIT (LEAKP_META_BIT << META_SHIFT) #define TRANSIENT_META_BIT (BIT << 3) @@ -136,6 +137,8 @@ #define IS_TRANSIENT(ptr) (TRACE_ID_PREDICATE(ptr, TRANSIENT_BIT)) #define IS_NOT_TRANSIENT(ptr) (!(IS_TRANSIENT(ptr))) #define SET_SERIALIZED(ptr) (TRACE_ID_META_TAG(ptr, SERIALIZED_META_BIT)) +#define IS_THIS_EPOCH_CLEARED_BIT_SET(ptr) (TRACE_ID_PREDICATE(ptr, (THIS_EPOCH_BIT << META_SHIFT))) +#define IS_PREVIOUS_EPOCH_CLEARED_BIT_SET(ptr) (TRACE_ID_PREDICATE(ptr, (PREVIOUS_EPOCH_BIT << META_SHIFT))) #define IS_SERIALIZED(ptr) (TRACE_ID_PREDICATE(ptr, SERIALIZED_BIT)) #define IS_NOT_SERIALIZED(ptr) (!(IS_SERIALIZED(ptr))) #define SHOULD_TAG(ptr) (NOT_USED_THIS_EPOCH(ptr)) @@ -161,5 +164,7 @@ #define CLEAR_THIS_EPOCH_METHOD_CLEARED_BIT(ptr) (METHOD_META_MASK_CLEAR(ptr,(~(THIS_EPOCH_BIT)))) #define IS_THIS_EPOCH_METHOD_CLEARED(ptr) (METHOD_FLAG_PREDICATE(method, THIS_EPOCH_BIT)) #define IS_PREVIOUS_EPOCH_METHOD_CLEARED(ptr) (METHOD_FLAG_PREDICATE(method, PREVIOUS_EPOCH_BIT)) +#define IS_THIS_EPOCH_METHOD_CLEARED_BIT_SET(ptr) (METHOD_FLAG_PREDICATE(ptr, (THIS_EPOCH_BIT << META_SHIFT))) +#define IS_PREVIOUS_EPOCH_METHOD_CLEARED_BIT_SET(ptr) (METHOD_FLAG_PREDICATE(ptr, (PREVIOUS_EPOCH_BIT << META_SHIFT))) #endif // SHARE_JFR_RECORDER_CHECKPOINT_TYPES_TRACEID_JFRTRACEIDMACROS_HPP diff --git a/src/hotspot/share/jfr/support/jfrTraceIdExtension.hpp b/src/hotspot/share/jfr/support/jfrTraceIdExtension.hpp index 353e5c3f07c..c73ece42645 100644 --- a/src/hotspot/share/jfr/support/jfrTraceIdExtension.hpp +++ b/src/hotspot/share/jfr/support/jfrTraceIdExtension.hpp @@ -44,11 +44,13 @@ #define REMOVE_METHOD_ID(method) JfrTraceId::remove(method); #define RESTORE_ID(k) JfrTraceId::restore(k); +static constexpr const uint16_t cleared_epoch_bits = 512 | 256; + class JfrTraceFlag { private: mutable uint16_t _flags; public: - JfrTraceFlag() : _flags(0) {} + JfrTraceFlag() : _flags(cleared_epoch_bits) {} bool is_set(uint16_t flag) const { return (_flags & flag) != 0; } @@ -96,9 +98,8 @@ class JfrTraceFlag { uint8_t* trace_meta_addr() const { \ return _trace_flags.meta_addr(); \ } \ - void copy_trace_flags(uint8_t src_flags) const { \ - uint8_t flags = *_trace_flags.flags_addr(); \ - _trace_flags.set_flags(flags | src_flags); \ + void copy_trace_flags(uint16_t rhs_flags) const { \ + _trace_flags.set_flags(_trace_flags.flags() | rhs_flags); \ } #endif // SHARE_JFR_SUPPORT_JFRTRACEIDEXTENSION_HPP diff --git a/src/hotspot/share/prims/jvmtiRedefineClasses.cpp b/src/hotspot/share/prims/jvmtiRedefineClasses.cpp index 84010493dbc..344ef7802a2 100644 --- a/src/hotspot/share/prims/jvmtiRedefineClasses.cpp +++ b/src/hotspot/share/prims/jvmtiRedefineClasses.cpp @@ -1174,7 +1174,7 @@ jvmtiError VM_RedefineClasses::compare_and_normalize_class_versions( } } } - JFR_ONLY(k_new_method->copy_trace_flags(*k_old_method->trace_flags_addr());) + JFR_ONLY(k_new_method->copy_trace_flags(k_old_method->trace_flags());) log_trace(redefine, class, normalize) ("Method matched: new: %s [%d] == old: %s [%d]", k_new_method->name_and_sig_as_C_string(), ni, k_old_method->name_and_sig_as_C_string(), oi); From 0c82e4bf19ab608406190429b6b55c54b299d962 Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Thu, 18 Jul 2024 04:46:59 +0000 Subject: [PATCH 2/3] 8336375: Crash on paste to JShell Reviewed-by: jvernee Backport-of: b9b0b8504ec989ad0687188de4bccfe2c04e5d64 --- .../jdk/internal/org/jline/terminal/impl/ffm/Kernel32.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jdk.internal.le/share/classes/jdk/internal/org/jline/terminal/impl/ffm/Kernel32.java b/src/jdk.internal.le/share/classes/jdk/internal/org/jline/terminal/impl/ffm/Kernel32.java index 0c485819f57..4e7adbe3355 100644 --- a/src/jdk.internal.le/share/classes/jdk/internal/org/jline/terminal/impl/ffm/Kernel32.java +++ b/src/jdk.internal.le/share/classes/jdk/internal/org/jline/terminal/impl/ffm/Kernel32.java @@ -236,7 +236,7 @@ public static int ScrollConsoleScreenBuffer( MethodHandle mh$ = requireNonNull(ScrollConsoleScreenBufferW$MH, "ScrollConsoleScreenBuffer"); try { return (int) - mh$.invokeExact(hConsoleOutput, lpScrollRectangle, lpClipRectangle, dwDestinationOrigin, lpFill); + mh$.invokeExact(hConsoleOutput, lpScrollRectangle.seg, lpClipRectangle.seg, dwDestinationOrigin.seg, lpFill.seg); } catch (Throwable ex$) { throw new AssertionError("should not reach here", ex$); } From e83e2b305e46d10a82ad30d58adabf676572576b Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Thu, 18 Jul 2024 04:52:31 +0000 Subject: [PATCH 3/3] 8335817: javac AssertionError addLocalVar checkNull Reviewed-by: mcimadamore Backport-of: 2b0adfc2decf47f6f49f072549c96f301f275285 --- .../sun/tools/javac/comp/TransPatterns.java | 9 +- .../MatchExceptionLambdaExpression.java | 150 ++++++++++++++++++ 2 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 test/langtools/tools/javac/patterns/MatchExceptionLambdaExpression.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 aed7b3265f8..2d522d01641 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 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 @@ -78,6 +78,7 @@ import com.sun.tools.javac.code.Symbol.RecordComponent; import com.sun.tools.javac.code.Type; import static com.sun.tools.javac.code.TypeTag.BOT; +import static com.sun.tools.javac.code.TypeTag.VOID; import com.sun.tools.javac.jvm.PoolConstant.LoadableConstant; import com.sun.tools.javac.jvm.Target; import com.sun.tools.javac.tree.JCTree; @@ -1260,7 +1261,11 @@ public void visitLambda(JCLambda tree) { tree.body = translate(tree.body); if (deconstructorCalls != null) { if (tree.body instanceof JCExpression value) { - tree.body = make.Block(0, List.of(make.Return(value))); + if (value.type.hasTag(VOID)) { + tree.body = make.Block(0, List.of(make.Exec(value))); + } else { + tree.body = make.Block(0, List.of(make.Return(value))); + } } if (tree.body instanceof JCBlock block) { preparePatternMatchingCatchIfNeeded(block); diff --git a/test/langtools/tools/javac/patterns/MatchExceptionLambdaExpression.java b/test/langtools/tools/javac/patterns/MatchExceptionLambdaExpression.java new file mode 100644 index 00000000000..f42564dab40 --- /dev/null +++ b/test/langtools/tools/javac/patterns/MatchExceptionLambdaExpression.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 + * @bug 8335817 + * @summary Verify synthetic catches for deconstruction patterns work properly in expression lambdas + * @compile MatchExceptionLambdaExpression.java + * @run main MatchExceptionLambdaExpression + */ +public class MatchExceptionLambdaExpression { + + public static void main(String[] args) { + try { + doRunPrimitiveVoid(new A("", true), o -> checkPrimitiveVoid(o instanceof A(String s, _), true)); + throw new AssertionError("Didn't gete the expected exception!"); + } catch (MatchException ex) { + if (ex.getCause() instanceof RequestedException) { + //correct + } else { + throw ex; + } + } + try { + doRunPrimitiveVoid(new A("", true), o -> checkVoidBox(o instanceof A(String s, _), true)); + throw new AssertionError("Didn't gete the expected exception!"); + } catch (MatchException ex) { + if (ex.getCause() instanceof RequestedException) { + //correct + } else { + throw ex; + } + } + try { + doRunPrimitiveVoid(new A("", true), o -> checkNonVoid(o instanceof A(String s, _), true)); + throw new AssertionError("Didn't gete the expected exception!"); + } catch (MatchException ex) { + if (ex.getCause() instanceof RequestedException) { + //correct + } else { + throw ex; + } + } + try { + doRunVoidBox(new A("", true), o -> checkVoidBox(o instanceof A(String s, _), true)); + throw new AssertionError("Didn't gete the expected exception!"); + } catch (MatchException ex) { + if (ex.getCause() instanceof RequestedException) { + //correct + } else { + throw ex; + } + } + try { + doRunNonVoid(new A("", true), o -> checkVoidBox(o instanceof A(String s, _), true)); + throw new AssertionError("Didn't gete the expected exception!"); + } catch (MatchException ex) { + if (ex.getCause() instanceof RequestedException) { + //correct + } else { + throw ex; + } + } + try { + doRunNonVoid(new A("", true), o -> checkNonVoid(o instanceof A(String s, _), true)); + throw new AssertionError("Didn't gete the expected exception!"); + } catch (MatchException ex) { + if (ex.getCause() instanceof RequestedException) { + //correct + } else { + throw ex; + } + } + } + + static void doRunPrimitiveVoid(Object inp, PrimitiveVoidFI toRun) { + toRun.run(inp); + } + + static void doRunVoidBox(Object inp, VoidBoxFI toRun) { + toRun.run(inp); + } + + static void doRunNonVoid(Object inp, NonVoidFI toRun) { + toRun.run(inp); + } + + static void checkPrimitiveVoid(boolean a, boolean shouldNotBeCalled) { + if (shouldNotBeCalled) { + throw new AssertionError("Should not be called."); + } + } + + static Void checkVoidBox(boolean a, boolean shouldNotBeCalled) { + if (shouldNotBeCalled) { + throw new AssertionError("Should not be called."); + } + return null; + } + + static Object checkNonVoid(boolean a, boolean shouldNotBeCalled) { + if (shouldNotBeCalled) { + throw new AssertionError("Should not be called."); + } + return null; + } + + interface PrimitiveVoidFI { + public void run(Object o); + } + + interface VoidBoxFI { + public Void run(Object o); + } + + interface NonVoidFI { + public Object run(Object o); + } + + record A(String s, boolean fail) { + public String s() { + if (fail) { + throw new RequestedException(); + } + return s; + } + } + + static class RequestedException extends RuntimeException {} +}