8014426: Original exception no longer thrown away when a finally rethrows
authorlagergren
Tue, 14 May 2013 19:56:35 +0200
changeset 17745 86e5a15b3b20
parent 17744 65849f87aa28
child 17746 bc27c77a3568
8014426: Original exception no longer thrown away when a finally rethrows Reviewed-by: attila, jlaskey
nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java
nashorn/src/jdk/nashorn/internal/codegen/Lower.java
nashorn/src/jdk/nashorn/internal/ir/CatchNode.java
nashorn/src/jdk/nashorn/internal/ir/ThrowNode.java
nashorn/src/jdk/nashorn/internal/parser/Parser.java
nashorn/test/script/basic/JDK-8014426.js
nashorn/test/script/basic/JDK-8014426.js.EXPECTED
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Tue May 14 19:18:17 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Tue May 14 19:56:35 2013 +0200
@@ -2028,6 +2028,13 @@
     public boolean enterThrowNode(final ThrowNode throwNode) {
         lineNumber(throwNode);
 
+        if (throwNode.isSyntheticRethrow()) {
+            //do not wrap whatever this is in an ecma exception, just rethrow it
+            load(throwNode.getExpression());
+            method.athrow();
+            return false;
+        }
+
         method._new(ECMAException.class).dup();
 
         final Source source     = getLexicalContext().getCurrentFunction().getSource();
@@ -2096,13 +2103,16 @@
                 }
                 @Override
                 protected void evaluate() {
+                    if (catchNode.isSyntheticRethrow()) {
+                        method.load(symbol);
+                        return;
+                    }
                     /*
                      * If caught object is an instance of ECMAException, then
                      * bind obj.thrown to the script catch var. Or else bind the
                      * caught object itself to the script catch var.
                      */
                     final Label notEcmaException = new Label("no_ecma_exception");
-
                     method.load(symbol).dup()._instanceof(ECMAException.class).ifeq(notEcmaException);
                     method.checkcast(ECMAException.class); //TODO is this necessary?
                     method.getField(ECMAException.THROWN);
--- a/nashorn/src/jdk/nashorn/internal/codegen/Lower.java	Tue May 14 19:18:17 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/Lower.java	Tue May 14 19:56:35 2013 +0200
@@ -288,10 +288,10 @@
 
         final IdentNode exception = new IdentNode(token, finish, getLexicalContext().getCurrentFunction().uniqueName("catch_all"));
 
-        final Block catchBody = new Block(lineNumber, token, finish, new ThrowNode(lineNumber, token, finish, new IdentNode(exception))).
+        final Block catchBody = new Block(lineNumber, token, finish, new ThrowNode(lineNumber, token, finish, new IdentNode(exception), ThrowNode.IS_SYNTHETIC_RETHROW)).
                 setIsTerminal(getLexicalContext(), true); //ends with throw, so terminal
 
-        final CatchNode catchAllNode  = new CatchNode(lineNumber, token, finish, new IdentNode(exception), null, catchBody);
+        final CatchNode catchAllNode  = new CatchNode(lineNumber, token, finish, new IdentNode(exception), null, catchBody, CatchNode.IS_SYNTHETIC_RETHROW);
         final Block     catchAllBlock = new Block(lineNumber, token, finish, catchAllNode);
 
         //catchallblock -> catchallnode (catchnode) -> exception -> throw
--- a/nashorn/src/jdk/nashorn/internal/ir/CatchNode.java	Tue May 14 19:18:17 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/CatchNode.java	Tue May 14 19:56:35 2013 +0200
@@ -42,6 +42,11 @@
     /** Catch body. */
     private final Block body;
 
+    private final int flags;
+
+    /** Is this block a synthethic rethrow created by finally inlining? */
+    public static final int IS_SYNTHETIC_RETHROW = 1;
+
     /**
      * Constructors
      *
@@ -51,19 +56,22 @@
      * @param exception          variable name of exception
      * @param exceptionCondition exception condition
      * @param body               catch body
+     * @param flags              flags
      */
-    public CatchNode(final int lineNumber, final long token, final int finish, final IdentNode exception, final Node exceptionCondition, final Block body) {
+    public CatchNode(final int lineNumber, final long token, final int finish, final IdentNode exception, final Node exceptionCondition, final Block body, final int flags) {
         super(lineNumber, token, finish);
         this.exception          = exception;
         this.exceptionCondition = exceptionCondition;
         this.body               = body;
+        this.flags              = flags;
     }
 
-    private CatchNode(final CatchNode catchNode, final IdentNode exception, final Node exceptionCondition, final Block body) {
+    private CatchNode(final CatchNode catchNode, final IdentNode exception, final Node exceptionCondition, final Block body, final int flags) {
         super(catchNode);
         this.exception          = exception;
         this.exceptionCondition = exceptionCondition;
         this.body               = body;
+        this.flags              = flags;
     }
 
     /**
@@ -124,7 +132,7 @@
         if (this.exceptionCondition == exceptionCondition) {
             return this;
         }
-        return new CatchNode(this, exception, exceptionCondition, body);
+        return new CatchNode(this, exception, exceptionCondition, body, flags);
     }
 
     /**
@@ -144,13 +152,25 @@
         if (this.exception == exception) {
             return this;
         }
-        return new CatchNode(this, exception, exceptionCondition, body);
+        return new CatchNode(this, exception, exceptionCondition, body, flags);
     }
 
     private CatchNode setBody(final Block body) {
         if (this.body == body) {
             return this;
         }
-        return new CatchNode(this, exception, exceptionCondition, body);
+        return new CatchNode(this, exception, exceptionCondition, body, flags);
     }
+
+    /**
+     * Is this catch block a non-JavaScript constructor, for example created as
+     * part of the rethrow mechanism of a finally block in Lower? Then we just
+     * pass the exception on and need not unwrap whatever is in the ECMAException
+     * object catch symbol
+     * @return true if a finally synthetic rethrow
+     */
+    public boolean isSyntheticRethrow() {
+        return (flags & IS_SYNTHETIC_RETHROW) == IS_SYNTHETIC_RETHROW;
+    }
+
 }
--- a/nashorn/src/jdk/nashorn/internal/ir/ThrowNode.java	Tue May 14 19:18:17 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/ThrowNode.java	Tue May 14 19:56:35 2013 +0200
@@ -36,6 +36,11 @@
     /** Exception expression. */
     private final Node expression;
 
+    private final int flags;
+
+    /** Is this block a synthethic rethrow created by finally inlining? */
+    public static final int IS_SYNTHETIC_RETHROW = 1;
+
     /**
      * Constructor
      *
@@ -43,15 +48,18 @@
      * @param token      token
      * @param finish     finish
      * @param expression expression to throw
+     * @param flags      flags
      */
-    public ThrowNode(final int lineNumber, final long token, final int finish, final Node expression) {
+    public ThrowNode(final int lineNumber, final long token, final int finish, final Node expression, final int flags) {
         super(lineNumber, token, finish);
         this.expression = expression;
+        this.flags = flags;
     }
 
-    private ThrowNode(final ThrowNode node, final Node expression) {
+    private ThrowNode(final ThrowNode node, final Node expression, final int flags) {
         super(node);
         this.expression = expression;
+        this.flags = flags;
     }
 
     @Override
@@ -98,7 +106,17 @@
         if (this.expression == expression) {
             return this;
         }
-        return new ThrowNode(this, expression);
+        return new ThrowNode(this, expression, flags);
+    }
+
+    /**
+     * Is this a throw a synthetic rethrow in a synthetic catch-all block
+     * created when inlining finally statements? In that case we never
+     * wrap whatever is thrown into an ECMAException, just rethrow it.
+     * @return true if synthetic throw node
+     */
+    public boolean isSyntheticRethrow() {
+        return (flags & IS_SYNTHETIC_RETHROW) == IS_SYNTHETIC_RETHROW;
     }
 
 }
--- a/nashorn/src/jdk/nashorn/internal/parser/Parser.java	Tue May 14 19:18:17 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/parser/Parser.java	Tue May 14 19:56:35 2013 +0200
@@ -1537,7 +1537,7 @@
 
         endOfLine();
 
-        appendStatement(new ThrowNode(throwLine, throwToken, finish, expression));
+        appendStatement(new ThrowNode(throwLine, throwToken, finish, expression, 0));
     }
 
     /**
@@ -1597,7 +1597,7 @@
                 try {
                     // Get CATCH body.
                     final Block catchBody = getBlock(true);
-                    final CatchNode catchNode = new CatchNode(catchLine, catchToken, finish, exception, ifExpression, catchBody);
+                    final CatchNode catchNode = new CatchNode(catchLine, catchToken, finish, exception, ifExpression, catchBody, 0);
                     appendStatement(catchNode);
                 } finally {
                     catchBlock = restoreBlock(catchBlock);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8014426.js	Tue May 14 19:56:35 2013 +0200
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2010, 2013, 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.
+ */
+
+/**
+ * Ensure catchall exceptions from finally inlining are rethrown as is
+ *
+ * @test
+ * @run
+ */
+
+function runScriptEngine() {
+    var fac    = new Packages.jdk.nashorn.api.scripting.NashornScriptEngineFactory();
+    var engine = fac.getScriptEngine();
+    engine.eval(
+"try {\n\
+  doIt();\n\
+} finally { \n\
+  var x = 17;\n\
+}\n\
+function doIt() {\n\
+  throw new TypeError('en stor graa noshoerning!');\n\
+}\n");
+}
+
+try {
+    runScriptEngine();
+} catch(e) {
+    print(e);
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8014426.js.EXPECTED	Tue May 14 19:56:35 2013 +0200
@@ -0,0 +1,1 @@
+javax.script.ScriptException: TypeError: en stor graa noshoerning! in <eval> at line number 7 at column number 2