8218628: Add detailed message to NullPointerException describing what is null

This is the implementation of JEP 358: Helpful NullPointerExceptions.

Reviewed-by: coleenp, clanger, rschmelter, rriggs, forax, mr

make/hotspot/symbols/symbols-unix

@@ -97,6 +97,7 @@ JVM_GetCPMethodSignatureUTF
 JVM_GetDeclaredClasses
 JVM_GetDeclaringClass
 JVM_GetEnclosingMethodInfo
+JVM_GetExtendedNPEMessage
 JVM_GetFieldIxModifiers
 JVM_GetFieldTypeAnnotations
 JVM_GetInheritedAccessControlContext

src/hotspot/share/classfile/javaClasses.cpp

@@ -1545,7 +1545,7 @@ bool java_lang_Class::offsets_computed = false;
 int  java_lang_Class::classRedefinedCount_offset = -1;
 
 #define CLASS_FIELDS_DO(macro) \
-  macro(classRedefinedCount_offset, k, "classRedefinedCount", int_signature,         false) ; \
+  macro(classRedefinedCount_offset, k, "classRedefinedCount", int_signature,         false); \
   macro(_class_loader_offset,       k, "classLoader",         classloader_signature, false); \
   macro(_component_mirror_offset,   k, "componentType",       class_signature,       false); \
   macro(_module_offset,             k, "module",              module_signature,      false); \
@@ -1938,7 +1938,6 @@ static inline bool version_matches(Method* method, int version) {
   return method != NULL && (method->constants()->version() == version);
 }
 
-
 // This class provides a simple wrapper over the internal structure of
 // exception backtrace to insulate users of the backtrace from needing
 // to know what it looks like.
