8043235: Type-based optimizations interfere with continuation methods
Reviewed-by: jlaskey, lagergren
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Wed May 14 17:05:08 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Thu May 15 15:28:51 2014 +0200
@@ -2383,6 +2383,14 @@
return false;
}
+ if(isDeoptimizedExpression(lhs)) {
+ // This is actually related to "lhs.getType().isPrimitive()" above: any expression being deoptimized in
+ // the current chain of rest-of compilations used to have a type narrower than Object (so it was primitive).
+ // We must not perform undefined check specialization for them, as then we'd violate the basic rule of
+ // "Thou shalt not alter the stack shape between a deoptimized method and any of its (transitive) rest-ofs."
+ return false;
+ }
+
final CompilationEnvironment env = compiler.getCompilationEnvironment();
// TODO: why?
if (env.isCompileRestOf()) {
@@ -2450,6 +2458,19 @@
return false;
}
+ if(isDeoptimizedExpression(lhs)) {
+ // This is actually related to "!lhs.getType().isObject()" above: any expression being deoptimized in
+ // the current chain of rest-of compilations used to have a type narrower than Object. We must not
+ // perform null check specialization for them, as then we'd no longer be loading aconst_null on stack
+ // and thus violate the basic rule of "Thou shalt not alter the stack shape between a deoptimized
+ // method and any of its (transitive) rest-ofs."
+ // NOTE also that if we had a representation for well-known constants (e.g. null, 0, 1, -1, etc.) in
+ // Label$Stack.localLoads then this wouldn't be an issue, as we would never (somewhat ridiculously)
+ // allocate a temporary local to hold the result of aconst_null before attempting an optimistic
+ // operation.
+ return false;
+ }
+
// this is a null literal check, so if there is implicit coercion
// involved like {D}x=null, we will fail - this is very rare
final Label trueLabel = new Label("trueLabel");
@@ -2505,6 +2526,39 @@
return true;
}
+ /**
+ * Was this expression or any of its subexpressions deoptimized in the current recompilation chain of rest-of methods?
+ * @param rootExpr the expression being tested
+ * @return true if the expression or any of its subexpressions was deoptimized in the current recompilation chain.
+ */
+ private boolean isDeoptimizedExpression(final Expression rootExpr) {
+ final CompilationEnvironment env = compiler.getCompilationEnvironment();
+ if(!env.isCompileRestOf()) {
+ return false;
+ }
+ return new Supplier<Boolean>() {
+ boolean contains;
+ @Override
+ public Boolean get() {
+ rootExpr.accept(new NodeVisitor<LexicalContext>(new LexicalContext()) {
+ @Override
+ public boolean enterFunctionNode(FunctionNode functionNode) {
+ return false;
+ }
+ @Override
+ public boolean enterDefault(final Node node) {
+ if(!contains && node instanceof Optimistic) {
+ final int pp = ((Optimistic)node).getProgramPoint();
+ contains = isValid(pp) && env.isContinuationEntryPoint(pp);
+ }
+ return !contains;
+ }
+ });
+ return contains;
+ }
+ }.get();
+ }
+
private void loadRuntimeNode(final RuntimeNode runtimeNode) {
final List<Expression> args = new ArrayList<>(runtimeNode.getArgs());
if (nullCheck(runtimeNode, args)) {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8043235.js Thu May 15 15:28:51 2014 +0200
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+
+/**
+ * JDK-8043235: Type-based optimizations interfere with continuation methods
+ *
+ * @test
+ * @run
+ */
+
+function g() {
+ return "Hello World!"
+}
+
+function f1() {
+ var c;
+ var paused = false;
+ // If we didn't disable nullCheck specialization for expressions
+ // containing a deoptimization site, we'd get an AssertionError
+ while (!paused && (null !== (c = g()))) {
+ print(c);
+ paused = true;
+ }
+}
+
+function f2() {
+ var c;
+ var paused = false;
+ // If we didn't disable undefinedCheck specialization for expressions
+ // containing a deoptimization site, we'd get an AssertionError
+ while (!paused && (undefined !== (c = g()))) {
+ print(c);
+ paused = true;
+ }
+}
+
+f1();
+f2();
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8043235.js.EXPECTED Thu May 15 15:28:51 2014 +0200
@@ -0,0 +1,2 @@
+Hello World!
+Hello World!