8031983: Error objects should capture stack at the constructor
authorsundar
Thu, 16 Jan 2014 21:26:21 +0530
changeset 22387 81639250d335
parent 22386 b421b9049d11
child 22388 1a18a66a182f
8031983: Error objects should capture stack at the constructor Reviewed-by: jlaskey, hannesw
nashorn/src/jdk/nashorn/api/scripting/NashornException.java
nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java
nashorn/src/jdk/nashorn/internal/objects/NativeError.java
nashorn/src/jdk/nashorn/internal/objects/NativeEvalError.java
nashorn/src/jdk/nashorn/internal/objects/NativeRangeError.java
nashorn/src/jdk/nashorn/internal/objects/NativeReferenceError.java
nashorn/src/jdk/nashorn/internal/objects/NativeSyntaxError.java
nashorn/src/jdk/nashorn/internal/objects/NativeTypeError.java
nashorn/src/jdk/nashorn/internal/objects/NativeURIError.java
nashorn/src/jdk/nashorn/internal/runtime/ECMAException.java
nashorn/test/script/basic/JDK-8031983.js
nashorn/test/script/basic/JDK-8031983.js.EXPECTED
nashorn/test/script/basic/NASHORN-441.js.EXPECTED
--- a/nashorn/src/jdk/nashorn/api/scripting/NashornException.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/api/scripting/NashornException.java	Thu Jan 16 21:26:21 2014 +0530
@@ -45,11 +45,11 @@
 @SuppressWarnings("serial")
 public abstract class NashornException extends RuntimeException {
     // script file name
-    private final String fileName;
+    private String fileName;
     // script line number
-    private final int line;
+    private int line;
     // script column number
-    private final int column;
+    private int column;
     // underlying ECMA error object - lazily initialized
     private Object ecmaError;
 
@@ -125,6 +125,15 @@
     }
 
     /**
+     * Set the source file name for this {@code NashornException}
+     *
+     * @param fileName the file name
+     */
+    public final void setFileName(final String fileName) {
+        this.fileName = fileName;
+    }
+
+    /**
      * Get the line number for this {@code NashornException}
      *
      * @return the line number
@@ -134,15 +143,33 @@
     }
 
     /**
+     * Set the line number for this {@code NashornException}
+     *
+     * @param line the line number
+     */
+    public final void setLineNumber(final int line) {
+        this.line = line;
+    }
+
+    /**
      * Get the column for this {@code NashornException}
      *
-     * @return the column
+     * @return the column number
      */
     public final int getColumnNumber() {
         return column;
     }
 
     /**
+     * Set the column for this {@code NashornException}
+     *
+     * @param column the column number
+     */
+    public final void setColumnNumber(final int column) {
+        this.column = column;
+    }
+
+    /**
      * Returns array javascript stack frames from the given exception object.
      *
      * @param exception exception from which stack frames are retrieved and filtered
@@ -208,9 +235,9 @@
 
         final Object thrown = getThrown();
         if (thrown instanceof ScriptObject) {
-            ecmaError = ScriptObjectMirror.wrap(thrown, global);
+            setEcmaError(ScriptObjectMirror.wrap(thrown, global));
         } else {
-            ecmaError = thrown;
+            setEcmaError(thrown);
         }
 
         return this;
@@ -225,4 +252,14 @@
     public Object getEcmaError() {
         return ecmaError;
     }
+
+    /**
+     * Return the underlying ECMA error object, if available.
+     *
+     * @param ecmaError underlying ECMA Error object's mirror or whatever was thrown
+     *         from script such as a String, Number or a Boolean.
+     */
+    public void setEcmaError(final Object ecmaError) {
+        this.ecmaError = ecmaError;
+    }
 }
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Thu Jan 16 21:26:21 2014 +0530
@@ -2023,8 +2023,6 @@
             return false;
         }
 
-        method._new(ECMAException.class).dup();
-
         final Source source     = lc.getCurrentFunction().getSource();
 
         final Expression expression = throwNode.getExpression();
@@ -2037,7 +2035,7 @@
         method.load(source.getName());
         method.load(line);
         method.load(column);
-        method.invoke(ECMAException.THROW_INIT);
+        method.invoke(ECMAException.CREATE);
 
         method.athrow();
 
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeError.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeError.java	Thu Jan 16 21:26:21 2014 +0530
@@ -38,7 +38,6 @@
 import jdk.nashorn.internal.objects.annotations.Property;
 import jdk.nashorn.internal.objects.annotations.ScriptClass;
 import jdk.nashorn.internal.objects.annotations.Where;
-import jdk.nashorn.internal.objects.ScriptFunctionImpl;
 import jdk.nashorn.internal.runtime.ECMAException;
 import jdk.nashorn.internal.runtime.JSType;
 import jdk.nashorn.internal.runtime.PropertyMap;
