Mercurial > hg > openjdk > jdk9 > nashorn
changeset 911:6ab069e48456
8047357: More precise synthetic return + unreachable throw
Reviewed-by: lagergren, sundar
line wrap: on
line diff
--- a/src/jdk/nashorn/internal/codegen/AssignSymbols.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/codegen/AssignSymbols.java Thu Jun 26 13:12:32 2014 +0200 @@ -209,7 +209,7 @@ if (varNode.isFunctionDeclaration()) { symbol.setIsFunctionDeclaration(); } - return varNode.setName((IdentNode)ident.setSymbol(symbol)); + return varNode.setName(ident.setSymbol(symbol)); } return varNode; } @@ -217,7 +217,7 @@ } private IdentNode compilerConstantIdentifier(final CompilerConstants cc) { - return (IdentNode)createImplicitIdentifier(cc.symbolName()).setSymbol(lc.getCurrentFunction().compilerConstant(cc)); + return createImplicitIdentifier(cc.symbolName()).setSymbol(lc.getCurrentFunction().compilerConstant(cc)); } /** @@ -263,7 +263,7 @@ final Symbol nameSymbol = fn.getBody().getExistingSymbol(name.getName()); assert nameSymbol != null; - return (VarNode)synthVar.setName((IdentNode)name.setSymbol(nameSymbol)).accept(this); + return (VarNode)synthVar.setName(name.setSymbol(nameSymbol)).accept(this); } private FunctionNode createSyntheticInitializers(final FunctionNode functionNode) { @@ -522,7 +522,7 @@ final Symbol paramSymbol = body.getExistingSymbol(param.getName()); assert paramSymbol != null; assert paramSymbol.isParam() : paramSymbol + " " + paramSymbol.getFlags(); - newParams.add((IdentNode)param.setSymbol(paramSymbol)); + newParams.add(param.setSymbol(paramSymbol)); // parameters should not be slots for a function that uses variable arity signature if (isVarArg) {
--- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java Thu Jun 26 13:12:32 2014 +0200 @@ -1097,7 +1097,7 @@ closeBlockVariables(block); lc.releaseSlots(); - assert !method.isReachable() || lc.getUsedSlotCount() == method.getFirstTemp(); + assert !method.isReachable() || (lc.isFunctionBody() ? 0 : lc.getUsedSlotCount()) == method.getFirstTemp(); return block; }
--- a/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Thu Jun 26 13:12:32 2014 +0200 @@ -25,10 +25,12 @@ package jdk.nashorn.internal.codegen; +import static jdk.nashorn.internal.codegen.CompilerConstants.RETURN; import static jdk.nashorn.internal.ir.Expression.isAlwaysFalse; import static jdk.nashorn.internal.ir.Expression.isAlwaysTrue; import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.Collections; import java.util.Deque; import java.util.HashSet; @@ -39,7 +41,6 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; - import jdk.nashorn.internal.codegen.types.Type; import jdk.nashorn.internal.ir.AccessNode; import jdk.nashorn.internal.ir.BaseNode; @@ -72,6 +73,7 @@ import jdk.nashorn.internal.ir.RuntimeNode; import jdk.nashorn.internal.ir.RuntimeNode.Request; import jdk.nashorn.internal.ir.SplitNode; +import jdk.nashorn.internal.ir.Statement; import jdk.nashorn.internal.ir.SwitchNode; import jdk.nashorn.internal.ir.Symbol; import jdk.nashorn.internal.ir.TernaryNode; @@ -356,6 +358,8 @@ private boolean reachable = true; // Return type of the function private Type returnType = Type.UNKNOWN; + // Synthetic return node that we must insert at the end of the function if it's end is reachable. + private ReturnNode syntheticReturn; // Topmost current split node (if any) private SplitNode topSplit; @@ -845,6 +849,10 @@ @Override public boolean enterThrowNode(final ThrowNode throwNode) { + if(!reachable) { + return false; + } + throwNode.getExpression().accept(this); jumpToCatchBlock(throwNode); doesNotContinueSequentially(); @@ -1031,6 +1039,15 @@ @Override public Node leaveBlock(final Block block) { if(lc.isFunctionBody()) { + if(reachable) { + // reachable==true means we can reach the end of the function without an explicit return statement. We + // need to insert a synthetic one then. This logic used to be in Lower.leaveBlock(), but Lower's + // reachability analysis (through Terminal.isTerminal() flags) is not precise enough so + // Lower$BlockLexicalContext.afterSetStatements will sometimes think the control flow terminates even + // when it didn't. Example: function() { switch((z)) { default: {break; } throw x; } }. + createSyntheticReturn(block); + assert !reachable; + } // We must calculate the return type here (and not in leaveFunctionNode) as it can affect the liveness of // the :return symbol and thus affect conversion type liveness calculations for it. calculateReturnType(); @@ -1089,6 +1106,23 @@ retSymbol.setNeedsSlot(true); } } + + private void createSyntheticReturn(final Block body) { + final FunctionNode functionNode = lc.getCurrentFunction(); + final long token = functionNode.getToken(); + final int finish = functionNode.getFinish(); + final List<Statement> statements = body.getStatements(); + final int lineNumber = statements.isEmpty() ? functionNode.getLineNumber() : statements.get(statements.size() - 1).getLineNumber(); + final IdentNode returnExpr; + if(functionNode.isProgram()) { + returnExpr = new IdentNode(token, finish, RETURN.symbolName()).setSymbol(getCompilerConstantSymbol(functionNode, RETURN)); + } else { + returnExpr = null; + } + syntheticReturn = new ReturnNode(lineNumber, token, finish, returnExpr); + syntheticReturn.accept(this); + } + /** * Leave a breakable node. If there's a join point associated with its break label (meaning there was at least one * break statement to the end of the node), insert the join point into the flow. @@ -1178,6 +1212,16 @@ } @Override + public Node leaveBlock(final Block block) { + if(inOuterFunction && syntheticReturn != null && lc.isFunctionBody()) { + final ArrayList<Statement> stmts = new ArrayList<>(block.getStatements()); + stmts.add((ReturnNode)syntheticReturn.accept(this)); + return block.setStatements(lc, stmts); + } + return super.leaveBlock(block); + } + + @Override public Node leaveFunctionNode(final FunctionNode nestedFunctionNode) { inOuterFunction = true; final FunctionNode newNestedFunction = (FunctionNode)nestedFunctionNode.accept(
--- a/src/jdk/nashorn/internal/codegen/Lower.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/codegen/Lower.java Thu Jun 26 13:12:32 2014 +0200 @@ -75,7 +75,6 @@ import jdk.nashorn.internal.runtime.CodeInstaller; import jdk.nashorn.internal.runtime.Context; import jdk.nashorn.internal.runtime.JSType; -import jdk.nashorn.internal.runtime.ScriptRuntime; import jdk.nashorn.internal.runtime.Source; import jdk.nashorn.internal.runtime.logging.DebugLogger; import jdk.nashorn.internal.runtime.logging.Loggable; @@ -160,30 +159,6 @@ } @Override - public Node leaveBlock(final Block block) { - //now we have committed the entire statement list to the block, but we need to truncate - //whatever is after the last terminal. block append won't append past it - - - if (lc.isFunctionBody()) { - final FunctionNode currentFunction = lc.getCurrentFunction(); - final boolean isProgram = currentFunction.isProgram(); - final Statement last = lc.getLastStatement(); - final ReturnNode returnNode = new ReturnNode( - last == null ? currentFunction.getLineNumber() : last.getLineNumber(), //TODO? - currentFunction.getToken(), - currentFunction.getFinish(), - isProgram ? - compilerConstant(RETURN) : - LiteralNode.newInstance(block, ScriptRuntime.UNDEFINED)); - - returnNode.accept(this); - } - - return block; - } - - @Override public boolean enterBreakNode(final BreakNode breakNode) { addStatement(breakNode); return false;
--- a/src/jdk/nashorn/internal/ir/Block.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/ir/Block.java Thu Jun 26 13:12:32 2014 +0200 @@ -41,7 +41,7 @@ * IR representation for a list of statements. */ @Immutable -public class Block extends Node implements BreakableNode, Flags<Block> { +public class Block extends Node implements BreakableNode, Terminal, Flags<Block> { /** List of statements */ protected final List<Statement> statements; @@ -231,6 +231,11 @@ return flags; } + /** + * Is this a terminal block, i.e. does it end control flow like ending with a throw or return? + * + * @return true if this node statement is terminal + */ @Override public boolean isTerminal() { return getFlag(IS_TERMINAL);
--- a/src/jdk/nashorn/internal/ir/CaseNode.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/ir/CaseNode.java Thu Jun 26 13:12:32 2014 +0200 @@ -36,7 +36,7 @@ * Case nodes are not BreakableNodes, but the SwitchNode is */ @Immutable -public final class CaseNode extends Node implements JoinPredecessor, Labels { +public final class CaseNode extends Node implements JoinPredecessor, Labels, Terminal { /** Test expression. */ private final Expression test; @@ -77,6 +77,11 @@ this.conversion = conversion; } + /** + * Is this a terminal case node, i.e. does it end control flow like having a throw or return? + * + * @return true if this node statement is terminal + */ @Override public boolean isTerminal() { return body.isTerminal();
--- a/src/jdk/nashorn/internal/ir/ExpressionStatement.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/ir/ExpressionStatement.java Thu Jun 26 13:12:32 2014 +0200 @@ -57,11 +57,6 @@ } @Override - public boolean isTerminal() { - return expression.isTerminal(); - } - - @Override public Node accept(final NodeVisitor<? extends LexicalContext> visitor) { if (visitor.enterExpressionStatement(this)) { return visitor.leaveExpressionStatement(setExpression((Expression)expression.accept(visitor)));
--- a/src/jdk/nashorn/internal/ir/IdentNode.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/ir/IdentNode.java Thu Jun 26 13:12:32 2014 +0200 @@ -110,7 +110,7 @@ * @return a temporary identifier for the symbol. */ public static IdentNode createInternalIdentifier(final Symbol symbol) { - return (IdentNode)new IdentNode(Token.toDesc(TokenType.IDENT, 0, 0), 0, symbol.getName()).setSymbol(symbol); + return new IdentNode(Token.toDesc(TokenType.IDENT, 0, 0), 0, symbol.getName()).setSymbol(symbol); } @Override @@ -180,7 +180,7 @@ * @param symbol the symbol * @return new node */ - public Expression setSymbol(final Symbol symbol) { + public IdentNode setSymbol(final Symbol symbol) { if (this.symbol == symbol) { return this; }
--- a/src/jdk/nashorn/internal/ir/Node.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/ir/Node.java Thu Jun 26 13:12:32 2014 +0200 @@ -144,15 +144,6 @@ public abstract void toString(final StringBuilder sb, final boolean printType); /** - * Check if this node has terminal flags, i.e. ends or breaks control flow - * - * @return true if terminal - */ - public boolean hasTerminalFlags() { - return isTerminal() || hasGoto(); - } - - /** * Get the finish position for this node in the source string * @return finish */ @@ -169,15 +160,6 @@ } /** - * Check if this function repositions control flow with goto like - * semantics, for example {@link BreakNode} or a {@link ForNode} with no test - * @return true if node has goto semantics - */ - public boolean hasGoto() { - return false; - } - - /** * Get start position for node * @return start position */ @@ -249,16 +231,6 @@ return token; } - /** - * Is this a terminal Node, i.e. does it end control flow like a throw or return - * expression does? - * - * @return true if this node is terminal - */ - public boolean isTerminal() { - return false; - } - //on change, we have to replace the entire list, that's we can't simple do ListIterator.set static <T extends Node> List<T> accept(final NodeVisitor<? extends LexicalContext> visitor, final Class<T> clazz, final List<T> list) { boolean changed = false;
--- a/src/jdk/nashorn/internal/ir/Statement.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/ir/Statement.java Thu Jun 26 13:12:32 2014 +0200 @@ -30,7 +30,7 @@ * made up of statements. The only node subclass that needs to keep token and * location information is the Statement */ -public abstract class Statement extends Node { +public abstract class Statement extends Node implements Terminal { private final int lineNumber; @@ -77,4 +77,32 @@ return lineNumber; } + /** + * Is this a terminal statement, i.e. does it end control flow like a throw or return? + * + * @return true if this node statement is terminal + */ + @Override + public boolean isTerminal() { + return false; + } + + /** + * Check if this statement repositions control flow with goto like + * semantics, for example {@link BreakNode} or a {@link ForNode} with no test + * @return true if statement has goto semantics + */ + public boolean hasGoto() { + return false; + } + + /** + * Check if this statement has terminal flags, i.e. ends or breaks control flow + * + * @return true if has terminal flags + */ + public final boolean hasTerminalFlags() { + return isTerminal() || hasGoto(); + } } +
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/jdk/nashorn/internal/ir/Terminal.java Thu Jun 26 13:12:32 2014 +0200 @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2010, 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. 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. + */ + +package jdk.nashorn.internal.ir; + +/** + * Interface for AST nodes that can have a flag determining if they can terminate function control flow. + */ +public interface Terminal { + /** + * Returns true if this AST node is (or contains) a statement that terminates function control flow. + * @return true if this AST node is (or contains) a statement that terminates function control flow. + */ + public boolean isTerminal(); +}
--- a/src/jdk/nashorn/internal/ir/debug/ASTWriter.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/ir/debug/ASTWriter.java Thu Jun 26 13:12:32 2014 +0200 @@ -38,7 +38,9 @@ import jdk.nashorn.internal.ir.Expression; import jdk.nashorn.internal.ir.IdentNode; import jdk.nashorn.internal.ir.Node; +import jdk.nashorn.internal.ir.Statement; import jdk.nashorn.internal.ir.Symbol; +import jdk.nashorn.internal.ir.Terminal; import jdk.nashorn.internal.ir.TernaryNode; import jdk.nashorn.internal.ir.annotations.Ignore; import jdk.nashorn.internal.ir.annotations.Reference; @@ -144,11 +146,11 @@ String status = ""; - if (node.isTerminal()) { + if (node instanceof Terminal && ((Terminal)node).isTerminal()) { status += " Terminal"; } - if (node.hasGoto()) { + if (node instanceof Statement && ((Statement)node).hasGoto()) { status += " Goto "; }
--- a/src/jdk/nashorn/internal/ir/debug/PrintVisitor.java Wed Jun 25 14:36:24 2014 +0200 +++ b/src/jdk/nashorn/internal/ir/debug/PrintVisitor.java Thu Jun 26 13:12:32 2014 +0200 @@ -182,9 +182,9 @@ final List<Statement> statements = block.getStatements(); - for (final Node statement : statements) { - if (printLineNumbers && (statement instanceof Statement)) { - final int lineNumber = ((Statement)statement).getLineNumber(); + for (final Statement statement : statements) { + if (printLineNumbers) { + final int lineNumber = statement.getLineNumber(); sb.append('\n'); if (lineNumber != lastLineNumber) { indent(); @@ -196,10 +196,6 @@ statement.accept(this); - if (statement instanceof FunctionNode) { - continue; - } - int lastIndex = sb.length() - 1; char lastChar = sb.charAt(lastIndex); while (Character.isWhitespace(lastChar) && lastIndex >= 0) {
--- a/test/script/basic/JDK-8047057.js Wed Jun 25 14:36:24 2014 +0200 +++ b/test/script/basic/JDK-8047057.js Thu Jun 26 13:12:32 2014 +0200 @@ -45,8 +45,8 @@ } } -// makeFuncAndCall("switch(0) { default: {break;} return }"); -// makeFuncAndCall("L: { { break L; } return; }"); +makeFuncAndCall("switch(0) { default: {break;} return }"); +makeFuncAndCall("L: { { break L; } return; }"); makeFuncAndCall("L: { while(0) break L; return; }"); makeFuncExpectError("L: {while(0) break L; return [](); }", TypeError); // makeFuncAndCall("do with({}) break ; while(0);");
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/script/basic/JDK-8047357.js Thu Jun 26 13:12:32 2014 +0200 @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2010, 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-8047357: More precise synthetic return + unreachable throw + * + * @test + * @run + */ + +print((function() { switch(0) { default: {var x; break ; } throw x; } })()); +print((function() { switch(0) { default: {break;} return; } })());