@@ -1950,7 +1949,11 @@ class BacktraceBuilder: public StackObj {
   typeArrayOop    _methods;
   typeArrayOop    _bcis;
   objArrayOop     _mirrors;
-  typeArrayOop    _names; // needed to insulate method name against redefinition
+  typeArrayOop    _names; // Needed to insulate method name against redefinition.
+  // This is set to a java.lang.Boolean(true) if the top frame
+  // of the backtrace is omitted because it shall be hidden.
+  // Else it is null.
+  oop             _has_hidden_top_frame;
   int             _index;
   NoSafepointVerifier _nsv;
 
@@ -1960,6 +1963,7 @@ class BacktraceBuilder: public StackObj {
     trace_mirrors_offset = java_lang_Throwable::trace_mirrors_offset,
     trace_names_offset   = java_lang_Throwable::trace_names_offset,
     trace_next_offset    = java_lang_Throwable::trace_next_offset,
+    trace_hidden_offset  = java_lang_Throwable::trace_hidden_offset,
     trace_size           = java_lang_Throwable::trace_size,
     trace_chunk_size     = java_lang_Throwable::trace_chunk_size
   };
@@ -1985,11 +1989,15 @@ class BacktraceBuilder: public StackObj {
     assert(names != NULL, "names array should be initialized in backtrace");
     return names;
   }
+  static oop get_has_hidden_top_frame(objArrayHandle chunk) {
+    oop hidden = chunk->obj_at(trace_hidden_offset);
+    return hidden;
+  }
 
  public:
 
   // constructor for new backtrace
-  BacktraceBuilder(TRAPS): _head(NULL), _methods(NULL), _bcis(NULL), _mirrors(NULL), _names(NULL) {
+  BacktraceBuilder(TRAPS): _head(NULL), _methods(NULL), _bcis(NULL), _mirrors(NULL), _names(NULL), _has_hidden_top_frame(NULL) {
     expand(CHECK);
     _backtrace = Handle(THREAD, _head);
     _index = 0;
@@ -2000,6 +2008,7 @@ class BacktraceBuilder: public StackObj {
     _bcis = get_bcis(backtrace);
     _mirrors = get_mirrors(backtrace);
     _names = get_names(backtrace);
+    _has_hidden_top_frame = get_has_hidden_top_frame(backtrace);
     assert(_methods->length() == _bcis->length() &&
            _methods->length() == _mirrors->length() &&
            _mirrors->length() == _names->length(),
@@ -2037,6 +2046,7 @@ class BacktraceBuilder: public StackObj {
     new_head->obj_at_put(trace_bcis_offset, new_bcis());
     new_head->obj_at_put(trace_mirrors_offset, new_mirrors());
     new_head->obj_at_put(trace_names_offset, new_names());
+    new_head->obj_at_put(trace_hidden_offset, NULL);
 
     _head    = new_head();
     _methods = new_methods();
@@ -2077,6 +2087,16 @@ class BacktraceBuilder: public StackObj {
     _index++;
   }
 
+  void set_has_hidden_top_frame(TRAPS) {
+    if (_has_hidden_top_frame == NULL) {
+      jvalue prim;
+      prim.z = 1;
+      PauseNoSafepointVerifier pnsv(&_nsv);
+      _has_hidden_top_frame = java_lang_boxing_object::create(T_BOOLEAN, &prim, CHECK);
+      _head->obj_at_put(trace_hidden_offset, _has_hidden_top_frame);
+    }
+  }
+
 };
 
 struct BacktraceElement : public StackObj {
@@ -2406,7 +2426,13 @@ void java_lang_Throwable::fill_in_stack_trace(Handle throwable, const methodHand
       }
     }
     if (method->is_hidden()) {
-      if (skip_hidden)  continue;
+      if (skip_hidden) {
+        if (total_count == 0) {
+          // The top frame will be hidden from the stack trace.
+          bt.set_has_hidden_top_frame(CHECK);
+        }
+        continue;
+      }
     }
     bt.push(method, bci, CHECK);
     total_count++;
@@ -2523,6 +2549,37 @@ void java_lang_Throwable::get_stack_trace_elements(Handle throwable,
   }
 }
 
+bool java_lang_Throwable::get_top_method_and_bci(oop throwable, Method** method, int* bci) {
+  Thread* THREAD = Thread::current();
+  objArrayHandle result(THREAD, objArrayOop(backtrace(throwable)));
+  BacktraceIterator iter(result, THREAD);
+  // 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 != NULL) {
+    return false;
+  }
+
+  // Get first backtrace element.
+  BacktraceElement bte = iter.next(THREAD);
+
+  InstanceKlass* holder = InstanceKlass::cast(java_lang_Class::as_Klass(bte._mirror()));
+  assert(holder != NULL, "first element should be non-null");
+  Method* m = holder->method_with_orig_idnum(bte._method_id, bte._version);
+
+  // Original version is no longer available.
+  if (m == NULL || !version_matches(m, bte._version)) {
+    return false;
+  }
+
+  *method = m;
+  *bci = bte._bci;
+  return true;
+}
+
 oop java_lang_StackTraceElement::create(const methodHandle& method, int bci, TRAPS) {
   // Allocate java.lang.StackTraceElement instance
   InstanceKlass* k = SystemDictionary::StackTraceElement_klass();

src/hotspot/share/classfile/javaClasses.hpp

@@ -518,7 +518,8 @@ class java_lang_Throwable: AllStatic {
     trace_mirrors_offset = 2,
     trace_names_offset   = 3,
     trace_next_offset    = 4,
-    trace_size           = 5,
+    trace_hidden_offset  = 5,
+    trace_size           = 6,
     trace_chunk_size     = 32
   };
 
@@ -568,6 +569,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);
 };
 
 

src/hotspot/share/classfile/systemDictionary.hpp

@@ -155,6 +155,7 @@ class GCTimer;
   do_klass(reflect_ConstantPool_klass,                  reflect_ConstantPool                                  ) \
   do_klass(reflect_UnsafeStaticFieldAccessorImpl_klass, reflect_UnsafeStaticFieldAccessorImpl                 ) \
   do_klass(reflect_CallerSensitive_klass,               reflect_CallerSensitive                               ) \
+  do_klass(reflect_NativeConstructorAccessorImpl_klass, reflect_NativeConstructorAccessorImpl                 ) \
                                                                                                                 \
   /* support for dynamic typing; it's OK if these are NULL in earlier JDKs */                                   \
   do_klass(DirectMethodHandle_klass,                    java_lang_invoke_DirectMethodHandle                   ) \

src/hotspot/share/classfile/vmSymbols.hpp

@@ -243,6 +243,7 @@
   template(reflect_Reflection,                        "jdk/internal/reflect/Reflection")              \
   template(reflect_CallerSensitive,                   "jdk/internal/reflect/CallerSensitive")         \
   template(reflect_CallerSensitive_signature,         "Ljdk/internal/reflect/CallerSensitive;")       \
+  template(reflect_NativeConstructorAccessorImpl,     "jdk/internal/reflect/NativeConstructorAccessorImpl")\
   template(checkedExceptions_name,                    "checkedExceptions")                        \
   template(clazz_name,                                "clazz")                                    \
   template(exceptionTypes_name,                       "exceptionTypes")                           \

src/hotspot/share/include/jvm.h

@@ -191,6 +191,13 @@ JVM_InitStackTraceElementArray(JNIEnv *env, jobjectArray elements, jobject throw
 JNIEXPORT void JNICALL
 JVM_InitStackTraceElement(JNIEnv* env, jobject element, jobject stackFrameInfo);
 
+/*
+ * java.lang.NullPointerException
+ */
+
+JNIEXPORT jstring JNICALL
+JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable);
+
 /*
  * java.lang.StackWalker
  */

src/hotspot/share/interpreter/bytecodeUtils.hpp

@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2019, 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.
+ *
+ * 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.
+ *
+ */
+
+#ifndef SHARE_INTERPRETER_BYTECODEUTILS_HPP
+#define SHARE_INTERPRETER_BYTECODEUTILS_HPP
+
+#include "memory/allocation.hpp"
+#include "utilities/globalDefinitions.hpp"
+
+class Method;
+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);
+};
+
+#endif // SHARE_INTERPRETER_BYTECODEUTILS_HPP