@@ -93,6 +92,7 @@
         return $nasgenmap$;
     }
 
+    @SuppressWarnings("LeakingThisInConstructor")
     private NativeError(final Object msg, final ScriptObject proto, final PropertyMap map) {
         super(proto, map);
         if (msg != UNDEFINED) {
@@ -100,6 +100,7 @@
         } else {
             this.delete(NativeError.MESSAGE, false);
         }
+        initException(this);
     }
 
     NativeError(final Object msg, final Global global) {
@@ -129,6 +130,14 @@
         return new NativeError(msg);
     }
 
+    // This is called NativeError, NativeTypeError etc. to
+    // associate a ECMAException with the ECMA Error object.
+    @SuppressWarnings("unused")
+    static void initException(final ScriptObject self) {
+        // ECMAException constructor has side effects
+        new ECMAException(self, null);
+    }
+
     /**
      * Nashorn extension: Error.captureStackTrace. Capture stack trace at the point of call into the Error object provided.
      *
@@ -136,16 +145,17 @@
      * @param errorObj the error object
      * @return undefined
      */
-    @SuppressWarnings("unused")
     @Function(attributes = Attribute.NOT_ENUMERABLE, where = Where.CONSTRUCTOR)
     public static Object captureStackTrace(final Object self, final Object errorObj) {
         Global.checkObject(errorObj);
         final ScriptObject sobj = (ScriptObject)errorObj;
-        new ECMAException(sobj, null); //constructor has side effects
-        sobj.delete("stack", false);
-        final ScriptFunction getStack = ScriptFunctionImpl.makeFunction("getStack", GET_STACK);
-        final ScriptFunction setStack = ScriptFunctionImpl.makeFunction("setStack", SET_STACK);
-        sobj.addOwnProperty("stack", Attribute.NOT_ENUMERABLE, getStack, setStack);
+        initException(sobj);
+        sobj.delete(STACK, false);
+        if (! sobj.has("stack")) {
+            final ScriptFunction getStack = ScriptFunctionImpl.makeFunction("getStack", GET_STACK);
+            final ScriptFunction setStack = ScriptFunctionImpl.makeFunction("setStack", SET_STACK);
+            sobj.addOwnProperty("stack", Attribute.NOT_ENUMERABLE, getStack, setStack);
+        }
         return UNDEFINED;
     }
 
@@ -226,7 +236,11 @@
     public static Object setLineNumber(final Object self, final Object value) {
         Global.checkObject(self);
         final ScriptObject sobj = (ScriptObject)self;
-        sobj.set(LINENUMBER, value, false);
+        if (sobj.hasOwnProperty(LINENUMBER)) {
+            sobj.put(LINENUMBER, value, false);
+        } else {
+            sobj.addOwnProperty(LINENUMBER, Attribute.NOT_ENUMERABLE, value);
+        }
         return value;
     }
 
@@ -254,7 +268,11 @@
     public static Object setColumnNumber(final Object self, final Object value) {
         Global.checkObject(self);
         final ScriptObject sobj = (ScriptObject)self;
-        sobj.set(COLUMNNUMBER, value, false);
+        if (sobj.hasOwnProperty(COLUMNNUMBER)) {
+            sobj.put(COLUMNNUMBER, value, false);
+        } else {
+            sobj.addOwnProperty(COLUMNNUMBER, Attribute.NOT_ENUMERABLE, value);
+        }
         return value;
     }
 
@@ -282,7 +300,11 @@
     public static Object setFileName(final Object self, final Object value) {
         Global.checkObject(self);
         final ScriptObject sobj = (ScriptObject)self;
-        sobj.set(FILENAME, value, false);
+        if (sobj.hasOwnProperty(FILENAME)) {
+            sobj.put(FILENAME, value, false);
+        } else {
+            sobj.addOwnProperty(FILENAME, Attribute.NOT_ENUMERABLE, value);
+        }
         return value;
     }
 
@@ -304,10 +326,12 @@
 
         final Object exception = ECMAException.getException(sobj);
         if (exception instanceof Throwable) {
-            return getScriptStackString(sobj, (Throwable)exception);
+            Object value = getScriptStackString(sobj, (Throwable)exception);
+            sobj.put(STACK, value, false);
+            return value;
         }
 
