RFR: 8364588: Export the NPE backtracking functionality to general null-checking APIs

[David Holmes]

On Fri, 1 Aug 2025 15:49:57 GMT, Chen Liang <liach at openjdk.org> wrote:

> Provide a general facility for our null check APIs like Objects::requireNonNull or future Checks::nullCheck (void), converting the existing infrastructure to start tracking from a given stack site (depth offset) and a given stack slot (offset value).

VM side seems reasonable. Java side looks a bit clunky but that is up to core-libs folk to evaluate.

General note: if comments start with a capital they should end with a period, and vice-versa. Extended comments should consist of full sentences. 

Thanks.

src/hotspot/share/classfile/javaClasses.cpp line 3078:

> 3076:   do {
> 3077:     // No backtrace available.
> 3078:     if (!iter.repeat()) return false;

Suggestion:

    if (!iter.repeat()) { 
       return false;
    }

src/hotspot/share/interpreter/bytecodeUtils.cpp line 346:

> 344: 
> 345:     if (found && is_parameter) {
> 346:       // Check MethodParameters for a name, if it carries a name

Suggestion:

      // check MethodParameters for a name, if it carries a name

src/hotspot/share/interpreter/bytecodeUtils.cpp line 354:

> 352:           char *var = cp->symbol_at(elem.name_cp_index)->as_C_string();
> 353:           os->print("%s", var);
> 354: 

No need for blank line

src/hotspot/share/interpreter/bytecodeUtils.cpp line 1483:

> 1481:   // Is an explicit slot given?
> 1482:   bool explicit_search = slot >= 0;
> 1483:   if (explicit_search) {

Suggestion:

  if (slot >= 0) {

No need for the temporary local.

src/hotspot/share/interpreter/bytecodeUtils.hpp line 40:

> 38:   // Slot can be nonnegative to indicate an explicit search for the source of null
> 39:   // If slot is negative (default), also search for the action that caused the NPE before
> 40:   // deriving the actual slot and source of null by code parsing

Suggestion:

  // 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.

Periods at the end of sentences in comments.

src/java.base/share/classes/java/lang/NullPointerException.java line 78:

> 76:     NullPointerException(int stackOffset, int searchSlot) {
> 77:         extendedMessageState = setupCustomBackTrace(stackOffset, searchSlot);
> 78:         this();

I thought `this()` had to come first in a constructor? Is this new in 25 or 26?

src/java.base/share/classes/java/lang/NullPointerException.java line 101:

> 99:             SEARCH_SLOT_MASK = SEARCH_SLOT_MAX << SEARCH_SLOT_SHIFT;
> 100: 
> 101:     // Access these fields in object monitor only

Suggestion:

    // Access these fields only while holding this object's monitor lock.

src/java.base/share/classes/java/lang/NullPointerException.java line 162:

> 160:     ///    and the search slot will be derived by bytecode tracing.  The message
> 161:     ///    will also include the action that caused the NPE besides the source of
> 162:     ///    the `null`.

Suggestion:

    ///    will also include the action that caused the NPE along with the source
    ///    of the `null`.

-------------

PR Review: https://git.openjdk.org/jdk/pull/26600#pullrequestreview-3087045581
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2253509165
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2253517408
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2253519550
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2253472224
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2253481631
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2253558481
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2253541806
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2253487877