# HG changeset patch
# User goetz
# Date 1593596705 -7200
# Node ID 4e130cdf75d504576d05947a90c80e95db1f44d9
# Parent  78c07dd7240412e60d8694e9dbfd46e57bd42ee0
8248476: No helpful NullPointerException message after calling fillInStackTrace
Summary: reported by christoph.dreis@freenet.de
Reviewed-by:

diff --git a/src/hotspot/share/interpreter/bytecodeUtils.cpp b/src/hotspot/share/interpreter/bytecodeUtils.cpp
--- a/src/hotspot/share/interpreter/bytecodeUtils.cpp
+++ b/src/hotspot/share/interpreter/bytecodeUtils.cpp
@@ -1152,7 +1152,11 @@
         // Assume the the call of a constructor can never cause a NullPointerException
         // (which is true in Java). This is mainly used to avoid generating wrong
         // messages for NullPointerExceptions created explicitly by new in Java code.
-        if (name != vmSymbols::object_initializer_name()) {
+        //
+        // Also, if the stack trace was filled by a user call, we don't print
+        // the message as it will print the code details where the stack trace
+        // was filled in, not the ones where the NPE was thrown.
+        if (name != vmSymbols::object_initializer_name()/* && name != vmSymbols::fillInStackTrace_name()*/) {
           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.
diff --git a/src/java.base/share/classes/java/lang/NullPointerException.java b/src/java.base/share/classes/java/lang/NullPointerException.java
--- a/src/java.base/share/classes/java/lang/NullPointerException.java
+++ b/src/java.base/share/classes/java/lang/NullPointerException.java
@@ -70,6 +70,31 @@
         super(s);
     }
 
+    private int numStackTracesFilledIn = 0;
+
+    /**
+     * Fills in the execution stack trace. This method records within this
+     * {@code NullPointerException} object information about the current state of
+     * the stack frames for the current thread.
+     *
+     * <p>If the stack trace of this {@code NullPointerException} {@linkplain
+     * Throwable#Throwable(String, Throwable, boolean, boolean) is not
+     * writable}, calling this method has no effect.
+     *
+     * @implNote
+     * This delegates to {@code super.fillInStackTrace()}. In addition,
+     * it records some internal information used to compute
+     * the verbose {@code getMessage()} return value.
+     *
+     * @return  a reference to this {@code Throwable} instance.
+     * @see     java.lang.Throwable#printStackTrace()
+     */
+    public synchronized Throwable fillInStackTrace() {
+        Throwable result = super.fillInStackTrace();
+        if (result == this) numStackTracesFilledIn++;
+        return result;
+    }
+
     /**
      * Returns the detail message string of this throwable.
      *
@@ -88,7 +113,9 @@
      */
     public String getMessage() {
         String message = super.getMessage();
-        if (message == null) {
+        // If the stack trace was changed the extended NPE algorithm
+        // will compute a wrong message.
+        if (message == null && numStackTracesFilledIn == 0) {
             return getExtendedNPEMessage();
         }
         return message;
diff --git a/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NullPointerExceptionTest.java b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NullPointerExceptionTest.java
--- a/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NullPointerExceptionTest.java
+++ b/test/hotspot/jtreg/runtime/exceptionMsgs/NullPointerException/NullPointerExceptionTest.java
@@ -28,7 +28,7 @@
  * @summary Test extended NullPointerException message for
  *   classfiles generated with debug information. In this case the name
  *   of the variable containing the array is printed.
- * @bug 8218628
+ * @bug 8218628 8248476
  * @modules java.base/java.lang:open
  *          java.base/jdk.internal.org.objectweb.asm
  * @library /test/lib
@@ -41,7 +41,7 @@
  * @summary Test extended NullPointerException message for class
  *   files generated without debugging information. The message lists
  *   detailed information about the entity that is null.
- * @bug 8218628
+ * @bug 8218628 8248476
  * @modules java.base/java.lang:open
  *          java.base/jdk.internal.org.objectweb.asm
  * @library /test/lib
@@ -1281,8 +1281,78 @@
         String msg = new String("A pointless message");
         Asserts.assertTrue(new NullPointerException(msg).getMessage() == msg);
 
+        // If the stack trace was set by the user, the message should not be
+        // created.
+        // This holds for explicitly crated NPEs, but also for implicilty
+        // thrown ones.
+        System.out.println(new NullPointerException().fillInStackTrace().getMessage());
+        System.out.println(new NullPointerException(msg).fillInStackTrace().getMessage());
+
+        Asserts.assertNull(new NullPointerException().fillInStackTrace().getMessage());
+        Asserts.assertTrue(new NullPointerException(msg).fillInStackTrace().getMessage() == msg);
+
+        NullPointerException ex = new NullPointerException();
+        Throwable t = ex.fillInStackTrace();
+
+        Asserts.assertNull(t.getMessage());
+
+        ex = new NullPointerException(msg);
+        t = ex.fillInStackTrace();
+        Asserts.assertTrue(t.getMessage() == msg);
+
+        F f = null;
+        try {
+            f.i = 17;
+        } catch (NullPointerException e) {
+            checkMessage(e, "f.i = 17;", e.getMessage(),
+                         "Cannot assign field \"i\" because " +
+                         (hasDebugInfo ? "\"f\"" : "\"<local4>\"") + " is null");
+            t = e.fillInStackTrace();
+        }
+        checkMessage(t, "e.fillInStackTrace()", t.getMessage(), null);
+
+        // Suppressing messages at fillInStackTrace also suppresses
+        // the generated message if fillInStackTrace was called on null.
+        ex = null;
+        try {
+            ex.fillInStackTrace();
+        } catch (NullPointerException e) {
+            // Without supressing the message we would see: 'Cannot invoke
+            // "java.lang.NullPointerException.fillInStackTrace()" because "ex" is null'.
+            checkMessage(e, "ex.fillInStackTrace()", e.getMessage(), //null);
+                         "Cannot invoke \"java.lang.NullPointerException.fillInStackTrace()\" because " +
+                         (hasDebugInfo ? "\"ex\"" : "\"<local2>\"") + " is null");
+        }
+
+        // setStackTrace does not affect computing the message.
+        // Message and stack trace won't match, though.
+        F f1 = null;
+        F f2 = null;
+        NullPointerException e1 = null;
+        NullPointerException e2 = null;
+        try {
+            f1.i = 18;
+        } catch (NullPointerException e) {
+            checkMessage(e, "f1.i = 18;", e.getMessage(),
+                         "Cannot assign field \"i\" because " +
+                         (hasDebugInfo ? "\"f1\"" : "\"<local5>\"") + " is null");
+            e1 = e;
+        }
+        try {
+            f2.i = 19;
+        } catch (NullPointerException e) {
+            checkMessage(e, "f2.i = 19;", e.getMessage(),
+                         "Cannot assign field \"i\" because " +
+                         (hasDebugInfo ? "\"f2\"" : "\"<local6>\"") + " is null");
+            e2 = e;
+        }
+        e1.setStackTrace(e2.getStackTrace());
+        checkMessage(e1, "f1.i = 18;", e1.getMessage(),
+                     "Cannot assign field \"i\" because " +
+                     (hasDebugInfo ? "\"f1\"" : "\"<local5>\"") + " is null");
+
         // If created via reflection, the message should not be generated.
-        Exception ex = NullPointerException.class.getDeclaredConstructor().newInstance();
+        ex = NullPointerException.class.getDeclaredConstructor().newInstance();
         Asserts.assertNull(ex.getMessage());
     }