-        return "";
+        return UNDEFINED;
     }
 
     /**
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeEvalError.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeEvalError.java	Thu Jan 16 21:26:21 2014 +0530
@@ -62,6 +62,7 @@
         return $nasgenmap$;
     }
 
+    @SuppressWarnings("LeakingThisInConstructor")
     private NativeEvalError(final Object msg, final ScriptObject proto, final PropertyMap map) {
         super(proto, map);
         if (msg != UNDEFINED) {
@@ -69,6 +70,7 @@
         } else {
             this.delete(NativeError.MESSAGE, false);
         }
+        NativeError.initException(this);
     }
 
     NativeEvalError(final Object msg, final Global global) {
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeRangeError.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeRangeError.java	Thu Jan 16 21:26:21 2014 +0530
@@ -62,6 +62,7 @@
         return $nasgenmap$;
     }
 
+    @SuppressWarnings("LeakingThisInConstructor")
     private NativeRangeError(final Object msg, final ScriptObject proto, final PropertyMap map) {
         super(proto, map);
         if (msg != UNDEFINED) {
@@ -69,6 +70,7 @@
         } else {
             this.delete(NativeError.MESSAGE, false);
         }
+        NativeError.initException(this);
     }
 
     NativeRangeError(final Object msg, final Global global) {
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeReferenceError.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeReferenceError.java	Thu Jan 16 21:26:21 2014 +0530
@@ -62,6 +62,7 @@
         return $nasgenmap$;
     }
 
+    @SuppressWarnings("LeakingThisInConstructor")
     private NativeReferenceError(final Object msg, final ScriptObject proto, final PropertyMap map) {
         super(proto, map);
         if (msg != UNDEFINED) {
@@ -69,6 +70,7 @@
         } else {
             this.delete(NativeError.MESSAGE, false);
         }
+        NativeError.initException(this);
     }
 
     NativeReferenceError(final Object msg, final Global global) {
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeSyntaxError.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeSyntaxError.java	Thu Jan 16 21:26:21 2014 +0530
@@ -62,6 +62,7 @@
         return $nasgenmap$;
     }
 
+    @SuppressWarnings("LeakingThisInConstructor")
     NativeSyntaxError(final Object msg, final Global global) {
         super(global.getSyntaxErrorPrototype(), global.getSyntaxErrorMap());
         if (msg != UNDEFINED) {
@@ -69,6 +70,7 @@
         } else {
             this.delete(NativeError.MESSAGE, false);
         }
+        NativeError.initException(this);
     }
 
     private NativeSyntaxError(final Object msg) {
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeTypeError.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeTypeError.java	Thu Jan 16 21:26:21 2014 +0530
@@ -62,6 +62,7 @@
         return $nasgenmap$;
     }
 
+    @SuppressWarnings("LeakingThisInConstructor")
     NativeTypeError(final Object msg, final Global global) {
         super(global.getTypeErrorPrototype(), global.getTypeErrorMap());
         if (msg != UNDEFINED) {
@@ -69,6 +70,7 @@
         } else {
             delete(NativeError.MESSAGE, false);
         }
+        NativeError.initException(this);
     }
 
     private NativeTypeError(final Object msg) {
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeURIError.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeURIError.java	Thu Jan 16 21:26:21 2014 +0530
@@ -61,6 +61,7 @@
         return $nasgenmap$;
     }
 
+    @SuppressWarnings("LeakingThisInConstructor")
     NativeURIError(final Object msg, final Global global) {
         super(global.getURIErrorPrototype(), global.getURIErrorMap());
         if (msg != UNDEFINED) {
@@ -68,6 +69,7 @@
         } else {
             this.delete(NativeError.MESSAGE, false);
         }
+        NativeError.initException(this);
     }
 
     private NativeURIError(final Object msg) {
--- a/nashorn/src/jdk/nashorn/internal/runtime/ECMAException.java	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ECMAException.java	Thu Jan 16 21:26:21 2014 +0530
@@ -25,7 +25,7 @@
 
 package jdk.nashorn.internal.runtime;
 
-import static jdk.nashorn.internal.codegen.CompilerConstants.constructorNoLookup;
+import static jdk.nashorn.internal.codegen.CompilerConstants.staticCallNoLookup;
 import static jdk.nashorn.internal.codegen.CompilerConstants.virtualField;
 import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED;
 
@@ -33,6 +33,7 @@
 import jdk.nashorn.api.scripting.NashornException;
 import jdk.nashorn.internal.codegen.CompilerConstants.Call;
 import jdk.nashorn.internal.codegen.CompilerConstants.FieldAccess;
+import jdk.nashorn.internal.objects.NativeError;
 
 /**
  * Exception used to implement ECMAScript "throw" from scripts. The actual thrown
@@ -44,9 +45,9 @@
 @SuppressWarnings("serial")
 public final class ECMAException extends NashornException {
     /**
-     * Method handle pointing to the constructor {@link ECMAException#ECMAException(Object, String, int, int)},
+     * Method handle pointing to the constructor {@link ECMAException#create(Object, String, int, int)},
      */