src/hotspot/share/prims/jvm.cpp

@@ -38,6 +38,7 @@
 #include "classfile/vmSymbols.hpp"
 #include "gc/shared/collectedHeap.inline.hpp"
 #include "interpreter/bytecode.hpp"
+#include "interpreter/bytecodeUtils.hpp"
 #include "jfr/jfrEvents.hpp"
 #include "logging/log.hpp"
 #include "memory/heapShared.hpp"
@@ -531,13 +532,37 @@ JVM_END
 
 // java.lang.Throwable //////////////////////////////////////////////////////
 
-
 JVM_ENTRY(void, JVM_FillInStackTrace(JNIEnv *env, jobject receiver))
   JVMWrapper("JVM_FillInStackTrace");
   Handle exception(thread, JNIHandles::resolve_non_null(receiver));
   java_lang_Throwable::fill_in_stack_trace(exception);
 JVM_END
 
+// java.lang.NullPointerException ///////////////////////////////////////////
+
+JVM_ENTRY(jstring, JVM_GetExtendedNPEMessage(JNIEnv *env, jthrowable throwable))
+  if (!ShowCodeDetailsInExceptionMessages) return NULL;
+
+  oop exc = JNIHandles::resolve_non_null(throwable);
+
+  Method* method;
+  int bci;
+  if (!java_lang_Throwable::get_top_method_and_bci(exc, &method, &bci)) {
+    return NULL;
+  }
+  if (method->is_native()) {
+    return NULL;
+  }
+
+  stringStream ss;
+  bool ok = BytecodeUtils::get_NPE_message_at(&ss, method, bci);
+  if (ok) {
+    oop result = java_lang_String::create_oop_from_str(ss.base(), CHECK_0);
+    return (jstring) JNIHandles::make_local(env, result);
+  } else {
+    return NULL;
+  }
+JVM_END
 
 // java.lang.StackTraceElement //////////////////////////////////////////////
 

src/hotspot/share/runtime/globals.hpp

@@ -643,6 +643,10 @@ const size_t minimumSymbolTableSize = 1024;
   product(bool, OmitStackTraceInFastThrow, true,                            \
           "Omit backtraces for some 'hot' exceptions in optimized code")    \
                                                                             \
+  manageable(bool, ShowCodeDetailsInExceptionMessages, false,               \
+          "Show exception messages from RuntimeExceptions that contain "    \
+          "snippets of the failing code. Disable this to improve privacy.") \
+                                                                            \
   product(bool, PrintWarnings, true,                                        \
           "Print JVM warnings to output stream")                            \
                                                                             \

src/java.base/share/classes/java/lang/NullPointerException.java

