8066225: NPE in MethodEmitter with duplicate integer switch cases
Reviewed-by: hannesw, lagergren
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/AssignSymbols.java Mon Dec 08 15:14:11 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/AssignSymbols.java Wed Dec 10 11:55:04 2014 +0100
@@ -917,7 +917,7 @@
@Override
public Node leaveSwitchNode(final SwitchNode switchNode) {
// We only need a symbol for the tag if it's not an integer switch node
- if(!switchNode.isInteger()) {
+ if(!switchNode.isUniqueInteger()) {
switchNode.setTag(newObjectInternal(SWITCH_TAG_PREFIX));
}
return switchNode;
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Dec 08 15:14:11 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Wed Dec 10 11:55:04 2014 +0100
@@ -2817,7 +2817,7 @@
Label defaultLabel = defaultCase != null ? defaultCase.getEntry() : breakLabel;
final boolean hasSkipConversion = LocalVariableConversion.hasLiveConversion(switchNode);
- if (switchNode.isInteger()) {
+ if (switchNode.isUniqueInteger()) {
// Tree for sorting values.
final TreeMap<Integer, Label> tree = new TreeMap<>();
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/FoldConstants.java Mon Dec 08 15:14:11 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/FoldConstants.java Wed Dec 10 11:55:04 2014 +0100
@@ -26,12 +26,16 @@
package jdk.nashorn.internal.codegen;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import jdk.nashorn.internal.codegen.types.Type;
import jdk.nashorn.internal.ir.BinaryNode;
import jdk.nashorn.internal.ir.Block;
import jdk.nashorn.internal.ir.BlockStatement;
+import jdk.nashorn.internal.ir.CaseNode;
import jdk.nashorn.internal.ir.EmptyNode;
+import jdk.nashorn.internal.ir.Expression;
import jdk.nashorn.internal.ir.FunctionNode;
import jdk.nashorn.internal.ir.FunctionNode.CompilationState;
import jdk.nashorn.internal.ir.IfNode;
@@ -40,6 +44,7 @@
import jdk.nashorn.internal.ir.LiteralNode.ArrayLiteralNode;
import jdk.nashorn.internal.ir.Node;
import jdk.nashorn.internal.ir.Statement;
+import jdk.nashorn.internal.ir.SwitchNode;
import jdk.nashorn.internal.ir.TernaryNode;
import jdk.nashorn.internal.ir.UnaryNode;
import jdk.nashorn.internal.ir.VarNode;
@@ -131,6 +136,32 @@
return ternaryNode;
}
+ @Override
+ public Node leaveSwitchNode(final SwitchNode switchNode) {
+ return switchNode.setUniqueInteger(lc, isUniqueIntegerSwitchNode(switchNode));
+ }
+
+ private static boolean isUniqueIntegerSwitchNode(final SwitchNode switchNode) {
+ final Set<Integer> alreadySeen = new HashSet<>();
+ for (final CaseNode caseNode : switchNode.getCases()) {
+ final Expression test = caseNode.getTest();
+ if (test != null && !isUniqueIntegerLiteral(test, alreadySeen)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static boolean isUniqueIntegerLiteral(final Expression expr, final Set<Integer> alreadySeen) {
+ if (expr instanceof LiteralNode) {
+ final Object value = ((LiteralNode<?>)expr).getValue();
+ if (value instanceof Integer) {
+ return alreadySeen.add((Integer)value);
+ }
+ }
+ return false;
+ }
+
/**
* Helper class to evaluate constant expressions at compile time This is
* also a simplifier used by BinaryNode visits, UnaryNode visits and
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Mon Dec 08 15:14:11 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Wed Dec 10 11:55:04 2014 +0100
@@ -709,7 +709,7 @@
// Control flow is different for all-integer cases where we dispatch by switch table, and for all other cases
// where we do sequential comparison. Note that CaseNode objects act as join points.
- final boolean isInteger = switchNode.isInteger();
+ final boolean isInteger = switchNode.isUniqueInteger();
final Label breakLabel = switchNode.getBreakLabel();
final boolean hasDefault = switchNode.getDefaultCase() != null;
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Lower.java Mon Dec 08 15:14:11 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Lower.java Wed Dec 10 11:55:04 2014 +0100
@@ -275,7 +275,7 @@
@Override
public Node leaveSwitchNode(final SwitchNode switchNode) {
- if(!switchNode.isInteger()) {
+ if(!switchNode.isUniqueInteger()) {
// Wrap it in a block so its internally created tag is restricted in scope
addStatementEnclosedInBlock(switchNode);
} else {
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/SwitchNode.java Mon Dec 08 15:14:11 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/SwitchNode.java Wed Dec 10 11:55:04 2014 +0100
@@ -48,6 +48,10 @@
/** Switch default index. */
private final int defaultCaseIndex;
+ /** True if all cases are 32-bit signed integer constants, without repetitions. It's a prerequisite for
+ * using a tableswitch/lookupswitch when generating code. */
+ private final boolean uniqueInteger;
+
/** Tag symbol. */
private Symbol tag;
@@ -66,15 +70,17 @@
this.expression = expression;
this.cases = cases;
this.defaultCaseIndex = defaultCase == null ? -1 : cases.indexOf(defaultCase);
+ this.uniqueInteger = false;
}
private SwitchNode(final SwitchNode switchNode, final Expression expression, final List<CaseNode> cases,
- final int defaultCaseIndex, final LocalVariableConversion conversion) {
+ final int defaultCaseIndex, final LocalVariableConversion conversion, final boolean uniqueInteger) {
super(switchNode, conversion);
this.expression = expression;
this.cases = cases;
this.defaultCaseIndex = defaultCaseIndex;
- this.tag = switchNode.getTag(); //TODO are symbols inhereted as references?
+ this.tag = switchNode.getTag(); //TODO are symbols inherited as references?
+ this.uniqueInteger = uniqueInteger;
}
@Override
@@ -83,7 +89,7 @@
for (final CaseNode caseNode : cases) {
newCases.add(new CaseNode(caseNode, caseNode.getTest(), caseNode.getBody(), caseNode.getLocalVariableConversion()));
}
- return Node.replaceInLexicalContext(lc, this, new SwitchNode(this, expression, newCases, defaultCaseIndex, conversion));
+ return Node.replaceInLexicalContext(lc, this, new SwitchNode(this, expression, newCases, defaultCaseIndex, conversion, uniqueInteger));
}
@Override
@@ -151,7 +157,7 @@
if (this.cases == cases) {
return this;
}
- return Node.replaceInLexicalContext(lc, this, new SwitchNode(this, expression, cases, defaultCaseIndex, conversion));
+ return Node.replaceInLexicalContext(lc, this, new SwitchNode(this, expression, cases, defaultCaseIndex, conversion, uniqueInteger));
}
/**
@@ -183,7 +189,7 @@
if (this.expression == expression) {
return this;
}
- return Node.replaceInLexicalContext(lc, this, new SwitchNode(this, expression, cases, defaultCaseIndex, conversion));
+ return Node.replaceInLexicalContext(lc, this, new SwitchNode(this, expression, cases, defaultCaseIndex, conversion, uniqueInteger));
}
/**
@@ -205,25 +211,30 @@
}
/**
- * Returns true if all cases of this switch statement are 32-bit signed integer constants.
- * @return true if all cases of this switch statement are 32-bit signed integer constants.
+ * Returns true if all cases of this switch statement are 32-bit signed integer constants, without repetitions.
+ * @return true if all cases of this switch statement are 32-bit signed integer constants, without repetitions.
*/
- public boolean isInteger() {
- for (final CaseNode caseNode : cases) {
- final Expression test = caseNode.getTest();
- if (test != null && !isIntegerLiteral(test)) {
- return false;
- }
+ public boolean isUniqueInteger() {
+ return uniqueInteger;
+ }
+
+ /**
+ * Sets whether all cases of this switch statement are 32-bit signed integer constants, without repetitions.
+ * @param lc lexical context
+ * @param uniqueInteger if true, all cases of this switch statement have been determined to be 32-bit signed
+ * integer constants, without repetitions.
+ * @return this switch node, if the value didn't change, or a new switch node with the changed value
+ */
+ public SwitchNode setUniqueInteger(final LexicalContext lc, final boolean uniqueInteger) {
+ if(this.uniqueInteger == uniqueInteger) {
+ return this;
}
- return true;
+ return Node.replaceInLexicalContext(lc, this, new SwitchNode(this, expression, cases, defaultCaseIndex, conversion, uniqueInteger));
}
@Override
JoinPredecessor setLocalVariableConversionChanged(final LexicalContext lc, final LocalVariableConversion conversion) {
- return Node.replaceInLexicalContext(lc, this, new SwitchNode(this, expression, cases, defaultCaseIndex, conversion));
+ return Node.replaceInLexicalContext(lc, this, new SwitchNode(this, expression, cases, defaultCaseIndex, conversion, uniqueInteger));
}
- private static boolean isIntegerLiteral(final Expression expr) {
- return expr instanceof LiteralNode && ((LiteralNode<?>)expr).getValue() instanceof Integer;
- }
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8066225.js Wed Dec 10 11:55:04 2014 +0100
@@ -0,0 +1,36 @@
+/*
+ * 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-8066225: NPE in MethodEmitter with duplicate integer switch cases
+ *
+ * @test
+ * @run
+ */
+
+(function (x){
+ switch(x) {
+ case 44: for (var x in {}) {x}; print("1");
+ case 44: print("2");
+ }
+})(44);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8066225.js.EXPECTED Wed Dec 10 11:55:04 2014 +0100
@@ -0,0 +1,2 @@
+1
+2