-    public static final Call THROW_INIT = constructorNoLookup(ECMAException.class, Object.class, String.class, int.class, int.class);
+    public static final Call CREATE = staticCallNoLookup(ECMAException.class, "create", ECMAException.class, Object.class, String.class, int.class, int.class);
 
     /** Field handle to the{@link ECMAException#thrown} field, so that it can be accessed from generated code */
     public static final FieldAccess THROWN = virtualField(ECMAException.class, "thrown", Object.class);
@@ -57,23 +58,21 @@
     public final Object thrown;
 
     /**
-     * Constructor. This is called from generated code to implement the {@code throw}
-     * instruction from generated script code
+     * Constructor. Called from the factory method 'create'.
      *
      * @param thrown    object to be thrown
      * @param fileName  script file name
      * @param line      line number of throw
      * @param column    column number of throw
      */
-    public ECMAException(final Object thrown, final String fileName, final int line, final int column) {
+    private ECMAException(final Object thrown, final String fileName, final int line, final int column) {
         super(ScriptRuntime.safeToString(thrown), asThrowable(thrown), fileName, line, column);
         this.thrown = thrown;
         setExceptionToThrown();
     }
 
     /**
-     * Constructor. This is called from runtime code in Nashorn to throw things like
-     * type errors.
+     * Constructor. This is called from the runtime code.
      *
      * @param thrown   object to be thrown
      * @param cause    Java exception that triggered this throw
@@ -85,6 +84,35 @@
     }
 
     /**
+     * Factory method to retrieve the underlying exception or create an exception.
+     * This method is called from the generated code.
+     *
+     * @param thrown    object to be thrown
+     * @param fileName  script file name
+     * @param line      line number of throw
+     * @param column    column number of throw
+     * @return ECMAException object
+     */
+    public static ECMAException create(final Object thrown, final String fileName, final int line, final int column) {
+        // If thrown object is an Error or sub-object like TypeError, then
+        // an ECMAException object has been already initialized at constructor.
+        if (thrown instanceof ScriptObject) {
+            ScriptObject sobj = (ScriptObject)thrown;
+            Object exception = getException(sobj);
+            if (exception instanceof ECMAException) {
+                // copy over file name, line number and column number.
+                final ECMAException ee = (ECMAException)exception;
+                ee.setFileName(fileName);
+                ee.setLineNumber(line);
+                ee.setColumnNumber(column);
+                return ee;
+            }
+        }
+
+        return new ECMAException(thrown, fileName, line, column);
+    }
+
+    /**
      * Get the thrown object
      * @return thrown object
      */
@@ -257,6 +285,8 @@
             final ScriptObject sobj = (ScriptObject)thrown;
             if (!sobj.has(EXCEPTION_PROPERTY)) {
                 sobj.addOwnProperty(EXCEPTION_PROPERTY, Property.NOT_ENUMERABLE, this);
+            } else {
+                sobj.set(EXCEPTION_PROPERTY, this, false);
             }
         }
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8031983.js	Thu Jan 16 21:26:21 2014 +0530
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2014, 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.
+ */
+
+/**
+ * JDK-8031983: Error objects should capture stack at the constructor 
+ *
+ * @test
+ * @run
+ */
+
+var e = new Error();
+print("hello");
+
+try {
+    throw e;
+} catch (e) {
+    print(e.lineNumber);
+    print(e.stack.replace(/\\/g, '/'));
+}
+
+Error.captureStackTrace(e);
+try {
+    throw e;
+} catch (e) {
+    print(e.lineNumber);
+    print(e.stack.replace(/\\/g, '/'));
+}
+
+var obj = {};
+Error.captureStackTrace(obj);
+try {
+    throw obj;
+} catch (e) {
+    print(e.stack.replace(/\\/g, '/'));
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8031983.js.EXPECTED	Thu Jan 16 21:26:21 2014 +0530
@@ -0,0 +1,9 @@
+hello
+35
+Error
+	at <program> (test/script/basic/JDK-8031983.js:31)
+43
+Error
+	at <program> (test/script/basic/JDK-8031983.js:41)
+[object Object]
+	at <program> (test/script/basic/JDK-8031983.js:50)
--- a/nashorn/test/script/basic/NASHORN-441.js.EXPECTED	Wed Jan 15 14:51:49 2014 +0530
+++ b/nashorn/test/script/basic/NASHORN-441.js.EXPECTED	Thu Jan 16 21:26:21 2014 +0530
@@ -12,6 +12,6 @@
 try 5
 rethrow 5
 finally 5
-Error: try 5 thrown in line 71
+Error: try 5 thrown in line 74
 try 6
 finally 6