stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8047357: More precise synthetic return + unreachable throw

Browse Source 
Reviewed-by: lagergren, sundar
This commit is contained in:
Attila Szegedi 2014-06-26 13:12:32 +02:00
parent ccb4ecb339
commit 9dde0bfa35
16 changed files with 173 additions and 80 deletions

8
nashorn/src/jdk/nashorn/internal/codegen/AssignSymbols.java

@ -209,7 +209,7 @@ final class AssignSymbols extends NodeOperatorVisitor<LexicalContext> implements
if (varNode.isFunctionDeclaration()) {
symbol.setIsFunctionDeclaration();
}
return varNode.setName((IdentNode)ident.setSymbol(symbol));
return varNode.setName(ident.setSymbol(symbol));
}
return varNode;
}
@ -217,7 +217,7 @@ final class AssignSymbols extends NodeOperatorVisitor<LexicalContext> implements
}
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 class AssignSymbols extends NodeOperatorVisitor<LexicalContext> implements
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 class AssignSymbols extends NodeOperatorVisitor<LexicalContext> implements
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) {

2
nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java

@ -1097,7 +1097,7 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
closeBlockVariables(block);
lc.releaseSlots();
assert !method.isReachable() || lc.getUsedSlotCount() == method.getFirstTemp();
assert !method.isReachable() || (lc.isFunctionBody() ? 0 : lc.getUsedSlotCount()) == method.getFirstTemp();
return block;
}

46
nashorn/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java

@ -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.List;
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.ReturnNode;
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 @@ final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
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 @@ final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
@Override
public boolean enterThrowNode(final ThrowNode throwNode) {
if(!reachable) {
return false;
}
throwNode.getExpression().accept(this);
jumpToCatchBlock(throwNode);
doesNotContinueSequentially();
@ -1031,6 +1039,15 @@ final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
@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 @@ final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
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.
@ -1177,6 +1211,16 @@ final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
return node;
}
@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;

25
nashorn/src/jdk/nashorn/internal/codegen/Lower.java

@ -75,7 +75,6 @@ import jdk.nashorn.internal.parser.TokenType;
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;
@ -159,30 +158,6 @@ final class Lower extends NodeOperatorVisitor<BlockLexicalContext> implements Lo
return context.getLogger(this.getClass());
}
@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);

7
nashorn/src/jdk/nashorn/internal/ir/Block.java

@ -41,7 +41,7 @@ import jdk.nashorn.internal.ir.visitor.NodeVisitor;
* 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 @@ public class Block extends Node implements BreakableNode, Flags<Block> {
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);

7
nashorn/src/jdk/nashorn/internal/ir/CaseNode.java

@ -36,7 +36,7 @@ import jdk.nashorn.internal.ir.visitor.NodeVisitor;
* 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 @@ public final class CaseNode extends Node implements JoinPredecessor, Labels {
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();

5
nashorn/src/jdk/nashorn/internal/ir/ExpressionStatement.java

@ -56,11 +56,6 @@ public final class ExpressionStatement extends Statement {
this.expression = expression;
}
@Override
public boolean isTerminal() {
return expression.isTerminal();
}
@Override
public Node accept(final NodeVisitor<? extends LexicalContext> visitor) {
if (visitor.enterExpressionStatement(this)) {

4
nashorn/src/jdk/nashorn/internal/ir/IdentNode.java

@ -110,7 +110,7 @@ public final class IdentNode extends Expression implements PropertyKey, Function
* @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 @@ public final class IdentNode extends Expression implements PropertyKey, Function
* @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;
}

28
nashorn/src/jdk/nashorn/internal/ir/Node.java

@ -143,15 +143,6 @@ public abstract class Node implements Cloneable {
*/
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
@ -168,15 +159,6 @@ public abstract class Node implements Cloneable {
this.finish = finish;
}
/**
* 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 @@ public abstract class Node implements Cloneable {
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;

30
nashorn/src/jdk/nashorn/internal/ir/Statement.java

@ -30,7 +30,7 @@ package jdk.nashorn.internal.ir;
* 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 @@ public abstract class Statement extends Node {
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();
}
}

37
nashorn/src/jdk/nashorn/internal/ir/Terminal.java Normal file

@ -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();
}

6
nashorn/src/jdk/nashorn/internal/ir/debug/ASTWriter.java

@ -38,7 +38,9 @@ import jdk.nashorn.internal.ir.Block;
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 @@ public final class ASTWriter {
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 ";
}

10
nashorn/src/jdk/nashorn/internal/ir/debug/PrintVisitor.java

@ -182,9 +182,9 @@ public final class PrintVisitor extends NodeVisitor<LexicalContext> {
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 @@ public final class PrintVisitor extends NodeVisitor<LexicalContext> {
statement.accept(this);
if (statement instanceof FunctionNode) {
continue;
}
int lastIndex = sb.length() - 1;
char lastChar = sb.charAt(lastIndex);
while (Character.isWhitespace(lastChar) && lastIndex >= 0) {

4
nashorn/test/script/basic/JDK-8047057.js

@ -45,8 +45,8 @@ function makeFuncExpectError(code, ErrorType) {
}
}
// 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);");

32
nashorn/test/script/basic/JDK-8047357.js Normal file

@ -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; } })());

2
nashorn/test/script/basic/JDK-8047357.js.EXPECTED Normal file

@ -0,0 +1,2 @@
undefined
undefined