From 1d4f30d9807e0f5fa5bb6c0156867ee4a0d9a78c Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Thu, 31 Jul 2025 16:27:21 -0500 Subject: [PATCH 01/11] Objects rNN better message --- src/hotspot/share/classfile/javaClasses.cpp | 26 ++++++---- src/hotspot/share/classfile/javaClasses.hpp | 4 +- src/hotspot/share/include/jvm.h | 2 +- .../share/interpreter/bytecodeUtils.cpp | 48 ++++++++++++++----- .../share/interpreter/bytecodeUtils.hpp | 2 + src/hotspot/share/prims/jvm.cpp | 12 +++-- .../java/lang/NullPointerException.java | 41 +++++++++------- .../share/classes/java/lang/System.java | 2 + .../share/classes/java/util/Objects.java | 3 +- .../jdk/internal/access/JavaLangAccess.java | 4 ++ .../native/libjava/NullPointerException.c | 4 +- .../java/util/Objects/BasicObjectsTest.java | 2 +- 12 files changed, 105 insertions(+), 45 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index 2dcfc43898c4b..7b6fe37120316 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -3039,21 +3039,31 @@ Handle java_lang_Throwable::create_initialization_error(JavaThread* current, Han return init_error; } -bool java_lang_Throwable::get_top_method_and_bci(oop throwable, Method** method, int* bci) { +bool java_lang_Throwable::get_method_and_bci(oop throwable, Method** method, int* bci, int depth) { JavaThread* current = JavaThread::current(); objArrayHandle result(current, objArrayOop(backtrace(throwable))); - BacktraceIterator iter(result, current); - // No backtrace available. - if (!iter.repeat()) return false; // If the exception happened in a frame that has been hidden, i.e., // omitted from the back trace, we can not compute the message. - oop hidden = ((objArrayOop)backtrace(throwable))->obj_at(trace_hidden_offset); - if (hidden != nullptr) { - return false; + // Restriction only exists for 1 depth; Objects.requireNonNull uses a hidden frame + if (depth == 1) { + oop hidden = ((objArrayOop)backtrace(throwable))->obj_at(trace_hidden_offset); + if (hidden != nullptr) { + return false; + } } - // Get first backtrace element. + BacktraceIterator iter(result, current); + // Get the backtrace element. + do { + // No backtrace available. + if (!iter.repeat()) return false; + + + if (--depth > 0) { + iter.next(current); + } + } while (depth > 0); BacktraceElement bte = iter.next(current); InstanceKlass* holder = InstanceKlass::cast(java_lang_Class::as_Klass(bte._mirror())); diff --git a/src/hotspot/share/classfile/javaClasses.hpp b/src/hotspot/share/classfile/javaClasses.hpp index 37ca22e92957b..eefe49096443e 100644 --- a/src/hotspot/share/classfile/javaClasses.hpp +++ b/src/hotspot/share/classfile/javaClasses.hpp @@ -690,8 +690,8 @@ class java_lang_Throwable: AllStatic { static void java_printStackTrace(Handle throwable, TRAPS); // Debugging friend class JavaClasses; - // Gets the method and bci of the top frame (TOS). Returns false if this failed. - static bool get_top_method_and_bci(oop throwable, Method** method, int* bci); + // Gets the method and bci of a particular frame (TOS). Returns false if this failed. + static bool get_method_and_bci(oop throwable, Method** method, int* bci, int depth); }; diff --git a/src/hotspot/share/include/jvm.h b/src/hotspot/share/include/jvm.h index f97374553cab8..bed901bf82294 100644 --- a/src/hotspot/share/include/jvm.h +++ b/src/hotspot/share/include/jvm.h @@ -241,7 +241,7 @@ JVM_InitStackTraceElement(JNIEnv* env, jobject element, jobject stackFrameInfo); */ JNIEXPORT jstring JNICALL -JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable); +JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, jboolean forObjectsRequireNonNull); /* * java.lang.StackWalker diff --git a/src/hotspot/share/interpreter/bytecodeUtils.cpp b/src/hotspot/share/interpreter/bytecodeUtils.cpp index eb4557d6ba09d..41285b29d6d6a 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.cpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.cpp @@ -192,7 +192,7 @@ class ExceptionMessageBuilder : public StackObj { int do_instruction(int bci); bool print_NPE_cause0(outputStream *os, int bci, int slot, int max_detail, - bool inner_expr = false, const char *prefix = nullptr); + bool inner_expr, bool because_clause = false); public: @@ -226,9 +226,10 @@ class ExceptionMessageBuilder : public StackObj { // slot: The slot on the operand stack that contains null. // The slots are numbered from TOS downwards, i.e., // TOS has the slot number 0, that below 1 and so on. + // because_clause: Whether to prefix with " because ..." // // Returns false if nothing was printed, else true. - bool print_NPE_cause(outputStream *os, int bci, int slot); + bool print_NPE_cause(outputStream *os, int bci, int slot, bool because_clause); // Prints a string describing the failed action. void print_NPE_failed_action(outputStream *os, int bci); @@ -1168,8 +1169,8 @@ int ExceptionMessageBuilder::get_NPE_null_slot(int bci) { return INVALID_BYTECODE_ENCOUNTERED; } -bool ExceptionMessageBuilder::print_NPE_cause(outputStream* os, int bci, int slot) { - if (print_NPE_cause0(os, bci, slot, _max_cause_detail, false, " because \"")) { +bool ExceptionMessageBuilder::print_NPE_cause(outputStream* os, int bci, int slot, bool because_clause) { + if (print_NPE_cause0(os, bci, slot, _max_cause_detail, false, because_clause)) { os->print("\" is null"); return true; } @@ -1182,8 +1183,8 @@ bool ExceptionMessageBuilder::print_NPE_cause(outputStream* os, int bci, int slo // at bytecode 'bci'. Compute a message for that bytecode. If // necessary (array, field), recur further. // At most do max_detail recursions. -// Prefix is used to print a proper beginning of the whole -// sentence. +// because_clause is used to print a "because" prefix when this is +// not inner_expr () // inner_expr is used to omit some text, like 'static' in // inner expressions like array subscripts. // @@ -1191,7 +1192,7 @@ bool ExceptionMessageBuilder::print_NPE_cause(outputStream* os, int bci, int slo // bool ExceptionMessageBuilder::print_NPE_cause0(outputStream* os, int bci, int slot, int max_detail, - bool inner_expr, const char *prefix) { + bool inner_expr, bool because_clause) { assert(bci >= 0, "BCI too low"); assert(bci < get_size(), "BCI too large"); @@ -1227,12 +1228,16 @@ bool ExceptionMessageBuilder::print_NPE_cause0(outputStream* os, int bci, int sl } if (max_detail == _max_cause_detail && - prefix != nullptr && + !inner_expr && code != Bytecodes::_invokevirtual && code != Bytecodes::_invokespecial && code != Bytecodes::_invokestatic && code != Bytecodes::_invokeinterface) { - os->print("%s", prefix); + if (because_clause) { + os->print(" because \""); + } else { + os->print("\""); + } } switch (code) { @@ -1347,7 +1352,12 @@ bool ExceptionMessageBuilder::print_NPE_cause0(outputStream* os, int bci, int sl case Bytecodes::_invokeinterface: { int cp_index = Bytes::get_native_u2(code_base + pos); if (max_detail == _max_cause_detail && !inner_expr) { - os->print(" because the return value of \""); + if (because_clause) { + os->print(" because t"); + } else { + os->print("T"); + } + os->print("he return value of \""); } print_method_name(os, _method, cp_index, code); return true; @@ -1472,10 +1482,26 @@ bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci // performed because of the null reference. emb.print_NPE_failed_action(ss, bci); // Print a description of what is null. - if (!emb.print_NPE_cause(ss, bci, slot)) { + if (!emb.print_NPE_cause(ss, bci, slot, true)) { // Nothing was printed. End the sentence without the 'because' // subordinate sentence. } } return true; } + +bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci, int slot) { + NoSafepointVerifier _nsv; // Cannot use this object over a safepoint. + + // If this NPE was created via reflection, we have no real NPE. + if (method->method_holder() == + vmClasses::reflect_DirectConstructorHandleAccessor_NativeAccessor_klass()) { + return false; + } + + // Analyse the bytecodes. + ResourceMark rm; + ExceptionMessageBuilder emb(method, bci); + + return emb.print_NPE_cause(ss, bci, slot, false); +} diff --git a/src/hotspot/share/interpreter/bytecodeUtils.hpp b/src/hotspot/share/interpreter/bytecodeUtils.hpp index ff23b9c000f40..025f9e9c908fe 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.hpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.hpp @@ -36,6 +36,8 @@ class BytecodeUtils : public AllStatic { public: // NPE extended message. Return true if string is printed. static bool get_NPE_message_at(outputStream* ss, Method* method, int bci); + // NPE extended message. Return true if string is printed. + static bool get_NPE_message_at(outputStream* ss, Method* method, int bci, int slot); }; #endif // SHARE_INTERPRETER_BYTECODEUTILS_HPP diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index 13d89b396fa40..4c35ab96f5a38 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -503,14 +503,15 @@ JVM_END // java.lang.NullPointerException /////////////////////////////////////////// -JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable)) +JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, jboolean forObjectsRequireNonNull)) if (!ShowCodeDetailsInExceptionMessages) return nullptr; oop exc = JNIHandles::resolve_non_null(throwable); Method* method; int bci; - if (!java_lang_Throwable::get_top_method_and_bci(exc, &method, &bci)) { + int depth = forObjectsRequireNonNull ? 2 : 1; // rNN, (hidden) JLA, 1-based + if (!java_lang_Throwable::get_method_and_bci(exc, &method, &bci, depth)) { return nullptr; } if (method->is_native()) { @@ -518,7 +519,12 @@ JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable)) } stringStream ss; - bool ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci); + bool ok; + if (forObjectsRequireNonNull) { + ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci, 0); + } else { + ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci); + } if (ok) { oop result = java_lang_String::create_oop_from_str(ss.base(), CHECK_NULL); return (jstring) JNIHandles::make_local(THREAD, result); diff --git a/src/java.base/share/classes/java/lang/NullPointerException.java b/src/java.base/share/classes/java/lang/NullPointerException.java index 7258b711b62d9..8a700ca39f60b 100644 --- a/src/java.base/share/classes/java/lang/NullPointerException.java +++ b/src/java.base/share/classes/java/lang/NullPointerException.java @@ -57,6 +57,7 @@ public class NullPointerException extends RuntimeException { */ public NullPointerException() { super(); + extendedMessageState |= CONSTRUCTOR_FINISHED; } /** @@ -67,11 +68,20 @@ public NullPointerException() { */ public NullPointerException(String s) { super(s); + extendedMessageState |= CONSTRUCTOR_FINISHED; } - // 0: no backtrace filled in, no message computed. - // 1: backtrace filled in, no message computed. - // 2: message computed + /// Internal constructor for Objects.requireNonNull + NullPointerException(Void sig) { + extendedMessageState = OBJECTS_REQUIRE_NON_NULL_HANDLING; + this(); + } + + // Access these fields in object monitor only + private static final int + CONSTRUCTOR_FINISHED = 0x1, + MESSAGE_COMPUTED = 0x2, + OBJECTS_REQUIRE_NON_NULL_HANDLING = 0x4; private transient int extendedMessageState; private transient String extendedMessage; @@ -81,12 +91,7 @@ public NullPointerException(String s) { public synchronized Throwable fillInStackTrace() { // If the stack trace is changed the extended NPE algorithm // will compute a wrong message. So compute it beforehand. - if (extendedMessageState == 0) { - extendedMessageState = 1; - } else if (extendedMessageState == 1) { - extendedMessage = getExtendedNPEMessage(); - extendedMessageState = 2; - } + ensureMessageComputed(); return super.fillInStackTrace(); } @@ -110,22 +115,26 @@ public String getMessage() { String message = super.getMessage(); if (message == null) { synchronized(this) { - if (extendedMessageState == 1) { - // Only the original stack trace was filled in. Message will - // compute correctly. - extendedMessage = getExtendedNPEMessage(); - extendedMessageState = 2; - } + ensureMessageComputed(); return extendedMessage; } } return message; } + // Methods below must be called in object monitor + + private void ensureMessageComputed() { + if ((extendedMessageState & (MESSAGE_COMPUTED | CONSTRUCTOR_FINISHED)) == CONSTRUCTOR_FINISHED) { + extendedMessage = getExtendedNPEMessage((extendedMessageState & OBJECTS_REQUIRE_NON_NULL_HANDLING) != 0); + extendedMessageState |= MESSAGE_COMPUTED; + } + } + /** * Get an extended exception message. This returns a string describing * the location and cause of the exception. It returns null for * exceptions where this is not applicable. */ - private native String getExtendedNPEMessage(); + private native String getExtendedNPEMessage(boolean forObjectsRequireNonNull); } diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java index 0175558d31348..b1c4cad892f04 100644 --- a/src/java.base/share/classes/java/lang/System.java +++ b/src/java.base/share/classes/java/lang/System.java @@ -81,6 +81,7 @@ import jdk.internal.vm.ContinuationScope; import jdk.internal.vm.StackableScope; import jdk.internal.vm.ThreadContainer; +import jdk.internal.vm.annotation.Hidden; import jdk.internal.vm.annotation.IntrinsicCandidate; import jdk.internal.vm.annotation.Stable; import sun.reflect.annotation.AnnotationType; @@ -2327,6 +2328,7 @@ public void copyToSegmentRaw(String string, MemorySegment segment, long offset) public boolean bytesCompatible(String string, Charset charset) { return string.bytesCompatible(charset); } + @Hidden public NullPointerException extendedNullPointerException() { return new NullPointerException((Void) null); } }); } } diff --git a/src/java.base/share/classes/java/util/Objects.java b/src/java.base/share/classes/java/util/Objects.java index d42a41915c9b5..ed032751dacf4 100644 --- a/src/java.base/share/classes/java/util/Objects.java +++ b/src/java.base/share/classes/java/util/Objects.java @@ -25,6 +25,7 @@ package java.util; +import jdk.internal.access.SharedSecrets; import jdk.internal.util.Preconditions; import jdk.internal.vm.annotation.ForceInline; @@ -217,7 +218,7 @@ public static int compare(T a, T b, Comparator c) { @ForceInline public static T requireNonNull(T obj) { if (obj == null) - throw new NullPointerException(); + throw SharedSecrets.getJavaLangAccess().extendedNullPointerException(); return obj; } diff --git a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java index e8343274caca7..9e80e2faed509 100644 --- a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java +++ b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java @@ -624,4 +624,8 @@ StackWalker newStackWalkerInstance(Set options, * Are the string bytes compatible with the given charset? */ boolean bytesCompatible(String string, Charset charset); + + /// Creates an extended NPE for Objects.requireNonNull. + /// The implementation is @Hidden to hide this JLA frame from the trace + NullPointerException extendedNullPointerException(); } diff --git a/src/java.base/share/native/libjava/NullPointerException.c b/src/java.base/share/native/libjava/NullPointerException.c index de0d4798c63b7..d5808015758a3 100644 --- a/src/java.base/share/native/libjava/NullPointerException.c +++ b/src/java.base/share/native/libjava/NullPointerException.c @@ -30,7 +30,7 @@ #include "java_lang_NullPointerException.h" JNIEXPORT jstring JNICALL -Java_java_lang_NullPointerException_getExtendedNPEMessage(JNIEnv *env, jobject throwable) +Java_java_lang_NullPointerException_getExtendedNPEMessage(JNIEnv *env, jobject throwable, jboolean forObjectsRequireNonNull) { - return JVM_GetExtendedNPEMessage(env, throwable); + return JVM_GetExtendedNPEMessage(env, throwable, forObjectsRequireNonNull); } diff --git a/test/jdk/java/util/Objects/BasicObjectsTest.java b/test/jdk/java/util/Objects/BasicObjectsTest.java index 51268d4cbaf4c..6e52cd22d8f72 100644 --- a/test/jdk/java/util/Objects/BasicObjectsTest.java +++ b/test/jdk/java/util/Objects/BasicObjectsTest.java @@ -208,7 +208,7 @@ private static int testRequireNonNull() { errors += testRNN_NonNull(rnn2, RNN_2); errors += testRNN_NonNull(rnn3, RNN_3); - errors += testRNN_Null(rnn1, RNN_1, null); + // No message constraint for 1-arg errors += testRNN_Null(rnn2, RNN_2, "trousers"); errors += testRNN_Null(rnn3, RNN_3, "trousers"); return errors; From e498b609304beedb84dbff8b5feeddec9fcdc002 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Thu, 31 Jul 2025 18:02:27 -0500 Subject: [PATCH 02/11] Tests, MPs based prints --- .../share/interpreter/bytecodeUtils.cpp | 16 ++- .../MethodParametersTest.java | 75 ++++++++++++ .../RequireNonNullTest.java | 107 ++++++++++++++++++ 3 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/MethodParametersTest.java create mode 100644 test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/RequireNonNullTest.java diff --git a/src/hotspot/share/interpreter/bytecodeUtils.cpp b/src/hotspot/share/interpreter/bytecodeUtils.cpp index 41285b29d6d6a..d4b7ae8bda8b2 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.cpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.cpp @@ -312,7 +312,7 @@ static void print_local_var(outputStream *os, unsigned int bci, Method* method, if ((bci >= start) && (bci < end) && (elem->slot == slot)) { ConstantPool* cp = method->constants(); - char *var = cp->symbol_at(elem->name_cp_index)->as_C_string(); + char *var = cp->symbol_at(elem->name_cp_index)->as_C_string(); os->print("%s", var); return; @@ -343,6 +343,20 @@ static void print_local_var(outputStream *os, unsigned int bci, Method* method, } if (found && is_parameter) { + // Check MethodParameters for a name, if it carries a name + int actual_param_index = param_index - 1; // 0 based + if (method->has_method_parameters() && actual_param_index < method->method_parameters_length()) { + MethodParametersElement elem = method->method_parameters_start()[actual_param_index]; + if (elem.name_cp_index != 0) { + ConstantPool* cp = method->constants(); + char *var = cp->symbol_at(elem.name_cp_index)->as_C_string(); + os->print("%s", var); + + return; + } + } + + // TODO we should use arg%d forms, 0-based, like core reflection os->print("", param_index); } else { // This is the best we can do. diff --git a/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/MethodParametersTest.java b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/MethodParametersTest.java new file mode 100644 index 0000000000000..a0094c62897b5 --- /dev/null +++ b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/MethodParametersTest.java @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2025, 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 Test the MethodParameters-based NPE messages. + * @bug 8233268 + * @library /test/lib + * @clean MethodParametersTest InnerClass + * @compile -parameters -g:none MethodParametersTest.java + * @run junit/othervm -XX:+ShowCodeDetailsInExceptionMessages MethodParametersTest + */ + +import java.lang.reflect.InvocationTargetException; +import java.util.Objects; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class MethodParametersTest { + + class InnerClass {} + + @Test + void testOuterThis() { + var npe = assertThrows(NullPointerException.class, () -> { + try { + InnerClass.class.getDeclaredConstructor(MethodParametersTest.class).newInstance((Object) null); + } catch (InvocationTargetException ex) { + throw ex.getCause(); + } + }); + assertEquals("\"this$0\" is null", npe.getMessage()); + } + + // Random slot to param index mappings, both raw and requireNonNull NPEs + // 0, 1, 3, 5 + static void myMethod(String firstArg, long l1, double l2, int[] lastArg) { + Objects.requireNonNull(firstArg); + System.out.println(lastArg.length); + Object a = l1 > 100 ? null : ""; // 6 + a.toString(); + } + + @Test + void testShuffles() { + var npe = assertThrows(NullPointerException.class, () -> myMethod(null, 2, 2, new int[0])); + assertEquals("\"firstArg\" is null", npe.getMessage()); + var msg = assertThrows(NullPointerException.class, () -> myMethod("", 2, 2, null)).getMessage(); + assertTrue(msg.endsWith("because \"lastArg\" is null"), msg); + msg = assertThrows(NullPointerException.class, () -> myMethod("", 2000, 2, new int[0])).getMessage(); + assertTrue(msg.endsWith("because \"\" is null"), msg); // No debug info + } +} \ No newline at end of file diff --git a/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/RequireNonNullTest.java b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/RequireNonNullTest.java new file mode 100644 index 0000000000000..f7a0844bb79ee --- /dev/null +++ b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/RequireNonNullTest.java @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2025, 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 Test the messages for 1-arg requireNonNull. + * @bug 8233268 + * @library /test/lib + * @compile -g RequireNonNullTest.java + * @run junit/othervm -XX:+ShowCodeDetailsInExceptionMessages RequireNonNullTest + */ + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.ParameterizedType; +import java.util.Objects; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class RequireNonNullTest { + + /// A simple NPE message for an expression + static String simpleMessage(String cause) { + return "\"" + cause + "\" is null"; + } + + /// An NPE message for an invocation result + static String invocationMessage(String cause) { + return "The return value of " + simpleMessage(cause); + } + + static void checkSimpleMessage(Executable action, String cause) { + var msg = assertThrows(NullPointerException.class, action).getMessage(); + assertEquals(simpleMessage(cause), msg); + } + + static void checkInvocationMessage(Executable action, String cause) { + var msg = assertThrows(NullPointerException.class, action).getMessage(); + assertEquals(invocationMessage(cause), msg); + } + + class Dummy { Object field; } + + static class One extends Dummy { + One(RequireNonNullTest rnnt) { + rnnt.super(); + } + } + + @Test + void test() { + checkSimpleMessage(() -> generateVariableNpe(null), "myA"); + checkSimpleMessage(() -> generateVariableNpe(new Dummy()), "myA.field"); + checkSimpleMessage(() -> { + RequireNonNullTest t = null; + t.new Dummy(); + }, "t"); + checkSimpleMessage(() -> new One(null), "rnnt"); + + var npe = assertThrows(NullPointerException.class, () -> { + try { + Dummy.class.getDeclaredConstructor(RequireNonNullTest.class).newInstance((Object) null); + } catch (InvocationTargetException ex) { + throw ex.getCause(); + } + }); + assertEquals("\"this$0\" is null", npe.getMessage()); + + checkInvocationMessage(() -> Objects.requireNonNull(int.class.getSuperclass()), "java.lang.Class.getSuperclass()"); + checkInvocationMessage(() -> { + switch (int.class.getGenericSuperclass()) { + case ParameterizedType pt -> {} + case Class cl -> {} + default -> {} + } + }, "java.lang.Class.getGenericSuperclass()"); + } + + // A method that generate NPE from variables + static void generateVariableNpe(Dummy myA) { + Objects.requireNonNull(myA); + Objects.requireNonNull(myA.field); + } +} \ No newline at end of file From 0e2e4e83d0a0e2c083422cbce97b07e5be9b1da6 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Thu, 31 Jul 2025 18:06:16 -0500 Subject: [PATCH 03/11] Perf concerns --- src/hotspot/share/prims/jvm.cpp | 2 +- src/java.base/share/classes/java/lang/System.java | 6 +++++- src/java.base/share/classes/java/util/Objects.java | 11 ++++++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index 4c35ab96f5a38..2af0dffa2b0f1 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -510,7 +510,7 @@ JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, Method* method; int bci; - int depth = forObjectsRequireNonNull ? 2 : 1; // rNN, (hidden) JLA, 1-based + int depth = forObjectsRequireNonNull ? 2 : 1; // extra rNN frame, 1-based; omits hidden frames if (!java_lang_Throwable::get_method_and_bci(exc, &method, &bci, depth)) { return nullptr; } diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java index b1c4cad892f04..269a04bba6476 100644 --- a/src/java.base/share/classes/java/lang/System.java +++ b/src/java.base/share/classes/java/lang/System.java @@ -2328,7 +2328,11 @@ public void copyToSegmentRaw(String string, MemorySegment segment, long offset) public boolean bytesCompatible(String string, Charset charset) { return string.bytesCompatible(charset); } - @Hidden public NullPointerException extendedNullPointerException() { return new NullPointerException((Void) null); } + + @Hidden + public NullPointerException extendedNullPointerException() { + return new NullPointerException((Void) null); + } }); } } diff --git a/src/java.base/share/classes/java/util/Objects.java b/src/java.base/share/classes/java/util/Objects.java index ed032751dacf4..de5e27fd84dc2 100644 --- a/src/java.base/share/classes/java/util/Objects.java +++ b/src/java.base/share/classes/java/util/Objects.java @@ -27,7 +27,9 @@ import jdk.internal.access.SharedSecrets; import jdk.internal.util.Preconditions; +import jdk.internal.vm.annotation.DontInline; import jdk.internal.vm.annotation.ForceInline; +import jdk.internal.vm.annotation.Hidden; import java.util.function.Supplier; @@ -218,10 +220,17 @@ public static int compare(T a, T b, Comparator c) { @ForceInline public static T requireNonNull(T obj) { if (obj == null) - throw SharedSecrets.getJavaLangAccess().extendedNullPointerException(); + throw extendedNullPointerException(); return obj; } + /// Squeeze code here to avoid blowing up ForceInline above with two calls + @Hidden + @DontInline + private static NullPointerException extendedNullPointerException() { + return SharedSecrets.getJavaLangAccess().extendedNullPointerException(); + } + /** * Checks that the specified object reference is not {@code null} and * throws a customized {@link NullPointerException} if it is. This method From 32474622db285e2d3ce9fdf9b98a249f9d842e04 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Fri, 1 Aug 2025 10:16:37 -0500 Subject: [PATCH 04/11] Try generify the NPE tracing API --- src/hotspot/share/classfile/javaClasses.cpp | 4 +- src/hotspot/share/classfile/javaClasses.hpp | 2 +- src/hotspot/share/include/jvm.h | 2 +- src/hotspot/share/prims/jvm.cpp | 11 +++-- .../java/lang/NullPointerException.java | 47 ++++++++++++++----- .../share/classes/java/lang/System.java | 4 +- .../share/classes/java/util/Objects.java | 3 +- .../jdk/internal/access/JavaLangAccess.java | 8 ++-- .../native/libjava/NullPointerException.c | 4 +- 9 files changed, 56 insertions(+), 29 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index 7b6fe37120316..26b40b377bd76 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -3039,14 +3039,14 @@ Handle java_lang_Throwable::create_initialization_error(JavaThread* current, Han return init_error; } -bool java_lang_Throwable::get_method_and_bci(oop throwable, Method** method, int* bci, int depth) { +bool java_lang_Throwable::get_method_and_bci(oop throwable, Method** method, int* bci, int depth, bool hidden_ok) { JavaThread* current = JavaThread::current(); objArrayHandle result(current, objArrayOop(backtrace(throwable))); // If the exception happened in a frame that has been hidden, i.e., // omitted from the back trace, we can not compute the message. // Restriction only exists for 1 depth; Objects.requireNonNull uses a hidden frame - if (depth == 1) { + if (!hidden_ok) { oop hidden = ((objArrayOop)backtrace(throwable))->obj_at(trace_hidden_offset); if (hidden != nullptr) { return false; diff --git a/src/hotspot/share/classfile/javaClasses.hpp b/src/hotspot/share/classfile/javaClasses.hpp index eefe49096443e..b38fb209e6e51 100644 --- a/src/hotspot/share/classfile/javaClasses.hpp +++ b/src/hotspot/share/classfile/javaClasses.hpp @@ -691,7 +691,7 @@ class java_lang_Throwable: AllStatic { // Debugging friend class JavaClasses; // Gets the method and bci of a particular frame (TOS). Returns false if this failed. - static bool get_method_and_bci(oop throwable, Method** method, int* bci, int depth); + static bool get_method_and_bci(oop throwable, Method** method, int* bci, int depth, bool hidden_ok); }; diff --git a/src/hotspot/share/include/jvm.h b/src/hotspot/share/include/jvm.h index bed901bf82294..3c2451fe7e5eb 100644 --- a/src/hotspot/share/include/jvm.h +++ b/src/hotspot/share/include/jvm.h @@ -241,7 +241,7 @@ JVM_InitStackTraceElement(JNIEnv* env, jobject element, jobject stackFrameInfo); */ JNIEXPORT jstring JNICALL -JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, jboolean forObjectsRequireNonNull); +JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, jint stack_offset, jint search_slot); /* * java.lang.StackWalker diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index 2af0dffa2b0f1..b6939419b7d4f 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -503,15 +503,16 @@ JVM_END // java.lang.NullPointerException /////////////////////////////////////////// -JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, jboolean forObjectsRequireNonNull)) +JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, jint stack_offset, jint search_slot)) if (!ShowCodeDetailsInExceptionMessages) return nullptr; oop exc = JNIHandles::resolve_non_null(throwable); + bool explicit_search = search_slot < 0; Method* method; int bci; - int depth = forObjectsRequireNonNull ? 2 : 1; // extra rNN frame, 1-based; omits hidden frames - if (!java_lang_Throwable::get_method_and_bci(exc, &method, &bci, depth)) { + int depth = explicit_search ? stack_offset : 0; // 1-based depth + if (!java_lang_Throwable::get_method_and_bci(exc, &method, &bci, depth + 1, explicit_search)) { return nullptr; } if (method->is_native()) { @@ -520,8 +521,8 @@ JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, stringStream ss; bool ok; - if (forObjectsRequireNonNull) { - ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci, 0); + if (explicit_search) { + ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci, search_slot); } else { ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci); } diff --git a/src/java.base/share/classes/java/lang/NullPointerException.java b/src/java.base/share/classes/java/lang/NullPointerException.java index 8a700ca39f60b..53ad219e180c0 100644 --- a/src/java.base/share/classes/java/lang/NullPointerException.java +++ b/src/java.base/share/classes/java/lang/NullPointerException.java @@ -71,17 +71,34 @@ public NullPointerException(String s) { extendedMessageState |= CONSTRUCTOR_FINISHED; } - /// Internal constructor for Objects.requireNonNull - NullPointerException(Void sig) { - extendedMessageState = OBJECTS_REQUIRE_NON_NULL_HANDLING; + /// Creates an NPE with a custom backtrace configuration. + /// The exception has no message if detailed NPE is not enabled. + NullPointerException(int stackOffset, int searchSlot) { + extendedMessageState = setupCustomBackTrace(stackOffset, searchSlot); this(); } - // Access these fields in object monitor only + private static int setupCustomBackTrace(int stackOffset, int searchSlot) { + if ((stackOffset & ~STACK_OFFSET_MAX) != 0 || (searchSlot & ~SEARCH_SLOT_MAX) != 0) + throw new InternalError(); // Bad arguments from trusted callers + return CUSTOM_TRACE + | ((stackOffset & STACK_OFFSET_MAX) << STACK_OFFSET_SHIFT) + | ((searchSlot & SEARCH_SLOT_MAX) << SEARCH_SLOT_SHIFT); + } + private static final int CONSTRUCTOR_FINISHED = 0x1, MESSAGE_COMPUTED = 0x2, - OBJECTS_REQUIRE_NON_NULL_HANDLING = 0x4; + CUSTOM_TRACE = 0x4; + private static final int + STACK_OFFSET_SHIFT = 4, + STACK_OFFSET_MAX = (1 << 4) - 1, + STACK_OFFSET_MASK = STACK_OFFSET_MAX << STACK_OFFSET_SHIFT, + SEARCH_SLOT_SHIFT = 8, + SEARCH_SLOT_MAX = (1 << 4) - 1, + SEARCH_SLOT_MASK = SEARCH_SLOT_MAX << SEARCH_SLOT_SHIFT; + + // Access these fields in object monitor only private transient int extendedMessageState; private transient String extendedMessage; @@ -126,15 +143,21 @@ public String getMessage() { private void ensureMessageComputed() { if ((extendedMessageState & (MESSAGE_COMPUTED | CONSTRUCTOR_FINISHED)) == CONSTRUCTOR_FINISHED) { - extendedMessage = getExtendedNPEMessage((extendedMessageState & OBJECTS_REQUIRE_NON_NULL_HANDLING) != 0); + int stackOffset = (extendedMessageState & STACK_OFFSET_MASK) >> STACK_OFFSET_SHIFT; + int searchSlot = (extendedMessageState & CUSTOM_TRACE) != 0 + ? (extendedMessageState & SEARCH_SLOT_MASK) >> SEARCH_SLOT_SHIFT + : -1; + extendedMessage = getExtendedNPEMessage(stackOffset, searchSlot); extendedMessageState |= MESSAGE_COMPUTED; } } - /** - * Get an extended exception message. This returns a string describing - * the location and cause of the exception. It returns null for - * exceptions where this is not applicable. - */ - private native String getExtendedNPEMessage(boolean forObjectsRequireNonNull); + /// Gets an extended exception message. There are two modes: + /// 1. `searchSlot == -1`, extend from the nullary constructor to find the + /// location and the cause of the exception. + /// 2. `searchSlot >= 0`, follow the explicit stack offset and search slot + /// configurations to trace how a particular argument, which turns out to + /// be `null`, was evaluated + /// If the backtracking cannot find a verifiable result, this method returns `null`. + private native String getExtendedNPEMessage(int stackOffset, int searchSlot); } diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java index 269a04bba6476..ab6d7a646e86e 100644 --- a/src/java.base/share/classes/java/lang/System.java +++ b/src/java.base/share/classes/java/lang/System.java @@ -2330,8 +2330,8 @@ public boolean bytesCompatible(String string, Charset charset) { } @Hidden - public NullPointerException extendedNullPointerException() { - return new NullPointerException((Void) null); + public NullPointerException extendedNullPointerException(int stackOffset, int searchSlot) { + return new NullPointerException(stackOffset, searchSlot); } }); } diff --git a/src/java.base/share/classes/java/util/Objects.java b/src/java.base/share/classes/java/util/Objects.java index de5e27fd84dc2..60ebbc3552293 100644 --- a/src/java.base/share/classes/java/util/Objects.java +++ b/src/java.base/share/classes/java/util/Objects.java @@ -228,7 +228,8 @@ public static T requireNonNull(T obj) { @Hidden @DontInline private static NullPointerException extendedNullPointerException() { - return SharedSecrets.getJavaLangAccess().extendedNullPointerException(); + // 1 offset for explicit requireNonNull, 0 slot for incoming argument + return SharedSecrets.getJavaLangAccess().extendedNullPointerException(1, 0); } /** diff --git a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java index 9e80e2faed509..5645d116d17ea 100644 --- a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java +++ b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java @@ -625,7 +625,9 @@ StackWalker newStackWalkerInstance(Set options, */ boolean bytesCompatible(String string, Charset charset); - /// Creates an extended NPE for Objects.requireNonNull. - /// The implementation is @Hidden to hide this JLA frame from the trace - NullPointerException extendedNullPointerException(); + /// Creates an extended NPE for general null-checking APIs. + /// The implementation is @Hidden to hide this JLA frame from the trace. + /// Stack offset is the number of non-hidden frames to skip, pointing to the null-checking API. + /// Search slot is the slot where the null-checked value is passed in. + NullPointerException extendedNullPointerException(int stackOffset, int searchSlot); } diff --git a/src/java.base/share/native/libjava/NullPointerException.c b/src/java.base/share/native/libjava/NullPointerException.c index d5808015758a3..3ffaf69ca5a61 100644 --- a/src/java.base/share/native/libjava/NullPointerException.c +++ b/src/java.base/share/native/libjava/NullPointerException.c @@ -30,7 +30,7 @@ #include "java_lang_NullPointerException.h" JNIEXPORT jstring JNICALL -Java_java_lang_NullPointerException_getExtendedNPEMessage(JNIEnv *env, jobject throwable, jboolean forObjectsRequireNonNull) +Java_java_lang_NullPointerException_getExtendedNPEMessage(JNIEnv *env, jobject throwable, jint stackOffset, jint searchSlot) { - return JVM_GetExtendedNPEMessage(env, throwable, forObjectsRequireNonNull); + return JVM_GetExtendedNPEMessage(env, throwable, stackOffset, searchSlot); } From 5fecff22e1b305a113d34ceef7ba82f9d627fde9 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Fri, 1 Aug 2025 10:36:39 -0500 Subject: [PATCH 05/11] Bork --- src/hotspot/share/prims/jvm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index b6939419b7d4f..7f89e00566f69 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -507,7 +507,7 @@ JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, if (!ShowCodeDetailsInExceptionMessages) return nullptr; oop exc = JNIHandles::resolve_non_null(throwable); - bool explicit_search = search_slot < 0; + bool explicit_search = search_slot >= 0; Method* method; int bci; From 807f3d34fb0a1933319e71101dab0fc6747063b3 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Sun, 3 Aug 2025 08:47:21 -0700 Subject: [PATCH 06/11] Review remarks --- src/hotspot/share/classfile/javaClasses.cpp | 4 +-- src/hotspot/share/classfile/javaClasses.hpp | 3 ++- .../share/interpreter/bytecodeUtils.cpp | 27 +++++++------------ .../share/interpreter/bytecodeUtils.hpp | 7 ++--- src/hotspot/share/prims/jvm.cpp | 9 +++---- .../java/lang/NullPointerException.java | 10 ++++--- 6 files changed, 27 insertions(+), 33 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index 3ca1b961b2979..05a22c8eaea8f 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -3057,14 +3057,14 @@ Handle java_lang_Throwable::create_initialization_error(JavaThread* current, Han return init_error; } -bool java_lang_Throwable::get_method_and_bci(oop throwable, Method** method, int* bci, int depth, bool hidden_ok) { +bool java_lang_Throwable::get_method_and_bci(oop throwable, Method** method, int* bci, int depth, bool allow_hidden) { JavaThread* current = JavaThread::current(); objArrayHandle result(current, objArrayOop(backtrace(throwable))); // If the exception happened in a frame that has been hidden, i.e., // omitted from the back trace, we can not compute the message. // Restriction only exists for 1 depth; Objects.requireNonNull uses a hidden frame - if (!hidden_ok) { + if (!allow_hidden) { oop hidden = ((objArrayOop)backtrace(throwable))->obj_at(trace_hidden_offset); if (hidden != nullptr) { return false; diff --git a/src/hotspot/share/classfile/javaClasses.hpp b/src/hotspot/share/classfile/javaClasses.hpp index 0463a09f82eef..c063c3045a206 100644 --- a/src/hotspot/share/classfile/javaClasses.hpp +++ b/src/hotspot/share/classfile/javaClasses.hpp @@ -695,7 +695,8 @@ class java_lang_Throwable: AllStatic { // Debugging friend class JavaClasses; // Gets the method and bci of a particular frame (TOS). Returns false if this failed. - static bool get_method_and_bci(oop throwable, Method** method, int* bci, int depth, bool hidden_ok); + // allow_hidden allows the caller of the NPE constructor to be a hidden frame. + static bool get_method_and_bci(oop throwable, Method** method, int* bci, int depth, bool allow_hidden); }; diff --git a/src/hotspot/share/interpreter/bytecodeUtils.cpp b/src/hotspot/share/interpreter/bytecodeUtils.cpp index d4b7ae8bda8b2..15aff00d612fb 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.cpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.cpp @@ -1464,7 +1464,7 @@ void ExceptionMessageBuilder::print_NPE_failed_action(outputStream *os, int bci) } // Main API -bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci) { +bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci, int slot) { NoSafepointVerifier _nsv; // Cannot use this object over a safepoint. @@ -1478,9 +1478,17 @@ bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci ResourceMark rm; ExceptionMessageBuilder emb(method, bci); + // Is an explicit slot given? + bool explicit_search = slot >= 0; + if (explicit_search) { + // Search from the given slot in bci in Method. + // Omit the failed action. + return emb.print_NPE_cause(ss, bci, slot, false); + } + // The slot of the operand stack that contains the null reference. // Also checks for NPE explicitly constructed and returns NPE_EXPLICIT_CONSTRUCTED. - int slot = emb.get_NPE_null_slot(bci); + slot = emb.get_NPE_null_slot(bci); // Build the message. if (slot == NPE_EXPLICIT_CONSTRUCTED) { @@ -1504,18 +1512,3 @@ bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci return true; } -bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci, int slot) { - NoSafepointVerifier _nsv; // Cannot use this object over a safepoint. - - // If this NPE was created via reflection, we have no real NPE. - if (method->method_holder() == - vmClasses::reflect_DirectConstructorHandleAccessor_NativeAccessor_klass()) { - return false; - } - - // Analyse the bytecodes. - ResourceMark rm; - ExceptionMessageBuilder emb(method, bci); - - return emb.print_NPE_cause(ss, bci, slot, false); -} diff --git a/src/hotspot/share/interpreter/bytecodeUtils.hpp b/src/hotspot/share/interpreter/bytecodeUtils.hpp index 025f9e9c908fe..6c285428ef5f9 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.hpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.hpp @@ -35,9 +35,10 @@ class outputStream; class BytecodeUtils : public AllStatic { public: // NPE extended message. Return true if string is printed. - static bool get_NPE_message_at(outputStream* ss, Method* method, int bci); - // NPE extended message. Return true if string is printed. - static bool get_NPE_message_at(outputStream* ss, Method* method, int bci, int slot); + // Slot can be nonnegative to indicate an explicit search for the source of null + // If slot is negative (default), also search for the action that caused the NPE before + // deriving the actual slot and source of null by code parsing + static bool get_NPE_message_at(outputStream* ss, Method* method, int bci, int slot = -1); }; #endif // SHARE_INTERPRETER_BYTECODEUTILS_HPP diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index 1d5d04ff1fec5..a6b877bac60e7 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -507,11 +507,13 @@ JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, if (!ShowCodeDetailsInExceptionMessages) return nullptr; oop exc = JNIHandles::resolve_non_null(throwable); + // If we are performing an explicit search instructed by alternative internal NPE constructor bool explicit_search = search_slot >= 0; Method* method; int bci; int depth = explicit_search ? stack_offset : 0; // 1-based depth + // The explicit search alternative internal NPE constructor is called from a @Hidden method, allow them if (!java_lang_Throwable::get_method_and_bci(exc, &method, &bci, depth + 1, explicit_search)) { return nullptr; } @@ -520,12 +522,7 @@ JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, } stringStream ss; - bool ok; - if (explicit_search) { - ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci, search_slot); - } else { - ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci); - } + bool ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci, search_slot); if (ok) { oop result = java_lang_String::create_oop_from_str(ss.base(), CHECK_NULL); return (jstring) JNIHandles::make_local(THREAD, result); diff --git a/src/java.base/share/classes/java/lang/NullPointerException.java b/src/java.base/share/classes/java/lang/NullPointerException.java index 53ad219e180c0..69239d99d48ce 100644 --- a/src/java.base/share/classes/java/lang/NullPointerException.java +++ b/src/java.base/share/classes/java/lang/NullPointerException.java @@ -153,11 +153,13 @@ private void ensureMessageComputed() { } /// Gets an extended exception message. There are two modes: - /// 1. `searchSlot == -1`, extend from the nullary constructor to find the - /// location and the cause of the exception. - /// 2. `searchSlot >= 0`, follow the explicit stack offset and search slot + /// 1. `searchSlot >= 0`, follow the explicit stack offset and search slot /// configurations to trace how a particular argument, which turns out to - /// be `null`, was evaluated + /// be `null`, was evaluated. + /// 2. `searchSlot < 0`, stack offset is 0 (a call to the nullary constructor) + /// and the search slot will be derived by bytecode tracing. The message + /// will also include the action that caused the NPE besides the source of + /// the `null`. /// If the backtracking cannot find a verifiable result, this method returns `null`. private native String getExtendedNPEMessage(int stackOffset, int searchSlot); } From 39c2d16fa98276319d18cf9334e1e9c794e56779 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Sun, 3 Aug 2025 16:58:00 -0700 Subject: [PATCH 07/11] Roll back Objects.rNN for now --- .../share/classes/java/util/Objects.java | 13 +---- .../MethodParametersTest.java | 17 +++++- ...NonNullTest.java => NullCheckAPITest.java} | 58 ++++++++++++++----- .../java/util/Objects/BasicObjectsTest.java | 2 +- 4 files changed, 61 insertions(+), 29 deletions(-) rename test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/{RequireNonNullTest.java => NullCheckAPITest.java} (66%) diff --git a/src/java.base/share/classes/java/util/Objects.java b/src/java.base/share/classes/java/util/Objects.java index 60ebbc3552293..d42a41915c9b5 100644 --- a/src/java.base/share/classes/java/util/Objects.java +++ b/src/java.base/share/classes/java/util/Objects.java @@ -25,11 +25,8 @@ package java.util; -import jdk.internal.access.SharedSecrets; import jdk.internal.util.Preconditions; -import jdk.internal.vm.annotation.DontInline; import jdk.internal.vm.annotation.ForceInline; -import jdk.internal.vm.annotation.Hidden; import java.util.function.Supplier; @@ -220,18 +217,10 @@ public static int compare(T a, T b, Comparator c) { @ForceInline public static T requireNonNull(T obj) { if (obj == null) - throw extendedNullPointerException(); + throw new NullPointerException(); return obj; } - /// Squeeze code here to avoid blowing up ForceInline above with two calls - @Hidden - @DontInline - private static NullPointerException extendedNullPointerException() { - // 1 offset for explicit requireNonNull, 0 slot for incoming argument - return SharedSecrets.getJavaLangAccess().extendedNullPointerException(1, 0); - } - /** * Checks that the specified object reference is not {@code null} and * throws a customized {@link NullPointerException} if it is. This method diff --git a/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/MethodParametersTest.java b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/MethodParametersTest.java index a0094c62897b5..38c2e44fcacd2 100644 --- a/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/MethodParametersTest.java +++ b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/MethodParametersTest.java @@ -26,6 +26,7 @@ * @summary Test the MethodParameters-based NPE messages. * @bug 8233268 * @library /test/lib + * @modules java.base/jdk.internal.access * @clean MethodParametersTest InnerClass * @compile -parameters -g:none MethodParametersTest.java * @run junit/othervm -XX:+ShowCodeDetailsInExceptionMessages MethodParametersTest @@ -34,14 +35,26 @@ import java.lang.reflect.InvocationTargetException; import java.util.Objects; +import jdk.internal.access.JavaLangAccess; +import jdk.internal.access.SharedSecrets; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; public class MethodParametersTest { + private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess(); + + // An arbitrary null-checking API + static void nullCheck(Object arg) { + if (arg == null) { + throw JLA.extendedNullPointerException(1, 0); + } + } class InnerClass {} + @Disabled("Requires javac's API support") @Test void testOuterThis() { var npe = assertThrows(NullPointerException.class, () -> { @@ -54,10 +67,10 @@ void testOuterThis() { assertEquals("\"this$0\" is null", npe.getMessage()); } - // Random slot to param index mappings, both raw and requireNonNull NPEs + // Random slot to param index mappings, both raw and null-check API NPEs // 0, 1, 3, 5 static void myMethod(String firstArg, long l1, double l2, int[] lastArg) { - Objects.requireNonNull(firstArg); + nullCheck(firstArg); System.out.println(lastArg.length); Object a = l1 > 100 ? null : ""; // 6 a.toString(); diff --git a/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/RequireNonNullTest.java b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NullCheckAPITest.java similarity index 66% rename from test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/RequireNonNullTest.java rename to test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NullCheckAPITest.java index f7a0844bb79ee..64d6c33858e0a 100644 --- a/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/RequireNonNullTest.java +++ b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NullCheckAPITest.java @@ -23,24 +23,48 @@ /* * @test - * @summary Test the messages for 1-arg requireNonNull. + * @summary Test the messages for arbitrary null-check APIs. * @bug 8233268 * @library /test/lib - * @compile -g RequireNonNullTest.java - * @run junit/othervm -XX:+ShowCodeDetailsInExceptionMessages RequireNonNullTest + * @modules java.base/jdk.internal.access + * @compile -g NullCheckAPITest.java + * @run junit/othervm -DnullCheckAPI.nestedThrow=true -XX:+ShowCodeDetailsInExceptionMessages NullCheckAPITest + * @run junit/othervm -DnullCheckAPI.nestedThrow=false -XX:+ShowCodeDetailsInExceptionMessages NullCheckAPITest */ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.ParameterizedType; -import java.util.Objects; +import jdk.internal.access.JavaLangAccess; +import jdk.internal.access.SharedSecrets; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -public class RequireNonNullTest { +public class NullCheckAPITest { + + private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess(); + private static final boolean NESTED_THROW = Boolean.getBoolean("nullCheckAPI.nestedThrow"); + + // An arbitrary null-checking API + static void nullCheck(Object arg) { + if (arg == null) { + if (NESTED_THROW) { + // 2 offset: nullCheck, throwNpe; + throwNpe(); + } else { + // 1 offset: nullCheck + throw JLA.extendedNullPointerException(1, 0); + } + } + } + + static void throwNpe() { + throw JLA.extendedNullPointerException(2, 0); + } /// A simple NPE message for an expression static String simpleMessage(String cause) { @@ -64,32 +88,38 @@ static void checkInvocationMessage(Executable action, String cause) { class Dummy { Object field; } + @Test + void test() { + checkSimpleMessage(() -> generateVariableNpe(null), "myA"); + checkSimpleMessage(() -> generateVariableNpe(new Dummy()), "myA.field"); + + checkInvocationMessage(() -> nullCheck(int.class.getSuperclass()), "java.lang.Class.getSuperclass()"); + } + static class One extends Dummy { - One(RequireNonNullTest rnnt) { + One(NullCheckAPITest rnnt) { rnnt.super(); } } @Test - void test() { - checkSimpleMessage(() -> generateVariableNpe(null), "myA"); - checkSimpleMessage(() -> generateVariableNpe(new Dummy()), "myA.field"); + @Disabled("Requires javac's API support") + void testRequireNonNull() { checkSimpleMessage(() -> { - RequireNonNullTest t = null; + NullCheckAPITest t = null; t.new Dummy(); }, "t"); checkSimpleMessage(() -> new One(null), "rnnt"); var npe = assertThrows(NullPointerException.class, () -> { try { - Dummy.class.getDeclaredConstructor(RequireNonNullTest.class).newInstance((Object) null); + Dummy.class.getDeclaredConstructor(NullCheckAPITest.class).newInstance((Object) null); } catch (InvocationTargetException ex) { throw ex.getCause(); } }); assertEquals("\"this$0\" is null", npe.getMessage()); - checkInvocationMessage(() -> Objects.requireNonNull(int.class.getSuperclass()), "java.lang.Class.getSuperclass()"); checkInvocationMessage(() -> { switch (int.class.getGenericSuperclass()) { case ParameterizedType pt -> {} @@ -101,7 +131,7 @@ void test() { // A method that generate NPE from variables static void generateVariableNpe(Dummy myA) { - Objects.requireNonNull(myA); - Objects.requireNonNull(myA.field); + nullCheck(myA); + nullCheck(myA.field); } } \ No newline at end of file diff --git a/test/jdk/java/util/Objects/BasicObjectsTest.java b/test/jdk/java/util/Objects/BasicObjectsTest.java index 6e52cd22d8f72..51268d4cbaf4c 100644 --- a/test/jdk/java/util/Objects/BasicObjectsTest.java +++ b/test/jdk/java/util/Objects/BasicObjectsTest.java @@ -208,7 +208,7 @@ private static int testRequireNonNull() { errors += testRNN_NonNull(rnn2, RNN_2); errors += testRNN_NonNull(rnn3, RNN_3); - // No message constraint for 1-arg + errors += testRNN_Null(rnn1, RNN_1, null); errors += testRNN_Null(rnn2, RNN_2, "trousers"); errors += testRNN_Null(rnn3, RNN_3, "trousers"); return errors; From 5735b8026ca07ad6a9dca6fe3dac7d1f3cec4863 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Sun, 3 Aug 2025 17:18:36 -0700 Subject: [PATCH 08/11] Years --- src/hotspot/share/interpreter/bytecodeUtils.hpp | 2 +- src/java.base/share/classes/java/lang/NullPointerException.java | 2 +- src/java.base/share/native/libjava/NullPointerException.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/interpreter/bytecodeUtils.hpp b/src/hotspot/share/interpreter/bytecodeUtils.hpp index 6c285428ef5f9..8823c6455eb6b 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.hpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2019, 2022 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * diff --git a/src/java.base/share/classes/java/lang/NullPointerException.java b/src/java.base/share/classes/java/lang/NullPointerException.java index 69239d99d48ce..dfbc313af567b 100644 --- a/src/java.base/share/classes/java/lang/NullPointerException.java +++ b/src/java.base/share/classes/java/lang/NullPointerException.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1994, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1994, 2025, 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 diff --git a/src/java.base/share/native/libjava/NullPointerException.c b/src/java.base/share/native/libjava/NullPointerException.c index 3ffaf69ca5a61..fe5c1b8bec325 100644 --- a/src/java.base/share/native/libjava/NullPointerException.c +++ b/src/java.base/share/native/libjava/NullPointerException.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2019 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * From 09233e9a2d7a9c0482212e9168d60f04317bd6d5 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Tue, 5 Aug 2025 07:29:07 -0700 Subject: [PATCH 09/11] Web review Co-authored-by: David Holmes <62092539+dholmes-ora@users.noreply.github.com> --- src/hotspot/share/classfile/javaClasses.cpp | 4 +++- src/hotspot/share/interpreter/bytecodeUtils.cpp | 7 ++----- src/hotspot/share/interpreter/bytecodeUtils.hpp | 4 ++-- .../share/classes/java/lang/NullPointerException.java | 10 +++++----- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index 05a22c8eaea8f..980a840fd1041 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -3075,7 +3075,9 @@ bool java_lang_Throwable::get_method_and_bci(oop throwable, Method** method, int // Get the backtrace element. do { // No backtrace available. - if (!iter.repeat()) return false; + if (!iter.repeat()) { + return false; + } if (--depth > 0) { diff --git a/src/hotspot/share/interpreter/bytecodeUtils.cpp b/src/hotspot/share/interpreter/bytecodeUtils.cpp index 15aff00d612fb..59b2d1583e78d 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.cpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.cpp @@ -343,7 +343,7 @@ static void print_local_var(outputStream *os, unsigned int bci, Method* method, } if (found && is_parameter) { - // Check MethodParameters for a name, if it carries a name + // check MethodParameters for a name, if it carries a name int actual_param_index = param_index - 1; // 0 based if (method->has_method_parameters() && actual_param_index < method->method_parameters_length()) { MethodParametersElement elem = method->method_parameters_start()[actual_param_index]; @@ -351,7 +351,6 @@ static void print_local_var(outputStream *os, unsigned int bci, Method* method, ConstantPool* cp = method->constants(); char *var = cp->symbol_at(elem.name_cp_index)->as_C_string(); os->print("%s", var); - return; } } @@ -1479,8 +1478,7 @@ bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci ExceptionMessageBuilder emb(method, bci); // Is an explicit slot given? - bool explicit_search = slot >= 0; - if (explicit_search) { + if (slot >= 0) { // Search from the given slot in bci in Method. // Omit the failed action. return emb.print_NPE_cause(ss, bci, slot, false); @@ -1511,4 +1509,3 @@ bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci } return true; } - diff --git a/src/hotspot/share/interpreter/bytecodeUtils.hpp b/src/hotspot/share/interpreter/bytecodeUtils.hpp index 8823c6455eb6b..997c2947ee48e 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.hpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.hpp @@ -35,9 +35,9 @@ class outputStream; class BytecodeUtils : public AllStatic { public: // NPE extended message. Return true if string is printed. - // Slot can be nonnegative to indicate an explicit search for the source of null + // Slot can be nonnegative to indicate an explicit search for the source of null. // If slot is negative (default), also search for the action that caused the NPE before - // deriving the actual slot and source of null by code parsing + // deriving the actual slot and source of null by code parsing. static bool get_NPE_message_at(outputStream* ss, Method* method, int bci, int slot = -1); }; diff --git a/src/java.base/share/classes/java/lang/NullPointerException.java b/src/java.base/share/classes/java/lang/NullPointerException.java index dfbc313af567b..a690a0d52dc38 100644 --- a/src/java.base/share/classes/java/lang/NullPointerException.java +++ b/src/java.base/share/classes/java/lang/NullPointerException.java @@ -71,8 +71,8 @@ public NullPointerException(String s) { extendedMessageState |= CONSTRUCTOR_FINISHED; } - /// Creates an NPE with a custom backtrace configuration. - /// The exception has no message if detailed NPE is not enabled. + // Creates an NPE with a custom backtrace configuration. + // The exception has no message if detailed NPE is not enabled. NullPointerException(int stackOffset, int searchSlot) { extendedMessageState = setupCustomBackTrace(stackOffset, searchSlot); this(); @@ -98,7 +98,7 @@ private static int setupCustomBackTrace(int stackOffset, int searchSlot) { SEARCH_SLOT_MAX = (1 << 4) - 1, SEARCH_SLOT_MASK = SEARCH_SLOT_MAX << SEARCH_SLOT_SHIFT; - // Access these fields in object monitor only + // Access these fields only while holding this object's monitor lock. private transient int extendedMessageState; private transient String extendedMessage; @@ -158,8 +158,8 @@ private void ensureMessageComputed() { /// be `null`, was evaluated. /// 2. `searchSlot < 0`, stack offset is 0 (a call to the nullary constructor) /// and the search slot will be derived by bytecode tracing. The message - /// will also include the action that caused the NPE besides the source of - /// the `null`. + /// will also include the action that caused the NPE along with the source + /// of the `null`. /// If the backtracking cannot find a verifiable result, this method returns `null`. private native String getExtendedNPEMessage(int stackOffset, int searchSlot); } From 9af7ee85a0a29c437078fddf4849deea7dd96264 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Tue, 5 Aug 2025 07:57:45 -0700 Subject: [PATCH 10/11] Use c++ enum classes per jdksjolen --- src/hotspot/share/classfile/javaClasses.cpp | 2 +- .../share/interpreter/bytecodeUtils.cpp | 44 +++++++++++-------- .../share/interpreter/bytecodeUtils.hpp | 11 ++++- src/hotspot/share/prims/jvm.cpp | 9 ++-- 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index f7aba4bffccea..2fa0245b56f4b 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -3074,7 +3074,7 @@ bool java_lang_Throwable::get_method_and_bci(oop throwable, Method** method, int // Get the backtrace element. do { // No backtrace available. - if (!iter.repeat()) { + if (!iter.repeat()) { return false; } diff --git a/src/hotspot/share/interpreter/bytecodeUtils.cpp b/src/hotspot/share/interpreter/bytecodeUtils.cpp index 59b2d1583e78d..d6b492304327e 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.cpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.cpp @@ -195,6 +195,14 @@ class ExceptionMessageBuilder : public StackObj { bool inner_expr, bool because_clause = false); public: + enum class BacktrackNullSlot : int { + // This NullPointerException is explicitly constructed + NPE_EXPLICIT_CONSTRUCTED = -2, + // There cannot be a NullPointerException at the given BCI + INVALID_BYTECODE_ENCOUNTERED = -1, + // A slot is found + FOUND + }; // Creates an ExceptionMessageBuilder object and runs the analysis // building SimulatedOperandStacks for each bytecode in the given @@ -214,7 +222,7 @@ class ExceptionMessageBuilder : public StackObj { // we return the nr of the slot holding the null reference. If this // NPE is created by hand, we return -2 as the slot. If there // cannot be a NullPointerException at the bci, -1 is returned. - int get_NPE_null_slot(int bci); + BacktrackNullSlot get_NPE_null_slot(int bci); // Prints a java-like expression for the bytecode that pushed // the value to the given slot being live at the given bci. @@ -1106,9 +1114,7 @@ int ExceptionMessageBuilder::do_instruction(int bci) { return len; } -#define INVALID_BYTECODE_ENCOUNTERED -1 -#define NPE_EXPLICIT_CONSTRUCTED -2 -int ExceptionMessageBuilder::get_NPE_null_slot(int bci) { +ExceptionMessageBuilder::BacktrackNullSlot ExceptionMessageBuilder::get_NPE_null_slot(int bci) { // Get the bytecode. address code_base = _method->constMethod()->code_base(); Bytecodes::Code code = Bytecodes::java_code_at(_method, code_base + bci); @@ -1124,7 +1130,7 @@ int ExceptionMessageBuilder::get_NPE_null_slot(int bci) { case Bytecodes::_athrow: case Bytecodes::_monitorenter: case Bytecodes::_monitorexit: - return 0; + return static_cast(0); case Bytecodes::_iaload: case Bytecodes::_faload: case Bytecodes::_aaload: @@ -1133,17 +1139,17 @@ int ExceptionMessageBuilder::get_NPE_null_slot(int bci) { case Bytecodes::_saload: case Bytecodes::_laload: case Bytecodes::_daload: - return 1; + return static_cast(1); case Bytecodes::_iastore: case Bytecodes::_fastore: case Bytecodes::_aastore: case Bytecodes::_bastore: case Bytecodes::_castore: case Bytecodes::_sastore: - return 2; + return static_cast(2); case Bytecodes::_lastore: case Bytecodes::_dastore: - return 3; + return static_cast(3); case Bytecodes::_putfield: { int cp_index = Bytes::get_native_u2(code_base + pos); ConstantPool* cp = _method->constants(); @@ -1151,7 +1157,7 @@ int ExceptionMessageBuilder::get_NPE_null_slot(int bci) { int type_index = cp->signature_ref_index_at(name_and_type_index); Symbol* signature = cp->symbol_at(type_index); BasicType bt = Signature::basic_type(signature); - return type2size[bt]; + return static_cast(type2size[bt]); } case Bytecodes::_invokevirtual: case Bytecodes::_invokespecial: @@ -1169,9 +1175,9 @@ int ExceptionMessageBuilder::get_NPE_null_slot(int bci) { int type_index = cp->signature_ref_index_at(name_and_type_index); Symbol* signature = cp->symbol_at(type_index); // The 'this' parameter was null. Return the slot of it. - return ArgumentSizeComputer(signature).size(); + return static_cast(ArgumentSizeComputer(signature).size()); } else { - return NPE_EXPLICIT_CONSTRUCTED; + return BacktrackNullSlot::NPE_EXPLICIT_CONSTRUCTED; } } @@ -1179,7 +1185,7 @@ int ExceptionMessageBuilder::get_NPE_null_slot(int bci) { break; } - return INVALID_BYTECODE_ENCOUNTERED; + return BacktrackNullSlot::INVALID_BYTECODE_ENCOUNTERED; } bool ExceptionMessageBuilder::print_NPE_cause(outputStream* os, int bci, int slot, bool because_clause) { @@ -1463,7 +1469,7 @@ void ExceptionMessageBuilder::print_NPE_failed_action(outputStream *os, int bci) } // Main API -bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci, int slot) { +bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci, NullSlot slot) { NoSafepointVerifier _nsv; // Cannot use this object over a safepoint. @@ -1478,21 +1484,21 @@ bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci ExceptionMessageBuilder emb(method, bci); // Is an explicit slot given? - if (slot >= 0) { + if (slot != NullSlot::SEARCH) { // Search from the given slot in bci in Method. // Omit the failed action. - return emb.print_NPE_cause(ss, bci, slot, false); + return emb.print_NPE_cause(ss, bci, static_cast(slot), false); } // The slot of the operand stack that contains the null reference. // Also checks for NPE explicitly constructed and returns NPE_EXPLICIT_CONSTRUCTED. - slot = emb.get_NPE_null_slot(bci); + ExceptionMessageBuilder::BacktrackNullSlot backtrackSlot = emb.get_NPE_null_slot(bci); // Build the message. - if (slot == NPE_EXPLICIT_CONSTRUCTED) { + if (backtrackSlot == ExceptionMessageBuilder::BacktrackNullSlot::NPE_EXPLICIT_CONSTRUCTED) { // We don't want to print a message. return false; - } else if (slot == INVALID_BYTECODE_ENCOUNTERED) { + } else if (backtrackSlot == ExceptionMessageBuilder::BacktrackNullSlot::INVALID_BYTECODE_ENCOUNTERED) { // We encountered a bytecode that does not dereference a reference. DEBUG_ONLY(ss->print("There cannot be a NullPointerException at bci %d of method %s", bci, method->external_name())); @@ -1502,7 +1508,7 @@ bool BytecodeUtils::get_NPE_message_at(outputStream* ss, Method* method, int bci // performed because of the null reference. emb.print_NPE_failed_action(ss, bci); // Print a description of what is null. - if (!emb.print_NPE_cause(ss, bci, slot, true)) { + if (!emb.print_NPE_cause(ss, bci, static_cast(backtrackSlot), true)) { // Nothing was printed. End the sentence without the 'because' // subordinate sentence. } diff --git a/src/hotspot/share/interpreter/bytecodeUtils.hpp b/src/hotspot/share/interpreter/bytecodeUtils.hpp index 997c2947ee48e..318d749331811 100644 --- a/src/hotspot/share/interpreter/bytecodeUtils.hpp +++ b/src/hotspot/share/interpreter/bytecodeUtils.hpp @@ -34,11 +34,18 @@ class outputStream; class BytecodeUtils : public AllStatic { public: + // The slot where the null value comes from. + enum class NullSlot : int { + // Unknown; perform a search at the given bci to check instead. + SEARCH = -1, + // A given slot value to search from. + EXPLICIT + }; // NPE extended message. Return true if string is printed. // Slot can be nonnegative to indicate an explicit search for the source of null. - // If slot is negative (default), also search for the action that caused the NPE before + // If slot is SEARCH (default), also search for the action that caused the NPE before // deriving the actual slot and source of null by code parsing. - static bool get_NPE_message_at(outputStream* ss, Method* method, int bci, int slot = -1); + static bool get_NPE_message_at(outputStream* ss, Method* method, int bci, NullSlot slot = NullSlot::SEARCH); }; #endif // SHARE_INTERPRETER_BYTECODEUTILS_HPP diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index af31f069375b7..cdb18765ec395 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -508,13 +508,14 @@ JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, oop exc = JNIHandles::resolve_non_null(throwable); // If we are performing an explicit search instructed by alternative internal NPE constructor - bool explicit_search = search_slot >= 0; + BytecodeUtils::NullSlot slot = search_slot >= 0 ? static_cast(search_slot) + : BytecodeUtils::NullSlot::SEARCH; Method* method; int bci; - int depth = explicit_search ? stack_offset : 0; // 1-based depth + int depth = slot != BytecodeUtils::NullSlot::SEARCH ? stack_offset : 0; // 1-based depth // The explicit search alternative internal NPE constructor is called from a @Hidden method, allow them - if (!java_lang_Throwable::get_method_and_bci(exc, &method, &bci, depth + 1, explicit_search)) { + if (!java_lang_Throwable::get_method_and_bci(exc, &method, &bci, depth + 1, slot != BytecodeUtils::NullSlot::SEARCH)) { return nullptr; } if (method->is_native()) { @@ -522,7 +523,7 @@ JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable, } stringStream ss; - bool ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci, search_slot); + bool ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci, slot); if (ok) { oop result = java_lang_String::create_oop_from_str(ss.base(), CHECK_NULL); return (jstring) JNIHandles::make_local(THREAD, result); From 4ba1f17cbe24efcb0b32af845aa7ceddba90129d Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Tue, 5 Aug 2025 08:13:52 -0700 Subject: [PATCH 11/11] Update NPE per roger review --- .../java/lang/NullPointerException.java | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/src/java.base/share/classes/java/lang/NullPointerException.java b/src/java.base/share/classes/java/lang/NullPointerException.java index a690a0d52dc38..ecefb47efe21f 100644 --- a/src/java.base/share/classes/java/lang/NullPointerException.java +++ b/src/java.base/share/classes/java/lang/NullPointerException.java @@ -79,11 +79,9 @@ public NullPointerException(String s) { } private static int setupCustomBackTrace(int stackOffset, int searchSlot) { - if ((stackOffset & ~STACK_OFFSET_MAX) != 0 || (searchSlot & ~SEARCH_SLOT_MAX) != 0) - throw new InternalError(); // Bad arguments from trusted callers return CUSTOM_TRACE - | ((stackOffset & STACK_OFFSET_MAX) << STACK_OFFSET_SHIFT) - | ((searchSlot & SEARCH_SLOT_MAX) << SEARCH_SLOT_SHIFT); + | encode(stackOffset, STACK_OFFSET_SIZE, STACK_OFFSET_SHIFT) + | encode(searchSlot, SEARCH_SLOT_SIZE, SEARCH_SLOT_SHIFT); } private static final int @@ -92,11 +90,21 @@ private static int setupCustomBackTrace(int stackOffset, int searchSlot) { CUSTOM_TRACE = 0x4; private static final int STACK_OFFSET_SHIFT = 4, - STACK_OFFSET_MAX = (1 << 4) - 1, - STACK_OFFSET_MASK = STACK_OFFSET_MAX << STACK_OFFSET_SHIFT, + STACK_OFFSET_SIZE = 4, SEARCH_SLOT_SHIFT = 8, - SEARCH_SLOT_MAX = (1 << 4) - 1, - SEARCH_SLOT_MASK = SEARCH_SLOT_MAX << SEARCH_SLOT_SHIFT; + SEARCH_SLOT_SIZE = 4; + + private static int encode(int data, int size, int shift) { + int max = (1 << size) - 1; + if ((data & ~max) != 0) + throw new InternalError(); // bad arguments from trusted callers + return ((data & max) << shift); + } + + private static int decode(int encoded, int size, int shift) { + int max = (1 << size) - 1; + return (encoded >> shift) & max; + } // Access these fields only while holding this object's monitor lock. private transient int extendedMessageState; @@ -139,27 +147,27 @@ public String getMessage() { return message; } - // Methods below must be called in object monitor - + // Must be called only while holding this object's monitor lock. private void ensureMessageComputed() { if ((extendedMessageState & (MESSAGE_COMPUTED | CONSTRUCTOR_FINISHED)) == CONSTRUCTOR_FINISHED) { - int stackOffset = (extendedMessageState & STACK_OFFSET_MASK) >> STACK_OFFSET_SHIFT; + int stackOffset = decode(extendedMessageState, STACK_OFFSET_SIZE, STACK_OFFSET_SHIFT); int searchSlot = (extendedMessageState & CUSTOM_TRACE) != 0 - ? (extendedMessageState & SEARCH_SLOT_MASK) >> SEARCH_SLOT_SHIFT + ? decode(extendedMessageState, SEARCH_SLOT_SIZE, SEARCH_SLOT_SHIFT) : -1; extendedMessage = getExtendedNPEMessage(stackOffset, searchSlot); extendedMessageState |= MESSAGE_COMPUTED; } } - /// Gets an extended exception message. There are two modes: - /// 1. `searchSlot >= 0`, follow the explicit stack offset and search slot - /// configurations to trace how a particular argument, which turns out to - /// be `null`, was evaluated. - /// 2. `searchSlot < 0`, stack offset is 0 (a call to the nullary constructor) - /// and the search slot will be derived by bytecode tracing. The message - /// will also include the action that caused the NPE along with the source - /// of the `null`. - /// If the backtracking cannot find a verifiable result, this method returns `null`. + // Must be called only while holding this object's monitor lock. + // Gets an extended exception message. There are two modes: + // 1. `searchSlot >= 0`, follow the explicit stack offset and search slot + // configurations to trace how a particular argument, which turns out to + // be `null`, was evaluated. + // 2. `searchSlot == -1`, stack offset is 0 (a call to the nullary constructor) + // and the search slot will be derived by bytecode tracing. The message + // will also include the action that caused the NPE along with the source + // of the `null`. + // If the backtracking cannot find a verifiable result, this method returns `null`. private native String getExtendedNPEMessage(int stackOffset, int searchSlot); }