@@ -70,4 +70,35 @@ class NullPointerException extends RuntimeException {
     public NullPointerException(String s) {
         super(s);
     }
+
+    /**
+     * Returns the detail message string of this throwable.
+     *
+     * <p> If a non-null message was supplied in a constructor it is
+     * returned. Otherwise, an implementation specific message or
+     * {@code null} is returned.
+     *
+     * @implNote
+     * If no explicit message was passed to the constructor, and as
+     * long as certain internal information is available, a verbose
+     * description of the null reference is returned.
+     * The internal information is not available in deserialized
+     * NullPointerExceptions.
+     *
+     * @return the detail message string, which may be {@code null}.
+     */
+    public String getMessage() {
+        String message = super.getMessage();
+        if (message == null) {
+            return getExtendedNPEMessage();
+        }
+        return message;
+    }
+
+    /**
+     * 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();
 }

src/java.base/share/native/libjava/NullPointerException.c

@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2019, 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.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include "jni.h"
+#include "jvm.h"
+
+#include "java_lang_NullPointerException.h"
+
+JNIEXPORT jstring JNICALL
+Java_java_lang_NullPointerException_getExtendedNPEMessage(JNIEnv *env, jobject throwable)
+{
+    return JVM_GetExtendedNPEMessage(env, throwable);
+}

test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NPEInHiddenTopFrameTest.java

@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2019, 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.
+ *
+ * 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 NullPointerException messages thrown in frames that
+ *   are hidden in the backtrace/stackTrace.
+ * @bug 8218628
+ * @library /test/lib
+ * @compile -g NPEInHiddenTopFrameTest.java
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:-ShowHiddenFrames -XX:+ShowCodeDetailsInExceptionMessages NPEInHiddenTopFrameTest hidden
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+ShowHiddenFrames -XX:+ShowCodeDetailsInExceptionMessages NPEInHiddenTopFrameTest visible
+ */
+
+import jdk.test.lib.Asserts;
+
+public class NPEInHiddenTopFrameTest {
+
+    @FunctionalInterface
+    private static interface SomeFunctionalInterface {
+        String someMethod(String a, String b);
+    }
+
+    public static void checkMessage(String expression,
+                                    String obtainedMsg, String expectedMsg) {
+        System.out.println();
+        System.out.println(" source code: " + expression);
+        System.out.println("  thrown msg: " + obtainedMsg);
+        if (obtainedMsg == null && expectedMsg == null) return;
+        System.out.println("expected msg: " + expectedMsg);
+        Asserts.assertEquals(expectedMsg, obtainedMsg);
+    }
+
+    public static void main(String[] args) throws Exception {
+        boolean framesAreHidden = false;
+        if (args.length > 0) {
+            String arg = args[0];
+            if (arg.equals("hidden")) framesAreHidden = true;
+        }
+
+        try {
+            final SomeFunctionalInterface concatter = String::concat;
+            final String nullString = null;
+            if (concatter != null) {
+                // This throws NPE from the lambda expression which is a hidden frame.
+                concatter.someMethod(nullString, "validString");
+            }
+        } catch (NullPointerException e) {
+            checkMessage("concatter.someMethod(nullString, \"validString\");", e.getMessage(),
+                         framesAreHidden ?
+                         // This is the message that would be printed if the wrong method/bci are used:
+                         // "Cannot invoke 'NPEInHiddenTopFrameTest$SomeFunctionalInterface.someMethod(String, String)'" +
+                         // " because 'concatter' is null."
+                         // But the NPE message generation now recognizes this situation and skips the
+                         // message. So we expect null:
+                         null :
+                         // This is the correct message, but it describes code generated on-the-fly.
+                         // You get it if you disable hiding frames (-XX:+ShowHiddenframes).
+                         "Cannot invoke \"String.concat(String)\" because \"<parameter1>\" is null" );
+            e.printStackTrace();
+        }
+    }
+}

test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/SuppressMessagesTest.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2019, 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.
+ *
+ * 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 that the default of flag ShowCodeDetailsInExceptionMessages is 'false',
+ *          i.e., make sure the VM does not print the message on default.
+ * @bug 8218628
+ * @library /test/lib
+ * @compile -g SuppressMessagesTest.java
+ * @run main/othervm SuppressMessagesTest noMessage
+ */
+/**
+ * @test
+ * @summary Test that the messages are suppressed if flag ShowCodeDetailsInExceptionMessages is 'false'.
+ * @bug 8218628
+ * @library /test/lib
+ * @compile -g SuppressMessagesTest.java
+ * @run main/othervm -XX:-ShowCodeDetailsInExceptionMessages SuppressMessagesTest noMessage
+ */
+/**
+ * @test
+ * @summary Test that the messages are printed if flag ShowCodeDetailsInExceptionMessages is 'true'.
+ * @bug 8218628
+ * @library /test/lib
+ * @compile -g SuppressMessagesTest.java
+ * @run main/othervm -XX:+ShowCodeDetailsInExceptionMessages SuppressMessagesTest printMessage
+ */
+
+import jdk.test.lib.Asserts;
+
+class A {
+    int aFld;
+}
+
+// Tests that the messages are suppressed by flag ShowCodeDetailsInExceptionMessages.
+public class SuppressMessagesTest {
+
+    public static void main(String[] args) throws Exception {
+        A a = null;
+
+        if (args.length != 1) {
+            Asserts.fail("You must specify one arg for this test");
+        }
+
+        try {
+            @SuppressWarnings("null")
+            int val = a.aFld;
+            System.out.println(val);
+            Asserts.fail();
+        } catch (NullPointerException e) {
+            System.out.println("Stacktrace of the expected exception:");
+            e.printStackTrace(System.out);
+            if (args[0].equals("noMessage")) {
+                Asserts.assertNull(e.getMessage());
+            } else {
+                Asserts.assertEquals(e.getMessage(), "Cannot read field \"aFld\" because \"a\" is null");
+            }
+        }
+    }
+}